feat: expose Desc error through public Err() method

[duricanikolic]

Adds a public Err() method to Desc type to allow users to check if an error occurred during descriptor construction. Previously, the error field was private and inaccessible.

Our use case is as follows:

• We create a Desc.
• We define and register a custom collector.
• In the collector’s Collect() method, we dynamically create a Metric using NewConstMetric() to report the current value.

However, if the Desc was constructed with an error (for example, due to an invalid label name), that error remains hidden until the collector’s Collect() method is invoked for the first time.

It would be much more practical if the Prometheus API exposed a method such as Desc.Err(), which would return any error that occurred during the creation of the Desc. That's the motivation for the present PR.

Signed-off-by: Yuri Nikolic [email protected]

[bboreham]

lgtm

[bwplotka]

Nice, thanks!

Hm, I don't see why not, other than opening a potentially bad pattern where users check for error always whenever desc is created, where collector checks it too 🤔 It's not necessarily bad, but perhaps users would think they HAVE to check it now on every NewDesc. At minimum a code comment would be nice that this is not mandatory to check this.

It would be much more practical if the Prometheus API exposed a method such as Desc.Err(), which would return any error that occurred during the creation of the Desc. That's the motivation for the present PR.

Can you elaborate on what "practical" means? Do you have some exact use case you got blocked on, without this PR?

[duricanikolic]

It would be much more practical if the Prometheus API exposed a method such as Desc.Err(), which would return any error that occurred during the creation of the Desc. That's the motivation for the present PR.

Can you elaborate on what "practical" means? Do you have some exact use case you got blocked on, without this PR?

Hi @bwplotka,
Thanks for the feedback! Here’s the issue we’re running into:

• We generate metric descriptors from user-defined labels at runtime.
• prometheus.NewDesc() does validate the labels internally, but it doesn’t return an error even if they’re invalid (for example, labels starting with __).
• The error only surfaces later, during the first call to Collect(), which happens when Prometheus scrapes the metrics:
  • If we use MustNewConstMetric(), it panics.
  • If we use NewConstMetric(), it returns an error.
  • In both cases, the failure occurs too late — users only discover their invalid configuration at scrape time.

This is why we need a way to validate the configuration earlier.

[duricanikolic]

Hm, I don't see why not, other than opening a potentially bad pattern where users check for error always whenever desc is created, where collector checks it too 🤔 It's not necessarily bad, but perhaps users would think they HAVE to check it now on every NewDesc. At minimum a code comment would be nice that this is not mandatory to check this.

A comment has been added in ec2ca21