Rebind log4j2 metrics if configuration is changed

[pativa]

Describe the issue

This PR is intended to fix #5756

When log4j2 metrics are reconfigured, metrics will no longer be recorded. For example, if monitorInterval (docs) is configured to 30 is used and configuration is reloaded after application start, metrics would only be recorded for the first 30 seconds.

---

By using a PropertyChangeListener, it looks like we can register the MetricsFilter in much the same way, but it will be rebound after reloading the configuration. All the previous tests seem to pass too when using this method of configuration.

[shakuzen]

Thank you for the quick turnaround time on submitting a pull request for this. It generally looks good to me. Will metrics filters still be configured on the old Configuration instance? Should that be the case? Does that not result in a memory leak of sorts?

[pativa]

Thank you for the quick turnaround time on submitting a pull request for this. It generally looks good to me. Will metrics filters still be configured on the old Configuration instance? Should that be the case? Does that not result in a memory leak of sorts?

Thanks for the review!

That is a good point! I don't think it would be any big issue, as the old configuration is already stopped here, but I do think it would be cleaner to clean up any old references to MetricsFilter when we rebind it.

I made a new commit here where I also do a cleanup of the old configuration instance. I also changed the changeListener to only do an update if the configuration instance is not the same object as the old instance (as no rebinding or cleanup should be needed then).

[pativa]

Kind of unrelated to this PR, so I haven't changed it, but the call to

loggerContext.updateLoggers(configuration);

Seems to be completely unnecessary. I don't know how expensive this operation is, but it seems to be unnecessary to trigger a reload as this class modifies the configuration in-place?

[jonatan-ivanov]

updateLoggers was removed in the past and was quickly restored it, see:

• Polish Log4j2Metrics #867
• Restore updateLoggers() in Log4j2Metrics.updateLoggers() #872

Basically it does two things:

1. It updates the internal "copy" of the config, since references are used in some cases, this might not be an issue but things can change (in Log4J2 or in Micrometer) so it's safer to call updateLoggers too
2. It emits a PropertyChangeEvent to notify components about the change

[shakuzen]

Shouldn't this be loggerConfig::removeFilter?

[pativa]

Yeah it should, I must have messed this up during the rebase... 😅 Fixed!