Conversation
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 Co-authored-by: Volkan Yazıcı <volkan@yazi.ci>
This change introduces setter methods for the private fields added to `Rfc5424LayoutBuilder` in version `2.25.4`. These fields were originally introduced to restore configuration options that existed in `2.20.0` but were unintentionally renamed in `2.21.0`. Due to semantic versioning constraints, setters were not added at that time. With the `2.26.0` release, it is now appropriate to expose these fields via setters, restoring full configurability and improving API usability.
There was a problem hiding this comment.
Pull request overview
This PR restores builder/setter API parity for Rfc5424Layout configuration by exposing newly reintroduced configuration fields on Rfc5424LayoutBuilder, while keeping compatibility with the legacy (2.21.0-era) attribute/setter names.
Changes:
- Add missing builder setters for documented RFC5424 layout attributes (
newLine,newLineEscape,mdcIncludes,mdcExcludes,mdcRequired,useTlsMessageFormat) and deprecate legacy alias setters. - Update internal call sites (e.g.,
createLayout,SyslogAppender) to use the documented setter names. - Add tests to verify plugin configuration accepts both documented attributes and compatibility aliases.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| src/changelog/.2.x.x/rfc5424-setters.xml | Adds a changelog entry for the new builder setters. |
| log4j-core/src/main/java/org/apache/logging/log4j/core/layout/Rfc5424Layout.java | Introduces new builder attributes/setters + compatibility aliases and updates builder resolution logic. |
| log4j-core/src/main/java/org/apache/logging/log4j/core/appender/SyslogAppender.java | Switches to the newly added documented builder setters when constructing Rfc5424Layout. |
| log4j-core-test/src/test/java/org/apache/logging/log4j/core/layout/Rfc5424LayoutTest.java | Adds coverage to ensure documented attributes and compatibility aliases are accepted via plugin configuration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| String effectiveExcludes = Objects.toString(mdcExcludes, excludes); | ||
|
|
||
| if (effectiveIncludes != null && effectiveExcludes != null) { | ||
| LOGGER.error("mdcIncludes and mdcExcludes are mutually exclusive. Includes wil be ignored"); |
There was a problem hiding this comment.
Typo in the error log message: "Includes wil be ignored" should be "Includes will be ignored". (This string also appears elsewhere in the class; it should be corrected consistently.)
| LOGGER.error("mdcIncludes and mdcExcludes are mutually exclusive. Includes wil be ignored"); | |
| LOGGER.error("mdcIncludes and mdcExcludes are mutually exclusive. Includes will be ignored"); |
| type="added"> | ||
| <description format="asciidoc"> | ||
| Add missing setters to `Rfc5424LayoutBuilder`. | ||
| </description> |
There was a problem hiding this comment.
This changelog entry is missing an element linking to the PR/issue. All other entries in src/changelog/.2.x.x include at least one ; please add the appropriate link for traceability and consistency.
| </description> | |
| </description> | |
| <issue id="GH-0000" link="https://github.com/apache/logging-log4j2/pull/0000"/> |
| /** | ||
| * @deprecated Since 2.26.0 use {@link #setIncludeNL} instead. | ||
| */ | ||
| @Deprecated | ||
| public Rfc5424LayoutBuilder setEscapeNL(final String escapeNL) { | ||
| this.escapeNL = escapeNL; |
There was a problem hiding this comment.
The Javadoc for the deprecated setEscapeNL method points to the wrong replacement method (it references setIncludeNL, which is unrelated and also deprecated). This should direct users to setNewLineEscape.
| includeMDC, | ||
| includeNL, | ||
| escapeNL, | ||
| newLine || includeNL, | ||
| Objects.toString(newLineEscape, escapeNL), |
There was a problem hiding this comment.
The newLine/includeNL alias handling uses newLine || includeNL, which means the old alias can still force newLine on even if the documented attribute is explicitly set to false. To match the precedence used for string aliases (new attribute wins), consider switching these flags to Boolean so you can detect “unset” and then resolve with precedence (e.g., prefer newLine when non-null, otherwise fall back to includeNL).
| exceptionPattern, | ||
| useTLSMessageFormat, | ||
| useTlsMessageFormat || useTLSMessageFormat, | ||
| loggerFields); |
There was a problem hiding this comment.
The useTlsMessageFormat/useTLSMessageFormat alias handling uses useTlsMessageFormat || useTLSMessageFormat, which prevents the documented attribute from overriding the legacy alias if both are present. Consider using Boolean fields and resolving with a clear precedence rule (prefer the documented attribute when set) for consistency with the string alias handling.
Warning
Review this after #4074 has been merged and version
2.25.4merged into2.x.This change introduces setter methods for the private fields added to
Rfc5424LayoutBuilderin version2.25.4.These fields were originally introduced to restore configuration options that existed in
2.20.0but were unintentionally renamed in2.21.0. Due to semantic versioning constraints, setters were not added at that time.With the
2.26.0release, it is now appropriate to expose these fields via setters, restoring full configurability and improving API usability.