[Prometheus Remote Write Exporter] Handling Staleness flag from OTLP. #6679
Conversation
…uge and sum metric data types
…ontrib into prwExporterStaleNaN_histogram_summary
…ontrib into prwExporterStaleNaN_histogram_summary
…ontrib into prwExporterStaleNaN_histogram_summary
…om:open-o11y/opentelemetry-collector-contrib into prwExporterStaleNaN_histogram_summary
| // check for staleness marker flag | ||
| if pt.Flags().HasFlag(pdata.MetricDataPointFlagNoRecordedValue) { | ||
| sample.Value = math.Float64frombits(value.StaleNaN) | ||
| } else { | ||
| switch pt.Type() { | ||
| case pdata.MetricValueTypeInt: | ||
| sample.Value = float64(pt.IntVal()) | ||
| case pdata.MetricValueTypeDouble: | ||
| sample.Value = pt.DoubleVal() | ||
| } |
There was a problem hiding this comment.
Mayor be better to extract this as a separate func.
| } else { | ||
| switch pt.Type() { | ||
| case pdata.MetricValueTypeInt: | ||
| sample.Value = float64(pt.IntVal()) |
There was a problem hiding this comment.
Not in this PR, but do we ever do this? I am sure that only Prometheus receiver does this stale marker, and they don't use the int.
There was a problem hiding this comment.
The switch statement is for the case when the datapoint has no OTLP flag, which means the data is not a staleness marker. I changed it to enhance the readability.
I also checked the prometheusreceiver part and there is no usage that the metric value type is integer at this moment, but should we leave this switch-case in case of we have the integer values from OTLP in the future? I found some test cases like uint64_counter
…ontrib into prwExporterStaleNaN_histogram_summary
| if pt.Flags().HasFlag(pdata.MetricDataPointFlagNoRecordedValue) { | ||
| bucket.Value = math.Float64frombits(value.StaleNaN) | ||
| } |
There was a problem hiding this comment.
Is this the protocol in Prometheus that you do this for all the buckets?
There was a problem hiding this comment.
I'm not sure whether it is documented, but the observed behavior is that the stale marker is produced for individual buckets that disappear. Thus I would expect that when the entire histogram disappears each bucket should be marked stale.
There was a problem hiding this comment.
@Aneurysm9 FYI you need a followup on our proto/specification to ensure that we send the buckets definition when we set the flag as well, because this was not clear to me at least.
Aneurysm9
added
the
ready to merge
Description:
This PR is to ensure the Prometheus Remote Write Exporter responds to OTLP flags from the Prometheus Receiver for all metric data types, including
gauge,sum,histogram, andsummary.This PR checks
MetricDataPointFlagNoRecordedValueflag and set theStaleNaNvalue to the sample of the data point.Link to tracking Issue:
#6620
Testing:
Test cases have been implemented to verify the sample value is set to
staleNaNif it has the OTLP flag.