fix: avoid updating exemplar values during subsequent metric changes #620
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
|
Sweet, thanks! Gauge and Histograms are not affected? |
Gauge's don't seem to have exemplars and I made the change for histograms in lib/histogram.js. |
|
👍 (I meant summary, but I see there's no exemplar support there either). Could you add a changelog entry? (also, CI is failing) |
psimk
force-pushed
the
master
branch
2 times, most recently
from
4e514e7 to
823b163
Compare
Added. |
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.
Problem
The current exemplars implementation expects that every time we update a metric, we will have fresh exemplar labels to set together with the metric. However, setups that have a low sampling rate for their exemplar labels (e.g. trace ids), this means that this implementation would be reseting exemplar labels.
This also creates a lot of noise in graphing solutions like Grafana as there are lots of exemplars displayed without any contextual data attached to them.
Solution
Before setting an exemplar, or even creating a brand new one, we first will check whether the the exemplar labels were configured with anything other that isn't the default empty object. If there are no exemplar labels, then we simply short circuit the function.
Note, this likely means that we could get rid off the
defaultExemplarLabelSetfield and just handle theundefinedvalue, but I wanted to have the least amount of changes in the PR.fixes #616
@SimenB