-
Notifications
You must be signed in to change notification settings - Fork 566
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
4.x Metrics followup #7547
Merged
Merged
4.x Metrics followup #7547
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
All contributors have signed the Oracle Contributor Agreement.
label
Sep 7, 2023
…names work; add clear logic to MetricsFeature so multiple tests in JVM work; change vendor GC collection count from gauge to functional counter
…Formatter; re-enable disabled tests
… mp-1 pom so expected base metrics are present
tjquinno
force-pushed
the
metrics-followup
branch
from
September 9, 2023 13:11
f00ee23
to
638a5ad
Compare
…nd do not set units at all if they do not apply
… will work; add unrelated explanatory comment
This was
linked to
issues
Sep 12, 2023
…the test to pass; also change to new enable/disable notation in config
…API and impl are transitive through observe-metrics
…mplementation and the system meters component
tomas-langer
requested changes
Sep 13, 2023
.../src/main/java/io/helidon/webserver/observe/metrics/KeyPerformanceIndicatorMetricsImpls.java
Outdated
Show resolved
Hide resolved
tomas-langer
approved these changes
Sep 13, 2023
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 #7506 (epic of TODO tasks from the M1 metrics work) and #7566 (issue for addressing all the epic task items).
This PR addresses the list of TODO items created as part of merging the neutral metrics API work into M2.
There are quite a few separate commits in this PR; I tried to separate the commits into related changes.
NOTE
In this PR components that depend on metrics, in many cases, now have these dependencies:
helidon-webserver-observe-metrics
(which supports the/metrics
endpoint and brings along thehelidon-metrics-api
andhelidon-metrics-providers-micrometer
artifacts as transitive dependencies)helidon-metrics-system-meters
(thebase
meters that expose JVM and OS measurements) for those components that want them.I made this adjustment after a discussion with several of the US folks but mostly Joe and Romain. This is consistent with the way components have dependencies on health in 4.x:
helidon-webserver-observe-health
supports/health
and brings alonghelidon-health
(the API and the implementation)helidon-health-checks
separately provides the Helidon-provided built-in health checks for those components that want them.As a separate, broader topic we should think about whether there are useful bundles for health and metrics that would make sense, esp. for developers who are new to Helidon.
General categories of changes
helidon-metrics-providers-micrometer
andhelidon-metrics-system-meters
in many cases.String
; now arePattern
)NoOpMeterRegistry
to notify listeners when meters are added (or removed), primarily so the MPRegistry
can update the map between MP metrics and delegates. That's needed so we can return the same MP metric (rather than creating new no-op meters all the time) if the client retrieves by metric ID, for example.mp_scope
andmp_app
).Documentation
No doc impact from this PR alone. Separate issue for metrics doc update.