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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
