Fix RW2 exemplars to be attached to samples per spec#18014
Open
AftAb-25 wants to merge 1 commit intoprometheus:mainfrom
Open
Fix RW2 exemplars to be attached to samples per spec#18014AftAb-25 wants to merge 1 commit intoprometheus:mainfrom
AftAb-25 wants to merge 1 commit intoprometheus:mainfrom
Conversation
Previously, exemplars were sent as separate TimeSeries entries without samples, which violates the Remote Write 2.0 specification requirement that 'At least one element in samples or in histograms MUST be provided'. This change modifies populateV2TimeSeries to: - Match exemplars with their corresponding samples/histograms by series labels - Attach exemplars to the same TimeSeries as their samples/histograms - Drop exemplars that don't have matching samples/histograms (per spec) The implementation now correctly follows the RW 2.0 spec where exemplars are attached to samples in the same TimeSeries message, similar to how scrape protocols and OTLP handle exemplars. Fixes prometheus#17857 Signed-off-by: aftab <[email protected]>
AftAb-25
requested review from
a team,
alexgreenbank,
bwplotka,
cstyan and
tomwilkie
as code owners
2 tasks
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
1 participant
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.
Fixes #17857
[BUGFIX] Remote Write: Fix RW2 exemplars to be attached to samples in the same TimeSeries per spec.
Which issue(s) does the PR fix:
Match exemplars with their corresponding samples/histograms by series labels
Attach exemplars to the same TimeSeries as their samples/histograms
Drop exemplars that don’t have matching samples/histograms (per spec)
2.Updated the TimeSeries count logic:
Changed from nPendingSamples + nPendingExemplars + nPendingHistograms
To nPendingSamples + nPendingHistograms (since exemplars no longer create separate entries)
3.Updated tests:
Modified the "exemplars only" test case for V2 to expect that exemplars are dropped when there are no matching samples
Does this PR introduce a user-facing change?
Yes, this PR introduces a user-facing change, but it's a bug fix.
What changes
1.Exemplars are now attached to samples in the same message (per spec)
2.Exemplars without samples are dropped (per spec)
Impact
1.Receivers that follow the RW 2.0 spec: no change needed
2.Receivers relying on the old buggy format: may need updates
The implementation now correctly follows the RW 2.0 spec where exemplars are attached to samples in the same TimeSeries message, similar to how scrape protocols and OTLP handle exemplars.