[sdk-metrics] Fix a race condition when a metric is deactivated and re-activated in the same collection cycle (open-telemetry#5908)

Co-authored-by: Piotr Kiełkowicz <[email protected]>

Author: CodeBlanch

src/OpenTelemetry/Metrics/Reader/MetricReaderExt.cs

@@ -15,7 +15,7 @@ namespace OpenTelemetry.Metrics;
 public abstract partial class MetricReader
 {
     private readonly HashSet<string> metricStreamNames = new(StringComparer.OrdinalIgnoreCase);
-    private readonly ConcurrentDictionary<MetricStreamIdentity, Metric> instrumentIdentityToMetric = new();
+    private readonly ConcurrentDictionary<MetricStreamIdentity, Metric?> instrumentIdentityToMetric = new();
     private readonly Lock instrumentCreationLock = new();
     private int metricLimit;
     private int cardinalityLimit;
@@ -201,7 +201,7 @@ internal void ApplyParentProviderSettings(
 
     private bool TryGetExistingMetric(in MetricStreamIdentity metricStreamIdentity, [NotNullWhen(true)] out Metric? existingMetric)
         => this.instrumentIdentityToMetric.TryGetValue(metricStreamIdentity, out existingMetric)
-            && existingMetric.Active;
+            && existingMetric != null && existingMetric.Active;
 
     private void CreateOrUpdateMetricStreamRegistration(in MetricStreamIdentity metricStreamIdentity)
     {
@@ -268,8 +268,12 @@ private void RemoveMetric(ref Metric? metric)
 
         OpenTelemetrySdkEventSource.Log.MetricInstrumentRemoved(metric!.Name, metric.MeterName);
 
-        var result = this.instrumentIdentityToMetric.TryRemove(metric.InstrumentIdentity, out var _);
-        Debug.Assert(result, "result was false");
+        // Note: This is using TryUpdate and NOT TryRemove because there is a
+        // race condition. If a metric is deactivated and then reactivated in
+        // the same collection cycle
+        // instrumentIdentityToMetric[metric.InstrumentIdentity] may already
+        // point to the new activated metric and not the old deactivated one.
+        this.instrumentIdentityToMetric.TryUpdate(metric.InstrumentIdentity, null, metric);
 
         // Note: metric is a reference to the array storage so
         // this clears the metric out of the array.