Bug fix: disposing meter with same name does not dispose the shared metric

src/OpenTelemetry/Metrics/Metric.cs

@@ -74,6 +74,10 @@ internal static readonly
 
     internal readonly AggregatorStore AggregatorStore;
 
+    private readonly Lock referenceLock = new();
+
+    private int referenceCount;
+
     internal Metric(
         MetricStreamIdentity instrumentIdentity,
         AggregationTemporality temporality,
@@ -268,4 +272,24 @@ internal void UpdateDouble(double value, ReadOnlySpan<KeyValuePair<string, objec
 
     internal int Snapshot()
         => this.AggregatorStore.Snapshot();
+
+    internal void AddReference()
+    {
+        lock (this.referenceLock)
+        {
+            this.referenceCount++;
+        }
+    }
+
+    internal void RemoveReference()
+    {
+        lock (this.referenceLock)
+        {
+            this.referenceCount--;
+            if (this.referenceCount == 0)
+            {
+                MetricReader.DeactivateMetric(this);
+            }
+        }
+    }
 }

src/OpenTelemetry/Metrics/MetricState.cs

@@ -24,11 +24,15 @@ private MetricState(
 
     internal delegate void RecordMeasurementAction<T>(T value, ReadOnlySpan<KeyValuePair<string, object?>> tags);
 
-    public static MetricState BuildForSingleMetric(Metric metric) =>
-        new(
-            completeMeasurement: () => MetricReader.DeactivateMetric(metric),
+    public static MetricState BuildForSingleMetric(Metric metric)
+    {
+        metric.AddReference();
+
+        return new(
+            completeMeasurement: metric.RemoveReference,
             recordMeasurementLong: metric.UpdateLong,
             recordMeasurementDouble: metric.UpdateDouble);
+    }
 
     public static MetricState BuildForMetricList(
         List<Metric> metrics)
@@ -38,12 +42,17 @@ public static MetricState BuildForMetricList(
         // Note: Use an array here to elide bounds checks.
         var metricsArray = metrics.ToArray();
 
+        for (var i = 0; i < metricsArray.Length; i++)
+        {
+            metricsArray[i].AddReference();
+        }
+
         return new(
             completeMeasurement: () =>
             {
                 for (var i = 0; i < metricsArray.Length; i++)
                 {
-                    MetricReader.DeactivateMetric(metricsArray[i]);
+                    metricsArray[i].RemoveReference();
                 }
             },
             recordMeasurementLong: (v, t) =>

test/OpenTelemetry.Tests/Metrics/MetricDisposeTests.cs

@@ -0,0 +1,149 @@
+// Copyright The OpenTelemetry Authors
+// SPDX-License-Identifier: Apache-2.0
+
+using System.Diagnostics.Metrics;
+using Xunit;
+
+namespace OpenTelemetry.Metrics.Tests;
+
+public class MetricDisposeTests : MetricTestsBase
+{
+    [Fact]
+    public void DisposingOneMeterWithSameNameDoesNotDeactivateSharedMetric()
+    {
+        var exportedItems = new List<Metric>();
+
+        using var m1 = new Meter("shared-meter");
+        using var m2 = new Meter("shared-meter");
+
+        using var container = BuildMeterProvider(out var meterProvider, builder => builder
+            .AddMeter("shared-meter")
+            .AddInMemoryExporter(exportedItems));
+
+        var counter1 = m1.CreateCounter<long>("my-counter");
+        var counter2 = m2.CreateCounter<long>("my-counter");
+
+        counter1.Add(10, new KeyValuePair<string, object?>("key", "value1"));
+        counter2.Add(20, new KeyValuePair<string, object?>("key", "value2"));
+
+        meterProvider.ForceFlush();
+
+        Assert.Single(exportedItems);
+
+        var sumValue1Before = GetSumForTag(exportedItems, "value1");
+        Assert.Equal(10, sumValue1Before);
+
+        exportedItems.Clear();
+
+        m2.Dispose();
+
+        counter1.Add(5, new KeyValuePair<string, object?>("key", "value1"));
+
+        meterProvider.ForceFlush();
+
+        Assert.Single(exportedItems);
+
+        var sumValue1After = GetSumForTag(exportedItems, "value1");
+        Assert.True(sumValue1After == sumValue1Before + 5, $"Expected sum for 'value1' to increase after m2 disposed. Before={sumValue1Before}, After={sumValue1After}");
+    }
+
+    [Fact]
+    public void DisposingAllMetersWithSameNameDeactivatesSharedMetric()
+    {
+        var exportedItems = new List<Metric>();
+
+        using var m1 = new Meter("shared-meter-all");
+        using var m2 = new Meter("shared-meter-all");
+
+        using var container = BuildMeterProvider(out var meterProvider, builder => builder
+            .AddMeter("shared-meter-all")
+            .AddInMemoryExporter(exportedItems, metricReaderOptions =>
+            {
+                metricReaderOptions.TemporalityPreference = MetricReaderTemporalityPreference.Delta;
+            }));
+
+        var counter1 = m1.CreateCounter<long>("my-counter");
+        var counter2 = m2.CreateCounter<long>("my-counter");
+
+        counter1.Add(10, new KeyValuePair<string, object?>("key", "value1"));
+        counter2.Add(20, new KeyValuePair<string, object?>("key", "value2"));
+
+        meterProvider.ForceFlush();
+        Assert.Single(exportedItems);
+
+        exportedItems.Clear();
+
+        m1.Dispose();
+        m2.Dispose();
+
+        counter1.Add(5, new KeyValuePair<string, object?>("key", "value1"));
+        counter2.Add(5, new KeyValuePair<string, object?>("key", "value2"));
+
+        meterProvider.ForceFlush();
+
+        Assert.Empty(exportedItems);
+    }
+
+    [Fact]
+    public void DisposingOneMeterWithMultipleReadersDoesNotDeactivateSharedMetric()
+    {
+        var exportedItems1 = new List<Metric>();
+        var exportedItems2 = new List<Metric>();
+
+        using var m1 = new Meter("multi-reader-meter");
+        using var m2 = new Meter("multi-reader-meter");
+
+        using var container = BuildMeterProvider(out var meterProvider, builder => builder
+            .AddMeter("multi-reader-meter")
+            .AddInMemoryExporter(exportedItems1)
+            .AddInMemoryExporter(exportedItems2));
+
+        var counter1 = m1.CreateCounter<long>("my-counter");
+        var counter2 = m2.CreateCounter<long>("my-counter");
+
+        counter1.Add(10, new KeyValuePair<string, object?>("key", "value1"));
+        counter2.Add(20, new KeyValuePair<string, object?>("key", "value2"));
+
+        meterProvider.ForceFlush();
+
+        Assert.Single(exportedItems1);
+        Assert.Single(exportedItems2);
+
+        var sumValue1Before = GetSumForTag(exportedItems1, "value1");
+        Assert.Equal(10, sumValue1Before);
+
+        exportedItems1.Clear();
+        exportedItems2.Clear();
+
+        m2.Dispose();
+
+        counter1.Add(5, new KeyValuePair<string, object?>("key", "value1"));
+
+        meterProvider.ForceFlush();
+
+        Assert.Single(exportedItems1);
+        Assert.Single(exportedItems2);
+
+        var sumReader1 = GetSumForTag(exportedItems1, "value1");
+        var sumReader2 = GetSumForTag(exportedItems2, "value1");
+
+        Assert.Equal(15, sumReader1);
+        Assert.Equal(15, sumReader2);
+    }
+
+    private static long GetSumForTag(List<Metric> exportedItems, string tagValue)
+    {
+        foreach (ref readonly var mp in exportedItems[0].GetMetricPoints())
+        {
+            foreach (var kv in mp.Tags)
+            {
+                if (kv.Key == "key" && kv.Value?.ToString() == tagValue)
+                {
+                    return mp.GetSumLong();
+                }
+            }
+        }
+
+        return 0;
+    }
+}