-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New Observation concept; revert Timer.Sample changes #2992
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
Merged
marcingrzejszczak
merged 7 commits into
micrometer-metrics:2.0.x
from
jonatan-ivanov:observation-api-more-stateful
Feb 3, 2022
Merged
New Observation concept; revert Timer.Sample changes #2992
marcingrzejszczak
merged 7 commits into
micrometer-metrics:2.0.x
from
jonatan-ivanov:observation-api-more-stateful
Feb 3, 2022
Conversation
This file contains hidden or 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
* We moved out all the getters - this API will provide a nice way to set things ON THE CONTEXT * We've added additionalLowCardinality and high cardinliaty tags on the context, tagsprovider is immutable * We removed timing information - there's no gain in unifying the time (e.g. same time for metrics & spans). It's up to the handlers to take control of doing measurements. If a handler is buggy we will see that in its timing information. * We removed the throwable getter to explicitly pass the throwable to the handler - that's the only parameter that will never change (onError - you have to have an error there)
…aning up (fixing imports, checkstyle issues, license, etc.)
…servation does not have any getters
Keeps only one registry implementation while still providing a specific type for ObservationRegistry where Observation-specific concerns can be defined. Forces instantiation of Observation through its static methods so there is no direct instantiation by users. This provides fewer ways to do the same thing and exposes API, which should help guide users and hide implementation details.
MeterRegistry implements ObservationRegistry, static factory methods
micrometer-api/src/main/java/io/micrometer/api/instrument/observation/ObservationRegistry.java
Show resolved
Hide resolved
| private final More more = new More(); | ||
| private final ThreadLocal<Timer.Sample> localSample = new ThreadLocal<>(); | ||
|
|
||
| private final ThreadLocal<Observation> localObservation = new ThreadLocal<>(); |
Contributor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ThreadLocalUsage: ThreadLocals should be stored in static fields (details)
(at-me in a reply with help or ignore)
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.
Adds a new concept and API for
Observationwhich has the behavior that was previously retrofitted ontoTimer.Sample. That retrofitting has been reverted and the relevant API introduced asObservation. We reached the limits of expanding whatTimer.Samplemeant. HopefullyObservationwill be reduce any confusion by offering a new API without any historical meaning.TODO: Add more description and linked issues.