diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index a11f9108e92..876cfd60ca5 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -29,6 +29,9 @@ Released 2025-Oct-01 } ``` +* Moved the `InstrumentNameRegex` from `MeterProviderBuilderSdk` to `Guard`. + ([#6457](https://github.com/open-telemetry/opentelemetry-dotnet/pull/6457)) + * Fixed parsing of `OTEL_TRACES_SAMPLER_ARG` decimal values to always use `.` as the delimiter when using the `traceidratio` sampler, preventing locale-specific parsing issues. diff --git a/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderExtensions.cs b/src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderExtensions.cs index 31ad29f7c5a..b18a1a6584d 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)); - } + 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/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..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 (!MeterProviderBuilderSdk.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 e366e63720a..3060a70f382 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)); - } + MetricGuard.ThrowIfInvalidViewName(value); this.name = value; } 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]