[TC Review - Metrics] Duplicate registration logic

[jsuereth]

The warning for duplicate instrument registration is incorrect. It should check for duplicate instruments with same name and different unit/description etc (distinct) rather than all fields being the same
Relevant spec
Relevant code

The specification verbage is a bit odd here, but from a user standpoint, we want to achieve the following:

• Metric name is the identifying part of an instrument. Description + Unit can actually change without breaking compatibility of the metric itself.
• The warning is meant to tell users when description or unit diverge between registrations of the same instrument.
• Calling create_counter multiple times from multiple locations for the same type of metric (e.g. http.server.duration) should be ok, e.g. if you're instrumenting different HTTP server libraries.

In Java we went so far as to distinguish conflicts resulting from View-based renames during instrument registration. The reason we did them all in a big bundle is an implementation detail.

Specifically, in python I'm worried that:

• Instrument Identity (for error reporting) is based on name, unit and description rather than just name. It's ok for identity to be this, by specification, but the error reporting should STILL report on conflicts by name.
• The error message should be outlined when there is a conflict in unit/description. Today it outputs on a (viable) use case.

[nstawski]

I would like to take this issue.

[davidji99]

Hmm, this seems to still be an issue even on 1.27.0?

[xrmx]

@jsuereth Currently we are just printing warnings (in the wrong case), once fixed should the call to create an instrument with different unit or description when from another one with the same name already registered fail?

[xrmx]

@jsuereth Currently we are just printing warnings (in the wrong case), once fixed should the call to create an instrument with different unit or description when from another one with the same name already registered fail?

Looking at other languages implementations this looks like more of a nice to have warning since I'm not finding this check anywhere.