From 454f7c83c27ee3dbf13216a003d41728c7486cdc Mon Sep 17 00:00:00 2001 From: hannahhaering Date: Thu, 28 Aug 2025 11:49:36 -0400 Subject: [PATCH 1/6] Improve instrument name validation and log messages --- src/OpenTelemetry/CHANGELOG.md | 4 + .../Builder/MeterProviderBuilderExtensions.cs | 5 +- .../Builder/MeterProviderBuilderSdk.cs | 42 ---------- src/OpenTelemetry/Metrics/MeterProviderSdk.cs | 2 +- .../Metrics/Reader/MetricReaderExt.cs | 17 +++- .../Metrics/View/MetricStreamConfiguration.cs | 5 +- src/Shared/Guard.cs | 77 +++++++++++++++++++ 7 files changed, 98 insertions(+), 54 deletions(-) diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index 36fcccbdde4..7a3d90a55fb 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -21,6 +21,10 @@ Notes](../../RELEASENOTES.md). } ``` +* Moved the `InstrumentNameRegex` from `MeterProviderBuilderSdk` to `Guard`. + This regex is used to validate the view and instrument name. If you overwrite + the validation regex using reflection you have to update the code. + ## 1.12.0 Released 2025-Apr-29 diff --git a/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderExtensions.cs b/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderExtensions.cs index 31ad29f7c5a..5892585d7f2 100644 --- a/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderExtensions.cs +++ b/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderExtensions.cs @@ -105,10 +105,7 @@ 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)); - } + Guard.ThrowIfInvalidCustomViewName(name); #pragma warning disable CA1062 // Validate arguments of public methods - needed for netstandard2.1 #if NET || NETSTANDARD2_1_OR_GREATER diff --git a/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderSdk.cs b/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderSdk.cs index 11084ec7f0d..03456bfa5f9 100644 --- a/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderSdk.cs +++ b/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderSdk.cs @@ -3,7 +3,6 @@ using System.Diagnostics; using System.Diagnostics.Metrics; -using System.Text.RegularExpressions; using Microsoft.Extensions.DependencyInjection; using OpenTelemetry.Internal; using OpenTelemetry.Resources; @@ -27,14 +26,6 @@ public MeterProviderBuilderSdk(IServiceProvider serviceProvider) this.serviceProvider = serviceProvider; } - // Note: We don't use static readonly here because some customers - // replace this using reflection which is not allowed on initonly static - // fields. See: https://github.com/dotnet/runtime/issues/11571. - // Customers: This is not guaranteed to work forever. We may change this - // mechanism in the future do this at your own risk. - public static Regex InstrumentNameRegex { get; set; } = new( - @"^[a-z][a-z0-9-._/]{0,254}$", RegexOptions.IgnoreCase | RegexOptions.Compiled); - public List Instrumentation { get; } = new(); public ResourceBuilder? ResourceBuilder { get; private set; } @@ -53,39 +44,6 @@ public MeterProviderBuilderSdk(IServiceProvider serviceProvider) public int CardinalityLimit { get; private set; } = DefaultCardinalityLimit; - /// - /// Returns whether the given instrument name is valid according to the specification. - /// - /// See specification: . - /// The instrument name. - /// Boolean indicating if the instrument is valid. - public static bool IsValidInstrumentName(string instrumentName) - { - if (string.IsNullOrWhiteSpace(instrumentName)) - { - return false; - } - - return InstrumentNameRegex.IsMatch(instrumentName); - } - - /// - /// Returns whether the given custom view name is valid according to the specification. - /// - /// See specification: . - /// The view name. - /// Boolean indicating if the instrument is valid. - public static bool IsValidViewName(string 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. - if (customViewName == null) - { - return true; - } - - return InstrumentNameRegex.IsMatch(customViewName); - } - public void RegisterProvider(MeterProviderSdk meterProvider) { Debug.Assert(meterProvider != null, "meterProvider was null"); diff --git a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs index e2b9077eb19..f3bd728bc47 100644 --- a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs +++ b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs @@ -260,7 +260,7 @@ internal static void MeasurementRecordedDouble(Instrument instrument, double val if (viewConfigCount <= 0) { - if (!MeterProviderBuilderSdk.IsValidInstrumentName(instrument.Name)) + if (!Guard.IsValidInstrumentName(instrument.Name)) { OpenTelemetrySdkEventSource.Log.MetricInstrumentIgnored( instrument.Name, diff --git a/src/OpenTelemetry/Metrics/Reader/MetricReaderExt.cs b/src/OpenTelemetry/Metrics/Reader/MetricReaderExt.cs index 6074dd072f6..0c72396ccb5 100644 --- a/src/OpenTelemetry/Metrics/Reader/MetricReaderExt.cs +++ b/src/OpenTelemetry/Metrics/Reader/MetricReaderExt.cs @@ -126,13 +126,24 @@ internal virtual List AddMetricWithViews(Instrument instrument, List this.name; set { - if (value != null && !MeterProviderBuilderSdk.IsValidViewName(value)) - { - throw new ArgumentException($"Custom view name {value} is invalid.", nameof(value)); - } + Guard.ThrowIfInvalidViewName(value); this.name = value; } diff --git a/src/Shared/Guard.cs b/src/Shared/Guard.cs index dbbe87e0e8f..8e141ece9bb 100644 --- a/src/Shared/Guard.cs +++ b/src/Shared/Guard.cs @@ -5,6 +5,7 @@ using System.Diagnostics.CodeAnalysis; using System.Globalization; using System.Runtime.CompilerServices; +using System.Text.RegularExpressions; #pragma warning disable SA1402 // File may only contain a single type #pragma warning disable SA1403 // File may only contain a single namespace @@ -48,6 +49,14 @@ namespace OpenTelemetry.Internal /// internal static class Guard { + // Note: We don't use static readonly here because some customers + // replace this using reflection which is not allowed on initonly static + // fields. See: https://github.com/dotnet/runtime/issues/11571. + // Customers: This is not guaranteed to work forever. We may change this + // mechanism in the future do this at your own risk. + public static Regex InstrumentNameRegex { get; set; } = new( + @"^[a-z][a-z0-9-._/]{0,254}$", RegexOptions.IgnoreCase | RegexOptions.Compiled); + /// /// Throw an exception if the value is null. /// @@ -178,6 +187,74 @@ public static T ThrowIfNotOfType([NotNull] object? value, [CallerArgumentExpr return result; } + /// + /// Throws an exception if the given view name is invalid according to the specification. + /// Null is valid because the instrument name will be used as the view name. + /// + /// See specification: . + /// The view name. + [DebuggerHidden] + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static void ThrowIfInvalidViewName(string? viewName) + { + if (!IsValidViewName(viewName)) + { + throw new ArgumentException($"View name {viewName} is invalid.", nameof(viewName)); + } + } + + /// + /// Throws an exception if the given custom view name is invalid according to the specification. + /// + /// See specification: . + /// The view name. + [DebuggerHidden] + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static void ThrowIfInvalidCustomViewName(string viewName) + { + if (!IsValidInstrumentName(viewName)) + { + throw new ArgumentException($"Custom view name {viewName} is invalid.", nameof(viewName)); + } + } + + /// + /// Returns whether the given instrument name is valid according to the specification. + /// + /// See specification: . + /// The instrument name. + /// Boolean indicating if the instrument is valid. + [DebuggerHidden] + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static bool IsValidInstrumentName(string instrumentName) + { + if (string.IsNullOrWhiteSpace(instrumentName)) + { + return false; + } + + return InstrumentNameRegex.IsMatch(instrumentName); + } + + /// + /// Returns whether the given custom view name is valid according to the specification. + /// + /// See specification: . + /// The view name. + /// Boolean indicating if the instrument is valid. + [DebuggerHidden] + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static bool IsValidViewName(string? 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. + if (customViewName == null) + { + return true; + } + + return InstrumentNameRegex.IsMatch(customViewName); + } + [DebuggerHidden] [MethodImpl(MethodImplOptions.AggressiveInlining)] private static void Range(T value, string? paramName, T min, T max, string? minName, string? maxName, string? message) From d61dc998f86a67de5b354b54319a1b844eb06570 Mon Sep 17 00:00:00 2001 From: hannahhaering Date: Thu, 28 Aug 2025 23:10:55 -0400 Subject: [PATCH 2/6] use RegEx source generator for net7_0_or_greater; add parameter name to argument exceptions --- .../Metrics/Reader/MetricReaderExt.cs | 16 +++++----- src/Shared/Guard.cs | 31 ++++++++++++++----- 2 files changed, 31 insertions(+), 16 deletions(-) diff --git a/src/OpenTelemetry/Metrics/Reader/MetricReaderExt.cs b/src/OpenTelemetry/Metrics/Reader/MetricReaderExt.cs index 0c72396ccb5..3a7e979c9ef 100644 --- a/src/OpenTelemetry/Metrics/Reader/MetricReaderExt.cs +++ b/src/OpenTelemetry/Metrics/Reader/MetricReaderExt.cs @@ -131,18 +131,18 @@ internal virtual List AddMetricWithViews(Instrument instrument, List /// Methods for guarding against exception throwing values. /// - internal static class Guard + internal static partial class Guard { // Note: We don't use static readonly here because some customers // replace this using reflection which is not allowed on initonly static // fields. See: https://github.com/dotnet/runtime/issues/11571. // Customers: This is not guaranteed to work forever. We may change this // mechanism in the future do this at your own risk. +#if NET7_0_OR_GREATER + [GeneratedRegex(@"^[a-z][a-z0-9-._/]{0,254}$", RegexOptions.IgnoreCase)] + public static partial Regex InstrumentNameRegex(); +#else public static Regex InstrumentNameRegex { get; set; } = new( @"^[a-z][a-z0-9-._/]{0,254}$", RegexOptions.IgnoreCase | RegexOptions.Compiled); +#endif /// /// Throw an exception if the value is null. @@ -193,13 +198,14 @@ public static T ThrowIfNotOfType([NotNull] object? value, [CallerArgumentExpr /// /// See specification: . /// The view name. + /// The parameter name to use in the thrown exception. [DebuggerHidden] [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static void ThrowIfInvalidViewName(string? viewName) + public static void ThrowIfInvalidViewName(string? viewName, [CallerArgumentExpression(nameof(viewName))] string? paramName = null) { if (!IsValidViewName(viewName)) { - throw new ArgumentException($"View name {viewName} is invalid.", nameof(viewName)); + throw new ArgumentException($"View name {viewName} is invalid.", paramName); } } @@ -208,15 +214,18 @@ public static void ThrowIfInvalidViewName(string? viewName) /// /// See specification: . /// The view name. + /// The parameter name to use in the thrown exception. [DebuggerHidden] [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static void ThrowIfInvalidCustomViewName(string viewName) + public static void ThrowIfInvalidCustomViewName([NotNull] string? viewName, [CallerArgumentExpression(nameof(viewName))] string? paramName = null) +#pragma warning disable CS8777 // Parameter must have a non-null value when exiting. { if (!IsValidInstrumentName(viewName)) { - throw new ArgumentException($"Custom view name {viewName} is invalid.", nameof(viewName)); + throw new ArgumentException($"Custom view name {viewName} is invalid.", paramName); } } +#pragma warning restore CS8777 // Parameter must have a non-null value when exiting. /// /// Returns whether the given instrument name is valid according to the specification. @@ -226,14 +235,17 @@ public static void ThrowIfInvalidCustomViewName(string viewName) /// Boolean indicating if the instrument is valid. [DebuggerHidden] [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static bool IsValidInstrumentName(string instrumentName) + public static bool IsValidInstrumentName(string? instrumentName) { if (string.IsNullOrWhiteSpace(instrumentName)) { return false; } - +#if NET7_0_OR_GREATER + return InstrumentNameRegex().IsMatch(instrumentName); +#else return InstrumentNameRegex.IsMatch(instrumentName); +#endif } /// @@ -251,8 +263,11 @@ private static bool IsValidViewName(string? customViewName) { return true; } - +#if NET7_0_OR_GREATER + return InstrumentNameRegex().IsMatch(customViewName); +#else return InstrumentNameRegex.IsMatch(customViewName); +#endif } [DebuggerHidden] From 210bfed04ae22f26815679b655c463d0046c5a03 Mon Sep 17 00:00:00 2001 From: Hannah Haering <157852144+hannahhaering@users.noreply.github.com> Date: Thu, 28 Aug 2025 23:14:24 -0400 Subject: [PATCH 3/6] Update src/OpenTelemetry/CHANGELOG.md Co-authored-by: Martin Costello --- src/OpenTelemetry/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index 7a3d90a55fb..10c540ebd7b 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -24,6 +24,7 @@ Notes](../../RELEASENOTES.md). * Moved the `InstrumentNameRegex` from `MeterProviderBuilderSdk` to `Guard`. This regex is used to validate the view and instrument name. If you overwrite the validation regex using reflection you have to update the code. + ([#6457](https://github.com/open-telemetry/opentelemetry-dotnet/pull/6457)) ## 1.12.0 From 8972d9db098a8a77ad218130a4eb2d7b9b641c45 Mon Sep 17 00:00:00 2001 From: Hannah Haering <157852144+hannahhaering@users.noreply.github.com> Date: Fri, 29 Aug 2025 10:24:48 -0400 Subject: [PATCH 4/6] Update src/OpenTelemetry/CHANGELOG.md Co-authored-by: Martin Costello --- src/OpenTelemetry/CHANGELOG.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index 10c540ebd7b..2315f71a2d1 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -22,8 +22,6 @@ Notes](../../RELEASENOTES.md). ``` * Moved the `InstrumentNameRegex` from `MeterProviderBuilderSdk` to `Guard`. - This regex is used to validate the view and instrument name. If you overwrite - the validation regex using reflection you have to update the code. ([#6457](https://github.com/open-telemetry/opentelemetry-dotnet/pull/6457)) ## 1.12.0 From 3933f30c4f63cec95b39debb083f9cd866393b87 Mon Sep 17 00:00:00 2001 From: hannahhaering Date: Tue, 9 Sep 2025 11:37:15 -0400 Subject: [PATCH 5/6] improved access for Regex --- src/Shared/Guard.cs | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/Shared/Guard.cs b/src/Shared/Guard.cs index 46cac3acb01..c7ae9125101 100644 --- a/src/Shared/Guard.cs +++ b/src/Shared/Guard.cs @@ -54,12 +54,14 @@ internal static partial class Guard // fields. See: https://github.com/dotnet/runtime/issues/11571. // Customers: This is not guaranteed to work forever. We may change this // mechanism in the future do this at your own risk. -#if NET7_0_OR_GREATER +#if NET [GeneratedRegex(@"^[a-z][a-z0-9-._/]{0,254}$", RegexOptions.IgnoreCase)] public static partial Regex InstrumentNameRegex(); #else - public static Regex InstrumentNameRegex { get; set; } = new( + private static readonly Regex InstrumentNameRegexField = new( @"^[a-z][a-z0-9-._/]{0,254}$", RegexOptions.IgnoreCase | RegexOptions.Compiled); + + public static Regex InstrumentNameRegex() => InstrumentNameRegexField; #endif /// @@ -241,11 +243,8 @@ public static bool IsValidInstrumentName(string? instrumentName) { return false; } -#if NET7_0_OR_GREATER + return InstrumentNameRegex().IsMatch(instrumentName); -#else - return InstrumentNameRegex.IsMatch(instrumentName); -#endif } /// @@ -263,11 +262,8 @@ private static bool IsValidViewName(string? customViewName) { return true; } -#if NET7_0_OR_GREATER + return InstrumentNameRegex().IsMatch(customViewName); -#else - return InstrumentNameRegex.IsMatch(customViewName); -#endif } [DebuggerHidden] From 2734a39311d54d22e3f6445c0f9c8c287dee7a23 Mon Sep 17 00:00:00 2001 From: hannahhaering Date: Tue, 16 Sep 2025 15:07:08 -0400 Subject: [PATCH 6/6] added metric guard and unit tests --- .../Builder/MeterProviderBuilderExtensions.cs | 2 +- src/OpenTelemetry/Metrics/MeterProviderSdk.cs | 2 +- src/OpenTelemetry/Metrics/MetricGuard.cs | 100 ++++++++++++++++++ .../Metrics/Reader/MetricReaderExt.cs | 2 +- .../Metrics/View/MetricStreamConfiguration.cs | 2 +- src/Shared/Guard.cs | 90 +--------------- .../Metrics/MetricGuardTests.cs | 68 ++++++++++++ .../Metrics/MetricViewTests.cs | 2 +- 8 files changed, 174 insertions(+), 94 deletions(-) create mode 100644 src/OpenTelemetry/Metrics/MetricGuard.cs create mode 100644 test/OpenTelemetry.Tests/Metrics/MetricGuardTests.cs diff --git a/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderExtensions.cs b/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderExtensions.cs index 5892585d7f2..b18a1a6584d 100644 --- a/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderExtensions.cs +++ b/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderExtensions.cs @@ -105,7 +105,7 @@ public static MeterProviderBuilder AddView(this MeterProviderBuilder meterProvid { Guard.ThrowIfNull(instrumentName); - Guard.ThrowIfInvalidCustomViewName(name); + MetricGuard.ThrowIfInvalidCustomViewName(name); #pragma warning disable CA1062 // Validate arguments of public methods - needed for netstandard2.1 #if NET || NETSTANDARD2_1_OR_GREATER diff --git a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs index f3bd728bc47..7a4f4785f72 100644 --- a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs +++ b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs @@ -260,7 +260,7 @@ internal static void MeasurementRecordedDouble(Instrument instrument, double val if (viewConfigCount <= 0) { - if (!Guard.IsValidInstrumentName(instrument.Name)) + if (!MetricGuard.IsValidInstrumentName(instrument.Name)) { OpenTelemetrySdkEventSource.Log.MetricInstrumentIgnored( instrument.Name, diff --git a/src/OpenTelemetry/Metrics/MetricGuard.cs b/src/OpenTelemetry/Metrics/MetricGuard.cs new file mode 100644 index 00000000000..50021598b93 --- /dev/null +++ b/src/OpenTelemetry/Metrics/MetricGuard.cs @@ -0,0 +1,100 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; +using System.Runtime.CompilerServices; +using System.Text.RegularExpressions; + +namespace OpenTelemetry.Metrics; + +/// +/// Methods for guarding against exception throwing values in Metrics. +/// +internal partial class MetricGuard +{ + // Note: We don't use static readonly here because some customers + // replace this using reflection which is not allowed on initonly static + // fields. See: https://github.com/dotnet/runtime/issues/11571. + // Customers: This is not guaranteed to work forever. We may change this + // mechanism in the future do this at your own risk. +#if NET + [GeneratedRegex(@"^[a-z][a-z0-9-._/]{0,254}$", RegexOptions.IgnoreCase)] + public static partial Regex InstrumentNameRegex(); +#else + private static readonly Regex InstrumentNameRegexField = new( + @"^[a-z][a-z0-9-._/]{0,254}$", RegexOptions.IgnoreCase | RegexOptions.Compiled); + + public static Regex InstrumentNameRegex() => InstrumentNameRegexField; +#endif + + /// + /// Throws an exception if the given view name is invalid according to the specification. + /// Null is valid because the instrument name will be used as the view name. + /// + /// See specification: . + /// The view name. + /// The parameter name to use in the thrown exception. + [DebuggerHidden] + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static void ThrowIfInvalidViewName(string? viewName, [CallerArgumentExpression(nameof(viewName))] string? paramName = null) + { + if (!IsValidViewName(viewName)) + { + throw new ArgumentException($"View name {viewName} is invalid.", paramName); + } + } + + /// + /// Throws an exception if the given custom view name is invalid according to the specification. + /// + /// See specification: . + /// The view name. + /// The parameter name to use in the thrown exception. + [DebuggerHidden] + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static void ThrowIfInvalidCustomViewName(string? viewName, [CallerArgumentExpression(nameof(viewName))] string? paramName = null) + { + if (!IsValidInstrumentName(viewName)) + { + throw new ArgumentException($"Custom view name {viewName} is invalid.", paramName); + } + } + + /// + /// Returns whether the given instrument name is valid according to the specification. + /// + /// See specification: . + /// The instrument name. + /// Boolean indicating if the instrument is valid. + [DebuggerHidden] + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static bool IsValidInstrumentName([NotNullWhen(true)] string? instrumentName) + { + if (string.IsNullOrWhiteSpace(instrumentName)) + { + return false; + } + + return InstrumentNameRegex().IsMatch(instrumentName); + } + + /// + /// Returns whether the given custom view name is valid according to the specification. + /// + /// See specification: . + /// The view name. + /// Boolean indicating if the instrument is valid. + [DebuggerHidden] + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static bool IsValidViewName(string? 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. + if (customViewName == null) + { + return true; + } + + return InstrumentNameRegex().IsMatch(customViewName); + } +} diff --git a/src/OpenTelemetry/Metrics/Reader/MetricReaderExt.cs b/src/OpenTelemetry/Metrics/Reader/MetricReaderExt.cs index 910a2f8d7ee..3060a70f382 100644 --- a/src/OpenTelemetry/Metrics/Reader/MetricReaderExt.cs +++ b/src/OpenTelemetry/Metrics/Reader/MetricReaderExt.cs @@ -126,7 +126,7 @@ internal virtual List AddMetricWithViews(Instrument instrument, List this.name; set { - Guard.ThrowIfInvalidViewName(value); + MetricGuard.ThrowIfInvalidViewName(value); this.name = value; } diff --git a/src/Shared/Guard.cs b/src/Shared/Guard.cs index c7ae9125101..dbbe87e0e8f 100644 --- a/src/Shared/Guard.cs +++ b/src/Shared/Guard.cs @@ -5,7 +5,6 @@ using System.Diagnostics.CodeAnalysis; using System.Globalization; using System.Runtime.CompilerServices; -using System.Text.RegularExpressions; #pragma warning disable SA1402 // File may only contain a single type #pragma warning disable SA1403 // File may only contain a single namespace @@ -47,23 +46,8 @@ namespace OpenTelemetry.Internal /// /// Methods for guarding against exception throwing values. /// - internal static partial class Guard + internal static class Guard { - // Note: We don't use static readonly here because some customers - // replace this using reflection which is not allowed on initonly static - // fields. See: https://github.com/dotnet/runtime/issues/11571. - // Customers: This is not guaranteed to work forever. We may change this - // mechanism in the future do this at your own risk. -#if NET - [GeneratedRegex(@"^[a-z][a-z0-9-._/]{0,254}$", RegexOptions.IgnoreCase)] - public static partial Regex InstrumentNameRegex(); -#else - private static readonly Regex InstrumentNameRegexField = new( - @"^[a-z][a-z0-9-._/]{0,254}$", RegexOptions.IgnoreCase | RegexOptions.Compiled); - - public static Regex InstrumentNameRegex() => InstrumentNameRegexField; -#endif - /// /// Throw an exception if the value is null. /// @@ -194,78 +178,6 @@ public static T ThrowIfNotOfType([NotNull] object? value, [CallerArgumentExpr return result; } - /// - /// Throws an exception if the given view name is invalid according to the specification. - /// Null is valid because the instrument name will be used as the view name. - /// - /// See specification: . - /// The view name. - /// The parameter name to use in the thrown exception. - [DebuggerHidden] - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static void ThrowIfInvalidViewName(string? viewName, [CallerArgumentExpression(nameof(viewName))] string? paramName = null) - { - if (!IsValidViewName(viewName)) - { - throw new ArgumentException($"View name {viewName} is invalid.", paramName); - } - } - - /// - /// Throws an exception if the given custom view name is invalid according to the specification. - /// - /// See specification: . - /// The view name. - /// The parameter name to use in the thrown exception. - [DebuggerHidden] - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static void ThrowIfInvalidCustomViewName([NotNull] string? viewName, [CallerArgumentExpression(nameof(viewName))] string? paramName = null) -#pragma warning disable CS8777 // Parameter must have a non-null value when exiting. - { - if (!IsValidInstrumentName(viewName)) - { - throw new ArgumentException($"Custom view name {viewName} is invalid.", paramName); - } - } -#pragma warning restore CS8777 // Parameter must have a non-null value when exiting. - - /// - /// Returns whether the given instrument name is valid according to the specification. - /// - /// See specification: . - /// The instrument name. - /// Boolean indicating if the instrument is valid. - [DebuggerHidden] - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static bool IsValidInstrumentName(string? instrumentName) - { - if (string.IsNullOrWhiteSpace(instrumentName)) - { - return false; - } - - return InstrumentNameRegex().IsMatch(instrumentName); - } - - /// - /// Returns whether the given custom view name is valid according to the specification. - /// - /// See specification: . - /// The view name. - /// Boolean indicating if the instrument is valid. - [DebuggerHidden] - [MethodImpl(MethodImplOptions.AggressiveInlining)] - private static bool IsValidViewName(string? 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. - if (customViewName == null) - { - return true; - } - - return InstrumentNameRegex().IsMatch(customViewName); - } - [DebuggerHidden] [MethodImpl(MethodImplOptions.AggressiveInlining)] private static void Range(T value, string? paramName, T min, T max, string? minName, string? maxName, string? message) diff --git a/test/OpenTelemetry.Tests/Metrics/MetricGuardTests.cs b/test/OpenTelemetry.Tests/Metrics/MetricGuardTests.cs new file mode 100644 index 00000000000..602a459730f --- /dev/null +++ b/test/OpenTelemetry.Tests/Metrics/MetricGuardTests.cs @@ -0,0 +1,68 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +using OpenTelemetry.Metrics; +using OpenTelemetry.Metrics.Tests; +using Xunit; + +namespace OpenTelemetry.Tests.Metrics; + +public class MetricGuardTests +{ + [Theory] + [MemberData(nameof(MetricTestData.InvalidInstrumentNames), MemberType = typeof(MetricTestData))] + [InlineData(null)] + public void IsValidInstrumentName_ReturnsFalse_ForInvalidNames(string? instrumentName) + { + Assert.False(MetricGuard.IsValidInstrumentName(instrumentName)); + } + + [Theory] + [MemberData(nameof(MetricTestData.ValidInstrumentNames), MemberType = typeof(MetricTestData))] + public void IsValidInstrumentName_ReturnsTrue_ForValidNames(string instrumentName) + { + Assert.True(MetricGuard.IsValidInstrumentName(instrumentName)); + } + + [Theory] + [MemberData(nameof(MetricTestData.InvalidInstrumentNames), MemberType = typeof(MetricTestData))] + public void ThrowIfInvalidViewName_ThrowsOnInvalid(string? viewName) + { + var ex = Assert.Throws(() => + MetricGuard.ThrowIfInvalidViewName(viewName)); + + Assert.Contains($"View name {viewName} is invalid.", ex.Message, StringComparison.Ordinal); + } + + [Theory] + [MemberData(nameof(MetricTestData.ValidInstrumentNames), MemberType = typeof(MetricTestData))] + [InlineData(null)] // null is valid because the instrument name will be used as the view name. + public void ThrowIfInvalidViewName_DoesNotThrowForValid(string? viewName) + { + var ex = Record.Exception(() => + MetricGuard.ThrowIfInvalidViewName(viewName)); + + Assert.Null(ex); + } + + [Theory] + [MemberData(nameof(MetricTestData.InvalidInstrumentNames), MemberType = typeof(MetricTestData))] + [InlineData(null)] // null is invalid for custom view names. + public void ThrowIfInvalidCustomViewName_ThrowsOnInvalid(string? customViewName) + { + var ex = Assert.Throws(() => + MetricGuard.ThrowIfInvalidCustomViewName(customViewName)); + + Assert.Contains($"Custom view name {customViewName} is invalid.", ex.Message, StringComparison.Ordinal); + } + + [Theory] + [MemberData(nameof(MetricTestData.ValidInstrumentNames), MemberType = typeof(MetricTestData))] + public void ThrowIfInvalidCustomViewName_DoesNotThrowForValid(string? customViewName) + { + var ex = Record.Exception(() => + MetricGuard.ThrowIfInvalidCustomViewName(customViewName)); + + Assert.Null(ex); + } +} diff --git a/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs b/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs index 96a1122be39..b1ea5f08753 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs @@ -52,7 +52,7 @@ public void AddViewWithInvalidNameThrowsArgumentException(string viewNewName) .AddView("name1", new MetricStreamConfiguration() { Name = viewNewName }) .AddInMemoryExporter(exportedItems))); - Assert.Contains($"Custom view name {viewNewName} is invalid.", ex.Message, StringComparison.Ordinal); + Assert.Contains($"View name {viewNewName} is invalid.", ex.Message, StringComparison.Ordinal); } [Fact]