Prometheus Stale NaN Values are Reported with 0 Value

[michaelli321]

Component(s)

receiver/prometheus

What happened?

Description

If a Stale NaN value is returned from the Prometheus job, it is converted to a NumberDataPoint with value 0 in the prometheusreciever

Steps to Reproduce

1. Create a prometheus server that doesn't always report a certain metric
2. Have the prometheusreceiver scrape that server
3. Observe that on scrapes where the metric disappears, a 0 value is exported for that metric

Expected Result

The metric should either not be present or that metric should have a value of NaN

Actual Result

A misleading 0 datapoint is exported for the metric

Collector version

v0.81.0

Environment information

Environment

Compiler(if manually compiled): go 1.20.6

OpenTelemetry Collector configuration

No response

Log output

No response

Additional context

When the prometheusreceiver is converting the prometheus values to OTLP, it never sets a value on the NumberDataPoint (ref).

• A new NumberDataPoint is created here (ref)
• When we encounter a stale NaN this block is hit which sets a flag on the NumberDataPoint
• The value field is never set so I believe we just serialize a 0 value since value is not marked as optional in the protobuf definition
  • protobuf default values
  • protobuf oneof semantics
• When the Metrics are then passed to the MetricsAdjuster, nothing happens for gauges, whereas other metric types will act upon the NoRecordedValue flag e.g. see here
• Another example of properly handling this case: The prometheusremotewrite exporter will detect the NoRecordedValue flag and set the value itself

[github-actions]

Pinging code owners:

• receiver/prometheus: @Aneurysm9 @dashpole

See Adding Labels via Comments if you do not have permissions to add labels yourself.

[dashpole]

The receiver uses the DATA_POINT_FLAGS_NO_RECORDED_VALUE data point flag to indicate that the point has no value. https://github.com/open-telemetry/opentelemetry-proto/blob/ac3242b03157295e4ee9e616af53b81517b06559/opentelemetry/proto/metrics/v1/metrics.proto#L332-L335

It seems like correct behavior to not record a value when setting the NO_RECORDED_VALUE flag, right?

The metric should either not be present or that metric should have a value of NaN

We need the metric to be present to allow prometheus exporters to work correctly. Setting the value to NaN is a possibility, and we used to do that IIRC, but it was much more confusing for exporters that didn't check the data point flag. I remember at least one case where an exporter was converting the value to an Int, and got a large negative number, which messed up graphs and rates.

I'll see if @Aneurysm9 has any thoughts, but I'm leaning towards keeping the current behavior.

[sirianni]

Thanks @dashpole!

to allow prometheus exporters to work correctly.

Right, this makes sense. I think the issue is that when using the prometheusreceiver with the otlpexporter a datapoint with a value of 0 is emitted. This is causing incorrect behavior in our backend when doing aggregations like average, etc.

[mx-psi]

Thanks @dashpole!

to allow prometheus exporters to work correctly.

Right, this makes sense. I think the issue is that when using the prometheusreceiver with the otlpexporter a datapoint with a value of 0 is emitted. This is causing incorrect behavior in our backend when doing aggregations like average, etc.

The OTLP exporter should also be exporting the NO_RECORDED_VALUE. Maybe your backend does not support NO_RECORDED_VALUE?

[sirianni]

Maybe your backend does not support NO_RECORDED_VALUE?

Ah, that's likely what is happening. Still, would it be better for the OTLP exporter to be emitting NaN instead of 0?

[michaelli321]

@mx-psi @dashpole Do you have any updates here? It may take sometime for the backends to implement the functionality to support NO_RECORDED_VALUE. Would the best option here be to write a custom processor for these metrics? OTTL doesn't work here since it doesn't support the bitwise & operator

[dashpole]

You could certainly write a custom processor. I don't know if it would be accepted in the contrib repo, but you could build it into your own collector to work around the issue until the backend handles the flag.

[crobert-1]

It seems like the conclusion of this issue is that everything collector-related is functioning as intended, but some backends do not yet support the NO_RECORDED_VALUE flag. Am I following this properly? If so, I believe we can close this issue.

[dashpole]

Agreed