Memory leak in global MeterProvider

[dashpole]

Description

Forked from open-telemetry/opentelemetry-go-contrib#5190

I am able to reproduce the issue with these benchmarks:

package bench

import (
	"testing"

	"go.opentelemetry.io/otel"
)

const (
	meterName   = "foo"
	metricName  = "bar"
)

func BenchmarkSimple(b *testing.B) {
	b.ReportAllocs()
	meter := otel.Meter(meterName)
	for i := 0; i < b.N; i++ {
		meter.Int64Counter(metricName)
	}
}

It has 1 allocation. Since we are repeatedly creating the same instrument, it should have 0 allocations. If I replace otel.Meter with a noop meterprovider or the real meter provider, it has 0 allocations. I believe the root cause is that we always append the instrument to the list of instruments for the global meter provider: e.g.

opentelemetry-go/internal/global/meter.go

Line 142 in d776f74

m.instruments = append(m.instruments, i)

We should instead maintain a map of instruments, and ensure that we only keep unique instruments, similar to what we did for the SDK.

Expected behavior

We should not leak memory when a user creates the same instrument repeatedly

[Jesse-Bonfire]

It appears that this change has broken metrics that are registered before otel.SetMeterProvider is called. We are getting into a spot where the delegate on the siUpDownCounter isn't getting set as it looks like with the new map we are creating new ones and throwing away the old. But the old counter has been registered with meter.RegisterCallback
https://github.com/open-telemetry/opentelemetry-go/blob/42fd8fe325c125a40a21796c3f14b599e5e5f586/internal/global/meter.go#L175C8-L175C9

I am pretty new to Otel and Go but this change seems to break the connection pools convention outlined here.
https://github.com/open-telemetry/semantic-conventions/blob/422eeb71f40c73d1b195723cc1cffeb09abf2391/docs/database/database-metrics.md#connection-pools

Currently I am seeing this when instrumenting a Redis Cluster as it creates instrumentation for the connection pool to each node in the cluster. https://github.com/redis/go-redis/blob/233f97accde15bcb2e3d86ee744109b4dce34171/extra/redisotel/metrics.go#L85

This problem is removed if we apply the instrumentation after we call otel.SetMeterProvider because the delegate just creates the counter. Not sure the best way to proceed.

[Jesse-Bonfire]

I am able to reproduce the exception with the following test case

package bench

import (
	"context"
	"testing"

	"go.opentelemetry.io/otel"
	"go.opentelemetry.io/otel/metric"
	sdkmetric "go.opentelemetry.io/otel/sdk/metric"
)

const (
	meterName  = "foo"
	metricName = "bar"
)

func BenchmarkSimple(b *testing.B) {
	b.ReportAllocs()
	meter := otel.Meter(meterName)
	for i := 0; i < 5; i++ {
		m, _ := meter.Int64ObservableUpDownCounter(metricName)
		meter.RegisterCallback(func(ctx context.Context, o metric.Observer) error {
			o.ObserveInt64(m, int64(10))
			return nil
		}, m)
	}
	p1 := sdkmetric.NewMeterProvider()
	otel.SetMeterProvider(p1)
}

[dashpole]

@Jesse-Bonfire I opened a new issue to track it: #5827