Prometheus exporter does not convert time units to seconds

[jonatan-ivanov]

Component(s)

exporter/prometheus

What happened?

Description

Prometheus uses seconds as time unit by default. If I send an OTLP histogram with a different time unit, the value will not be converted to seconds (as it should be) but will be used as-is.

Steps to Reproduce

Send a histogram with unit: "milliseconds" to the OTel collector where the receiver is otlp/http/protobuf (but I think any otlp receiver should produce the same result) and the exporter is prometheus. Then check the Prometheus /metrics endpoint.
E.g.:

metrics {
  name: "test.timer"
  unit: "milliseconds"
  histogram {
    data_points {
      start_time_unix_nano: 1677210838494000000
      time_unix_nano: 1677210839021000000
      count: 1
      sum: 123.0
    }
    aggregation_temporality: AGGREGATION_TEMPORALITY_CUMULATIVE
  }
}

Expected Result

test_timer_sum{...} 0.123

Actual Result

test_timer_sum{...} 123

Collector version

otel/opentelemetry-collector-contrib:cdf47846a7ff

Environment information

Environment

OS: MacOS 13.2.1

OpenTelemetry Collector configuration

receivers:
  otlp:
    protocols:
      grpc:
      http:

exporters:
  prometheus:
    endpoint: '0.0.0.0:9090'
    metric_expiration: 1m
    enable_open_metrics: true
    resource_to_telemetry_conversion:
      enabled: true

service:
  pipelines:
    metrics:
      receivers: [otlp]
      exporters: [prometheus]

Log output

No response

Additional context

No response

[github-actions]

Pinging code owners:

• exporter/prometheus: @Aneurysm9

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

[Aneurysm9]

The specification requires that the unit be handled as follows:

The Unit of an OTLP metric point SHOULD be converted to the equivalent unit in Prometheus when possible. This includes:

• Converting from abbreviations to full words (e.g. "ms" to "milliseconds").
• Dropping the portions of the Unit within brackets (e.g. {packets}). Brackets MUST NOT be included in the resulting unit. A "count of foo" is considered unitless in Prometheus.
• Special case: Converting "1" to "ratio".
• Converting "foo/bar" to "foo_per_bar".

The resulting unit SHOULD be added to the metric as OpenMetrics UNIT metadata and as a suffix to the metric name unless the metric name already contains the unit, or the unit MUST be omitted. The unit suffix comes before any type-specific suffixes.

That does not include changing the unit to a different unit or modifying the value in any way.

[jonatan-ivanov]

Since the Prometheus exporter is the concern of the collector, I think the client should never know that the data that it published in OTLP format will be converted to Prometheus format. Because of this, I think any unit that is supported by OTLP should work and the client should not care. Maybe the Prometheus exporter is not configured right now but it will be starting from tomorrow. I think making a change on the exporters should not involve changing all the clients.

Can this behavior lead to impossible scenarios?

• Can it happen that there are two receivers that mandate different units?
• Can it happen that there are two exporters that mandate different units?

[shakuzen]

The resulting unit SHOULD be added to the metric as OpenMetrics UNIT metadata and as a suffix to the metric name unless the metric name already contains the unit

The bold part is not happening. The included actual result above shows the metric name from the Prometheus exporter is test_timer_sum - no unit in the name. A consumer of the exporter in Prometheus format has no way to know what the unit is. I can't speak to whether the former part is happening or not because I was never able to get the Prometheus exporter to return OpenMetrics format, even when setting enable_open_metrics: true. Regardless, the UNIT metadata is not part of Prometheus format, so it wouldn't help consumers of the Prometheus exporter that are scraping Prometheus format rather than OpenMetrics format.

[github-actions]

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

• exporter/prometheus: @Aneurysm9

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

[jonatan-ivanov]

@Aneurysm9 Could you please check the last two comments and mark this issue so that it won't be auto-closed?