Improve readability and reduce number of if-checks in UniqueInstrumentMeterImpl

[bogdandrutu]

Signed-off-by: Bogdan Drutu [email protected]

[codecov]

Codecov Report

Merging #2455 (2bb303c) into main (43daab9) will decrease coverage by 0.0%.
The diff coverage is 76.0%.

@@           Coverage Diff           @@
##            main   #2455     +/-   ##
=======================================
- Coverage   76.3%   76.3%   -0.1%     
=======================================
  Files        173     173             
  Lines      12010   11999     -11     
=======================================
- Hits        9168    9157     -11     
  Misses      2597    2597             
  Partials     245     245             

Impacted Files	Coverage Δ
internal/metric/registry/registry.go	84.0% <76.0%> (-2.9%)	⬇️

[MadVikingGod]

This leaves an easy way to panic. You can test this by adding a test:

func TestDoubleNewInstrument(t *testing.T) {
	reg := registry.NewUniqueInstrumentMeterImpl(
		metrictest.NewMeterProvider().Meter("meter").MeterImpl(),
	)
	_, err := reg.NewAsyncInstrument(sdkapi.NewNoopAsyncInstrument().Descriptor(), nil)
	require.Nil(t, err)
	_, err = reg.NewSyncInstrument(sdkapi.NewNoopSyncInstrument().Descriptor())
	require.Nil(t, err)
}

[bogdandrutu]

@MadVikingGod as you said this is not a new problem and as the title says "Improve readability and reduce number of if-checks in UniqueInstrumentMeterImpl" this PR achieved the goal to improve the readability and identify this issue. I don't think this needs to be resolved in a cleanup PR, happy to followup.

[bogdandrutu]

@MadVikingGod also your test is wrong because you rely on a bug (the fact that the NewNoop instruments actually do not set the correct instrument type, which causes the "compatible" to return true even though it should not happen in reality since a sync and async instruments cannot have same type.

[bogdandrutu]

Here is the bug that causes your test to fail (which should not) #2460

[bogdandrutu]

@MadVikingGod @Aneurysm9 you had a question about things being "documented", here is the best that I can find https://github.com/open-telemetry/opentelemetry-go/blob/main/metric/sdkapi/instrumentkind.go#L41

[bogdandrutu]

Closing since maintainers do not believe the cleanup is good enough.