You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by "ppkarwasz (via GitHub)" <gi...@apache.org> on 2026/03/24 18:41:40 UTC

[PR] Restore support for documented `Rfc5424Layout` attributes [logging-log4j2]

ppkarwasz opened a new pull request, #4074:
URL: https://github.com/apache/logging-log4j2/pull/4074

   In `2.21.0`, `Rfc5424Layout` was migrated from a factory method to the builder pattern. During this change, the recognized names of several configuration attributes unintentionally diverged from the documented ones.
   
   As a result, some documented attributes were no longer recognized, while new, undocumented names were introduced.
   
   This change restores support for the documented attribute names while continuing to accept the names introduced in `2.21.0` for backward compatibility.
   
   Fixes #4022
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Restore support for documented `Rfc5424Layout` attributes [logging-log4j2]

Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #4074:
URL: https://github.com/apache/logging-log4j2/pull/4074#discussion_r2983852523


##########
log4j-core-test/src/test/java/org/apache/logging/log4j/core/layout/Rfc5424LayoutTest.java:
##########
@@ -792,4 +801,123 @@ void testFQDN() throws UnknownHostException {
         final Rfc5424Layout layout = Rfc5424Layout.newBuilder().build();
         assertThat(layout.getLocalHostName()).isEqualTo(fqdn);
     }
+
+    private static Map<String, String> attributeMap(String... keyValuePairs) {
+        Map<String, String> result = new HashMap<>();
+        for (int i = 0; i < keyValuePairs.length; i += 2) {
+            result.put(keyValuePairs[i], keyValuePairs[i + 1]);
+        }
+        return result;
+    }
+
+    private static Rfc5424Layout buildRfc5424Layout(Map<String, String> attributes) {
+        PluginManager manager = new PluginManager(Node.CATEGORY);
+        manager.collectPlugins();
+

Review Comment:
   Nice catch! :100:
   
   Fixed in https://github.com/apache/logging-log4j2/pull/4074/commits/d411716e195f4bd3ea47d3afda3bb5afcb5abddb



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Restore support for documented `Rfc5424Layout` attributes [logging-log4j2]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #4074:
URL: https://github.com/apache/logging-log4j2/pull/4074#discussion_r2983748780


##########
log4j-core/src/main/java/org/apache/logging/log4j/core/layout/Rfc5424Layout.java:
##########
@@ -880,19 +1047,19 @@ public Rfc5424Layout build() {
                     id,
                     ein,
                     includeMDC,
-                    includeNL,
-                    escapeNL,
+                    newLine || includeNL,
+                    Objects.toString(newLineEscape, escapeNL),
                     mdcId,
                     mdcPrefix,
                     eventPrefix,
                     appName,
                     messageId,
-                    excludes,
-                    includes,
-                    required,
+                    effectiveExcludes,
+                    effectiveIncludes,
+                    Objects.toString(mdcRequired, required),
                     charset != null ? charset : StandardCharsets.UTF_8,
                     exceptionPattern,
-                    useTLSMessageFormat,
+                    useTlsMessageFormat || useTLSMessageFormat,
                     loggerFields);

Review Comment:
   The boolean alias handling uses logical OR (e.g., `newLine || includeNL`, `useTlsMessageFormat || useTLSMessageFormat`). If both attribute names are present and one is explicitly set to `false`, the other being `true` will still force `true`, which makes it impossible for the documented attribute to override the legacy alias. Consider using `Boolean` fields (nullable) and applying a clear precedence rule (e.g., prefer documented attribute when provided, otherwise fall back to the compatibility alias).



##########
log4j-core/src/main/java/org/apache/logging/log4j/core/layout/Rfc5424Layout.java:
##########
@@ -860,9 +1024,12 @@ public Rfc5424LayoutBuilder setEnterpriseNumber(Integer enterpriseNumber) {
 
         @Override
         public Rfc5424Layout build() {
-            if (includes != null && excludes != null) {
+            String effectiveIncludes = Objects.toString(mdcIncludes, includes);
+            String effectiveExcludes = Objects.toString(mdcExcludes, excludes);
+
+            if (effectiveIncludes != null && effectiveExcludes != null) {
                 LOGGER.error("mdcIncludes and mdcExcludes are mutually exclusive. Includes wil be ignored");

Review Comment:
   The log message has a spelling typo: "wil" should be "will". Since this message is used when rejecting mutually-exclusive configuration attributes, fixing it helps avoid propagating the typo into status logs and documentation/searches.
   ```suggestion
                   LOGGER.error("mdcIncludes and mdcExcludes are mutually exclusive. Includes will be ignored");
   ```



##########
log4j-core-test/src/test/java/org/apache/logging/log4j/core/layout/Rfc5424LayoutTest.java:
##########
@@ -792,4 +801,123 @@ void testFQDN() throws UnknownHostException {
         final Rfc5424Layout layout = Rfc5424Layout.newBuilder().build();
         assertThat(layout.getLocalHostName()).isEqualTo(fqdn);
     }
+
+    private static Map<String, String> attributeMap(String... keyValuePairs) {
+        Map<String, String> result = new HashMap<>();
+        for (int i = 0; i < keyValuePairs.length; i += 2) {
+            result.put(keyValuePairs[i], keyValuePairs[i + 1]);
+        }
+        return result;
+    }
+
+    private static Rfc5424Layout buildRfc5424Layout(Map<String, String> attributes) {
+        PluginManager manager = new PluginManager(Node.CATEGORY);
+        manager.collectPlugins();
+

Review Comment:
   `buildRfc5424Layout` creates a new `PluginManager` and calls `collectPlugins()` for every parameterized test invocation, which can significantly slow down the test suite. Consider caching the `PluginType`/`PluginManager` (e.g., static field initialized once) and reusing it across these tests while still building a fresh `Node` per case.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Restore support for documented `Rfc5424Layout` attributes [logging-log4j2]

Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz merged PR #4074:
URL: https://github.com/apache/logging-log4j2/pull/4074


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Restore support for documented `Rfc5424Layout` attributes [logging-log4j2]

Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #4074:
URL: https://github.com/apache/logging-log4j2/pull/4074#discussion_r2983819184


##########
log4j-core/src/main/java/org/apache/logging/log4j/core/layout/Rfc5424Layout.java:
##########
@@ -880,19 +1047,19 @@ public Rfc5424Layout build() {
                     id,
                     ein,
                     includeMDC,
-                    includeNL,
-                    escapeNL,
+                    newLine || includeNL,
+                    Objects.toString(newLineEscape, escapeNL),
                     mdcId,
                     mdcPrefix,
                     eventPrefix,
                     appName,
                     messageId,
-                    excludes,
-                    includes,
-                    required,
+                    effectiveExcludes,
+                    effectiveIncludes,
+                    Objects.toString(mdcRequired, required),
                     charset != null ? charset : StandardCharsets.UTF_8,
                     exceptionPattern,
-                    useTLSMessageFormat,
+                    useTlsMessageFormat || useTLSMessageFormat,
                     loggerFields);

Review Comment:
   One of the attributes is deprecated, they can not be both set.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org