Stop validating View-provided stream Name against instrument name syntax

src/OpenTelemetry/CHANGELOG.md

@@ -6,6 +6,11 @@ Notes](../../RELEASENOTES.md).
 
 ## Unreleased
 
+* Stop validating View-provided metric stream `Name` against the instrument
+  name syntax, per
+  [spec clarification](https://github.com/open-telemetry/opentelemetry-specification/pull/5094).
+  ([#7300](https://github.com/open-telemetry/opentelemetry-dotnet/pull/7300))
+
 * Fix incorrect validation of `OTEL_BSP_*` and `OTEL_BLRP_*` environment
   variables.
   ([#7187](https://github.com/open-telemetry/opentelemetry-dotnet/pull/7187))

src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderExtensions.cs

@@ -105,10 +105,10 @@ public static MeterProviderBuilder AddView(this MeterProviderBuilder meterProvid
     {
         Guard.ThrowIfNull(instrumentName);
 
-        if (!MeterProviderBuilderSdk.IsValidInstrumentName(name))
-        {
-            throw new ArgumentException($"Custom view name {name} is invalid.", nameof(name));
-        }
+        // Per the OpenTelemetry specification, the View-provided stream name
+        // is not subject to the instrument name syntax and the SDK MUST NOT
+        // validate it against that syntax. See:
+        // https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#view
 
 #pragma warning disable CA1062 // Validate arguments of public methods - needed for netstandard2.1
 #if NET || NETSTANDARD2_1_OR_GREATER

src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderSdk.cs

@@ -62,15 +62,6 @@ public MeterProviderBuilderSdk(IServiceProvider serviceProvider)
     public static bool IsValidInstrumentName(string instrumentName)
         => !string.IsNullOrWhiteSpace(instrumentName) && InstrumentNameRegex.IsMatch(instrumentName);
 
-    /// <summary>
-    /// Returns whether the given custom view name is valid according to the specification.
-    /// </summary>
-    /// <remarks>See specification: <see href="https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/api.md#instrument"/>.</remarks>
-    /// <param name="customViewName">The view name.</param>
-    /// <returns>Boolean indicating if the instrument is valid.</returns>
-    public static bool IsValidViewName(string customViewName) =>
-        customViewName == null || InstrumentNameRegex.IsMatch(customViewName); // Only validate the view name in case it's not null. In case it's null, the view name will be the instrument name as per the spec.
-
     public void RegisterProvider(MeterProviderSdk meterProvider)
     {
         Debug.Assert(meterProvider != null, "meterProvider was null");

src/OpenTelemetry/Metrics/Reader/MetricReaderExt.cs

@@ -131,12 +131,18 @@ internal virtual List<Metric> AddMetricWithViews(Instrument instrument, List<Met
                     ? this.exemplarFilterForHistograms ?? this.exemplarFilter
                     : this.exemplarFilter;
 
-                if (!MeterProviderBuilderSdk.IsValidInstrumentName(metricStreamIdentity.InstrumentName))
+                // Per the OpenTelemetry specification, View-provided names are
+                // not subject to the instrument name syntax. Only validate the
+                // name when it originates from the instrument itself (no View
+                // rename). The Meter API does not validate instrument names,
+                // so the SDK must validate them here.
+                if (metricStreamConfig?.Name == null
+                    && !MeterProviderBuilderSdk.IsValidInstrumentName(metricStreamIdentity.InstrumentName))
                 {
                     OpenTelemetrySdkEventSource.Log.MetricInstrumentIgnored(
                         metricStreamIdentity.InstrumentName,
                         metricStreamIdentity.MeterName,
-                        metricStreamConfig?.Name == null ? "Instrument name is invalid." : "View name is invalid.",
+                        "Instrument name is invalid.",
                         "The name must comply with the OpenTelemetry specification.");
 
                     continue;

src/OpenTelemetry/Metrics/View/MetricStreamConfiguration.cs

@@ -26,21 +26,14 @@ public class MetricStreamConfiguration
     /// Gets or sets the optional name of the metric stream.
     /// </summary>
     /// <remarks>
-    /// Note: If not provided the instrument name will be used.
+    /// <para>Note: If not provided the instrument name will be used.</para>
+    /// <para>The name supplied here is used as-is; it is not required to
+    /// follow the instrument name syntax. This lets you rename instruments
+    /// to names that the OpenTelemetry instrument naming rules would
+    /// otherwise reject (for example, legacy or third-party names, or
+    /// OS-level counter names).</para>
     /// </remarks>
-    public string? Name
-    {
-        get;
-        set
-        {
-            if (value != null && !MeterProviderBuilderSdk.IsValidViewName(value))
-            {
-                throw new ArgumentException($"Custom view name {value} is invalid.", nameof(value));
-            }
-
-            field = value;
-        }
-    }
+    public string? Name { get; set; }
 
     /// <summary>
     /// Gets or sets the optional description of the metric stream.

test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs

@@ -34,25 +34,45 @@ public void ViewToRenameMetric()
 
     [Theory]
     [MemberData(nameof(MetricTestData.InvalidInstrumentNames), MemberType = typeof(MetricTestData))]
-    public void AddViewWithInvalidNameThrowsArgumentException(string viewNewName)
+    public void AddViewWithNonInstrumentSyntaxNameAccepted_StringOverload(string viewNewName)
     {
+        // Per spec, View-provided stream names are NOT subject to the
+        // instrument name syntax. Names that would be invalid as instrument
+        // names (and previously threw ArgumentException) MUST now be accepted.
         var exportedItems = new List<Metric>();
 
-        using var meter1 = new Meter("AddViewWithInvalidNameThrowsArgumentException");
-
-        var ex = Assert.Throws<ArgumentException>(() => BuildMeterProvider(out var meterProvider, builder => builder
-            .AddMeter(meter1.Name)
+        using var meter = new Meter("AddViewWithNonInstrumentSyntaxNameAccepted_StringOverload");
+        using var container = BuildMeterProvider(out var meterProvider, builder => builder
+            .AddMeter(meter.Name)
             .AddView("name1", viewNewName)
-            .AddInMemoryExporter(exportedItems)));
+            .AddInMemoryExporter(exportedItems));
 
-        Assert.Contains($"Custom view name {viewNewName} is invalid.", ex.Message, StringComparison.Ordinal);
+        var counter = meter.CreateCounter<long>("name1");
+        counter.Add(10);
+        meterProvider.ForceFlush(MaxTimeToAllowForFlush);
 
-        ex = Assert.Throws<ArgumentException>(() => BuildMeterProvider(out var meterProvider, builder => builder
-            .AddMeter(meter1.Name)
+        Assert.Single(exportedItems);
+        Assert.Equal(viewNewName, exportedItems[0].Name);
+    }
+
+    [Theory]
+    [MemberData(nameof(MetricTestData.InvalidInstrumentNames), MemberType = typeof(MetricTestData))]
+    public void AddViewWithNonInstrumentSyntaxNameAccepted_ConfigOverload(string viewNewName)
+    {
+        var exportedItems = new List<Metric>();
+
+        using var meter = new Meter("AddViewWithNonInstrumentSyntaxNameAccepted_ConfigOverload");
+        using var container = BuildMeterProvider(out var meterProvider, builder => builder
+            .AddMeter(meter.Name)
             .AddView("name1", new MetricStreamConfiguration() { Name = viewNewName })
-            .AddInMemoryExporter(exportedItems)));
+            .AddInMemoryExporter(exportedItems));
 
-        Assert.Contains($"Custom view name {viewNewName} is invalid.", ex.Message, StringComparison.Ordinal);
+        var counter = meter.CreateCounter<long>("name1");
+        counter.Add(10);
+        meterProvider.ForceFlush(MaxTimeToAllowForFlush);
+
+        Assert.Single(exportedItems);
+        Assert.Equal(viewNewName, exportedItems[0].Name);
     }
 
     [Fact]
@@ -295,21 +315,20 @@ public void ViewToRenameMetricConditionally()
 
     [Theory]
     [MemberData(nameof(MetricTestData.InvalidInstrumentNames), MemberType = typeof(MetricTestData))]
-    public void ViewWithInvalidNameIgnoredConditionally(string viewNewName)
+    public void ViewWithNonInstrumentSyntaxNameAcceptedConditionally(string viewNewName)
     {
+        // Per spec, View-provided stream names are NOT subject to the
+        // instrument name syntax. Names that would be invalid as instrument
+        // names MUST still be accepted when supplied via a View.
         using var meter1 = new Meter("ViewToRenameMetricConditionallyTest");
         var exportedItems = new List<Metric>();
         using var container = BuildMeterProvider(out var meterProvider, builder => builder
             .AddMeter(meter1.Name)
-
-            // since here it's a func, we can't validate the name right away
-            // so the view is allowed to be added, but upon instrument creation it's going to be ignored.
             .AddView((instrument) =>
             {
                 if (instrument.Meter.Name.Equals(meter1.Name, StringComparison.OrdinalIgnoreCase)
                     && instrument.Name.Equals("name1", StringComparison.OrdinalIgnoreCase))
                 {
-                    // invalid instrument name as per the spec
                     return new MetricStreamConfiguration() { Name = viewNewName, Description = "new description" };
                 }
                 else
@@ -319,14 +338,14 @@ public void ViewWithInvalidNameIgnoredConditionally(string viewNewName)
             })
             .AddInMemoryExporter(exportedItems));
 
-        // Because the MetricStreamName passed is invalid, the view is ignored,
-        // and default aggregation is used.
         var counter1 = meter1.CreateCounter<long>("name1", "unit", "original_description");
         counter1.Add(10);
 
         meterProvider.ForceFlush(MaxTimeToAllowForFlush);
 
         Assert.Single(exportedItems);
+        Assert.Equal(viewNewName, exportedItems[0].Name);
+        Assert.Equal("new description", exportedItems[0].Description);
     }
 
     [Theory]