Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[exporters/prometheus] / [exporters/prometheusremotewrite] do not add _total suffix to Sum metric types #11837

Closed
povilasv opened this issue Jun 30, 2022 · 6 comments
Labels
bug Something isn't working comp:prometheus Prometheus related issues data:metrics Metric related issues priority:p2 Medium

Comments

@povilasv
Copy link
Contributor

povilasv commented Jun 30, 2022

Describe the bug

Currently exporters/prometheus and exporters/prometheusremotewrite do not add _total suffix metric for OpenTelemetry Sum type Metrics.

The spec states:
image

Ref: https://opentelemetry.io/docs/reference/specification/metrics/datamodel/#sums-1

Steps to reproduce

Openetelemtry collector exports the following metric in its :8888/metrics telemetry endpoint:

otelcol_exporter_enqueue_failed_metric_points{exporter="prometheus",...}

When it converts from Prometheus to OTel and then back to Prometheus it should become:

otelcol_exporter_enqueue_failed_metric_points_total{exporter="prometheus",...}

Because the counter is converted to Otel SUM Type:

A Prometheus Counter MUST be converted to an OTLP Sum with is_monotonic equal to true. If the counter has a _total suffix, it MUST be removed.

What did you expect to see?

Sum type metrics having _total suffix.

What did you see instead?

No _total suffix.

What version did you use?
Version: 877066b

What config did you use?

config:
  exporters:
    prometheus:
      endpoint: "0.0.0.0:9090"
    prometheusremotewrite:
      endpoint: "http://prometheus:9090/api/v1/write"
  extensions:
    health_check: {}
    memory_ballast: {}
  processors:
    batch: {}
    memory_limiter: null
  
  receivers:
    otlp:
      protocols:
        grpc:
          endpoint: 0.0.0.0:4417
          include_metadata: true
        http:
          endpoint: 0.0.0.0:4318
    prometheus:
      config:
        scrape_configs:
        - job_name: 'otelcol'
          scrape_interval: 10s
          static_configs:
          - targets: ['0.0.0.0:8888']

Environment

Additional context
Add any other context about the problem here.

@povilasv povilasv added the bug Something isn't working label Jun 30, 2022
@mx-psi mx-psi added comp:prometheus Prometheus related issues data:metrics Metric related issues labels Jun 30, 2022
@povilasv
Copy link
Contributor Author

The other question is if this is intentional in the spec or not?

If the Prometheus metric never had the _total suffix and we do Prometheus -> Otelp -> Prometheus, it will add it.

@povilasv
Copy link
Contributor Author

povilasv commented Jun 30, 2022

Folks let me know if you agree and have any feedback, I can work on a fix -> #11838 👯‍♂️

@TylerHelmuth
Copy link
Member

pinging @Aneurysm9 as code owner

@TylerHelmuth TylerHelmuth added the priority:p2 Medium label Jul 1, 2022
@paivagustavo
Copy link
Member

@dashpole This is a duplicate of #8950.

@povilasv this was added by #10028 under the --feature-gates=pkg.translator.prometheus.NormalizeName featuregate.

@dashpole
Copy link
Contributor

dashpole commented Nov 4, 2022

I need to double-check, but we should probably flip that feature-gate to enabled by default.

@povilasv
Copy link
Contributor Author

povilasv commented Nov 4, 2022

closing as it's duplicate issue of #8950

@povilasv povilasv closed this as completed Nov 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working comp:prometheus Prometheus related issues data:metrics Metric related issues priority:p2 Medium
Projects
None yet
Development

No branches or pull requests

5 participants