Move check for drop configuration

open-telemetry · Apr 1, 2022 · 462d1af · 462d1af
1 parent 2d47c2d
commit 462d1af
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 7 deletions.
diff --git a/src/OpenTelemetry/Metrics/MetricReaderExt.cs b/src/OpenTelemetry/Metrics/MetricReaderExt.cs
@@ -132,6 +132,12 @@ internal List<Metric> AddMetricsListWithViews(Instrument instrument, List<Metric
                         continue;
                     }
 
+                    if (metricStreamConfig == MetricStreamConfiguration.Drop)
+                    {
+                        OpenTelemetrySdkEventSource.Log.MetricInstrumentIgnored(metricName, instrument.Meter.Name, "View configuration asks to drop this instrument.", "Modify view configuration to allow this instrument, if desired.");
+                        continue;
+                    }
+
                     if (this.instrumentIdentityToMetric.TryGetValue(instrumentIdentity, out var existingMetric))
                     {
                         if (metrics.Count == 0)
@@ -150,13 +156,6 @@ internal List<Metric> AddMetricsListWithViews(Instrument instrument, List<Metric
                         continue;
                     }
 
-                    // TODO: I think this check needs to be moved up...
-                    if (metricStreamConfig == MetricStreamConfiguration.Drop)
-                    {
-                        OpenTelemetrySdkEventSource.Log.MetricInstrumentIgnored(metricName, instrument.Meter.Name, "View configuration asks to drop this instrument.", "Modify view configuration to allow this instrument, if desired.");
-                        continue;
-                    }
-
                     var index = ++this.metricIndex;
                     if (index >= this.maxMetricStreams)
                     {

diff --git a/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs b/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs
@@ -707,6 +707,44 @@ public void DuplicateInstrumentRegistration_WithViews_TwoInstruments_OneView_Res
             CheckTagsForNthMetricPoint(metric2, tags2, 1);
         }
 
+        [Fact]
+        public void DuplicateInstrumentRegistration_WithViews_TwoInstruments_WouldResultInConflictButSecondInstrumentIsDropped()
+        {
+            var exportedItems = new List<Metric>();
+
+            using var meter = new Meter($"{Utils.GetCurrentMethodName()}");
+            var meterProviderBuilder = Sdk.CreateMeterProviderBuilder()
+                .AddMeter(meter.Name)
+                .AddView((instrument) =>
+                {
+                    if (instrument.Name == "name")
+                    {
+                        return new MetricStreamConfiguration { Name = "othername" };
+                    }
+                    else
+                    {
+                        return MetricStreamConfiguration.Drop;
+                    }
+                })
+                .AddInMemoryExporter(exportedItems);
+
+            using var meterProvider = meterProviderBuilder.Build();
+
+            var instrument1 = meter.CreateCounter<long>("name");
+            var instrument2 = meter.CreateCounter<long>("othername");
+
+            instrument1.Add(10);
+            instrument2.Add(20);
+
+            meterProvider.ForceFlush(MaxTimeToAllowForFlush);
+
+            Assert.Single(exportedItems);
+            var metric1 = new List<Metric>() { exportedItems[0] };
+
+            Assert.Equal("othername", exportedItems[0].Name);
+            Assert.Equal(10, GetLongSum(metric1));
+        }
+
         [Fact]
         public void DuplicateInstrumentNamesFromDifferentMetersWithSameNameDifferentVersion()
         {