Properly handle disabled metrics in MP #8908
Merged
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
Signed-off-by: Tim Quinn <tim.quinn@oracle.com>
…ch metric type; add test
oracle-contributor-agreement
bot
added
the
OCA Verified
spericas
approved these changes
Jun 26, 2024
barchetta
pushed a commit
to barchetta/helidon
that referenced
this pull request
Jul 12, 2024
* Fix MP handling of disabled metrics Signed-off-by: Tim Quinn <tim.quinn@oracle.com> * A little refactoring to reduce the need to duplicate some code for each metric type; add test --------- Signed-off-by: Tim Quinn <tim.quinn@oracle.com>
barchetta
added a commit
that referenced
this pull request
Jul 12, 2024
* Fix MP handling of disabled metrics * A little refactoring to reduce the need to duplicate some code for each metric type; add test --------- Signed-off-by: Tim Quinn <tim.quinn@oracle.com> Co-authored-by: Tim Quinn <tim.quinn@oracle.com>
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 #8906
In Helidon 3.x, if the metrics configuration disabled a metric then Helidon created a no-op metric and registered it in the registry.
For Helidon 4.x, after discussion and in conformance with common practice, Helidon SE no longer registers a disabled meter in the meter registry or stores it in the related data structures. It simply returns a no-op meter when one is looked up or registered.
Helidon MP metrics needs additional data structures beyond the SE meter registry. When code invokes the MP metrics API to get a metric, MP metrics consults those MP data structures. If the requested metric does not exist, the MP code delegates to the SE code to register a meter and then via an "on-create" callback updates its data structures and returns an MP metric wrapper around the new SE meter.
In this processing, though, the MP code did not check whether the created SE meter was a no-op or not and this led to the MP data structures being out of sync with the SE meter registry and led to the error reported in the issue.
The key change in the PR is that Helidon MP metrics is now sensitive to whether a searched-for metric is disabled or not. If enabled, it uses the old code path which always worked for that case. If disabled, it now bypasses the attempt to look-up the SE delegate and returns a Helidon MP wrapper around the disabled SE meter delegate.
To accomplish this, each Helidon MP metric type (
HelidonCounter, etc.) add a new staticcreatefactory method which accepts the corresponding HelidonMetertype (Counter, etc.) as a delegate. Those methods delegate to some new common code which encapsulates the decision-making about whether the delegate is configured to be enabled or not.One new test makes sure that selectively disabled metrics are handled correctly.
Documentation
Bug fix; no doc impact