OTLP receiver: Generate target_info samples between the earliest and latest samples per resource
#16737
Conversation
target_info samples between the earliest and latest samples per resource
a9795be to
3d4a41a
Compare
1bc16ac to
13d83d2
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
13d83d2 to
26e5dcb
Compare
There was a problem hiding this comment.
Pull Request Overview
Adds support for generating multiple target_info samples between the earliest and latest OTLP metric samples for each resource, spaced at half the lookback delta.
- Introduce
LookbackDeltaconfiguration throughout handlers and API. - Track earliest and latest timestamps per resource in the OTLP-to-remote-write converter.
- Generate a
target_infotime series sample at everylookbackDelta/2interval plus the final timestamp. - Update tests to cover the multi-sample
target_infobehavior and bump changelog.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| web/web.go | Pass LookbackDelta into NewAPI |
| web/api/v1/errors_test.go | Add lookbackDelta parameter to test helper invocation |
| web/api/v1/api.go | Wire lookbackDelta through to OTLP write handler |
| storage/remote/write_handler.go | Extend OTLPOptions and exporter to carry lookbackDelta |
| storage/remote/otlptranslator/prometheusremotewrite/metrics_to_prw.go | Add earliest/latest timestamp tracking in converter |
| storage/remote/otlptranslator/prometheusremotewrite/helper.go | Implement multi-sample logic in addResourceTargetInfo |
| storage/remote/otlptranslator/prometheusremotewrite/metrics_to_prw_test.go | New test for spaced target_info samples |
| CHANGELOG.md | Document the bugfix |
Comments suppressed due to low confidence (3)
storage/remote/otlptranslator/prometheusremotewrite/metrics_to_prw.go:55
- [nitpick] Comment should start with the field name
LookbackDeltato follow Go doc conventions, e.g.// LookbackDelta is the PromQL engine lookback delta used for target_info spacing.
// Lookback delta is the PromQL engine lookback delta.
storage/remote/otlptranslator/prometheusremotewrite/helper.go:658
- [nitpick] The variable name
tsis ambiguous (could be confused with timestamp). Consider renaming toseriesortargetSeriesfor clarity.
ts, _ := converter.getOrCreateTimeSeries(labels)
storage/remote/otlptranslator/prometheusremotewrite/helper.go:438
- The calls to
minandmaxrely on helper functions that aren’t shown here. Ensure thatmin(a,b)andmax(a,b)are defined forpcommon.Timestamp, otherwise these will fail to compile.
minTimestamp = min(minTimestamp, ts)
e19cbd6 to
4567c6d
Compare
…latest samples per resource Modify the OTLP receiver to generate target_info samples between the earliest and latest samples per resource instead of only one for the latest timestamp. The samples are spaced lookback delta/2 apart. Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
4567c6d to
d5761d7
Compare
@beorn7 My algorithm doesn't exceed the latest timestamp, it ends exactly at the latest timestamp. Is there any benefit to modify it, so it instead potentially exceeds the latest timestamp? I don't see it at least. |
|
What I mean is to not add a sample that exceed the latest timestamp, but also don't add any sample that doesn't match the lookbackdelta/2 repeating pattern. Just add one at the beginning of the range, and then every lookbackdelta/2 duration, until you have added the last sample before the end of the range. |
@beorn7 Isn't the problem with this approach that you can miss the latest |
|
Oh yes, you are right. Now I got it. (Maybe it should be explained in a way so that even people like me will understand it. :) So this looks plausible to me, then. I'll leave the codelevel review to the OTLP receiver experts. |
@beorn7 Hahahaha! :) I can update the PR description to be explicit about this, thanks for the feedback! |
Modify the OTLP receiver to generate
target_infosamples starting at the earliest and ending at the latest samples per resource, instead of only one for the latest timestamp. Between those samples, one sample is generated every lookback delta/2 timestamps. The idea behind generatingtarget_infosamples spaced lookback delta/2 apart is to ensure thattarget_infois found for every connected time series sample generated from the same request.The motivation behind this fix is that I realized when debugging strange
infoPromQL function behaviour that the reason was thattarget_infostarted one minute later than the time series it was being joined with (byinfo). See Grafana Mimir issue for reference.