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

[exp/prometheusremotewrite] Exporting "target" instead of "target_info" #12079

Closed
gouthamve opened this issue Jul 5, 2022 · 8 comments
Closed

Comments

@gouthamve
Copy link
Member

Describe the bug

When pushing data through Prometheus Remote Write, we should expose the Resource Attributes using the target_info metric, but we are currently pushing a target metric.

Steps to reproduce
Run a simple collector deployment with the following config:

exporters:
  prometheusremotewrite:
    endpoint: "http://prometheus:9090/api/v1/write"

processors:
  batch: {}
receivers:
  prometheus:
    config:
      global:
        scrape_interval: 5s
        external_labels:
          scraped_by: otel-collector 

      scrape_configs:
        - job_name: collector
          static_configs:
            - targets: ['localhost:8888']

service:
  pipelines:
    metrics:
      receivers: [prometheus]
      processors: [batch]
      exporters: [prometheusremotewrite]

What did you expect to see?
target_info in Prometheus.

What did you see instead?
target

What version did you use?
Version: v0.54.0

@povilasv
Copy link
Contributor

povilasv commented Jul 5, 2022

Based on OTEL spec https://opentelemetry.io/docs/reference/specification/metrics/datamodel/#resource-attributes-1

I believe the metric should named target

Other resource attributes SHOULD be converted to a “target” info metric family, or MUST be dropped. The “target” info metric family is an info-typed metric family whose labels MUST include the resource attributes, and MUST NOT include any other labels other than job and instance

Note: the quotes around the word target

@gouthamve
Copy link
Member Author

Yes, but Info type metrics should have an _info appended to them according to OpenMetrics spec: https://github.com/OpenObservability/OpenMetrics/blob/main/specification/OpenMetrics.md#info-1

@povilasv
Copy link
Contributor

povilasv commented Jul 5, 2022

Agree, I believe we need to fix the otel spec first before merging your change

@gouthamve
Copy link
Member Author

I think the OTEL Spec is correct already imo. The metric family for target_info is target.

Just that when exporting a metric called target of the info type in OpenMetrics, we should append _info to the end.

@codeboten
Copy link
Contributor

@Aneurysm9 please take a look at this issue

@dashpole
Copy link
Contributor

@gouthamve is correct. For other metric types (e.g. histograms), suffixes are appended when translating from pdata to PRW. For example, here we add _sum to the histogram sum sample. But since this series is manufactured from the resource, rather than from a pdata data point, it needs to handle type suffixing itself. So the OpenMetrics metric family name is target (as described in the spec), but the type suffix is _info, and is currently missing.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 9, 2022

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:

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

@github-actions github-actions bot added the Stale label Nov 9, 2022
@dashpole
Copy link
Contributor

dashpole commented Nov 9, 2022

This was fixed in #12086

@dashpole dashpole closed this as completed Nov 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants