Ensure metric name is present on histogram_quantile annotations#15828
Merged
beorn7 merged 3 commits intoprometheus:mainfrom Jan 22, 2025
Merged
Ensure metric name is present on histogram_quantile annotations#15828beorn7 merged 3 commits intoprometheus:mainfrom
beorn7 merged 3 commits intoprometheus:mainfrom
Conversation
Previously the series __name__ label was dropped before the annotation was emitted causing the annotation message to have a blank value. This fix also allows delayed name removal for classic histograms. Fixes: prometheus#15411 Signed-off-by: Joshua Hesketh <josh@nitrotech.org>
This annotation could be split into two to be clearer for the user (one for missing, and one for malformed). This is outside of the scope of this change. Signed-off-by: Joshua Hesketh <josh@nitrotech.org>
b1ee4a8 to
28e37c7
Compare
beorn7
approved these changes
Jan 22, 2025
Member
beorn7
left a comment
There was a problem hiding this comment.
Very nice. Just my usual nit about a missing period (I'll submit the suggestion before merging, so no need for you to fix it).
I really like that this also fixes the delayed name removal for classic histograms. (I wasn't aware that it didn't work.)
The sad news is that the metric name will still be missing in the annotations in the very common case that you first rate the histogram before applying histogram_quantile, while not having --enable-feature=promql-delayed-name-removal activated. Currently, I see no easy way of fixing that. But maybe it's fine "as is". I hope anyway that we can make delayed name removal the default for v4.
To be squashed. Signed-off-by: Björn Rabenstein <github@rabenste.in>
Member
|
#15855 to make delayed name removal the default in v4. |
charleskorn
added a commit
to grafana/mimir
that referenced
this pull request
May 5, 2025
This issue was fixed in prometheus/prometheus#15828.
charleskorn
added a commit
to grafana/mimir
that referenced
this pull request
May 16, 2025
This issue was fixed in prometheus/prometheus#15828.
charleskorn
added a commit
to grafana/mimir
that referenced
this pull request
May 16, 2025
* Upgrade mimir-prometheus * Fix breaking change in `NewAPI` * Sync upstream PromQL engine test cases * Modify `histogram_stddev` and `histogram_stdvar` to match new behaviour introduced in prometheus/prometheus#16444 * Simplify code * Remove outdated comment This issue was fixed in prometheus/prometheus#15828. * Remove unnecessary variable * Rename operator and files * Extract `histogram_quantile`-specific logic * Add support for classic histograms in `histogram_fraction` See prometheus/prometheus#16095 * Bring in changes from grafana/mimir-prometheus#871 * Invert build tag logic to match prometheus/prometheus#16436 * Sync query engine tests with upstream * Fix breaking changes * Regenerate OTLP code
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Previously the series name label was dropped before the annotation was emitted causing the annotation message to have a blank value.
This fix also allows delayed name removal for classic histograms.
Fixes: #15411