Inconsistent Prometheus exporter timestamp handling between Java and C++

[nluk]

Is your feature request related to a problem? Please describe.

The C++ Prometheus exporter has an issue resolved where they disable timestamps, referencing the specification.

The same behaviour is present in Java Prometheus exporter 1.31.0 , possibly enabled on request (not sure if that's the case). Not all metrics have timestamps set.

# TYPE logback_events_total counter
# HELP logback_events_total Number of logback events of given level
logback_events_total{otel_scope_name="io.opentelemetry.micrometer-1.5",level="debug"} 5.0 1699559811248
# TYPE jetty_connections_bytes_out histogram
# HELP jetty_connections_bytes_out Bytes sent by tracked connections
jetty_connections_bytes_out_count{otel_scope_name="io.opentelemetry.micrometer-1.5",connector_name="unnamed"} 116.0 1699559811248
# TYPE otel_scope_info info
# HELP otel_scope_info Scope metadata
otel_scope_info{otel_scope_name="io.opentelemetry.java-http-client",otel_scope_version="1.31.0-alpha"} 1

Describe the solution you'd like

1. Define which option is compliant to the spec.
2. If this feature is non-compliant and related to the linked feature request, hide it behind a feature flag (someone had requested it - the probably need it).
3. If not related to the feature request we could dispose of it.

If the team at C++ repo is wrong, let them know 😸

Describe alternatives you've considered

No response

Additional context

Java Agent 1.31.0

[jack-berg]

Moving to opentelemetry-java, since opentelemetry-java-instrumentation doesn't contain the prometheus exporter.

[jack-berg]

@dashpole / @jsuereth Could use some guidance on this. #3700 added timestamps to the prometheus exporter after discussion in #3696, but that appears to be in conflict with OpenMetrics, and maybe the prometheus compatibility spec.

Is this behavior something that should be deleted, or opt-in, or adjusted in the spec?

[dashpole]

Prometheus recommends against using explicit timestamps. We should clarify the correct behavior in the spec (I thought it was, but can't find it). Using explicit timestamps is reserved for "collector-like" scenarios, where data collected elsewhere is being re-exposed. For an application with its own instrumentation, explicit timestamps are almost never the right thing to do.

One example of where it would possibly be appropriate to use explicit timestamps would be if you were periodically (e.g. every 10 minutes) querying something, caching the result, and then had an async callback that returned the cached value. But unless there is a significant difference between when something was actually observed, and when the scrape happens, you wouldn't want to add explicit timestamps.

I'm OK with allowing explicit timestamps to be turned on, but it needs to be off by default. exporters shouldn't be required to support that configuration.

[jack-berg]

I'm OK with allowing explicit timestamps to be turned on, but it needs to be off by default. exporters shouldn't be required to support that configuration.

👍 Seems reasonable to me. We can make this an opt in feature of prometheus exporter.

[fstab]

You're right, the timestamp represents the scrape time, and this should be set by the Prometheus server, not by the client library.

One example of where it would possibly be appropriate to use explicit timestamps would be if you were periodically (e.g. every 10 minutes) querying something, caching the result, and then had an async callback that returned the cached value.

This example makes sense. Note however that in that case the timestamp should be the time when the callback was called, not the scrape timestamp. Afaik OpenTelemetry does not provide API to set the timestamp explicitly, therefore this cannot be implemented anyway.

opt in feature

I think an opt in feature only makes sense if there is API to explicitly set the timestamp. An opt in feature where the scrape time is set automatically during scrape wouldn't be useful.

Anyway, Just a heads-up: I'm currently finalizing #5940, and if that gets merged it will remove the timestamp.

[fstab]

PR #6015 would fix this.

[dashpole]

Afaik OpenTelemetry does not provide API to set the timestamp explicitly, therefore this cannot be implemented anyway.

Thats a good point. I support prohibiting SDK prometheus exporters from adding timestamps in that case.