registry: drop duplicate checks

[lukedirtwalker]

In processMetrics drop duplicate checks. If the metricFamily exists we already have a type check with the call to checkMetricConsistency. The help string is already checked in the checkDescConsistency albeit only if pedantic mode is enabled. However this is probably what is desired anyway.

[lukedirtwalker]

I took the freedom to remove the description consistency check in non-pedantic mode. This was causing issues in opentelemetry-collector prometheus exporter (see https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/28617 for more details)

[beorn7]

My memories of implementation details are pretty much gone… I would be surprised if there were too many checks, but who knows…

Note that the Pushgateway has the same problem (of diverging help strings from different sources). It did not need a change of client_golang to cope.

[beorn7]

Generally it would epic if there was a simple solution for callers like Otel to dedup and decide what help you want to pick. Have you considered that? Then registry can maintain semantics of consistent help for better UX and contract, plus you can see this invariance is tied to various places of this library.

This sounds very reasonable to me and in line with my vague memories (but I might misremember something).

I've just checked what PGW is doing. It is doing the help string check on its own indeed, and then fixes it on its own and logs about it.

[lukedirtwalker]

Thank you both for the quick feedback. If I read it right, you both seem to prefer to close this PR and solve the problem on the client side? Works for me as well, it's certainly possible to fix this in the code that uses client_golang (otel in this case).

Let me know if you like to leave it here as is, then I close the PR.

[bwplotka]

I mean it would epic to help you in the easiest possible way for everyone.

I looked a bit, plus we discussed on Slack too. I noticed you are using unchecked collector, which makes sense for otel: aggregating data from various sources. I think it would make sense to do this help check ONLY if registeredDescIDs != nil (representing case of checked collector), given it's already "unchecked". This might allow us to generally take it lightly and not need to redesign the whole desc consistency model.

However... flapping help issue remains...maybe, probably depending on order of code movings metrics into channel. We could have a simple framework on client_golang side e.g. take the longer one / strings.Compare > 0 or so, but it's pretty important to have it somewhat stable (if possible).

On client side all of this is possible, just a bit more overhead. Ideas welcome, we are not saying no, but some tests would be mandatory here!

[stale[bot]]

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.