Fail-fast when using AddView with guaranteed conflict (#2785)

Author: cijothomas

src/OpenTelemetry/CHANGELOG.md

@@ -5,6 +5,9 @@
 * Make `MetricPoint` of `MetricPointAccessor` readonly.
   ([2736](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2736))
 
+* Fail-fast when using AddView with guaranteed conflict.
+  ([2751](https://github.com/open-telemetry/opentelemetry-dotnet/issues/2751))
+
 ## 1.2.0-rc1
 
 Released 2021-Nov-29

src/OpenTelemetry/Metrics/MeterProviderBuilderExtensions.cs

@@ -57,6 +57,15 @@ public static MeterProviderBuilder AddView(this MeterProviderBuilder meterProvid
                 throw new ArgumentException($"Custom view name {name} is invalid.", nameof(name));
             }
 
+            if (instrumentName.IndexOf('*') != -1)
+            {
+                throw new ArgumentException(
+                    $"Instrument selection criteria is invalid. Instrument name '{instrumentName}' " +
+                    $"contains a wildcard character. This is not allowed when using a view to " +
+                    $"rename a metric stream as it would lead to conflicting metric stream names.",
+                    nameof(instrumentName));
+            }
+
             if (meterProviderBuilder is MeterProviderBuilderBase meterProviderBuilderBase)
             {
                 return meterProviderBuilderBase.AddView(instrumentName, name);
@@ -86,6 +95,15 @@ public static MeterProviderBuilder AddView(this MeterProviderBuilder meterProvid
                 throw new ArgumentException($"Custom view name {metricStreamConfiguration.Name} is invalid.", nameof(metricStreamConfiguration.Name));
             }
 
+            if (metricStreamConfiguration.Name != null && instrumentName.IndexOf('*') != -1)
+            {
+                throw new ArgumentException(
+                    $"Instrument selection criteria is invalid. Instrument name '{instrumentName}' " +
+                    $"contains a wildcard character. This is not allowed when using a view to " +
+                    $"rename a metric stream as it would lead to conflicting metric stream names.",
+                    nameof(instrumentName));
+            }
+
             if (metricStreamConfiguration is ExplicitBucketHistogramConfiguration histogramConfiguration)
             {
                 // Validate histogram boundaries

test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs

@@ -92,6 +92,34 @@ public void AddViewWithNullMetricStreamConfigurationThrowsArgumentnullException(
                .Build());
         }
 
+        [Fact]
+        public void AddViewWithNameThrowsInvalidArgumentExceptionWhenConflict()
+        {
+            var exportedItems = new List<Metric>();
+
+            using var meter1 = new Meter("AddViewWithGuaranteedConflictThrowsInvalidArgumentException");
+
+            Assert.Throws<ArgumentException>(() => Sdk.CreateMeterProviderBuilder()
+               .AddMeter(meter1.Name)
+               .AddView("instrumenta.*", name: "newname")
+               .AddInMemoryExporter(exportedItems)
+               .Build());
+        }
+
+        [Fact]
+        public void AddViewWithNameInMetricStreamConfigurationThrowsInvalidArgumentExceptionWhenConflict()
+        {
+            var exportedItems = new List<Metric>();
+
+            using var meter1 = new Meter("AddViewWithGuaranteedConflictThrowsInvalidArgumentException");
+
+            Assert.Throws<ArgumentException>(() => Sdk.CreateMeterProviderBuilder()
+               .AddMeter(meter1.Name)
+               .AddView("instrumenta.*", new MetricStreamConfiguration() { Name = "newname" })
+               .AddInMemoryExporter(exportedItems)
+               .Build());
+        }
+
         [Theory]
         [MemberData(nameof(MetricTestData.InvalidHistogramBoundaries), MemberType = typeof(MetricTestData))]
         public void AddViewWithInvalidHistogramBoundsThrowsArgumentException(double[] boundaries)
@@ -277,33 +305,6 @@ public void ViewWithNullCustomNameTakesInstrumentName()
             Assert.Equal(counter1.Name, metric.Name);
         }
 
-        [Fact]
-        public void ViewToRenameMetricWildCardMatch()
-        {
-            using var meter = new Meter(Utils.GetCurrentMethodName());
-            var exportedItems = new List<Metric>();
-            using var meterProvider = Sdk.CreateMeterProviderBuilder()
-                .AddMeter(meter.Name)
-                .AddView("counter*", "renamed")
-                .AddInMemoryExporter(exportedItems)
-                .Build();
-
-            // Expecting one metric stream.
-            var counter1 = meter.CreateCounter<long>("counterA");
-            counter1.Add(10);
-            var counter2 = meter.CreateCounter<long>("counterB");
-            counter2.Add(10);
-            var counter3 = meter.CreateCounter<long>("counterC");
-            counter3.Add(10);
-            meterProvider.ForceFlush(MaxTimeToAllowForFlush);
-
-            // counter* matches all 3 instruments which all
-            // becomes "renamed" and only 1st one is exported.
-            Assert.Single(exportedItems);
-            var metric = exportedItems[0];
-            Assert.Equal("renamed", metric.Name);
-        }
-
         [Fact]
         public void ViewToProduceMultipleStreamsFromInstrument()
         {