Renaming metrics with prometheus receiver produces unknown-type metrics

[dashpole]

Describe the bug
If the __name__ label is changed using prometheus relabel configs, the prometheus receiver fails to populate metric metadata.

The root cause is that the prometheus receiver looks up metric metadata using the final metric name. However, the prometheus server updates the metadata cache before applying relabel rules, meaning the metadata cache stores metadata based on the initial metric name. The metadata lookup fails, and it is left empty.

Steps to reproduce
Run a collector with the config below.

The collector produces the logs:

2020-12-16T21:04:44.840Z	info	internal/metrics_adjuster.go:357	Adjust - skipping unexpected point	{"component_kind": "receiver", "component_type": "prometheus", "component_name": "prometheus", "type": "UNSPECIFIED"}
2020-12-16T21:04:45.035Z	INFO	loggingexporter/logging_exporter.go:361	MetricsExporter	{"#metrics": 1}
2020-12-16T21:04:45.035Z	DEBUG	loggingexporter/logging_exporter.go:388	ResourceMetrics #0
Resource labels:
     -> service.name: STRING(prometheusreceiver)
     -> host.name: STRING(localhost)
     -> port: STRING(8888)
     -> scheme: STRING(http)
InstrumentationLibraryMetrics #0
InstrumentationLibrary
Metric #0
Descriptor:
     -> Name:
     -> Description:
     -> Unit:
     -> DataType: None

Note the empty Name, Description, Unit, and DataType.

What did you expect to see?
I expected metrics to be renamed, and otherwise emitted normally.

What did you see instead?
Metrics are dropped during conversion from OpenCensus format to OpenTelemetry format because the metric descriptor type is Unknown.

What version did you use?
Version: otel/opentelemetry-collector-contrib-dev@sha256:0dbc61590cc04678997173fb378c875e2733ff2e443d75a7957a340d4b2bb9ef (latest)

What config did you use?
Config: (e.g. the yaml config file)

receivers:
  prometheus:
    config:
      global:
        scrape_interval: 10s
      scrape_configs:
      - job_name: 'prometheusreceiver'
        static_configs:
        - targets: [localhost:8888]
        metric_relabel_configs:
        # filter out all metrics except otelcol_process_cpu_seconds so it is easier to read logs
        - source_labels: [ __name__ ]
          regex: "otelcol_process_cpu_seconds"
          action: keep
        # rename otelcol_process_cpu_seconds to process_cpu_seconds by replacing the __name__ label
        - source_labels: [ __name__ ]
          regex: "otelcol_(.*)"
          action: replace
          target_label: __name__
exporters:
  logging:
    logLevel: debug
service:
  pipelines:
    metrics:
      receivers: [prometheus]
      exporters: [logging]

Additional context
I was able to determine the root cause above by adding additional debug statements and rebuilding the collector.

[dashpole]

Given the metadata cache (and prometheus' target metadata endpoint) is intended to be used to attach metadata to samples, I opened an issue with upstream prometheus to see if they are interested in changing the name that is included in metadata. In the mean time, we should probably validate the prometheus configuration to ensure users don't set __name__ as a target label, since that won't work.

[dashpole]

Re-linking open-telemetry/opentelemetry-collector#3410, which is related, and the issue transfer seems to have dropped the link

[dashpole]

Discussed this at the 9/22 meeting:

The prometheus server will also produce untyped metrics if you rename the metric, so we aren't necessarily out of compliance for producing untyped metrics when a metric was renamed.

Given that, to "fix" this, we just need to:

1. Make sure unknown typed metrics produced by the receiver can be correctly sent through the collector to the remote write exporter
   1. In the description, I was seeing "Metrics are dropped during conversion from OpenCensus format to OpenTelemetry format because the metric descriptor type is Unknown". We need to make sure this is fixed.
2. Remove the validation added in Disallow renaming metrics using the prometheus receiver opentelemetry-collector#3410

[dashpole]

I believe step 1 would be fixed by #4743.

[Mario-Hofstaetter]

@dashpole Is there any progress on this?
I just hit the metric renaming using metric_relabel_configs is disallowed error introduced by #3410 while trying to use the feature.

[dashpole]

I don't think we have proper support for untyped metrics right now. There is some discussion here about how best to accomplish that: open-telemetry/prometheus-interoperability-spec#69

[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.

[Mario-Hofstaetter]

Push, Edit: push again?

[KovtunV]

Hi, is any progress?

[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.

[Mario-Hofstaetter]

Push

[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.

[Mario-Hofstaetter]

@dashpole should we continue to push this or let it go stale? Seems to be low prio.

Too bad there is no __type__ metadata label that could be utilized to just set the type again after renaming. Is there any other workaround?

[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.

[Mario-Hofstaetter]

Bump

[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.

[dashpole]

I don't think we can fix the underlying issue. But we can at least not fail when users try and do this. We convert unknown-typed metrics to gauges (without unit or description) today, which is better than failing entirely.

[github-actions]

Pinging code owners for receiver/prometheus: @Aneurysm9 @dashpole. See Adding Labels via Comments if you do not have permissions to add labels yourself.