Prometheus Receiver: failed to register service discovery metrics

[cpheps]

Component(s)

receiver/prometheus

What happened?

Description

When using the collector as an API (the process doesn't correlate to the lifetime of the collector) the Prometheus receiver returns an error when its Start function is called twice within the same process.

In our collector distribution we wrap the collector and run it as an API. We are able to change out the config and restart the collector via opamp without the process going down. When you reload a config twice that thas a prometheus receiver you hit the following error:

cannot start pipelines: failed to register service discovery metrics: failed to create service discovery metrics

The CreateAndRegisterSDMetrics method called here causes the issue as SDMetrics can only be registered once per process according to the library here.

I guess a starting question does the receiver use the Service Discovery metrics or can those be omitted?

Collector version

v0.97.0

Environment information

Environment

OS: macOS 14.4.1 (23E224)
Compiler(if manually compiled): go version go1.22.0 darwin/arm64

OpenTelemetry Collector configuration

No response

Log output

No response

Additional context

No response

[github-actions]

Pinging code owners:

• receiver/prometheus: @Aneurysm9 @dashpole

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

[dashpole]

We plan to add the service discovery metrics in the future.

We should probably unregister metrics during shutdown to fix this. DiscovererMetrics has an Unregister function:

https://github.com/prometheus/prometheus/blob/9b7de4778732ca2f2ab5028e9d1955109f440c4c/discovery/manager.go#L70

https://github.com/prometheus/prometheus/blob/9b7de4778732ca2f2ab5028e9d1955109f440c4c/discovery/discovery.go#L45

You can actually register the same metric twice in a process, as long as it has a constant label that is unique. We use the receiver name to do this today:

opentelemetry-collector-contrib/receiver/prometheusreceiver/metrics_receiver.go

Line 67 in 4e9baa7

prometheus.Labels{"receiver": set.ID.String()},

. But if you create two receivers with the same name during the lifetime of the process, that will currently break

[cpheps]

But if you create two receivers with the same name during the lifetime of the process, that will currently break

I think this is what we're seeing. I can take a look at unregistering service discovery metrics on shutdown.

[cpheps]

So I'm not sure if unregistering metrics is fully possible. I did a spike where I unregistered the Service Discovery metrics then hit a snag in discovery.NewManager. It eventually ends up here where it registers new metrics. Because those metrics are never unregistered you get an error because the metrics are already registered. The error is logged and a nil manager is returned resulting in a panic.

I don't see a way to access those metrics to unregister them. It does seem like this is designed to be run once per process.

@dashpole So I don't think there's a way to reuse a register based on this. I wanted to propos adding a uuid or some random string onto the register name to guarantee it's unique every time. The downside of this is we are leaking registers over the lifetime of the process. Though this may not be any worse than using a config with receivers with entirely new names as the old ones would be lost too.

[dashpole]

My attempt at a fix: prometheus/prometheus#13896