Skip to content
4 changes: 4 additions & 0 deletions src/OpenTelemetry/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
42 changes: 0 additions & 42 deletions src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderSdk.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<InstrumentationRegistration> Instrumentation { get; } = new();

public ResourceBuilder? ResourceBuilder { get; private set; }
Expand All @@ -53,39 +44,6 @@ public MeterProviderBuilderSdk(IServiceProvider serviceProvider)

public int CardinalityLimit { get; private set; } = DefaultCardinalityLimit;

/// <summary>
/// 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>
public static bool IsValidInstrumentName(string instrumentName)
{
if (string.IsNullOrWhiteSpace(instrumentName))
{
return false;
}

return 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)
{
// 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");
Expand Down
2 changes: 1 addition & 1 deletion src/OpenTelemetry/Metrics/MeterProviderSdk.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
17 changes: 14 additions & 3 deletions src/OpenTelemetry/Metrics/Reader/MetricReaderExt.cs
Original file line number Diff line number Diff line change
Expand Up @@ -126,13 +126,24 @@ internal virtual List<Metric> AddMetricWithViews(Instrument instrument, List<Met
? this.exemplarFilterForHistograms ?? this.exemplarFilter
: this.exemplarFilter;

if (!MeterProviderBuilderSdk.IsValidInstrumentName(metricStreamIdentity.InstrumentName))
if (!Guard.IsValidInstrumentName(metricStreamIdentity.InstrumentName))
{
OpenTelemetrySdkEventSource.Log.MetricInstrumentIgnored(
if (metricStreamConfig?.Name == null)
{
OpenTelemetrySdkEventSource.Log.MetricInstrumentIgnored(
metricStreamIdentity.InstrumentName,
metricStreamIdentity.MeterName,
"Metric name is invalid.",
"Instrument name is invalid.",
"The name must comply with the OpenTelemetry specification.");
}
else
{
OpenTelemetrySdkEventSource.Log.MetricInstrumentIgnored(
metricStreamIdentity.InstrumentName,
metricStreamIdentity.MeterName,
"View name is invalid.",
"The name must comply with the OpenTelemetry specification.");
}

continue;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,7 @@ public string? Name
get => 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;
}
Expand Down
77 changes: 77 additions & 0 deletions src/Shared/Guard.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -48,6 +49,14 @@ namespace OpenTelemetry.Internal
/// </summary>
internal static class Guard
{
// Note: We don't use static readonly here because some customers
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more thing, for now, new logic seems to be related only to metrics. Maybe you could put all this stuff to separate class under src/OpenTelemetry/Metrics/MetricsGuard.cs (better name more than weslcomed)?

Do you think that we need some new unit tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm currently working on this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the other hand, this doesn't need to be a regular expression. Length check and IndexOfAny would do the job.

// 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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to be broking #4136.

In general, it is not against OTel promises, but it might be still useful for @dpk83.

@dpk83, what is your perspective on changes?

@CodeBlanch, FYI as you introduced this functionality.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I make the changes in the contrib repo or is there anything else left to do?

public static Regex InstrumentNameRegex { get; set; } = new(
@"^[a-z][a-z0-9-._/]{0,254}$", RegexOptions.IgnoreCase | RegexOptions.Compiled);

/// <summary>
/// Throw an exception if the value is null.
/// </summary>
Expand Down Expand Up @@ -178,6 +187,74 @@ public static T ThrowIfNotOfType<T>([NotNull] object? value, [CallerArgumentExpr
return result;
}

/// <summary>
/// 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.
/// </summary>
/// <remarks>See specification: <see href="https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/api.md#instrument"/>.</remarks>
/// <param name="viewName">The view name.</param>
[DebuggerHidden]
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static void ThrowIfInvalidViewName(string? viewName)
{
if (!IsValidViewName(viewName))
{
throw new ArgumentException($"View name {viewName} is invalid.", nameof(viewName));
}
}

/// <summary>
/// Throws an exception if the given custom view name is invalid 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="viewName">The view name.</param>
[DebuggerHidden]
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static void ThrowIfInvalidCustomViewName(string viewName)
{
if (!IsValidInstrumentName(viewName))
{
throw new ArgumentException($"Custom view name {viewName} is invalid.", nameof(viewName));
}
}

/// <summary>
/// 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>
[DebuggerHidden]
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static bool IsValidInstrumentName(string instrumentName)
{
if (string.IsNullOrWhiteSpace(instrumentName))
{
return false;
}

return 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>
[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>(T value, string? paramName, T min, T max, string? minName, string? maxName, string? message)
Expand Down