Metrics generator: make status_message a default dimension #1794
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
The metric label status_message is generated by default, it is no longer required to add the corresponding dimension to the user defined dimensions
When user defined dimensions correspond to the same label as one of the intrinsic dimensions (e.g. status.code) the label for the user defined dimension is prefixed with double underscore
stoewer
requested review from
knylander-grafana,
joe-elliott,
annanay25,
mdisibio,
mapno,
yvrhdn and
zalegrala
as code owners
zalegrala
reviewed
Oct 12, 2022
joe-elliott
reviewed
Oct 12, 2022
There was a problem hiding this comment.
some minor questions/suggestions. looks good!
can we add some details in the span metrics docs about the list of "special" attributes and how we handle collisions?
something in here:
https://github.com/grafana/tempo/tree/main/docs/tempo/website/metrics-generator
stoewer
force-pushed
the
status-message-metric-label
branch
from
846f02a to
e951758
Compare
Good point, I added a paragraph about default labels and collisions with dimensions to the metrics generator docs ( |
joe-elliott
approved these changes
Oct 13, 2022
There was a problem hiding this comment.
Looks good! Going to leave for a second in case @knylander-grafana has thoughts on the docs changes.
3 tasks
stoewer
added a commit
to stoewer/tempo
that referenced
this pull request
Dec 19, 2022
stoewer
added a commit
to stoewer/tempo
that referenced
this pull request
Dec 19, 2022
mapno
pushed a commit
that referenced
this pull request
Dec 21, 2022
…default (#1960) * Add intrinsic dimenstions to spanmetrics config * Remove deprecated linters from glangci-lint config The linters deadcode, structcheck, and varcheck are deprecated since golangci-lint v1.49.0. They are replace by unused which is already enabled in our config * Make intrinsic dimensions configurable for spanmetrics * Disable intrinsic dimension status_message by default * Add overrides for intrinsic dimension configuration * Docs for the configuration of intrinsic dimensions * Add PR to changelog * Regenerate configuration schema in manifest.md * Handle dimensions as conflicts when intrinsic dimension is absent Custom dimensions from span attributs are also treated as conflicts when the conflicting intrinsic dimension is not actually present. This prevents those dimensions from being renamed when the intrinsic dimension configuration changes * Mark change #1794 as breaking in changelog * Unit test for intrinsic dimension override * Fail on invalid intrinsic dimension override keys * Fix dimension prefix in changelog
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.
What this PR does:
status_messageis now generated by default, it is no longer required to add the corresponding dimension to the user defined dimensions. PR Metrics generator: extract status_message field from spans #1786 addressed the same problem of making the status message available as a metric label. However, addingstatus_messageas a default dimension is more consistent with the closely related default dimensionstatus_code.Which issue(s) this PR fixes:
Fixes #1764
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]