Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restore the public access level of the Log4jMarker class to its pre-2.19.0 state #1414

Closed
AlexxeyKa opened this issue Apr 7, 2023 · 2 comments
Assignees
Labels
api Affects the public API bug Incorrect, unexpected, or unintended behavior of existing code
Milestone

Comments

@AlexxeyKa
Copy link

Hello,

I would like to request that the org.apache.logging.slf4j.Log4jMarker class be made public once again, as it was prior to version 2.19.0.

Having markers in the LogEvent is a powerful mechanism for passing custom markers with custom fields inside. Previously, it was possible to implement a custom marker and pass it through the log4j logger as a Log4jMarker, which could then be caught in a custom FieldProvider, analyzed, and acted upon.

However, with the release of version 2.19.0, this functionality was removed by making the Log4jMarker class package-private. As a result, all custom markers are converted into Log4jMarker inside the Log4jMarkerFactory, and only the name and parents fields are retained, which is unacceptable for our use case.

Therefore, I request that the Log4jMarker class be made public once again, in order to allow for the same level of flexibility and customization that was available prior to version 2.19.0. Additionally, I would like to note that the Log4jMarker constructor remains public, so making the class itself public should not pose any security concerns.

Thank you for your consideration.

@ppkarwasz ppkarwasz added bug Incorrect, unexpected, or unintended behavior of existing code api Affects the public API labels Apr 7, 2023
@ppkarwasz ppkarwasz self-assigned this Apr 7, 2023
@jvz jvz added this to the 2.21.0 milestone Apr 10, 2023
@ppkarwasz ppkarwasz modified the milestones: 2.21.0, 2.20.1 Aug 1, 2023
@vy
Copy link
Member

vy commented Oct 5, 2023

Even though I don't agree with the way you use Log4jMarker, we should indeed not break the backward compatibility. Hence, I have fixed the issue. (Had a talk with @ppkarwasz and he will drop our further remarks here.)

All in all, feedback is feedback. Thanks for sharing the issue with us. 🙇

@vy vy closed this as completed Oct 5, 2023
@ppkarwasz
Copy link
Contributor

@AlexxeyKa,

The API breakage in 2.19.0 was unintentional and we'll revert it in the 2.x branch. As for the 3.x branch I would keep the class hidden.

The reasons behind this are largely explained in #770: using markers to convey additional data is IMHO a hack to workaround SLF4J's limitations. Unlike the Log4j API that allows to log any kind of Message, for a very long time SLF4J only allowed to log Strings (or the equivalent of ParameterizeMessage).

The situation changed with the release of SLF4J 2.x:

As discussed in #1813 the bridges between SLF4J and Log4j API will (probably) map KeyValuePairs to entries in a MapMessage and viceversa.

@rgoers, am I missing something?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Affects the public API bug Incorrect, unexpected, or unintended behavior of existing code
Projects
None yet
Development

No branches or pull requests

4 participants