Skip to content

Structured properties API enhancement proposal #1144

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

Closed
wants to merge 66 commits into from
Closed

Conversation

jvz
Copy link
Member

@jvz jvz commented Nov 21, 2022

This PR has initial work toward implementing https://cwiki.apache.org/confluence/display/LOGGING/Properties+Enhancement with updates to the property parsing API. This is proposed to replace PropertiesUtil in general as the semantics for keys have changed.

@jvz jvz requested a review from rgoers November 21, 2022 20:04
@jvz
Copy link
Member Author

jvz commented Nov 21, 2022

Wow, CodeQL is a little paranoid there. Let me see what I can do about that.

Signed-off-by: Matt Sicker <[email protected]>
Copy link
Member

@rgoers rgoers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So far this looks good. I don't see any tie in between LoggerContext specific properties but that is OK for a first pass.

@jvz
Copy link
Member Author

jvz commented Nov 24, 2022

The context parameter in the new API is the main action for that. Properties of the form log4j2.*.foo.bar are filled in with the context name if specified (hence the unit test demos as migrating everything over to this API isn't done yet).

jvz added 23 commits December 10, 2022 18:19
Signed-off-by: Matt Sicker <[email protected]>
Signed-off-by: Matt Sicker <[email protected]>
This will help remove the need to specify a context name when looking up properties. Hopefully.

Signed-off-by: Matt Sicker <[email protected]>
This requires the introduction of a few new classes (see ConfigurationResolver and HttpSourceLoader for non-trivial refactoring) for allowing dependency injection or otherwise cleaning up static state. Some APIs are expected to be removed once the migration is completed.

This adds default properties values, though the properties for specifying class names are currently deprecated and expected to be migrated to use the dependency injection system entirely.

This adds a way to specify the logger context name for tests along with updates to ContextSelector to support this.

Signed-off-by: Matt Sicker <[email protected]>
This could have been done back when we switched to a Java 7 baseline, but here we are updating it much later.

Signed-off-by: Matt Sicker <[email protected]>
Signed-off-by: Matt Sicker <[email protected]>
Signed-off-by: Matt Sicker <[email protected]>
jvz added 19 commits January 1, 2023 18:21
Signed-off-by: Matt Sicker <[email protected]>
Signed-off-by: Matt Sicker <[email protected]>
This is no longer relevant as LogEvent is not serializable.

Signed-off-by: Matt Sicker <[email protected]>
Signed-off-by: Matt Sicker <[email protected]>
Signed-off-by: Matt Sicker <[email protected]>
Signed-off-by: Matt Sicker <[email protected]>
Signed-off-by: Matt Sicker <[email protected]>
Signed-off-by: Matt Sicker <[email protected]>
@jvz jvz requested a review from ppkarwasz January 2, 2023 19:24
@jvz
Copy link
Member Author

jvz commented Jan 2, 2023

@ppkarwasz I've still got things to fix here, but the general shape of the changes are coming into place.

@jvz
Copy link
Member Author

jvz commented Jan 9, 2023

I'm going to begin working on breaking out the various entangled changes in this PR into independent changes. A partial list of things to extract to separate commits/PRs:

  • Move JsonReader API to log4j-api in the util package
  • Automatic spotless apply in build
  • Replace NullOutputStream with OutputStream::nullOutputStream
  • Remove Serializable from Layout API (we already tend to use custom interfaces to serializing to bytes)
  • Remove Serializable from LogEvent API
  • Update Lazy::relaxed
  • Move Recycler API to log4j-api in util
  • Move AbstractLifeCycle methods to default methods in LifeCycle
  • Update TriggeringPolicy to extend LifeCycle (marked as a TODO from long ago)

@rgoers
Copy link
Member

rgoers commented Jan 10, 2023

I am thrilled you are breaking out all the independent changes!

@jvz
Copy link
Member Author

jvz commented Jan 10, 2023

I hope you continue to remain thrilled when I submit the next PR as this one changes a bunch of files: LOG4J2-3228. We're removing Serializable from a few interfaces which lets us remove a bunch of serialization proxy classes (but not ThrowableProxy amusingly enough; that seems to be reused by JSON et al.).

@jvz jvz added this to the 3.0.0 milestone Jan 13, 2023
@vy vy deleted the branch apache:master February 28, 2023 15:02
@vy vy closed this Feb 28, 2023
@ppkarwasz ppkarwasz modified the milestones: 3.0.0, 3.0.0-beta1 Feb 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants