-
Notifications
You must be signed in to change notification settings - Fork 163
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
experimental: metric handles #240
Merged
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: Toby Lawrence <[email protected]>
Signed-off-by: Toby Lawrence <[email protected]>
Signed-off-by: Toby Lawrence <[email protected]>
34ebf20
to
eddb428
Compare
Signed-off-by: Toby Lawrence <[email protected]>
Signed-off-by: Toby Lawrence <[email protected]>
Signed-off-by: Toby Lawrence <[email protected]>
Signed-off-by: Toby Lawrence <[email protected]>
Signed-off-by: Toby Lawrence <[email protected]>
…ions Signed-off-by: Toby Lawrence <[email protected]>
Signed-off-by: Toby Lawrence <[email protected]>
This was referenced Dec 23, 2021
mnpw
pushed a commit
to mnpw/metrics
that referenced
this pull request
Mar 8, 2024
* new design for handle-based metrics Signed-off-by: Toby Lawrence <[email protected]>
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.
Context
Prior to the current design, there used to be a crate called
metrics-runtime
which encapsulated essentially all of the functionality that we expose today viametrics
and associated exporters. Specifically, it provided a mechanism to get a "handle" to a metric: instead of blindly using the macros, you could hold a tangible object that gave you access to increment counters or set gauges, and so on.This provided a meaningful performance boost when you could deal with the limitations of that -- can't have dynamic labels -- but was ultimately a little too coupled for my taste, and I wasn't yet a good enough developer to find a suitably decoupled design to continue providing it.
Present day
This PR explores a design that moves back towards what
metrics-runtime
provided -- metric handles -- while fitting into the current ecosystem in such a way that the handles can be decoupled and backed by an arbitrary implementation.This shows up in a few ways:
Recorder
now only hands back handles, it does not deal with updating a metric at allCounter
,Gauge
, andHistogram
) with concrete sets of methods specific to the metricGaugeFn
trait requires a way to increment, decrement, or set the absolute value of the metricdescribe_*!
whileregister_*
now create metric handles for a given metricsCrucially, the macros for updating a metric have not changed, so this change is backwards-compatible for many use cases where metrics were never being "registered". For use cases where metrics were "registered" ahead of time, they'll now have to change to "describing" those metrics instead.
For static metrics -- no dynamic labels -- this represents a significant performance boost opportunity when the handles can be stored or held. In cases where the macros are still used normally i.e.
counter!(...)
, there will be a slight performance decrease as we're creating a new handle just to perform the operation, as well looking up the metric in the registry. There's a pathway to store the handle in a local static so that we can reduce the overhead to simply an atomic load, when compared to holding the handle and updating it directly.For dynamic metrics, the performance should be on par.
Drawbacks
In some cases, using handles will provide a reduction/regression in behavior. Take the
metrics-tracing-context
layer, for example: it grabs the fields of the currenttracing
span, if there is one, and augments the labels of the metric being emitted using those fields.When using metric handles, those fields would only be grabbed when the handle is created. In cases where the tracing fields represented something dynamic, like the user ID for a given web request, or something else of that nature, the ability to implicitly capture that would be lost if the metric handle was held as a field on a struct that was created before the point where the metric was expected to be updated.
In most cases, stateless layers -- make all metrics start with a specific prefix, or don't emit metrics whose name matches a list of patterns, etc -- will experience no difference in behavior.