Skip to content

Handle programatically added log4j2 LoggerConfig #5902

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

Conversation

pativa
Copy link
Contributor

@pativa pativa commented Feb 6, 2025

The needed change for being able to bind to programmatically added loggers is simply to remove listener.getOldValue() != listener.getNewValue().

However, when we do that it leads to three things:

  • We need to always check if the MetricsFilter is already added to avoid adding it again on configuration reload
  • We need some way to tell when to call updateLoggers() to avoid an endless loop (as the listener will be triggered again when calling updateLoggers()
  • We need to move the check if its a new Configuration to the conditional before removeMetricsFilter to avoid removing all filters when we reload with the same Configuration instance

I did this by returning whether a MetricsFilter was added or not from registerMetricsFilter(). I tried out some different ways of doing this, but I ended up refactoring it a bit and I think it ended up quite readable.

I have also included a test that adds a logger programmatically (which would fail without these changes). It also verifies that updateLoggers() is called once with the new MetricsFilter configured, as expected.

Resolves #5901

* The goal of this is to be able to bind to a newly added `LoggerConfig` even if the `Configuration` is the same
* This means we always need to check if the filter was already added to avoid adding it twice
* We need to keep track of if anything was changed to avoid an endless loop when we call `updateLoggers()`

Signed-off-by: Patrik Ivarsson <[email protected]>
Copy link
Member

@shakuzen shakuzen left a comment

Choose a reason for hiding this comment

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

Looks good. Thank you for the complete pull request. I just had one comment about the test.

@shakuzen shakuzen merged commit e4c9658 into micrometer-metrics:main Feb 18, 2025
9 checks passed
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.

Log4j2Metrics does not work with programatically added LoggerConfig
2 participants