PR Suggestions

open-telemetry · Oct 9, 2021 · af86f5e · af86f5e
1 parent 465af0e
commit af86f5e
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 14 deletions.
diff --git a/src/OpenTelemetry/Metrics/MeterProviderBuilderExtensions.cs b/src/OpenTelemetry/Metrics/MeterProviderBuilderExtensions.cs
@@ -53,7 +53,7 @@ public static MeterProviderBuilder AddReader(this MeterProviderBuilder meterProv
         public static MeterProviderBuilder AddView(this MeterProviderBuilder meterProviderBuilder, string instrumentName, string name)
         {
             // we only need to validate the custom view name in case something was actually provided
-            if (!string.IsNullOrWhiteSpace(name) && !MeterProviderBuilderSdk.IsViewNameValid(name))
+            if (!string.IsNullOrWhiteSpace(name) && !MeterProviderBuilderSdk.IsValidViewName(name))
             {
                 throw new ArgumentException($"Custom view name {name} is invalid.", nameof(name));
             }
@@ -78,7 +78,7 @@ public static MeterProviderBuilder AddView(this MeterProviderBuilder meterProvid
         public static MeterProviderBuilder AddView(this MeterProviderBuilder meterProviderBuilder, string instrumentName, MetricStreamConfiguration metricStreamConfiguration)
         {
             // we only need to validate the custom view name in case something was actually provided
-            if (!string.IsNullOrWhiteSpace(metricStreamConfiguration.Name) && !MeterProviderBuilderSdk.IsViewNameValid(metricStreamConfiguration.Name))
+            if (!string.IsNullOrWhiteSpace(metricStreamConfiguration.Name) && !MeterProviderBuilderSdk.IsValidViewName(metricStreamConfiguration.Name))
             {
                 throw new ArgumentException($"Custom view name {metricStreamConfiguration.Name} is invalid.", nameof(metricStreamConfiguration.Name));
             }

diff --git a/src/OpenTelemetry/Metrics/MeterProviderBuilderSdk.cs b/src/OpenTelemetry/Metrics/MeterProviderBuilderSdk.cs
@@ -24,12 +24,12 @@ internal class MeterProviderBuilderSdk : MeterProviderBuilderBase
             @"^[a-zA-Z][-.\w]{0,62}$", RegexOptions.IgnoreCase | RegexOptions.Compiled);
 
         /// <summary>
-        /// Returns whether the given instrument name is valid according with the specification.
+        /// Returns whether the given instrument 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="instrumentName">The instrument name.</param>
         /// <returns>Boolean indicating if the instrument is valid.</returns>
-        internal static bool IsInstrumentNameValid(string instrumentName)
+        internal static bool IsValidInstrumentName(string instrumentName)
         {
             if (string.IsNullOrWhiteSpace(instrumentName))
             {
@@ -40,12 +40,12 @@ internal static bool IsInstrumentNameValid(string instrumentName)
         }
 
         /// <summary>
-        /// Returns whether the given custom view name is valid according with the specification.
+        /// 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>
-        internal static bool IsViewNameValid(string customViewName)
+        internal static bool IsValidViewName(string customViewName)
         {
             return InstrumentNameRegex.IsMatch(customViewName);
         }

diff --git a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs
@@ -139,9 +139,14 @@ internal MeterProviderSdk(
                                 var metricStreamConfig = metricStreamConfigs[i];
                                 var metricStreamName = metricStreamConfig?.Name ?? instrument.Name;
 
-                                if (!MeterProviderBuilderSdk.IsInstrumentNameValid(metricStreamName))
+                                if (!MeterProviderBuilderSdk.IsValidInstrumentName(metricStreamName))
                                 {
-                                    OpenTelemetrySdkEventSource.Log.MetricInstrumentIgnored(instrument.Name, instrument.Meter.Name, "Metric name is invalid.", "The name must comply with the OpenTelemetry specification: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/api.md#instrument");
+                                    OpenTelemetrySdkEventSource.Log.MetricInstrumentIgnored(
+                                        instrument.Name,
+                                        instrument.Meter.Name,
+                                        "Metric name is invalid.",
+                                        "The name must comply with the OpenTelemetry specification.");
+
                                     continue;
                                 }
 
@@ -206,14 +211,19 @@ internal MeterProviderSdk(
                 {
                     try
                     {
-                        if (!MeterProviderBuilderSdk.IsInstrumentNameValid(instrument.Name))
-                        {
-                            OpenTelemetrySdkEventSource.Log.MetricInstrumentIgnored(instrument.Name, instrument.Meter.Name, "Metric name is invalid.", "The name must comply with the OpenTelemetry specification: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/api.md#instrument");
-                            return;
-                        }
-
                         if (meterSourcesToSubscribe.ContainsKey(instrument.Meter.Name))
                         {
+                            if (!MeterProviderBuilderSdk.IsValidInstrumentName(instrument.Name))
+                            {
+                                OpenTelemetrySdkEventSource.Log.MetricInstrumentIgnored(
+                                    instrument.Name,
+                                    instrument.Meter.Name,
+                                    "Metric name is invalid.",
+                                    "The name must comply with the OpenTelemetry specification");
+
+                                return;
+                            }
+
                             var metricName = instrument.Name;
                             Metric metric = null;
                             lock (this.instrumentCreationLock)