Impossible to create Instrument with the same name from the different MeterProvider

[morozovcookie]

Description

More that year have passed since I wrote about this in Slack and it is still here.
I am trying to use Meter API, count some data and collect it with Prometheus. It is possible situation, when you need the same Instrument name with the different prefixes.
I guess that Meter name is the prefix, but not. And If I will try to create two Meters and for each one create Instrument with the same name than I will get an error.

Environment

• OS: OS X
• Architecture: x86_64
• Go Version: 1.19
• opentelemetry-go version: v1.8.0

Steps To Reproduce

config := prometheus.Config{}
	ctrl := controller.New(
		processor.NewFactory(
			selector.NewWithHistogramDistribution(
				histogram.WithExplicitBoundaries(config.DefaultHistogramBoundaries),
			),
			aggregation.CumulativeTemporalitySelector(),
			processor.WithMemory(true),
		),
	)

	exporter, err := prometheus.New(config, ctrl)
	if err != nil {
		return fmt.Errorf("setup monitor routes: %w", err)
	}

	provider := exporter.MeterProvider()

	httpInstrumentProvider := provider.Meter("http").SyncInt64()
	httpErrorCounter, err := httpInstrumentProvider.Counter("error_count", instrument.WithUnit(unit.Dimensionless),
		instrument.WithDescription("count of http request errors"))
	if err != nil {
		return fmt.Errorf("setup monitor routes: %w", err)
	}

	httpErrorCounter.Add(ctx, 1)

	sqlInstrumentProvider := provider.Meter("sql").SyncInt64()
	sqlErrorCounter, err := sqlInstrumentProvider.Counter("error_count", instrument.WithUnit(unit.Dimensionless),
		instrument.WithDescription("count of sql query errors"))
	if err != nil {
		return fmt.Errorf("setup monitor routes: %w", err)
	}

	sqlErrorCounter.Add(ctx, 1)

	be.monitorRouter.HandleFunc("/metrics", exporter.ServeHTTP)

Expected behavior

I expect that I could create different Meters which isolate their Instruments (like namespace) and Instruments with the same name will not conflict between themselves in case if it was created with different Meters.

[jmacd]

Blocked by open-telemetry/opentelemetry-specification#2703

[fatsheep9146]

as discussed in #3357 (review)

this issue is also blocked by open-telemetry/opentelemetry-specification#2890

[MrAlias]

this issue is also blocked by open-telemetry/opentelemetry-specification#2890

@fatsheep9146 this issue should not be blocked. The referenced issue is merged in the specification and we should implement a solution that address it.

Let me know if you are still able to work on this.

[fatsheep9146]

this issue is also blocked by open-telemetry/opentelemetry-specification#2890

@fatsheep9146 this issue should not be blocked. The referenced issue is merged in the specification and we should implement a solution that address it.

Let me know if you are still able to work on this.

Yes, I am working on this. =D @MrAlias

[eddycharly]

Where is exporter.ServeHTTP gone ?

How are we supposed to serve metrics to prometheus now ?
Am I missing something ?

[fatsheep9146]

Where is exporter.ServeHTTP gone ?

How are we supposed to serve metrics to prometheus now ? Am I missing something ?

I think you can see this example as reference. @eddycharly
https://github.com/open-telemetry/opentelemetry-go/tree/main/example/prometheus

[eddycharly]

@fatsheep9146 thanks, I did the same as in https://github.com/open-telemetry/opentelemetry-go/tree/main/example/prometheus.

I now see crazy metrics like kyverno_policy_changes_total_total_total_total_total_total_total_total_total_total.
Any idea what's going on here ?

[fatsheep9146]

@fatsheep9146 thanks, I did the same as in https://github.com/open-telemetry/opentelemetry-go/tree/main/example/prometheus.

I now see crazy metrics like kyverno_policy_changes_total_total_total_total_total_total_total_total_total_total. Any idea what's going on here ?

Could you paste your code? @eddycharly

[eddycharly]

Basically the same as in the example, my metric is created like this:

	m.policyChangesMetric, err = meter.SyncInt64().Counter("kyverno_policy_changes_total", instrument.WithDescription("can be used to track all the changes associated with the Kyverno policies present on the cluster such as creation, updates and deletions"))
	if err != nil {
		m.Log.Error(err, "Failed to create instrument, kyverno_policy_changes_total")
		return nil, err
	}

[eddycharly]

The code initialising otel is here https://github.com/eddycharly/kyverno/blob/a8a496c8b231e1587797dc4af464ce747a524384/pkg/metrics/metrics.go#L169-L203

[fatsheep9146]

kyverno_policy_changes_total_total_total_total_total_total_total_total_total_total

@eddycharly

First of all, the best practice to create a counter which is monotonically is not include the "total " suffix when you call Counter. The suffix will be supplied automatically by sdk itself.

So I suggest you change your code into

	m.policyChangesMetric, err = meter.SyncInt64().Counter("kyverno_policy_changes", instrument.WithDescription("can be used to track all the changes associated with the Kyverno policies present on the cluster such as creation, updates and deletions"))
	if err != nil {
		m.Log.Error(err, "Failed to create instrument, kyverno_policy_changes_total")
		return nil, err
	}

Then finally you can get the metric name kyverno_policy_changes_total

[eddycharly]

@fatsheep9146 the suggested changes didn't fix the issue, I still observe metrics like this:

# TYPE kyverno_policy_changes_total_total_total_total_total_total_total_total_total_total_total_total_total counter            

kyverno_policy_changes_total_total_total_total_total_total_total_total_total_total_total_total_total{policy_background_mode="true",policy_change_type="deleted",policy_name="tasks-no-extractor",policy_namespace="-",policy_type="cluster",policy_validation_mode="enforce"} 1

[eddycharly]

@fatsheep9146 it looks like it's fixed on main but not yet in v1.11.1

[eddycharly]

Offending code (should have been out of the for loop)

opentelemetry-go/exporters/prometheus/exporter.go

Lines 207 to 210 in 2fe8861

	if sum.IsMonotonic {
	// Add _total suffix for counters
	name += counterSuffix
	}

[fatsheep9146]

@eddycharly
Yes, I think this bug maybe fixed by #3369

[eddycharly]

Any idea when the next release containing the fix might come out ?

[fatsheep9146]

Any idea when the next release containing the fix might come out ?

I think the milestone is tracked here. https://github.com/open-telemetry/opentelemetry-go/milestone/34

@eddycharly

[eddycharly]

Thank you so much 🙏