More carefully handle re-creation of global meter registry #8389
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
oracle-contributor-agreement
bot
added
the
OCA Verified
tomas-langer
approved these changes
Feb 22, 2024
hrstoyanov
pushed a commit
to hrstoyanov/helidon
that referenced
this pull request
Feb 23, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Resolves #8382
The global meter registry can be reconfigured with new
MetricsConfigeven if it has been previously initialized.This should be rare but can happen. In such cases, the Helidon metrics implementation using Micrometer did not remove previously-registered meters from the Micrometer meter registry. It just cleared out the Helidon data structures.
When Helidon creates a new meter registry, it automatically registers any pre-defined meters (such as the base metrics for various JVM quantities). The warnings reported in the issue resulted when Helidon, as it was registering the predefined meters for the new meter registry, unexpectedly found those meters already registered in the Micrometer meter registry because of the prior initialization of a prior global registry.
This PR does several things:
When the global meter registry is reconfigured, Helidon now removes any previously-registered meters as part of closing the Helidon meter registry. This removes them from the Micrometer meter registry as well so when predefined meters are added again via the Helidon metrics API they do not already exist in the Micrometer registry.
As an optimization, the code skips the reconfiguration of the global registry (and the removal and reregistration of predefined meters) if the
MetricsConfigprovided for the re-configuration is consistent with theMetricsConfigused when the previous global registry was created.As part of this optimization, the
configsetting forMetricsConfigis now marked as redundant so it is not used in the generatedequalsorhashCodemethods. To decide if the previous and newMetricsConfigobjects are consistent we don't care if the configuration objects (if any) are equal--just that the behavior induced by theMetricsConfigsettings will be the same. For example, this can happen if one config instance explicitly sets values that happen to be the defaults there are used if the config node isMISSING.The warning message (and some other related ones not involved in this issue) used the Micrometer
Meter.toString()which did not have a helpful implementation. Now those messages log the meter ID which is at least readable.Documentation
Bug fix; no doc impact