diff --git a/Directory.Packages.props b/Directory.Packages.props index c0f17252d6..353391fdaf 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -119,6 +119,7 @@ + @@ -151,11 +152,13 @@ + + diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentation.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentation.cs index d7d02ecdc3..a9ec2c8939 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentation.cs +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentation.cs @@ -19,7 +19,7 @@ internal sealed class AspNetCoreInstrumentation : IDisposable "Microsoft.AspNetCore.Hosting.UnhandledException" ]; - private readonly Func isEnabled = (eventName, _, _) + private readonly Func isEnabled = static (eventName, _, _) => DiagnosticSourceEvents.Contains(eventName); private readonly DiagnosticSourceSubscriber diagnosticSourceSubscriber; @@ -32,7 +32,5 @@ public AspNetCoreInstrumentation(HttpInListener httpInListener) /// public void Dispose() - { - this.diagnosticSourceSubscriber?.Dispose(); - } + => this.diagnosticSourceSubscriber?.Dispose(); } diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationTracerProviderBuilderExtensions.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationTracerProviderBuilderExtensions.cs index d18e17357a..f44442a9ef 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationTracerProviderBuilderExtensions.cs +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationTracerProviderBuilderExtensions.cs @@ -75,9 +75,7 @@ public static TracerProviderBuilder AddAspNetCoreInstrumentation( return builder.AddInstrumentation(sp => { var options = sp.GetRequiredService>().Get(name); - - return new AspNetCoreInstrumentation( - new HttpInListener(options)); + return new AspNetCoreInstrumentation(new HttpInListener(options)); }); } @@ -102,9 +100,9 @@ private static void AddAspNetCoreInstrumentationSources( string optionsName, IServiceProvider? serviceProvider = null) { - // For .NET7.0 onwards activity will be created using activitySource. + // For .NET 7.0+ the activity will be created using activitySource. // https://github.com/dotnet/aspnetcore/blob/bf3352f2422bf16fa3ca49021f0e31961ce525eb/src/Hosting/Hosting/src/Internal/HostingApplicationDiagnostics.cs#L327 - // For .NET6.0 and below, we will continue to use legacy way. + // For .NET 6.0 and below, we will continue to use legacy way. if (HttpInListener.Net7OrGreater) { // TODO: Check with .NET team to see if this can be prevented @@ -122,7 +120,7 @@ private static void AddAspNetCoreInstrumentationSources( } else { - builder.AddSource(HttpInListener.ActivitySourceName); + builder.AddSource(HttpInListener.ActivitySource.Name); builder.AddLegacySource(HttpInListener.ActivityOperationName); // for the activities created by AspNetCore } diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md index c6813e4a42..3590e2a409 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md @@ -11,6 +11,15 @@ * Updated OpenTelemetry core component version(s) to `1.15.2`. ([#4080](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/4080)) +* Avoid duplicative work to add tags to traces when they are already natively supported + by ASP.NET Core itself. When using ASP.NET Core 10, performance can be + improved by setting the `Microsoft.AspNetCore.Hosting.SuppressActivityOpenTelemetryData` + AppContext switch to `false` (its default value is `true`). + ([#3993](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/3993)) + +* Add instrumentation schema URL to traces for `netstandard2.0`. + ([#4066](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/4066)) + ## 1.15.1 Released 2026-Mar-11 diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs index c452c1fad9..58628e0f4c 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs @@ -1,9 +1,10 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 +using System.Collections.Concurrent; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; -using System.Reflection; +using System.Globalization; using System.Runtime.CompilerServices; using Microsoft.AspNetCore.Http; using OpenTelemetry.Context.Propagation; @@ -23,15 +24,13 @@ internal class HttpInListener : ListenerHandler // https://github.com/dotnet/aspnetcore/blob/8d6554e655b64da75b71e0e20d6db54a3ba8d2fb/src/Hosting/Hosting/src/GenericHost/GenericWebHostBuilder.cs#L85 internal const string AspNetCoreActivitySourceName = "Microsoft.AspNetCore"; - internal static readonly AssemblyName AssemblyName = typeof(HttpInListener).Assembly.GetName(); -#pragma warning disable IDE0370 // Suppression is unnecessary - internal static readonly string ActivitySourceName = AssemblyName.Name!; - internal static readonly Version Version = AssemblyName.Version!; -#pragma warning restore IDE0370 // Suppression is unnecessary - internal static readonly ActivitySource ActivitySource = new(ActivitySourceName, Version.ToString()); + internal static readonly Version SemanticConventionsVersion = new(1, 40, 0); + internal static readonly ActivitySource ActivitySource = ActivitySourceFactory.Create(SemanticConventionsVersion); internal static readonly bool Net7OrGreater = Environment.Version.Major >= 7; + internal static readonly bool Net10OrGreater = Environment.Version.Major >= 10; private const string DiagnosticSourceName = "Microsoft.AspNetCore"; + private const string CreatedByInstrumentationPropertyName = "OpenTelemetry.AspNetCore.CreatedByInstrumentation"; private static readonly Func> HttpRequestHeaderValuesGetter = (request, name) => { @@ -45,8 +44,14 @@ internal class HttpInListener : ListenerHandler }; private static readonly PropertyFetcher ExceptionPropertyFetcher = new("Exception"); + private static readonly object CreatedByInstrumentationMarker = new(); + + // Caches the display name, rpc.service, and rpc.method derived from the raw gRPC method string. + // The set of distinct gRPC method strings is bounded by the number of gRPC endpoints in the app. + private static readonly ConcurrentDictionary GrpcMethodCache = new(); private readonly AspNetCoreTraceInstrumentationOptions options; + private readonly bool nativeAspNetCoreOpenTelemetrySuppressed; public HttpInListener(AspNetCoreTraceInstrumentationOptions options) : base(DiagnosticSourceName) @@ -54,6 +59,7 @@ public HttpInListener(AspNetCoreTraceInstrumentationOptions options) Guard.ThrowIfNull(options); this.options = options; + this.nativeAspNetCoreOpenTelemetrySuppressed = IsOpenTelemetryActivityDataSuppressed(); } public override void OnEventWritten(string name, object? payload) @@ -63,24 +69,18 @@ public override void OnEventWritten(string name, object? payload) switch (name) { case OnStartEvent: - { - this.OnStartActivity(activity, payload); - } - + this.OnStartActivity(activity, payload); break; - case OnStopEvent: - { - this.OnStopActivity(activity, payload); - } + case OnStopEvent: + this.OnStopActivity(activity, payload); break; + case OnUnhandledHostingExceptionEvent: case OnUnHandledDiagnosticsExceptionEvent: - { - this.OnException(activity, payload); - } - + this.OnException(activity, payload); break; + default: break; } @@ -135,7 +135,7 @@ public void OnStartActivity(Activity activity, object? payload) newOne!.TraceStateString = ctx.ActivityContext.TraceState; - newOne.SetTag("IsCreatedByInstrumentation", bool.TrueString); + newOne.SetCustomProperty(CreatedByInstrumentationPropertyName, CreatedByInstrumentationMarker); // Starting the new activity make it the Activity.Current one. newOne.Start(); @@ -152,23 +152,26 @@ public void OnStartActivity(Activity activity, object? payload) // is favorable. if (activity.IsAllDataRequested) { - try + if (this.options.Filter is { } filter) { - if (this.options.Filter?.Invoke(context) == false) + try + { + if (!filter(context)) + { + AspNetCoreInstrumentationEventSource.Log.RequestIsFilteredOut(nameof(HttpInListener), nameof(this.OnStartActivity), activity.OperationName); + activity.IsAllDataRequested = false; + activity.ActivityTraceFlags &= ~ActivityTraceFlags.Recorded; + return; + } + } + catch (Exception ex) { - AspNetCoreInstrumentationEventSource.Log.RequestIsFilteredOut(nameof(HttpInListener), nameof(this.OnStartActivity), activity.OperationName); + AspNetCoreInstrumentationEventSource.Log.RequestFilterException(nameof(HttpInListener), nameof(this.OnStartActivity), activity.OperationName, ex); activity.IsAllDataRequested = false; activity.ActivityTraceFlags &= ~ActivityTraceFlags.Recorded; return; } } - catch (Exception ex) - { - AspNetCoreInstrumentationEventSource.Log.RequestFilterException(nameof(HttpInListener), nameof(this.OnStartActivity), activity.OperationName, ex); - activity.IsAllDataRequested = false; - activity.ActivityTraceFlags &= ~ActivityTraceFlags.Recorded; - return; - } if (!Net7OrGreater) { @@ -176,19 +179,36 @@ public void OnStartActivity(Activity activity, object? payload) ActivityInstrumentationHelper.SetKindProperty(activity, ActivityKind.Server); } - var path = (request.PathBase.HasValue || request.Path.HasValue) ? (request.PathBase + request.Path).ToString() : "/"; - TelemetryHelper.RequestDataHelper.SetActivityDisplayName(activity, request.Method); + // See the spec: https://github.com/open-telemetry/semantic-conventions/blob/v1.40.0/docs/http/http-spans.md + var path = GetRequestPath(request); - // see the spec https://github.com/open-telemetry/semantic-conventions/blob/v1.23.0/docs/http/http-spans.md + // ASP.NET Core 10 does not support OTEL_INSTRUMENTATION_HTTP_KNOWN_METHODS so we + // still need to set the HTTP method tag so that any override by the user is honoured. + TelemetryHelper.RequestDataHelper.SetActivityDisplayNameAndHttpMethodTag(activity, request.Method); - if (request.Host.HasValue) + if (!Net10OrGreater || this.nativeAspNetCoreOpenTelemetrySuppressed) { - activity.SetTag(SemanticConventions.AttributeServerAddress, request.Host.Host); + if (request.Host.HasValue) + { + activity.SetTag(SemanticConventions.AttributeServerAddress, request.Host.Value); + + if (request.Host.Port is { } port) + { + activity.SetTag(SemanticConventions.AttributeServerPort, port); + } + } - if (request.Host.Port.HasValue) + if (request.Headers.TryGetValue("User-Agent", out var values)) { - activity.SetTag(SemanticConventions.AttributeServerPort, request.Host.Port.Value); + var userAgent = values.Count > 0 ? values[0] : null; + if (!string.IsNullOrEmpty(userAgent)) + { + activity.SetTag(SemanticConventions.AttributeUserAgentOriginal, userAgent); + } } + + activity.SetTag(SemanticConventions.AttributeUrlScheme, request.Scheme); + activity.SetTag(SemanticConventions.AttributeUrlPath, path); } if (request.QueryString.HasValue) @@ -203,28 +223,18 @@ public void OnStartActivity(Activity activity, object? payload) } } - TelemetryHelper.RequestDataHelper.SetHttpMethodTag(activity, request.Method); - - activity.SetTag(SemanticConventions.AttributeUrlScheme, request.Scheme); - activity.SetTag(SemanticConventions.AttributeUrlPath, path); activity.SetTag(SemanticConventions.AttributeNetworkProtocolVersion, RequestDataHelper.GetHttpProtocolVersion(request.Protocol)); - if (request.Headers.TryGetValue("User-Agent", out var values)) + if (this.options.EnrichWithHttpRequest is { } enricher) { - var userAgent = values.Count > 0 ? values[0] : null; - if (!string.IsNullOrEmpty(userAgent)) + try { - activity.SetTag(SemanticConventions.AttributeUserAgentOriginal, userAgent); + enricher(activity, request); + } + catch (Exception ex) + { + AspNetCoreInstrumentationEventSource.Log.EnrichmentException(nameof(HttpInListener), nameof(this.OnStartActivity), activity.OperationName, ex); } - } - - try - { - this.options.EnrichWithHttpRequest?.Invoke(activity, request); - } - catch (Exception ex) - { - AspNetCoreInstrumentationEventSource.Log.EnrichmentException(nameof(HttpInListener), nameof(this.OnStartActivity), activity.OperationName, ex); } } } @@ -252,9 +262,37 @@ public void OnStopActivity(Activity activity, object? payload) activity.SetTag(SemanticConventions.AttributeHttpResponseStatusCode, TelemetryHelper.GetBoxedStatusCode(response.StatusCode)); - if (this.options.EnableGrpcAspNetCoreSupport && TryGetGrpcMethod(activity, out var grpcMethod)) + if (this.options.EnableGrpcAspNetCoreSupport && IsGrpcRequestProtocol(context.Request.Protocol)) { - AddGrpcAttributes(activity, grpcMethod, context); + // Single pass over the tag collection to retrieve both gRPC tags, + // avoiding two separate GetTagValue iterations. + string? grpcMethod = null; + int grpcStatusCode = -1; + var hasGrpcStatusCode = false; + + var tagEnumerator = activity.EnumerateTagObjects(); + while (tagEnumerator.MoveNext()) + { + ref readonly var tag = ref tagEnumerator.Current; + if (grpcMethod is null && tag.Key == GrpcTagHelper.GrpcMethodTagName) + { + grpcMethod = tag.Value as string; + } + else if (!hasGrpcStatusCode && tag.Key == GrpcTagHelper.GrpcStatusCodeTagName) + { + hasGrpcStatusCode = int.TryParse(tag.Value as string, NumberStyles.None, CultureInfo.InvariantCulture, out grpcStatusCode); + } + + if (grpcMethod is not null && hasGrpcStatusCode) + { + break; + } + } + + if (!string.IsNullOrEmpty(grpcMethod)) + { + AddGrpcAttributes(activity, grpcMethod!, context, grpcStatusCode, hasGrpcStatusCode); + } } if (activity.Status == ActivityStatusCode.Unset) @@ -262,31 +300,23 @@ public void OnStopActivity(Activity activity, object? payload) activity.SetStatus(SpanHelper.ResolveActivityStatusForHttpStatusCode(activity.Kind, response.StatusCode)); } - try - { - this.options.EnrichWithHttpResponse?.Invoke(activity, response); - } - catch (Exception ex) + if (this.options.EnrichWithHttpResponse is { } enricher) { - AspNetCoreInstrumentationEventSource.Log.EnrichmentException(nameof(HttpInListener), nameof(this.OnStopActivity), activity.OperationName, ex); + try + { + enricher(activity, response); + } + catch (Exception ex) + { + AspNetCoreInstrumentationEventSource.Log.EnrichmentException(nameof(HttpInListener), nameof(this.OnStopActivity), activity.OperationName, ex); + } } } - object? tagValue; - if (Net7OrGreater) - { - tagValue = activity.GetTagValue("IsCreatedByInstrumentation"); - } - else - { - _ = activity.TryCheckFirstTag("IsCreatedByInstrumentation", out tagValue); - } - - if (ReferenceEquals(tagValue, bool.TrueString)) + if (ReferenceEquals(activity.GetCustomProperty(CreatedByInstrumentationPropertyName), CreatedByInstrumentationMarker)) { // If instrumentation started a new Activity, it must // be stopped here. - activity.SetTag("IsCreatedByInstrumentation", null); activity.Stop(); // After the activity.Stop() code, Activity.Current becomes null. @@ -322,13 +352,16 @@ public void OnException(Activity activity, object? payload) activity.SetStatus(ActivityStatusCode.Error); - try + if (this.options.EnrichWithException is { } enricher) { - this.options.EnrichWithException?.Invoke(activity, exc); - } - catch (Exception ex) - { - AspNetCoreInstrumentationEventSource.Log.EnrichmentException(nameof(HttpInListener), nameof(this.OnException), activity.OperationName, ex); + try + { + enricher(activity, exc); + } + catch (Exception ex) + { + AspNetCoreInstrumentationEventSource.Log.EnrichmentException(nameof(HttpInListener), nameof(this.OnException), activity.OperationName, ex); + } } } @@ -345,19 +378,20 @@ static bool TryFetchException(object? payload, [NotNullWhen(true)] out Exception } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private static bool TryGetGrpcMethod(Activity activity, [NotNullWhen(true)] out string? grpcMethod) + private static void AddGrpcAttributes(Activity activity, string grpcMethod, HttpContext context, int grpcStatusCode, bool validStatusCode) { - grpcMethod = GrpcTagHelper.GetGrpcMethodFromActivity(activity); - return !string.IsNullOrEmpty(grpcMethod); - } + var details = GrpcMethodCache.GetOrAdd(grpcMethod, static (method) => + { + var displayName = method.Length > 0 && method[0] == '/' ? method.Substring(1) : method; + var isParsed = GrpcTagHelper.TryParseRpcServiceAndRpcMethod(method, out var serviceName, out var methodName); + + return new(displayName, isParsed ? serviceName : null, isParsed ? methodName : null, isParsed); + }); - [MethodImpl(MethodImplOptions.AggressiveInlining)] - private static void AddGrpcAttributes(Activity activity, string grpcMethod, HttpContext context) - { // The RPC semantic conventions indicate the span name // should not have a leading forward slash. // https://github.com/open-telemetry/semantic-conventions/blob/main/docs/rpc/rpc-spans.md#span-name - activity.DisplayName = grpcMethod.TrimStart('/'); + activity.DisplayName = details.DisplayName; activity.SetTag(SemanticConventions.AttributeRpcSystem, GrpcTagHelper.RpcSystemGrpc); @@ -370,16 +404,15 @@ private static void AddGrpcAttributes(Activity activity, string grpcMethod, Http activity.SetTag(SemanticConventions.AttributeClientPort, context.Connection.RemotePort); - var validConversion = GrpcTagHelper.TryGetGrpcStatusCodeFromActivity(activity, out var status); - if (validConversion) + if (validStatusCode) { - activity.SetStatus(GrpcTagHelper.ResolveSpanStatusForGrpcStatusCodeOnServer(status)); + activity.SetStatus(GrpcTagHelper.ResolveSpanStatusForGrpcStatusCodeOnServer(grpcStatusCode)); } - if (GrpcTagHelper.TryParseRpcServiceAndRpcMethod(grpcMethod, out var rpcService, out var rpcMethod)) + if (details.IsParsed) { - activity.SetTag(SemanticConventions.AttributeRpcService, rpcService); - activity.SetTag(SemanticConventions.AttributeRpcMethod, rpcMethod); + activity.SetTag(SemanticConventions.AttributeRpcService, details.RpcService); + activity.SetTag(SemanticConventions.AttributeRpcMethod, details.RpcMethod); // Remove the grpc.method tag added by the gRPC .NET library activity.SetTag(GrpcTagHelper.GrpcMethodTagName, null); @@ -387,11 +420,50 @@ private static void AddGrpcAttributes(Activity activity, string grpcMethod, Http // Remove the grpc.status_code tag added by the gRPC .NET library activity.SetTag(GrpcTagHelper.GrpcStatusCodeTagName, null); - if (validConversion) + if (validStatusCode) { // setting rpc.grpc.status_code - activity.SetTag(SemanticConventions.AttributeRpcGrpcStatusCode, status); + activity.SetTag(SemanticConventions.AttributeRpcGrpcStatusCode, grpcStatusCode); } } } + + // ASP.NET Core 10 does not generate OpenTelemetry tags by default so we can only take the optimal + // path if the user has not explicitly opted into ASP.NET Core 10's native OpenTelemetry support. + // https://github.com/dotnet/aspnetcore/blob/7387de91234d3ef751fa50b3d1bfede4130213ff/src/Hosting/Hosting/src/Internal/HostingApplicationDiagnostics.cs#L58-L67 + + private static bool IsOpenTelemetryActivityDataSuppressed() => +#if NET10_0 + !AppContext.TryGetSwitch("Microsoft.AspNetCore.Hosting.SuppressActivityOpenTelemetryData", out var enabled) || enabled; +#else + false; +#endif + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static string GetRequestPath(HttpRequest request) => !request.PathBase.HasValue + ? request.Path.Value ?? "/" + : !request.Path.HasValue ? request.PathBase.Value : string.Concat(request.PathBase.Value, request.Path.Value); + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static bool IsGrpcRequestProtocol(string protocol) => + protocol == "HTTP/2" || protocol == "HTTP/3"; + + private readonly struct GrpcMethodDetails + { + public GrpcMethodDetails(string displayName, string? rpcService, string? rpcMethod, bool isParsed) + { + this.DisplayName = displayName; + this.RpcService = rpcService; + this.RpcMethod = rpcMethod; + this.IsParsed = isParsed; + } + + public readonly string DisplayName { get; } + + public readonly string? RpcService { get; } + + public readonly string? RpcMethod { get; } + + public readonly bool IsParsed { get; } + } } diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/OpenTelemetry.Instrumentation.AspNetCore.csproj b/src/OpenTelemetry.Instrumentation.AspNetCore/OpenTelemetry.Instrumentation.AspNetCore.csproj index 9dc42087ae..f941c259a8 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/OpenTelemetry.Instrumentation.AspNetCore.csproj +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/OpenTelemetry.Instrumentation.AspNetCore.csproj @@ -13,15 +13,17 @@ - - - - - + + + + + + + diff --git a/src/Shared/GrpcStatusCanonicalCode.cs b/src/Shared/GrpcStatusCanonicalCode.cs index 8abad05f25..aa37a40c2c 100644 --- a/src/Shared/GrpcStatusCanonicalCode.cs +++ b/src/Shared/GrpcStatusCanonicalCode.cs @@ -132,4 +132,9 @@ internal enum GrpcStatusCanonicalCode /// The request does not have valid authentication credentials for the operation. /// Unauthenticated = 16, + + /// + /// The maximum valid GRPC status code. + /// + MaxValue = Unauthenticated, // Update this value if new status codes are added. } diff --git a/src/Shared/GrpcTagHelper.cs b/src/Shared/GrpcTagHelper.cs index 722b1847c8..352b4c248f 100644 --- a/src/Shared/GrpcTagHelper.cs +++ b/src/Shared/GrpcTagHelper.cs @@ -2,12 +2,11 @@ // SPDX-License-Identifier: Apache-2.0 using System.Diagnostics; -using System.Text.RegularExpressions; using OpenTelemetry.Trace; namespace OpenTelemetry.Instrumentation; -internal static partial class GrpcTagHelper +internal static class GrpcTagHelper { public const string RpcSystemGrpc = "grpc"; @@ -28,19 +27,25 @@ public static bool TryGetGrpcStatusCodeFromActivity(Activity activity, out int s public static bool TryParseRpcServiceAndRpcMethod(string grpcMethod, out string rpcService, out string rpcMethod) { - var match = GrpcMethodRegex().Match(grpcMethod); - if (match.Success) + var span = grpcMethod.AsSpan(); + + if (!span.IsEmpty && span[0] is '/') { - rpcService = match.Groups["service"].Value; - rpcMethod = match.Groups["method"].Value; - return true; + span = span.Slice(1); } - else + + var lastSlash = span.LastIndexOf('/'); + if (lastSlash < 0) { rpcService = string.Empty; rpcMethod = string.Empty; return false; } + + rpcService = span.Slice(0, lastSlash).ToString(); + rpcMethod = span.Slice(lastSlash + 1).ToString(); + + return true; } /// @@ -54,7 +59,7 @@ public static ActivityStatusCode ResolveSpanStatusForGrpcStatusCodeOnClient(int { var status = ActivityStatusCode.Error; - if (typeof(GrpcStatusCanonicalCode).IsEnumDefined(statusCode)) + if (statusCode >= 0 && statusCode <= (int)GrpcStatusCanonicalCode.MaxValue) { status = (GrpcStatusCanonicalCode)statusCode switch { @@ -92,7 +97,7 @@ public static ActivityStatusCode ResolveSpanStatusForGrpcStatusCodeOnClient(int /// Resolved span for the Grpc status code. public static ActivityStatusCode ResolveSpanStatusForGrpcStatusCodeOnServer(int statusCode) { - if (typeof(GrpcStatusCanonicalCode).IsEnumDefined(statusCode)) + if (statusCode >= 0 && statusCode <= (int)GrpcStatusCanonicalCode.MaxValue) { #pragma warning disable IDE0072 // Add missing cases return (GrpcStatusCanonicalCode)statusCode switch @@ -111,16 +116,4 @@ public static ActivityStatusCode ResolveSpanStatusForGrpcStatusCodeOnServer(int // Unknown status code, treat as error return ActivityStatusCode.Error; } - -#if NET - [GeneratedRegex(@"^/?(?.*)/(?.*)$")] - private static partial Regex GrpcMethodRegex(); -#else -#pragma warning disable SA1201 // A field should not follow a method - private static readonly Regex GrpcMethodRegexField = new(@"^/?(?.*)/(?.*)$", RegexOptions.Compiled); -#pragma warning restore SA1201 // A field should not follow a method - - private static Regex GrpcMethodRegex() => GrpcMethodRegexField; -#endif - } diff --git a/src/Shared/RequestDataHelper.cs b/src/Shared/RequestDataHelper.cs index 5e0ff85ddd..7c3905220e 100644 --- a/src/Shared/RequestDataHelper.cs +++ b/src/Shared/RequestDataHelper.cs @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 #if NET +using System.Collections.Concurrent; using System.Collections.Frozen; #endif using System.Diagnostics; @@ -25,6 +26,12 @@ internal sealed class RequestDataHelper private readonly Dictionary knownHttpMethods; #endif +#if NET + // Caches the final display name string for each (normalizedMethod, httpRoute) pair. + // The number of distinct combinations is bounded by the number of (HTTP methods * routes) in the app. + private readonly ConcurrentDictionary<(string Method, string Route), string> displayNameCache = new(); +#endif + public RequestDataHelper(bool configureByHttpKnownMethodsEnvironmentalVariable) { var suppliedKnownMethods = configureByHttpKnownMethodsEnvironmentalVariable ? Environment.GetEnvironmentVariable(KnownHttpMethodsEnvironmentVariable) @@ -79,6 +86,19 @@ public void SetHttpMethodTag(Activity activity, string originalHttpMethod) } } + public void SetActivityDisplayNameAndHttpMethodTag(Activity activity, string originalHttpMethod) + { + var normalizedHttpMethod = this.GetNormalizedHttpMethod(originalHttpMethod); + + activity.DisplayName = normalizedHttpMethod == OtherHttpMethod ? "HTTP" : normalizedHttpMethod; + activity.SetTag(SemanticConventions.AttributeHttpRequestMethod, normalizedHttpMethod); + + if (originalHttpMethod != normalizedHttpMethod) + { + activity.SetTag(SemanticConventions.AttributeHttpRequestMethodOriginal, originalHttpMethod); + } + } + public void SetHttpMethodTag(ref TagList tags, string originalHttpMethod) { var normalizedHttpMethod = this.GetNormalizedHttpMethod(originalHttpMethod); @@ -103,9 +123,18 @@ public string GetActivityDisplayName(string originalHttpMethod, string? httpRout // https://github.com/open-telemetry/semantic-conventions/blob/v1.24.0/docs/http/http-spans.md#name var normalizedHttpMethod = this.GetNormalizedHttpMethod(originalHttpMethod); - var namePrefix = normalizedHttpMethod == "_OTHER" ? "HTTP" : normalizedHttpMethod; + var namePrefix = normalizedHttpMethod == OtherHttpMethod ? "HTTP" : normalizedHttpMethod; + + if (string.IsNullOrEmpty(httpRoute)) + { + return namePrefix; + } - return string.IsNullOrEmpty(httpRoute) ? namePrefix : $"{namePrefix} {httpRoute}"; +#if NET + return this.displayNameCache.GetOrAdd((namePrefix, httpRoute), static kv => $"{kv.Method} {kv.Route}"); +#else + return $"{namePrefix} {httpRoute}"; +#endif } internal static string GetHttpProtocolVersion(Version httpVersion) => httpVersion switch diff --git a/test/OpenTelemetry.Contrib.Shared.Tests/GrpcTagHelperTests.cs b/test/OpenTelemetry.Contrib.Shared.Tests/GrpcTagHelperTests.cs index 070ac9f6a9..88ed2ffaa3 100644 --- a/test/OpenTelemetry.Contrib.Shared.Tests/GrpcTagHelperTests.cs +++ b/test/OpenTelemetry.Contrib.Shared.Tests/GrpcTagHelperTests.cs @@ -54,6 +54,7 @@ public void GrpcTagHelper_GetGrpcStatusCodeFromActivity() [Theory] [InlineData(0, ActivityStatusCode.Unset)] // Ok + [InlineData(-1, ActivityStatusCode.Error)] // Invalid negative status code [InlineData(1, ActivityStatusCode.Error)] // Cancelled [InlineData(2, ActivityStatusCode.Error)] // Unknown [InlineData(3, ActivityStatusCode.Error)] // InvalidArgument @@ -79,6 +80,7 @@ public void GrpcTagHelper_ResolveSpanStatusForGrpcStatusCodeOnClient(int grpcSta [Theory] [InlineData(0, ActivityStatusCode.Unset)] // Ok + [InlineData(-1, ActivityStatusCode.Error)] // Invalid negative status code [InlineData(1, ActivityStatusCode.Unset)] // Cancelled [InlineData(2, ActivityStatusCode.Error)] // Unknown [InlineData(3, ActivityStatusCode.Unset)] // InvalidArgument diff --git a/test/OpenTelemetry.Contrib.Shared.Tests/RequestDataHelperTests.cs b/test/OpenTelemetry.Contrib.Shared.Tests/RequestDataHelperTests.cs index 565db87367..10155e84c1 100644 --- a/test/OpenTelemetry.Contrib.Shared.Tests/RequestDataHelperTests.cs +++ b/test/OpenTelemetry.Contrib.Shared.Tests/RequestDataHelperTests.cs @@ -1,6 +1,8 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 +using System.Diagnostics; +using OpenTelemetry.Trace; using Xunit; namespace OpenTelemetry.Internal.Tests; @@ -75,6 +77,47 @@ public void MethodMappingWorksIfEnvironmentalVariableConfigurationIsDisabled() } } + [Theory] + [InlineData("GET", null, "GET")] + [InlineData("POST", "/orders/{id}", "POST /orders/{id}")] + [InlineData("CUSTOM", "/orders/{id}", "HTTP /orders/{id}")] + public void GetActivityDisplayNameReturnsExpectedValue(string method, string? route, string expected) + { + var requestHelper = new RequestDataHelper(configureByHttpKnownMethodsEnvironmentalVariable: false); + + var actual = requestHelper.GetActivityDisplayName(method, route); + + Assert.Equal(expected, actual); + } + + [Theory] + [InlineData("GET", "GET", null)] + [InlineData("CUSTOM", "HTTP", "CUSTOM")] + public void SetActivityDisplayNameAndHttpMethodTagSetsExpectedValues(string method, string expectedDisplayName, string? expectedOriginalMethod) + { + var requestHelper = new RequestDataHelper(configureByHttpKnownMethodsEnvironmentalVariable: false); + using var activity = new Activity("operation"); + + requestHelper.SetActivityDisplayNameAndHttpMethodTag(activity, method); + + Assert.Equal(expectedDisplayName, activity.DisplayName); + Assert.Equal(expectedDisplayName == "HTTP" ? "_OTHER" : method, activity.GetTagItem(SemanticConventions.AttributeHttpRequestMethod)); + Assert.Equal(expectedOriginalMethod, activity.GetTagItem(SemanticConventions.AttributeHttpRequestMethodOriginal)); + } + +#if NET + [Fact] + public void GetActivityDisplayNameCachesRouteDisplayNames() + { + var requestHelper = new RequestDataHelper(configureByHttpKnownMethodsEnvironmentalVariable: false); + + var first = requestHelper.GetActivityDisplayName("GET", "/orders/{id}"); + var second = requestHelper.GetActivityDisplayName("GET", "/orders/{id}"); + + Assert.Same(first, second); + } +#endif + [Theory] [InlineData("HTTP/1.1", "1.1")] [InlineData("HTTP/2", "2")] diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Benchmarks/AspNetCoreBenchmarks.cs b/test/OpenTelemetry.Instrumentation.AspNetCore.Benchmarks/AspNetCoreBenchmarks.cs new file mode 100644 index 0000000000..9a778ad346 --- /dev/null +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Benchmarks/AspNetCoreBenchmarks.cs @@ -0,0 +1,146 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +using BenchmarkDotNet.Attributes; +using Greet; +using Grpc.Core; +using Grpc.Net.Client; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.TestHost; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using OpenTelemetry.Metrics; +using OpenTelemetry.Trace; + +namespace OpenTelemetry.Instrumentation.AspNetCore.Benchmarks; + +[MemoryDiagnoser] +public class AspNetCoreBenchmarks +{ + private static readonly Uri BaseAddress = new("/", UriKind.Relative); + + private readonly HelloRequest helloRequest = new(); + private HttpClient? httpClient; + private Greeter.GreeterClient? grpcClient; + private WebApplication? app; + private TracerProvider? tracerProvider; + private MeterProvider? meterProvider; + + [Flags] + public enum EnableInstrumentationOption + { + /// + /// Instrumentation is not enabled for any signal. + /// + None = 0, + + /// + /// Instrumentation is enabled only for Traces. + /// + Traces = 1, + + /// + /// Instrumentation is enabled only for Metrics. + /// + Metrics = 2, + } + + [Params(EnableInstrumentationOption.None, EnableInstrumentationOption.Traces, EnableInstrumentationOption.Metrics, EnableInstrumentationOption.Traces | EnableInstrumentationOption.Metrics)] + public EnableInstrumentationOption EnableInstrumentation { get; set; } + + [GlobalSetup] + public async Task StartServer() + { + await this.StartWebApplicationAsync(); + + KeyValuePair[] config = + [ + new("OTEL_DOTNET_EXPERIMENTAL_ASPNETCORE_ENABLE_GRPC_INSTRUMENTATION", "true"), + ]; + + IConfiguration configuration = new ConfigurationBuilder() + .AddInMemoryCollection(config) + .Build(); + + if (this.EnableInstrumentation.HasFlag(EnableInstrumentationOption.Traces)) + { + this.tracerProvider = Sdk.CreateTracerProviderBuilder() + .ConfigureServices((services) => services.AddSingleton(configuration)) + .AddAspNetCoreInstrumentation() + .Build(); + } + + if (this.EnableInstrumentation.HasFlag(EnableInstrumentationOption.Metrics)) + { + var exportedItems = new List(); + + this.meterProvider = Sdk.CreateMeterProviderBuilder() + .ConfigureServices((services) => services.AddSingleton(configuration)) + .AddAspNetCoreInstrumentation() + .AddInMemoryExporter(exportedItems) + .Build(); + } + } + + [GlobalCleanup] + public async Task StopServer() + { + this.httpClient?.Dispose(); + + if (this.app != null) + { + await this.app.StopAsync(); + await this.app.DisposeAsync(); + } + + this.tracerProvider?.Dispose(); + this.meterProvider?.Dispose(); + } + + [Benchmark] + public async Task HttpGet() + { + using var httpResponse = await this.httpClient!.GetAsync(BaseAddress).ConfigureAwait(false); + httpResponse.EnsureSuccessStatusCode(); + } + + [Benchmark] + public async Task GrpcGet() => + await this.grpcClient!.SayHelloAsync(this.helloRequest); + + private async Task StartWebApplicationAsync() + { + var builder = WebApplication.CreateBuilder(); + + builder.Logging.ClearProviders(); + builder.WebHost.UseTestServer(); + + builder.Services.AddGrpc(); + + var app = builder.Build(); + + app.MapGet("/", async context => await context.Response.WriteAsync("Hello World!")); + + app.MapGrpcService(); + + await app.StartAsync(); + + this.app = app; + this.httpClient = app.GetTestClient(); + + var channel = GrpcChannel.ForAddress("http://localhost", new() + { + HttpClient = this.httpClient, + }); + + this.grpcClient = new Greeter.GreeterClient(channel); + } + + private sealed class GreeterService : Greeter.GreeterBase + { + public override Task SayHello(HelloRequest request, ServerCallContext context) => + Task.FromResult(new HelloReply { Message = "Hello " + request.Name }); + } +} diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Benchmarks/Instrumentation/AspNetCoreInstrumentationBenchmarks.cs b/test/OpenTelemetry.Instrumentation.AspNetCore.Benchmarks/Instrumentation/AspNetCoreInstrumentationBenchmarks.cs deleted file mode 100644 index a826051513..0000000000 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Benchmarks/Instrumentation/AspNetCoreInstrumentationBenchmarks.cs +++ /dev/null @@ -1,200 +0,0 @@ -// Copyright The OpenTelemetry Authors -// SPDX-License-Identifier: Apache-2.0 - -using BenchmarkDotNet.Attributes; -using Microsoft.AspNetCore.Builder; -using Microsoft.AspNetCore.Http; -using Microsoft.Extensions.Logging; -using OpenTelemetry.Metrics; -using OpenTelemetry.Trace; - -/* -BenchmarkDotNet=v0.13.5, OS=Windows 11 (10.0.22621.1702/22H2/2022Update/SunValley2) -Intel Core i7-8850H CPU 2.60GHz (Coffee Lake), 1 CPU, 12 logical and 6 physical cores -.NET SDK=7.0.203 - [Host] : .NET 7.0.5 (7.0.523.17405), X64 RyuJIT AVX2 - DefaultJob : .NET 7.0.5 (7.0.523.17405), X64 RyuJIT AVX2 - - -| Method | EnableInstrumentation | Mean | Error | StdDev | Gen0 | Allocated | -|--------------------------- |---------------------- |---------:|--------:|--------:|-------:|----------:| -| GetRequestForAspNetCoreApp | None | 136.8 us | 1.56 us | 1.46 us | 0.4883 | 2.45 KB | -| GetRequestForAspNetCoreApp | Traces | 148.1 us | 0.88 us | 0.82 us | 0.7324 | 3.57 KB | -| GetRequestForAspNetCoreApp | Metrics | 144.4 us | 1.16 us | 1.08 us | 0.4883 | 2.92 KB | -| GetRequestForAspNetCoreApp | Traces, Metrics | 163.0 us | 1.60 us | 1.49 us | 0.7324 | 3.63 KB | - -Allocation details for .NET 7: - -// Traces -* Activity creation + `Activity.Start()` = 416 B -* Casting of the struct `Microsoft.Extensions.Primitives.StringValues` to `IEnumerable` by `HttpRequestHeaderValuesGetter` - - `TraceContextPropagator.Extract` = 24 B - - `BaggageContextPropagator.Extract` = 24 B -* String creation for `HttpRequest.HostString.Host` = 40 B -* `Activity.TagsLinkedList` (this is allocated on the first Activity.SetTag call) = 40 B -* Boxing of `Port` number when adding it as a tag = 24 B -* String creation in `GetUri` method for adding the http url tag = 66 B -* Setting `Baggage` (Setting AsyncLocal values causes allocation) - - `BaggageHolder` creation = 24 B - - `System.Threading.AsyncLocalValueMap.TwoElementAsyncLocalValueMap` = 48 B - - `System.Threading.ExecutionContext` = 40 B -* `DiagNode>` - - This is allocated eight times for the eight tags that are added = 8 * 40 = 320 B -* `Activity.Stop()` trying to set `Activity.Current` (This happens because of setting another AsyncLocal variable which is `Baggage` - - System.Threading.AsyncLocalValueMap.OneElementAsyncLocalValueMap = 32 B - - System.Threading.ExecutionContext = 40 B - -Baseline = 2.45 KB -With Traces = 2.45 + (1138 / 1024) = 2.45 + 1.12 = 3.57 KB - - -// Metrics -* Activity creation + `Activity.Start()` = 416 B -* Boxing of `Port` number when adding it as a tag = 24 B -* String creation for `HttpRequest.HostString.Host` = 40 B - -Baseline = 2.45 KB -With Metrics = 2.45 + (416 + 40 + 24) / 1024 = 2.45 + 0.47 = 2.92 KB - -// With Traces and Metrics - -Baseline = 2.45 KB -With Traces and Metrics = Baseline + With Traces + (With Metrics - (Activity creation + `Acitivity.Stop()`)) (they use the same activity) - = 2.45 + (1138 + 64) / 1024 = 2.45 + 1.17 = ~3.63KB -*/ - -namespace OpenTelemetry.Instrumentation.AspNetCore.Benchmark.Instrumentation; - -public class AspNetCoreInstrumentationBenchmarks -{ - private HttpClient? httpClient; - private WebApplication? app; - private TracerProvider? tracerProvider; - private MeterProvider? meterProvider; - - [Flags] - public enum EnableInstrumentationOption - { - /// - /// Instrumentation is not enabled for any signal. - /// - None = 0, - - /// - /// Instrumentation is enabled only for Traces. - /// - Traces = 1, - - /// - /// Instrumentation is enabled only for Metrics. - /// - Metrics = 2, - } - - [Params(EnableInstrumentationOption.None, EnableInstrumentationOption.Traces, EnableInstrumentationOption.Metrics, EnableInstrumentationOption.Traces | EnableInstrumentationOption.Metrics)] - public EnableInstrumentationOption EnableInstrumentation { get; set; } - - [GlobalSetup(Target = nameof(GetRequestForAspNetCoreApp))] - public void GetRequestForAspNetCoreAppGlobalSetup() - { - if (this.EnableInstrumentation == EnableInstrumentationOption.None) - { - this.StartWebApplication(); - this.httpClient = new HttpClient(); - } - else if (this.EnableInstrumentation == EnableInstrumentationOption.Traces) - { - this.StartWebApplication(); - this.httpClient = new HttpClient(); - - this.tracerProvider = Sdk.CreateTracerProviderBuilder() - .AddAspNetCoreInstrumentation() - .Build(); - } - else if (this.EnableInstrumentation == EnableInstrumentationOption.Metrics) - { - this.StartWebApplication(); - this.httpClient = new HttpClient(); - - this.meterProvider = Sdk.CreateMeterProviderBuilder() - .AddAspNetCoreInstrumentation() - .Build(); - } - else if (this.EnableInstrumentation.HasFlag(EnableInstrumentationOption.Traces) && - this.EnableInstrumentation.HasFlag(EnableInstrumentationOption.Metrics)) - { - this.StartWebApplication(); - this.httpClient = new HttpClient(); - - this.tracerProvider = Sdk.CreateTracerProviderBuilder() - .AddAspNetCoreInstrumentation() - .Build(); - - this.meterProvider = Sdk.CreateMeterProviderBuilder() - .AddAspNetCoreInstrumentation() - .Build(); - } - } - - [GlobalCleanup(Target = nameof(GetRequestForAspNetCoreApp))] - public async Task GetRequestForAspNetCoreAppGlobalCleanup() - { - if (this.EnableInstrumentation == EnableInstrumentationOption.None) - { - this.httpClient?.Dispose(); - if (this.app != null) - { - await this.app.DisposeAsync(); - } - } - else if (this.EnableInstrumentation == EnableInstrumentationOption.Traces) - { - this.httpClient?.Dispose(); - if (this.app != null) - { - await this.app.DisposeAsync(); - } - - this.tracerProvider?.Dispose(); - } - else if (this.EnableInstrumentation == EnableInstrumentationOption.Metrics) - { - this.httpClient?.Dispose(); - if (this.app != null) - { - await this.app.DisposeAsync(); - } - - this.meterProvider?.Dispose(); - } - else if (this.EnableInstrumentation.HasFlag(EnableInstrumentationOption.Traces) && - this.EnableInstrumentation.HasFlag(EnableInstrumentationOption.Metrics)) - { - this.httpClient?.Dispose(); - if (this.app != null) - { - await this.app.DisposeAsync(); - } - - this.meterProvider?.Dispose(); - } - } - - [Benchmark] - public async Task GetRequestForAspNetCoreApp() - { - var httpResponse = await this.httpClient!.GetAsync(new Uri("http://localhost:5000")).ConfigureAwait(false); - httpResponse.EnsureSuccessStatusCode(); - } - - private void StartWebApplication() - { - var builder = WebApplication.CreateBuilder(); - builder.Logging.ClearProviders(); - var app = builder.Build(); - app.MapGet("/", async context => await context.Response.WriteAsync($"Hello World!")); - app.RunAsync(); - - this.app = app; - } -} diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Benchmarks/Instrumentation/AspNetCoreInstrumentationNewBenchmarks.cs b/test/OpenTelemetry.Instrumentation.AspNetCore.Benchmarks/Instrumentation/AspNetCoreInstrumentationNewBenchmarks.cs deleted file mode 100644 index 5e42243f74..0000000000 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Benchmarks/Instrumentation/AspNetCoreInstrumentationNewBenchmarks.cs +++ /dev/null @@ -1,222 +0,0 @@ -// Copyright The OpenTelemetry Authors -// SPDX-License-Identifier: Apache-2.0 - -using BenchmarkDotNet.Attributes; -using Microsoft.AspNetCore.Builder; -using Microsoft.AspNetCore.Http; -using Microsoft.Extensions.Configuration; -using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Logging; -using OpenTelemetry.Metrics; -using OpenTelemetry.Trace; - -/* -// * Summary * - -BenchmarkDotNet=v0.13.5, OS=Windows 11 (10.0.22621.1992/22H2/2022Update/SunValley2), VM=Hyper-V -AMD EPYC 7763, 1 CPU, 16 logical and 8 physical cores -.NET SDK=7.0.306 - [Host] : .NET 7.0.9 (7.0.923.32018), X64 RyuJIT AVX2 - DefaultJob : .NET 7.0.9 (7.0.923.32018), X64 RyuJIT AVX2 - - -| Method | EnableInstrumentation | Mean | Error | StdDev | Allocated | -|--------------------------- |---------------------- |---------:|--------:|--------:|----------:| -| GetRequestForAspNetCoreApp | None | 150.7 us | 1.68 us | 1.57 us | 2.45 KB | -| GetRequestForAspNetCoreApp | Traces | 156.6 us | 3.12 us | 6.37 us | 3.46 KB | -| GetRequestForAspNetCoreApp | Metrics | 148.8 us | 2.87 us | 2.69 us | 2.92 KB | -| GetRequestForAspNetCoreApp | Traces, Metrics | 164.0 us | 3.19 us | 6.22 us | 3.52 KB | - -Allocation details for .NET 7: - -// Traces -* Activity creation + `Activity.Start()` = 416 B -* Casting of the struct `Microsoft.Extensions.Primitives.StringValues` to `IEnumerable` by `HttpRequestHeaderValuesGetter` - - `TraceContextPropagator.Extract` = 24 B - - `BaggageContextPropagator.Extract` = 24 B -* String creation for `HttpRequest.HostString.Host` = 40 B -* `Activity.TagsLinkedList` (this is allocated on the first Activity.SetTag call) = 40 B -* Boxing of `Port` number when adding it as a tag = 24 B -* Setting `Baggage` (Setting AsyncLocal values causes allocation) - - `BaggageHolder` creation = 24 B - - `System.Threading.AsyncLocalValueMap.TwoElementAsyncLocalValueMap` = 48 B - - `System.Threading.ExecutionContext` = 40 B -* `DiagNode>` - - This is allocated seven times for the seven (eight if query string is available) tags that are added = 7 * 40 = 280 B -* `Activity.Stop()` trying to set `Activity.Current` (This happens because of setting another AsyncLocal variable which is `Baggage` - - System.Threading.AsyncLocalValueMap.OneElementAsyncLocalValueMap = 32 B - - System.Threading.ExecutionContext = 40 B - -Baseline = 2.45 KB -With Traces = 2.45 + (1032 / 1024) = 2.45 + 1.01 = 3.46 KB - - -// Metrics -* Activity creation + `Activity.Start()` = 416 B -* Boxing of `Port` number when adding it as a tag = 24 B -* String creation for `HttpRequest.HostString.Host` = 40 B - -Baseline = 2.45 KB -With Metrics = 2.45 + (416 + 40 + 24) / 1024 = 2.45 + 0.47 = 2.92 KB - -// With Traces and Metrics - -Baseline = 2.45 KB -With Traces and Metrics = Baseline + With Traces + (With Metrics - (Activity creation + `Acitivity.Stop()`)) (they use the same activity) - = 2.45 + (1032 + 64) / 1024 = 2.45 + 1.07 = ~3.52KB -*/ -namespace OpenTelemetry.Instrumentation.AspNetCore.Benchmarks.Instrumentation; - -public class AspNetCoreInstrumentationNewBenchmarks -{ - private HttpClient? httpClient; - private WebApplication? app; - private TracerProvider? tracerProvider; - private MeterProvider? meterProvider; - - [Flags] - public enum EnableInstrumentationOption - { - /// - /// Instrumentation is not enabled for any signal. - /// - None = 0, - - /// - /// Instrumentation is enabled only for Traces. - /// - Traces = 1, - - /// - /// Instrumentation is enabled only for Metrics. - /// - Metrics = 2, - } - - [Params(EnableInstrumentationOption.None, EnableInstrumentationOption.Traces, EnableInstrumentationOption.Metrics, EnableInstrumentationOption.Traces | EnableInstrumentationOption.Metrics)] - public EnableInstrumentationOption EnableInstrumentation { get; set; } - - [GlobalSetup(Target = nameof(GetRequestForAspNetCoreApp))] - public void GetRequestForAspNetCoreAppGlobalSetup() - { - KeyValuePair[] config = []; - var configuration = new ConfigurationBuilder() - .AddInMemoryCollection(config) - .Build(); - - if (this.EnableInstrumentation == EnableInstrumentationOption.None) - { - this.StartWebApplication(); - this.httpClient = new HttpClient(); - } - else if (this.EnableInstrumentation == EnableInstrumentationOption.Traces) - { - this.StartWebApplication(); - this.httpClient = new HttpClient(); - - this.tracerProvider = Sdk.CreateTracerProviderBuilder() - .ConfigureServices(services => services.AddSingleton(configuration)) - .AddAspNetCoreInstrumentation() - .Build(); - } - else if (this.EnableInstrumentation == EnableInstrumentationOption.Metrics) - { - this.StartWebApplication(); - this.httpClient = new HttpClient(); - - var exportedItems = new List(); - this.meterProvider = Sdk.CreateMeterProviderBuilder() - .ConfigureServices(services => services.AddSingleton(configuration)) - .AddAspNetCoreInstrumentation() - .AddInMemoryExporter(exportedItems, metricReaderOptions => - { - metricReaderOptions.PeriodicExportingMetricReaderOptions.ExportIntervalMilliseconds = 1000; - }) - .Build(); - } - else if (this.EnableInstrumentation.HasFlag(EnableInstrumentationOption.Traces) && - this.EnableInstrumentation.HasFlag(EnableInstrumentationOption.Metrics)) - { - this.StartWebApplication(); - this.httpClient = new HttpClient(); - - this.tracerProvider = Sdk.CreateTracerProviderBuilder() - .ConfigureServices(services => services.AddSingleton(configuration)) - .AddAspNetCoreInstrumentation() - .Build(); - - var exportedItems = new List(); - this.meterProvider = Sdk.CreateMeterProviderBuilder() - .ConfigureServices(services => services.AddSingleton(configuration)) - .AddAspNetCoreInstrumentation() - .AddInMemoryExporter(exportedItems, metricReaderOptions => - { - metricReaderOptions.PeriodicExportingMetricReaderOptions.ExportIntervalMilliseconds = 1000; - }) - .Build(); - } - } - - [GlobalCleanup(Target = nameof(GetRequestForAspNetCoreApp))] - public async Task GetRequestForAspNetCoreAppGlobalCleanup() - { - if (this.EnableInstrumentation == EnableInstrumentationOption.None) - { - this.httpClient?.Dispose(); - if (this.app != null) - { - await this.app.DisposeAsync(); - } - } - else if (this.EnableInstrumentation == EnableInstrumentationOption.Traces) - { - this.httpClient?.Dispose(); - if (this.app != null) - { - await this.app.DisposeAsync(); - } - - this.tracerProvider?.Dispose(); - } - else if (this.EnableInstrumentation == EnableInstrumentationOption.Metrics) - { - this.httpClient?.Dispose(); - if (this.app != null) - { - await this.app.DisposeAsync(); - } - - this.meterProvider?.Dispose(); - } - else if (this.EnableInstrumentation.HasFlag(EnableInstrumentationOption.Traces) && - this.EnableInstrumentation.HasFlag(EnableInstrumentationOption.Metrics)) - { - this.httpClient?.Dispose(); - if (this.app != null) - { - await this.app.DisposeAsync(); - } - - this.tracerProvider?.Dispose(); - this.meterProvider?.Dispose(); - } - } - - [Benchmark] - public async Task GetRequestForAspNetCoreApp() - { - var httpResponse = await this.httpClient!.GetAsync(new Uri("http://localhost:5000")).ConfigureAwait(false); - httpResponse.EnsureSuccessStatusCode(); - } - - private void StartWebApplication() - { - var builder = WebApplication.CreateBuilder(); - builder.Logging.ClearProviders(); - var app = builder.Build(); - app.MapGet("/", async context => await context.Response.WriteAsync($"Hello World!")); - app.RunAsync(); - - this.app = app; - } -} diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Benchmarks/OpenTelemetry.Instrumentation.AspNetCore.Benchmarks.csproj b/test/OpenTelemetry.Instrumentation.AspNetCore.Benchmarks/OpenTelemetry.Instrumentation.AspNetCore.Benchmarks.csproj index ea0aa66c99..ba091755cd 100644 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Benchmarks/OpenTelemetry.Instrumentation.AspNetCore.Benchmarks.csproj +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Benchmarks/OpenTelemetry.Instrumentation.AspNetCore.Benchmarks.csproj @@ -5,12 +5,21 @@ Exe + + + + + + + + + diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Benchmarks/Program.cs b/test/OpenTelemetry.Instrumentation.AspNetCore.Benchmarks/Program.cs index 5e1c1a3d17..9be0226363 100644 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Benchmarks/Program.cs +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Benchmarks/Program.cs @@ -2,10 +2,6 @@ // SPDX-License-Identifier: Apache-2.0 using BenchmarkDotNet.Running; +using OpenTelemetry.Instrumentation.AspNetCore.Benchmarks; -namespace OpenTelemetry.Instrumentation.AspNetCore.Benchmarks; - -internal class Program -{ - private static void Main(string[] args) => BenchmarkSwitcher.FromAssembly(typeof(Program).Assembly).Run(args); -} +BenchmarkSwitcher.FromAssembly(typeof(AspNetCoreBenchmarks).Assembly).Run(args); diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Benchmarks/Proto/greet.proto b/test/OpenTelemetry.Instrumentation.AspNetCore.Benchmarks/Proto/greet.proto new file mode 100644 index 0000000000..c9ab263f18 --- /dev/null +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Benchmarks/Proto/greet.proto @@ -0,0 +1,29 @@ +// Copyright 2019 The gRPC Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +syntax = "proto3"; + +package greet; + +service Greeter { + rpc SayHello (HelloRequest) returns (HelloReply); +} + +message HelloRequest { + string name = 1; +} + +message HelloReply { + string message = 1; +} diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Benchmarks/README.md b/test/OpenTelemetry.Instrumentation.AspNetCore.Benchmarks/README.md index 797799c290..e08fd21f38 100644 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Benchmarks/README.md +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Benchmarks/README.md @@ -1,11 +1,8 @@ # OpenTelemetry ASP.NET Core Instrumentation Benchmarks -Navigate to `./test/OpenTelemetry.Instrumentation.AspNetCore.Benchmarks` directory -and run the following command: +Navigate to the `./test/OpenTelemetry.Instrumentation.AspNetCore.Benchmarks` +directory and run the following command to run all of the benchmarks: ```sh -dotnet run -c Release -f net10.0 -- -m -`` - -Then choose the benchmark class that you want to run by entering the required -option number from the list of options shown on the Console window. +dotnet run --configuration Release --framework net10.0 -- --filter "*" +``` diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs index 2e9d1b07d9..e2d2689d25 100644 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs @@ -725,7 +725,6 @@ public async Task ActivitiesStartedInMiddlewareBySettingHostActivityToNullShould Assert.Equal("Microsoft.AspNetCore.Hosting.HttpRequestIn", aspnetcoreframeworkactivity.OperationName); } -#if NET [Fact] public async Task UserRegisteredActivitySourceIsUsedForActivityCreationByAspNetCore() { @@ -766,7 +765,6 @@ void ConfigureTestServices(IServiceCollection services) Assert.Equal("UserRegisteredActivitySource", activity.Source.Name); } -#endif [Theory] [InlineData(1)] @@ -1332,14 +1330,9 @@ private static void WaitForActivityExport(List exportedItems, int coun private static void ValidateAspNetCoreActivity(Activity activityToValidate, string expectedHttpPath) { Assert.Equal(ActivityKind.Server, activityToValidate.Kind); -#if NET Assert.Equal(HttpInListener.AspNetCoreActivitySourceName, activityToValidate.Source.Name); Assert.NotNull(activityToValidate.Source.Version); Assert.Empty(activityToValidate.Source.Version); -#else - Assert.Equal(HttpInListener.ActivitySourceName, activityToValidate.Source.Name); - Assert.Equal(HttpInListener.Version.ToString(), activityToValidate.Source.Version); -#endif Assert.Equal(expectedHttpPath, activityToValidate.GetTagValue(SemanticConventions.AttributeUrlPath) as string); } diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/GrpcTests.cs b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/GrpcTests.cs new file mode 100644 index 0000000000..609b8deb45 --- /dev/null +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/GrpcTests.cs @@ -0,0 +1,120 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +using System.Diagnostics; +using System.Net; +using Microsoft.AspNetCore.Http; +using OpenTelemetry.Instrumentation.AspNetCore.Implementation; +using OpenTelemetry.Trace; +using Xunit; + +namespace OpenTelemetry.Instrumentation.AspNetCore.Tests; + +public class GrpcTests +{ + [Fact] + public void OnStopActivityAddsGrpcAttributesForParsedMethodAndValidStatusCode() + { + // Arrange + var listener = CreateListener(); + using var activity = CreateActivity("/package.Service/Method", "13"); + var context = CreateContext(IPAddress.Loopback, remotePort: 4317); + + // Act + listener.OnStopActivity(activity, context); + + // Assert + Assert.Equal("package.Service/Method", activity.DisplayName); + Assert.Equal(ActivityStatusCode.Error, activity.Status); + + AssertTag(activity, GrpcTagHelper.GrpcMethodTagName, null); + AssertTag(activity, GrpcTagHelper.GrpcStatusCodeTagName, null); + AssertTag(activity, SemanticConventions.AttributeClientAddress, "127.0.0.1"); + AssertTag(activity, SemanticConventions.AttributeClientPort, 4317); + AssertTag(activity, SemanticConventions.AttributeHttpResponseStatusCode, 200); + AssertTag(activity, SemanticConventions.AttributeRpcGrpcStatusCode, 13); + AssertTag(activity, SemanticConventions.AttributeRpcMethod, "Method"); + AssertTag(activity, SemanticConventions.AttributeRpcService, "package.Service"); + AssertTag(activity, SemanticConventions.AttributeRpcSystem, "grpc"); + } + + [Fact] + public void OnStopActivityPreservesGrpcTagsWhenMethodCannotBeParsedAndStatusIsInvalid() + { + // Arrange + var listener = CreateListener(); + using var activity = CreateActivity("Invalid", "invalid"); + var context = CreateContext(remoteIpAddress: null, remotePort: 4317); + + // Act + listener.OnStopActivity(activity, context); + + // Assert + Assert.Equal("Invalid", activity.DisplayName); + Assert.Equal(ActivityStatusCode.Unset, activity.Status); + + AssertTag(activity, GrpcTagHelper.GrpcMethodTagName, "Invalid"); + AssertTag(activity, GrpcTagHelper.GrpcStatusCodeTagName, "invalid"); + AssertTag(activity, SemanticConventions.AttributeClientAddress, null); + AssertTag(activity, SemanticConventions.AttributeClientPort, 4317); + AssertTag(activity, SemanticConventions.AttributeRpcGrpcStatusCode, null); + AssertTag(activity, SemanticConventions.AttributeRpcMethod, null); + AssertTag(activity, SemanticConventions.AttributeRpcService, null); + AssertTag(activity, SemanticConventions.AttributeRpcSystem, "grpc"); + } + + [Fact] + public void OnStopActivityIgnoresEmptyGrpcMethodTag() + { + // Arrange + var listener = CreateListener(); + using var activity = CreateActivity(string.Empty, "13"); + var context = CreateContext(IPAddress.Loopback, remotePort: 4317); + + // Act + listener.OnStopActivity(activity, context); + + // Assert + Assert.Equal("operation", activity.DisplayName); + Assert.Equal(ActivityStatusCode.Unset, activity.Status); + + AssertTag(activity, GrpcTagHelper.GrpcMethodTagName, string.Empty); + AssertTag(activity, GrpcTagHelper.GrpcStatusCodeTagName, "13"); + AssertTag(activity, SemanticConventions.AttributeRpcGrpcStatusCode, null); + AssertTag(activity, SemanticConventions.AttributeRpcSystem, null); + } + + private static void AssertTag(Activity activity, string name, object? expected) => + Assert.Equal(expected, activity.GetTagValue(name)); + + private static HttpInListener CreateListener() => + new(new AspNetCoreTraceInstrumentationOptions + { + EnableGrpcAspNetCoreSupport = true, + }); + + private static Activity CreateActivity(string grpcMethod, string grpcStatusCode) + { + var activity = new Activity("operation") + { + IsAllDataRequested = true, + }; + + activity.SetTag(GrpcTagHelper.GrpcMethodTagName, grpcMethod); + activity.SetTag(GrpcTagHelper.GrpcStatusCodeTagName, grpcStatusCode); + + return activity; + } + + private static DefaultHttpContext CreateContext(IPAddress? remoteIpAddress, int remotePort) + { + var context = new DefaultHttpContext(); + + context.Connection.RemoteIpAddress = remoteIpAddress; + context.Connection.RemotePort = remotePort; + context.Request.Protocol = "HTTP/2"; + context.Response.StatusCode = StatusCodes.Status200OK; + + return context; + } +} diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/MetricTests.cs b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/MetricTests.cs index 0ea35508eb..68853382f2 100644 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/MetricTests.cs +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/MetricTests.cs @@ -1,22 +1,13 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 -#if NET using System.Threading.RateLimiting; using Microsoft.AspNetCore.Builder; -#endif using Microsoft.AspNetCore.Hosting; -#if NET using Microsoft.AspNetCore.Http; -#endif using Microsoft.AspNetCore.Mvc.Testing; -#if NET using Microsoft.AspNetCore.RateLimiting; -#endif -#if NET using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Hosting; -#endif using Microsoft.Extensions.Logging; using OpenTelemetry.Metrics; using OpenTelemetry.Trace; @@ -38,7 +29,6 @@ public void AddAspNetCoreInstrumentation_BadArgs() Assert.Throws(builder!.AddAspNetCoreInstrumentation); } -#if NET [Fact] public async Task ValidateNetMetricsAsync() { @@ -178,7 +168,6 @@ static string GetTicks() await app.DisposeAsync(); } -#endif [Theory] [InlineData("/api/values/2", "api/Values/{id}", null, 200)] diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/TestApplication/TestApplicationFactory.cs b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/TestApplication/TestApplicationFactory.cs index a0e99e5775..478fe00c05 100644 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/TestApplication/TestApplicationFactory.cs +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/TestApplication/TestApplicationFactory.cs @@ -129,11 +129,9 @@ private static WebApplication CreateMinimalApiApplication() app.MapGet("/MinimalApi", () => Results.Ok()); app.MapGet("/MinimalApi/{id}", (int id) => Results.Ok()); -#if NET var api = app.MapGroup("/MinimalApiUsingMapGroup"); api.MapGet("/", () => Results.Ok()); api.MapGet("/{id}", (int id) => Results.Ok()); -#endif return app; }