From 2629aa6bfbbf44846184338c787bbc566ca58584 Mon Sep 17 00:00:00 2001 From: martincostello Date: Thu, 7 May 2026 10:32:51 +0100 Subject: [PATCH] [AspNetCore] Optimize HttpInListener Add optimizations, recommended by Copilot, to reduce allocations in `HttpInListener.OnStopActivity()`: - Cache activity display names. - Uses slices instead of `Regex`. - Avoid enum reflection. - Null check delegates to avoid try-catch blocks. - Use SetCustomProperty to check for who made the instrumentation. - Avoid running gRPC-related code for HTTP 1.1. - Avoid additional check for the normalized HTTP method. - Use pattern matching on nullable. --- .../Implementation/HttpInListener.cs | 207 ++++++++++++------ src/Shared/GrpcStatusCanonicalCode.cs | 5 + src/Shared/GrpcTagHelper.cs | 37 ++-- src/Shared/RequestDataHelper.cs | 33 ++- .../GrpcTagHelperTests.cs | 2 + .../RequestDataHelperTests.cs | 43 ++++ .../GrpcTests.cs | 120 ++++++++++ 7 files changed, 360 insertions(+), 87 deletions(-) create mode 100644 test/OpenTelemetry.Instrumentation.AspNetCore.Tests/GrpcTests.cs diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs index c58d22e54d..96cb77adee 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs @@ -1,8 +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.Globalization; using System.Runtime.CompilerServices; using Microsoft.AspNetCore.Http; using OpenTelemetry.Context.Propagation; @@ -29,6 +31,7 @@ internal class HttpInListener : ListenerHandler internal static readonly bool Net11OrGreater = Environment.Version.Major >= 11; private const string DiagnosticSourceName = "Microsoft.AspNetCore"; + private const string CreatedByInstrumentationPropertyName = "OpenTelemetry.AspNetCore.CreatedByInstrumentation"; private static readonly Func> HttpRequestHeaderValuesGetter = (request, name) => { @@ -42,6 +45,11 @@ 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 GrpcMethodDetailsCache GrpcMethodCache = new(); private readonly AspNetCoreTraceInstrumentationOptions options; private readonly bool nativeAspNetCoreOpenTelemetryEnabled; @@ -128,7 +136,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(); @@ -145,23 +153,30 @@ 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); + DisableActivity(activity); + return; + } + } + catch (Exception ex) + { + AspNetCoreInstrumentationEventSource.Log.RequestFilterException(nameof(HttpInListener), nameof(this.OnStartActivity), activity.OperationName, ex); + DisableActivity(activity); + return; + } + + static void DisableActivity(Activity activity) { - AspNetCoreInstrumentationEventSource.Log.RequestIsFilteredOut(nameof(HttpInListener), nameof(this.OnStartActivity), activity.OperationName); 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) { @@ -169,12 +184,10 @@ public void OnStartActivity(Activity activity, object? payload) ActivityInstrumentationHelper.SetKindProperty(activity, ActivityKind.Server); } - TelemetryHelper.RequestDataHelper.SetActivityDisplayName(activity, request.Method); - // 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. // See https://github.com/dotnet/aspnetcore/issues/65873. - TelemetryHelper.RequestDataHelper.SetHttpMethodTag(activity, request.Method); + TelemetryHelper.RequestDataHelper.SetActivityDisplayNameAndHttpMethodTag(activity, request.Method); if (!Net10OrGreater || !this.nativeAspNetCoreOpenTelemetryEnabled) { @@ -218,13 +231,16 @@ public void OnStartActivity(Activity activity, object? payload) activity.SetTag(SemanticConventions.AttributeNetworkProtocolVersion, RequestDataHelper.GetHttpProtocolVersion(request.Protocol)); - try + if (this.options.EnrichWithHttpRequest is { } enricher) { - this.options.EnrichWithHttpRequest?.Invoke(activity, request); - } - catch (Exception ex) - { - AspNetCoreInstrumentationEventSource.Log.EnrichmentException(nameof(HttpInListener), nameof(this.OnStartActivity), activity.OperationName, ex); + try + { + enricher(activity, request); + } + catch (Exception ex) + { + AspNetCoreInstrumentationEventSource.Log.EnrichmentException(nameof(HttpInListener), nameof(this.OnStartActivity), activity.OperationName, ex); + } } } } @@ -252,9 +268,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 +306,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 +358,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 +384,14 @@ 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.Get(grpcMethod); - [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,10 +420,10 @@ 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); } } } @@ -419,4 +452,52 @@ private static bool AspNetCoreHasNativeOpenTelemetryTags() // see https://github.com/dotnet/aspnetcore/blob/655f41d52f2fc75992eac41496b8e9cc119e1b54/src/Hosting/Hosting/src/Internal/HostingApplicationDiagnostics.cs#L59-L67. return Net11OrGreater; } + + [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; } + } + + private sealed class GrpcMethodDetailsCache + { + private const int MaxCacheSize = 512; + private readonly ConcurrentDictionary cache = new(); + + public GrpcMethodDetails Get(string grpcMethod) + { + if (this.cache.TryGetValue(grpcMethod, out var details)) + { + return details; + } + + // If the cache has reached its maximum size, just create a value without caching + return this.cache.Count >= MaxCacheSize ? Create(grpcMethod) : this.cache.GetOrAdd(grpcMethod, Create); + } + + private static GrpcMethodDetails Create(string 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); + } + } } 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 84112bd1da..7b17a1b74f 100644 --- a/src/Shared/GrpcTagHelper.cs +++ b/src/Shared/GrpcTagHelper.cs @@ -3,12 +3,11 @@ using System.Diagnostics; using System.Globalization; -using System.Text.RegularExpressions; using OpenTelemetry.Trace; namespace OpenTelemetry.Instrumentation; -internal static partial class GrpcTagHelper +internal static class GrpcTagHelper { public const string RpcSystemGrpc = "grpc"; @@ -30,19 +29,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; } /// @@ -56,7 +61,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 { @@ -94,7 +99,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 @@ -113,16 +118,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..c263192884 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 (namePrefix, httpRoute) pair. + // The number of distinct combinations is bounded by the number of (HTTP method name prefixes * 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.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; + } +}