diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationMeterProviderBuilderExtensions.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationMeterProviderBuilderExtensions.cs index 3da8c1de59d..c4eec2ca5a6 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationMeterProviderBuilderExtensions.cs +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationMeterProviderBuilderExtensions.cs @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 #if !NET8_0_OR_GREATER +using Microsoft.Extensions.DependencyInjection; using OpenTelemetry.Instrumentation.AspNetCore; using OpenTelemetry.Instrumentation.AspNetCore.Implementation; #endif @@ -27,13 +28,15 @@ public static MeterProviderBuilder AddAspNetCoreInstrumentation( #if NET8_0_OR_GREATER return builder.ConfigureMeters(); #else - // Note: Warm-up the status code and method mapping. + // Note: Warm-up the status code. _ = TelemetryHelper.BoxedStatusCodes; - _ = RequestMethodHelper.KnownMethods; + + builder.ConfigureServices(RequestMethodHelper.RegisterServices); builder.AddMeter(HttpInMetricsListener.InstrumentationName); - builder.AddInstrumentation(new AspNetCoreMetrics()); + builder.AddInstrumentation(sp => new AspNetCoreMetrics( + sp.GetRequiredService())); return builder; #endif diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationTracerProviderBuilderExtensions.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationTracerProviderBuilderExtensions.cs index d37d77ae3fa..52b1e7ebf1c 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationTracerProviderBuilderExtensions.cs +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationTracerProviderBuilderExtensions.cs @@ -50,9 +50,8 @@ public static TracerProviderBuilder AddAspNetCoreInstrumentation( { Guard.ThrowIfNull(builder); - // Note: Warm-up the status code and method mapping. + // Note: Warm-up the status code. _ = TelemetryHelper.BoxedStatusCodes; - _ = RequestMethodHelper.KnownMethods; name ??= Options.DefaultName; @@ -64,6 +63,7 @@ public static TracerProviderBuilder AddAspNetCoreInstrumentation( } services.RegisterOptionsFactory(configuration => new AspNetCoreTraceInstrumentationOptions(configuration)); + RequestMethodHelper.RegisterServices(services); }); if (builder is IDeferredTracerProviderBuilder deferredTracerProviderBuilder) @@ -76,10 +76,10 @@ public static TracerProviderBuilder AddAspNetCoreInstrumentation( return builder.AddInstrumentation(sp => { - var options = sp.GetRequiredService>().Get(name); - return new AspNetCoreInstrumentation( - new HttpInListener(options)); + new HttpInListener( + sp.GetRequiredService>().Get(name), + sp.GetRequiredService())); }); } diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreMetrics.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreMetrics.cs index a819d561a9e..333a282b49d 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreMetrics.cs +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreMetrics.cs @@ -3,6 +3,7 @@ #if !NET8_0_OR_GREATER using OpenTelemetry.Instrumentation.AspNetCore.Implementation; +using OpenTelemetry.Internal; namespace OpenTelemetry.Instrumentation.AspNetCore; @@ -25,9 +26,9 @@ internal sealed class AspNetCoreMetrics : IDisposable private readonly DiagnosticSourceSubscriber diagnosticSourceSubscriber; - internal AspNetCoreMetrics() + internal AspNetCoreMetrics(RequestMethodHelper requestMethodHelper) { - var metricsListener = new HttpInMetricsListener("Microsoft.AspNetCore"); + var metricsListener = new HttpInMetricsListener("Microsoft.AspNetCore", requestMethodHelper); this.diagnosticSourceSubscriber = new DiagnosticSourceSubscriber(metricsListener, this.isEnabled, AspNetCoreInstrumentationEventSource.Log.UnknownErrorProcessingEvent); this.diagnosticSourceSubscriber.Subscribe(); } diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs index a9aefacc005..9817b37d4a6 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs @@ -58,13 +58,18 @@ internal class HttpInListener : ListenerHandler private readonly PropertyFetcher beforeActionTemplateFetcher = new("Template"); #endif private readonly AspNetCoreTraceInstrumentationOptions options; + private readonly RequestMethodHelper requestMethodHelper; - public HttpInListener(AspNetCoreTraceInstrumentationOptions options) + public HttpInListener( + AspNetCoreTraceInstrumentationOptions options, + RequestMethodHelper requestMethodHelper) : base(DiagnosticSourceName) { - Guard.ThrowIfNull(options); + Debug.Assert(options != null, "options was null"); + Debug.Assert(requestMethodHelper != null, "requestMethodHelper was null"); this.options = options; + this.requestMethodHelper = requestMethodHelper; } public override void OnEventWritten(string name, object payload) @@ -105,8 +110,7 @@ public void OnStartActivity(Activity activity, object payload) // By this time, samplers have already run and // activity.IsAllDataRequested populated accordingly. - HttpContext context = payload as HttpContext; - if (context == null) + if (payload is not HttpContext context) { AspNetCoreInstrumentationEventSource.Log.NullPayload(nameof(HttpInListener), nameof(this.OnStartActivity), activity.OperationName); return; @@ -176,7 +180,7 @@ public void OnStartActivity(Activity activity, object payload) #endif var path = (request.PathBase.HasValue || request.Path.HasValue) ? (request.PathBase + request.Path).ToString() : "/"; - activity.DisplayName = GetDisplayName(request.Method); + activity.DisplayName = this.GetDisplayName(request.Method); // see the spec https://github.com/open-telemetry/semantic-conventions/blob/v1.23.0/docs/http/http-spans.md @@ -196,7 +200,7 @@ public void OnStartActivity(Activity activity, object payload) activity.SetTag(SemanticConventions.AttributeUrlQuery, request.QueryString.Value); } - RequestMethodHelper.SetHttpMethodTag(activity, request.Method); + this.requestMethodHelper.SetHttpMethodTag(activity, request.Method); activity.SetTag(SemanticConventions.AttributeUrlScheme, request.Scheme); activity.SetTag(SemanticConventions.AttributeUrlPath, path); @@ -226,8 +230,7 @@ public void OnStopActivity(Activity activity, object payload) { if (activity.IsAllDataRequested) { - HttpContext context = payload as HttpContext; - if (context == null) + if (payload is not HttpContext context) { AspNetCoreInstrumentationEventSource.Log.NullPayload(nameof(HttpInListener), nameof(this.OnStopActivity), activity.OperationName); return; @@ -239,7 +242,7 @@ public void OnStopActivity(Activity activity, object payload) var routePattern = (context.GetEndpoint() as RouteEndpoint)?.RoutePattern.RawText; if (!string.IsNullOrEmpty(routePattern)) { - activity.DisplayName = GetDisplayName(context.Request.Method, routePattern); + activity.DisplayName = this.GetDisplayName(context.Request.Method, routePattern); activity.SetTag(SemanticConventions.AttributeHttpRoute, routePattern); } #endif @@ -382,9 +385,9 @@ private static void AddGrpcAttributes(Activity activity, string grpcMethod, Http } } - private static string GetDisplayName(string httpMethod, string httpRoute = null) + private string GetDisplayName(string httpMethod, string httpRoute = null) { - var normalizedMethod = RequestMethodHelper.GetNormalizedHttpMethod(httpMethod); + var normalizedMethod = this.requestMethodHelper.GetNormalizedHttpMethod(httpMethod); return string.IsNullOrEmpty(httpRoute) ? normalizedMethod diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInMetricsListener.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInMetricsListener.cs index c2afcd02a81..11fab9b0a68 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInMetricsListener.cs +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInMetricsListener.cs @@ -37,12 +37,39 @@ internal sealed class HttpInMetricsListener : ListenerHandler private static readonly Histogram HttpServerRequestDuration = Meter.CreateHistogram(HttpServerRequestDurationMetricName, "s", "Duration of HTTP server requests."); - internal HttpInMetricsListener(string name) + private readonly RequestMethodHelper requestMethodHelper; + + internal HttpInMetricsListener( + string name, + RequestMethodHelper requestMethodHelper) : base(name) { + Debug.Assert(requestMethodHelper != null, "requestMethodHelper was null"); + + this.requestMethodHelper = requestMethodHelper; + } + + public override void OnEventWritten(string name, object payload) + { + switch (name) + { + case OnUnhandledDiagnosticsExceptionEvent: + case OnUnhandledHostingExceptionEvent: + { + OnExceptionEventWritten(name, payload); + } + + break; + case OnStopEvent: + { + this.OnStopEventWritten(name, payload); + } + + break; + } } - public static void OnExceptionEventWritten(string name, object payload) + private static void OnExceptionEventWritten(string name, object payload) { // We need to use reflection here as the payload type is not a defined public type. if (!TryFetchException(payload, out Exception exc) || !TryFetchHttpContext(payload, out HttpContext ctx)) @@ -68,12 +95,11 @@ static bool TryFetchHttpContext(object payload, out HttpContext ctx) => HttpContextPropertyFetcher.TryFetch(payload, out ctx) && ctx != null; } - public static void OnStopEventWritten(string name, object payload) + private void OnStopEventWritten(string name, object payload) { - var context = payload as HttpContext; - if (context == null) + if (payload is not HttpContext context) { - AspNetCoreInstrumentationEventSource.Log.NullPayload(nameof(HttpInMetricsListener), nameof(OnStopEventWritten), HttpServerRequestDurationMetricName); + AspNetCoreInstrumentationEventSource.Log.NullPayload(nameof(HttpInMetricsListener), nameof(this.OnStopEventWritten), HttpServerRequestDurationMetricName); return; } @@ -84,8 +110,9 @@ public static void OnStopEventWritten(string name, object payload) tags.Add(new KeyValuePair(SemanticConventions.AttributeUrlScheme, context.Request.Scheme)); tags.Add(new KeyValuePair(SemanticConventions.AttributeHttpResponseStatusCode, TelemetryHelper.GetBoxedStatusCode(context.Response.StatusCode))); - var httpMethod = RequestMethodHelper.GetNormalizedHttpMethod(context.Request.Method); - tags.Add(new KeyValuePair(SemanticConventions.AttributeHttpRequestMethod, httpMethod)); + tags.Add(new KeyValuePair( + SemanticConventions.AttributeHttpRequestMethod, + this.requestMethodHelper.GetNormalizedHttpMethod(context.Request.Method))); #if NET6_0_OR_GREATER var route = (context.GetEndpoint() as RouteEndpoint)?.RoutePattern.RawText; @@ -104,24 +131,4 @@ public static void OnStopEventWritten(string name, object payload) // TODO: Follow up with .NET team if we can continue to rely on this behavior. HttpServerRequestDuration.Record(Activity.Current.Duration.TotalSeconds, tags); } - - public override void OnEventWritten(string name, object payload) - { - switch (name) - { - case OnUnhandledDiagnosticsExceptionEvent: - case OnUnhandledHostingExceptionEvent: - { - OnExceptionEventWritten(name, payload); - } - - break; - case OnStopEvent: - { - OnStopEventWritten(name, payload); - } - - break; - } - } } diff --git a/src/OpenTelemetry.Instrumentation.Http/HttpClientInstrumentation.cs b/src/OpenTelemetry.Instrumentation.Http/HttpClientInstrumentation.cs index 80a84bb57c3..b96ee807837 100644 --- a/src/OpenTelemetry.Instrumentation.Http/HttpClientInstrumentation.cs +++ b/src/OpenTelemetry.Instrumentation.Http/HttpClientInstrumentation.cs @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 using OpenTelemetry.Instrumentation.Http.Implementation; +using OpenTelemetry.Internal; namespace OpenTelemetry.Instrumentation.Http; @@ -31,11 +32,9 @@ internal sealed class HttpClientInstrumentation : IDisposable private readonly Func isEnabledNet7OrGreater = (eventName, _, _) => !ExcludedDiagnosticSourceEventsNet7OrGreater.Contains(eventName); - /// - /// Initializes a new instance of the class. - /// - /// Configuration options for HTTP client instrumentation. - public HttpClientInstrumentation(HttpClientTraceInstrumentationOptions options) + public HttpClientInstrumentation( + HttpClientTraceInstrumentationOptions options, + RequestMethodHelper requestMethodHelper) { // For .NET7.0 activity will be created using activitySource. // https://github.com/dotnet/runtime/blob/main/src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHandler.cs @@ -45,11 +44,17 @@ public HttpClientInstrumentation(HttpClientTraceInstrumentationOptions options) // so that the sampler's decision is respected. if (HttpHandlerDiagnosticListener.IsNet7OrGreater) { - this.diagnosticSourceSubscriber = new DiagnosticSourceSubscriber(new HttpHandlerDiagnosticListener(options), this.isEnabledNet7OrGreater, HttpInstrumentationEventSource.Log.UnknownErrorProcessingEvent); + this.diagnosticSourceSubscriber = new DiagnosticSourceSubscriber( + new HttpHandlerDiagnosticListener(options, requestMethodHelper), + this.isEnabledNet7OrGreater, + HttpInstrumentationEventSource.Log.UnknownErrorProcessingEvent); } else { - this.diagnosticSourceSubscriber = new DiagnosticSourceSubscriber(new HttpHandlerDiagnosticListener(options), this.isEnabled, HttpInstrumentationEventSource.Log.UnknownErrorProcessingEvent); + this.diagnosticSourceSubscriber = new DiagnosticSourceSubscriber( + new HttpHandlerDiagnosticListener(options, requestMethodHelper), + this.isEnabled, + HttpInstrumentationEventSource.Log.UnknownErrorProcessingEvent); } this.diagnosticSourceSubscriber.Subscribe(); diff --git a/src/OpenTelemetry.Instrumentation.Http/HttpClientInstrumentationMeterProviderBuilderExtensions.cs b/src/OpenTelemetry.Instrumentation.Http/HttpClientInstrumentationMeterProviderBuilderExtensions.cs index 24b9524696f..e90632f1e08 100644 --- a/src/OpenTelemetry.Instrumentation.Http/HttpClientInstrumentationMeterProviderBuilderExtensions.cs +++ b/src/OpenTelemetry.Instrumentation.Http/HttpClientInstrumentationMeterProviderBuilderExtensions.cs @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 #if !NET8_0_OR_GREATER +using Microsoft.Extensions.DependencyInjection; #if !NETFRAMEWORK using OpenTelemetry.Instrumentation.Http; #endif @@ -32,16 +33,26 @@ public static MeterProviderBuilder AddHttpClientInstrumentation( .AddMeter("System.Net.Http") .AddMeter("System.Net.NameResolution"); #else - // Note: Warm-up the status code and method mapping. + // Note: Warm-up the status code. _ = TelemetryHelper.BoxedStatusCodes; - _ = RequestMethodHelper.KnownMethods; + + builder.ConfigureServices(RequestMethodHelper.RegisterServices); #if NETFRAMEWORK builder.AddMeter(HttpWebRequestActivitySource.MeterName); + + if (builder is IDeferredMeterProviderBuilder deferredMeterProviderBuilder) + { + deferredMeterProviderBuilder.Configure((sp, builder) => + { + HttpWebRequestActivitySource.RequestMethodHelper = sp.GetRequiredService(); + }); + } #else builder.AddMeter(HttpHandlerMetricsDiagnosticListener.MeterName); - builder.AddInstrumentation(new HttpClientMetrics()); + builder.AddInstrumentation(sp => new HttpClientMetrics( + sp.GetRequiredService())); #endif return builder; #endif diff --git a/src/OpenTelemetry.Instrumentation.Http/HttpClientInstrumentationTracerProviderBuilderExtensions.cs b/src/OpenTelemetry.Instrumentation.Http/HttpClientInstrumentationTracerProviderBuilderExtensions.cs index 6015c183f2c..e70d1d9e9c2 100644 --- a/src/OpenTelemetry.Instrumentation.Http/HttpClientInstrumentationTracerProviderBuilderExtensions.cs +++ b/src/OpenTelemetry.Instrumentation.Http/HttpClientInstrumentationTracerProviderBuilderExtensions.cs @@ -47,9 +47,8 @@ public static TracerProviderBuilder AddHttpClientInstrumentation( { Guard.ThrowIfNull(builder); - // Note: Warm-up the status code and method mapping. + // Note: Warm-up the status code. _ = TelemetryHelper.BoxedStatusCodes; - _ = RequestMethodHelper.KnownMethods; name ??= Options.DefaultName; @@ -59,6 +58,8 @@ public static TracerProviderBuilder AddHttpClientInstrumentation( { services.Configure(name, configureHttpClientTraceInstrumentationOptions); } + + RequestMethodHelper.RegisterServices(services); }); #if NETFRAMEWORK @@ -68,9 +69,8 @@ public static TracerProviderBuilder AddHttpClientInstrumentation( { deferredTracerProviderBuilder.Configure((sp, builder) => { - var options = sp.GetRequiredService>().Get(name); - - HttpWebRequestActivitySource.TracingOptions = options; + HttpWebRequestActivitySource.TracingOptions = sp.GetRequiredService>().Get(name); + HttpWebRequestActivitySource.RequestMethodHelper = sp.GetRequiredService(); }); } #else @@ -78,9 +78,9 @@ public static TracerProviderBuilder AddHttpClientInstrumentation( builder.AddInstrumentation(sp => { - var options = sp.GetRequiredService>().Get(name); - - return new HttpClientInstrumentation(options); + return new HttpClientInstrumentation( + sp.GetRequiredService>().Get(name), + sp.GetRequiredService()); }); #endif return builder; diff --git a/src/OpenTelemetry.Instrumentation.Http/HttpClientMetrics.cs b/src/OpenTelemetry.Instrumentation.Http/HttpClientMetrics.cs index f3773ed8fdc..373892f87a8 100644 --- a/src/OpenTelemetry.Instrumentation.Http/HttpClientMetrics.cs +++ b/src/OpenTelemetry.Instrumentation.Http/HttpClientMetrics.cs @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 using OpenTelemetry.Instrumentation.Http.Implementation; +using OpenTelemetry.Internal; namespace OpenTelemetry.Instrumentation.Http; @@ -21,13 +22,10 @@ internal sealed class HttpClientMetrics : IDisposable private readonly Func isEnabled = (activityName, obj1, obj2) => !ExcludedDiagnosticSourceEvents.Contains(activityName); - /// - /// Initializes a new instance of the class. - /// - public HttpClientMetrics() + public HttpClientMetrics(RequestMethodHelper requestMethodHelper) { this.diagnosticSourceSubscriber = new DiagnosticSourceSubscriber( - new HttpHandlerMetricsDiagnosticListener("HttpHandlerDiagnosticListener"), + new HttpHandlerMetricsDiagnosticListener("HttpHandlerDiagnosticListener", requestMethodHelper), this.isEnabled, HttpInstrumentationEventSource.Log.UnknownErrorProcessingEvent); this.diagnosticSourceSubscriber.Subscribe(); diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs index e0625b420db..84474a7f604 100644 --- a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs +++ b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs @@ -15,7 +15,9 @@ namespace OpenTelemetry.Instrumentation.Http.Implementation; -internal sealed class HttpHandlerDiagnosticListener : ListenerHandler +internal sealed class HttpHandlerDiagnosticListener( + HttpClientTraceInstrumentationOptions options, + RequestMethodHelper requestMethodHelper) : ListenerHandler("HttpHandlerDiagnosticListener") { internal static readonly AssemblyName AssemblyName = typeof(HttpHandlerDiagnosticListener).Assembly.GetName(); internal static readonly bool IsNet7OrGreater; @@ -34,7 +36,8 @@ internal sealed class HttpHandlerDiagnosticListener : ListenerHandler private static readonly PropertyFetcher StopResponseFetcher = new("Response"); private static readonly PropertyFetcher StopExceptionFetcher = new("Exception"); private static readonly PropertyFetcher StopRequestStatusFetcher = new("RequestTaskStatus"); - private readonly HttpClientTraceInstrumentationOptions options; + private readonly HttpClientTraceInstrumentationOptions options = options; + private readonly RequestMethodHelper requestMethodHelper = requestMethodHelper; static HttpHandlerDiagnosticListener() { @@ -48,12 +51,6 @@ static HttpHandlerDiagnosticListener() } } - public HttpHandlerDiagnosticListener(HttpClientTraceInstrumentationOptions options) - : base("HttpHandlerDiagnosticListener") - { - this.options = options; - } - public override void OnEventWritten(string name, object payload) { switch (name) @@ -135,7 +132,7 @@ public void OnStartActivity(Activity activity, object payload) return; } - RequestMethodHelper.SetHttpClientActivityDisplayName(activity, request.Method.Method); + this.requestMethodHelper.SetHttpClientActivityDisplayName(activity, request.Method.Method); if (!IsNet7OrGreater) { @@ -144,7 +141,7 @@ public void OnStartActivity(Activity activity, object payload) } // see the spec https://github.com/open-telemetry/semantic-conventions/blob/v1.23.0/docs/http/http-spans.md - RequestMethodHelper.SetHttpMethodTag(activity, request.Method.Method); + this.requestMethodHelper.SetHttpMethodTag(activity, request.Method.Method); activity.SetTag(SemanticConventions.AttributeServerAddress, request.RequestUri.Host); if (!request.RequestUri.IsDefaultPort) diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs index c6a710e2dd0..6f6b1328544 100644 --- a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs +++ b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs @@ -15,7 +15,9 @@ namespace OpenTelemetry.Instrumentation.Http.Implementation; -internal sealed class HttpHandlerMetricsDiagnosticListener : ListenerHandler +internal sealed class HttpHandlerMetricsDiagnosticListener( + string name, + RequestMethodHelper requestMethodHelper) : ListenerHandler(name) { internal const string OnStopEvent = "System.Net.Http.HttpRequestOut.Stop"; @@ -34,20 +36,75 @@ internal sealed class HttpHandlerMetricsDiagnosticListener : ListenerHandler private static readonly HttpRequestOptionsKey HttpRequestOptionsErrorKey = new(SemanticConventions.AttributeErrorType); #endif - public HttpHandlerMetricsDiagnosticListener(string name) - : base(name) + private readonly RequestMethodHelper requestMethodHelper = requestMethodHelper; + + public override void OnEventWritten(string name, object payload) + { + if (name == OnStopEvent) + { + this.OnStopEventWritten(Activity.Current, payload); + } + else if (name == OnUnhandledExceptionEvent) + { + OnExceptionEventWritten(Activity.Current, payload); + } + } + + private static void OnExceptionEventWritten(Activity activity, object payload) { + if (!TryFetchException(payload, out Exception exc) || !TryFetchRequest(payload, out HttpRequestMessage request)) + { + HttpInstrumentationEventSource.Log.NullPayload(nameof(HttpHandlerMetricsDiagnosticListener), nameof(OnExceptionEventWritten)); + return; + } + +#if !NET6_0_OR_GREATER + request.Properties.Add(SemanticConventions.AttributeErrorType, exc.GetType().FullName); +#else + request.Options.Set(HttpRequestOptionsErrorKey, exc.GetType().FullName); +#endif + + // The AOT-annotation DynamicallyAccessedMembers in System.Net.Http library ensures that top-level properties on the payload object are always preserved. + // see https://github.com/dotnet/runtime/blob/f9246538e3d49b90b0e9128d7b1defef57cd6911/src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHandler.cs#L325 +#if NET6_0_OR_GREATER + [UnconditionalSuppressMessage("Trimming", "IL2026", Justification = "The System.Net.Http library guarantees that top-level properties are preserved")] +#endif + static bool TryFetchException(object payload, out Exception exc) + { + if (!StopExceptionFetcher.TryFetch(payload, out exc) || exc == null) + { + return false; + } + + return true; + } + + // The AOT-annotation DynamicallyAccessedMembers in System.Net.Http library ensures that top-level properties on the payload object are always preserved. + // see https://github.com/dotnet/runtime/blob/f9246538e3d49b90b0e9128d7b1defef57cd6911/src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHandler.cs#L325 +#if NET6_0_OR_GREATER + [UnconditionalSuppressMessage("Trimming", "IL2026", Justification = "The System.Net.Http library guarantees that top-level properties are preserved")] +#endif + static bool TryFetchRequest(object payload, out HttpRequestMessage request) + { + if (!RequestFetcher.TryFetch(payload, out request) || request == null) + { + return false; + } + + return true; + } } - public static void OnStopEventWritten(Activity activity, object payload) + private void OnStopEventWritten(Activity activity, object payload) { if (TryFetchRequest(payload, out HttpRequestMessage request)) { // see the spec https://github.com/open-telemetry/semantic-conventions/blob/v1.23.0/docs/http/http-metrics.md TagList tags = default; - var httpMethod = RequestMethodHelper.GetNormalizedHttpMethod(request.Method.Method); - tags.Add(new KeyValuePair(SemanticConventions.AttributeHttpRequestMethod, httpMethod)); + tags.Add(new KeyValuePair( + SemanticConventions.AttributeHttpRequestMethod, + this.requestMethodHelper.GetNormalizedHttpMethod(request.Method.Method))); tags.Add(new KeyValuePair(SemanticConventions.AttributeServerAddress, request.RequestUri.Host)); tags.Add(new KeyValuePair(SemanticConventions.AttributeUrlScheme, request.RequestUri.Scheme)); @@ -108,61 +165,4 @@ static bool TryFetchRequest(object payload, out HttpRequestMessage request) => static bool TryFetchResponse(object payload, out HttpResponseMessage response) => StopResponseFetcher.TryFetch(payload, out response) && response != null; } - - public static void OnExceptionEventWritten(Activity activity, object payload) - { - if (!TryFetchException(payload, out Exception exc) || !TryFetchRequest(payload, out HttpRequestMessage request)) - { - HttpInstrumentationEventSource.Log.NullPayload(nameof(HttpHandlerMetricsDiagnosticListener), nameof(OnExceptionEventWritten)); - return; - } - -#if !NET6_0_OR_GREATER - request.Properties.Add(SemanticConventions.AttributeErrorType, exc.GetType().FullName); -#else - request.Options.Set(HttpRequestOptionsErrorKey, exc.GetType().FullName); -#endif - - // The AOT-annotation DynamicallyAccessedMembers in System.Net.Http library ensures that top-level properties on the payload object are always preserved. - // see https://github.com/dotnet/runtime/blob/f9246538e3d49b90b0e9128d7b1defef57cd6911/src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHandler.cs#L325 -#if NET6_0_OR_GREATER - [UnconditionalSuppressMessage("Trimming", "IL2026", Justification = "The System.Net.Http library guarantees that top-level properties are preserved")] -#endif - static bool TryFetchException(object payload, out Exception exc) - { - if (!StopExceptionFetcher.TryFetch(payload, out exc) || exc == null) - { - return false; - } - - return true; - } - - // The AOT-annotation DynamicallyAccessedMembers in System.Net.Http library ensures that top-level properties on the payload object are always preserved. - // see https://github.com/dotnet/runtime/blob/f9246538e3d49b90b0e9128d7b1defef57cd6911/src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHandler.cs#L325 -#if NET6_0_OR_GREATER - [UnconditionalSuppressMessage("Trimming", "IL2026", Justification = "The System.Net.Http library guarantees that top-level properties are preserved")] -#endif - static bool TryFetchRequest(object payload, out HttpRequestMessage request) - { - if (!RequestFetcher.TryFetch(payload, out request) || request == null) - { - return false; - } - - return true; - } - } - - public override void OnEventWritten(string name, object payload) - { - if (name == OnStopEvent) - { - OnStopEventWritten(Activity.Current, payload); - } - else if (name == OnUnhandledExceptionEvent) - { - OnExceptionEventWritten(Activity.Current, payload); - } - } } diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs index ea8e88abb5f..4a6600db4fa 100644 --- a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs +++ b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs @@ -9,6 +9,7 @@ using System.Reflection; using System.Reflection.Emit; using System.Runtime.CompilerServices; +using Microsoft.Extensions.Configuration; using OpenTelemetry.Context.Propagation; using OpenTelemetry.Internal; using OpenTelemetry.Trace; @@ -38,6 +39,7 @@ internal static class HttpWebRequestActivitySource private static readonly Histogram HttpClientRequestDuration = WebRequestMeter.CreateHistogram("http.client.request.duration", "s", "Measures the duration of outbound HTTP requests."); private static HttpClientTraceInstrumentationOptions tracingOptions; + private static RequestMethodHelper requestMethodHelper; // Fields for reflection private static FieldInfo connectionGroupListField; @@ -75,6 +77,7 @@ static HttpWebRequestActivitySource() PerformInjection(); TracingOptions = new HttpClientTraceInstrumentationOptions(); + RequestMethodHelper = new RequestMethodHelper(new ConfigurationBuilder().AddEnvironmentVariables().Build()); } catch (Exception ex) { @@ -88,10 +91,23 @@ internal static HttpClientTraceInstrumentationOptions TracingOptions get => tracingOptions; set { + Debug.Assert(value != null, "value was null"); + tracingOptions = value; } } + internal static RequestMethodHelper RequestMethodHelper + { + get => requestMethodHelper; + set + { + Debug.Assert(value != null, "value was null"); + + requestMethodHelper = value; + } + } + [MethodImpl(MethodImplOptions.AggressiveInlining)] private static void AddRequestTagsAndInstrumentRequest(HttpWebRequest request, Activity activity) { @@ -359,12 +375,11 @@ private static void ProcessResult(IAsyncResult asyncResult, AsyncCallback asyncC // Disposed HttpWebResponse throws when accessing properties, so let's make a copy of the data to ensure that doesn't happen. HttpWebResponse responseCopy = httpWebResponseCtor( - new object[] - { + [ uriAccessor(response), verbAccessor(response), coreResponseDataAccessor(response), mediaTypeAccessor(response), usesProxySemanticsAccessor(response), DecompressionMethods.None, isWebSocketResponseAccessor(response), connectionGroupNameAccessor(response), - }); + ]); if (activity != null) { @@ -425,8 +440,9 @@ private static void ProcessResult(IAsyncResult asyncResult, AsyncCallback asyncC TagList tags = default; - var httpMethod = RequestMethodHelper.GetNormalizedHttpMethod(request.Method); - tags.Add(new KeyValuePair(SemanticConventions.AttributeHttpRequestMethod, httpMethod)); + tags.Add(new KeyValuePair( + SemanticConventions.AttributeHttpRequestMethod, + RequestMethodHelper.GetNormalizedHttpMethod(request.Method))); tags.Add(SemanticConventions.AttributeServerAddress, request.RequestUri.Host); tags.Add(SemanticConventions.AttributeUrlScheme, request.RequestUri.Scheme); diff --git a/src/OpenTelemetry.Instrumentation.Http/OpenTelemetry.Instrumentation.Http.csproj b/src/OpenTelemetry.Instrumentation.Http/OpenTelemetry.Instrumentation.Http.csproj index 55158028cde..b50c28698ed 100644 --- a/src/OpenTelemetry.Instrumentation.Http/OpenTelemetry.Instrumentation.Http.csproj +++ b/src/OpenTelemetry.Instrumentation.Http/OpenTelemetry.Instrumentation.Http.csproj @@ -12,9 +12,11 @@ + - + + @@ -28,6 +30,10 @@ + + + + diff --git a/src/Shared/RequestMethodHelper.cs b/src/Shared/RequestMethodHelper.cs index 05a3f022511..e7a20c5601f 100644 --- a/src/Shared/RequestMethodHelper.cs +++ b/src/Shared/RequestMethodHelper.cs @@ -1,59 +1,80 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 +#nullable enable + #if NET8_0_OR_GREATER using System.Collections.Frozen; #endif using System.Diagnostics; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.DependencyInjection.Extensions; using OpenTelemetry.Trace; namespace OpenTelemetry.Internal; -internal static class RequestMethodHelper +internal sealed class RequestMethodHelper { // The value "_OTHER" is used for non-standard HTTP methods. // https://github.com/open-telemetry/semantic-conventions/blob/v1.22.0/docs/http/http-spans.md#common-attributes public const string OtherHttpMethod = "_OTHER"; -#if NET8_0_OR_GREATER - internal static readonly FrozenDictionary KnownMethods; -#else - internal static readonly Dictionary KnownMethods; -#endif + private static readonly List DefaultKnownMethods = new() + { + { "GET" }, + { "PUT" }, + { "POST" }, + { "DELETE" }, + { "HEAD" }, + { "OPTIONS" }, + { "TRACE" }, + { "PATCH" }, + { "CONNECT" }, + }; - static RequestMethodHelper() + public RequestMethodHelper(IConfiguration configuration) { - var knownMethodSet = new Dictionary(StringComparer.OrdinalIgnoreCase) + Debug.Assert(configuration != null, "configuration was null"); + + if (configuration!.TryGetStringValue("OTEL_INSTRUMENTATION_HTTP_KNOWN_METHODS", out var knownHttpMethods)) { - { "GET", "GET" }, - { "PUT", "PUT" }, - { "POST", "POST" }, - { "DELETE", "DELETE" }, - { "HEAD", "HEAD" }, - { "OPTIONS", "OPTIONS" }, - { "TRACE", "TRACE" }, - { "PATCH", "PATCH" }, - { "CONNECT", "CONNECT" }, - }; + var splitArray = knownHttpMethods!.Split(',') + .Select(x => x.Trim()) + .Where(x => !string.IsNullOrEmpty(x)) + .ToList(); + + this.KnownMethods = GetKnownMethods(splitArray); + } + else + { + this.KnownMethods = GetKnownMethods(DefaultKnownMethods); + } + } - // KnownMethods ignores case. Use the value returned by the dictionary to have a consistent case. #if NET8_0_OR_GREATER - KnownMethods = FrozenDictionary.ToFrozenDictionary(knownMethodSet, StringComparer.OrdinalIgnoreCase); + public FrozenDictionary KnownMethods { get; private set; } #else - KnownMethods = knownMethodSet; + public Dictionary KnownMethods { get; private set; } #endif + + public static void RegisterServices(IServiceCollection services) + { + Debug.Assert(services != null, "services was null"); + + services!.TryAddSingleton(); } - public static string GetNormalizedHttpMethod(string method) + public string GetNormalizedHttpMethod(string method) { - return KnownMethods.TryGetValue(method, out var normalizedMethod) + return this.KnownMethods.TryGetValue(method, out var normalizedMethod) ? normalizedMethod : OtherHttpMethod; } - public static void SetHttpMethodTag(Activity activity, string method) + public void SetHttpMethodTag(Activity activity, string method) { - if (KnownMethods.TryGetValue(method, out var normalizedMethod)) + if (this.KnownMethods.TryGetValue(method, out var normalizedMethod)) { activity?.SetTag(SemanticConventions.AttributeHttpRequestMethod, normalizedMethod); } @@ -64,9 +85,26 @@ public static void SetHttpMethodTag(Activity activity, string method) } } - public static void SetHttpClientActivityDisplayName(Activity activity, string method) + public void SetHttpClientActivityDisplayName(Activity activity, string method) { // https://github.com/open-telemetry/semantic-conventions/blob/v1.23.0/docs/http/http-spans.md#name - activity.DisplayName = KnownMethods.TryGetValue(method, out var httpMethod) ? httpMethod : "HTTP"; + activity.DisplayName = this.KnownMethods.TryGetValue(method, out var httpMethod) ? httpMethod : "HTTP"; + } + +#if NET8_0_OR_GREATER + private static FrozenDictionary GetKnownMethods(List configuredKnownMethods) +#else + private static Dictionary GetKnownMethods(List configuredKnownMethods) +#endif + { + Debug.Assert(configuredKnownMethods != null, "configuredKnownMethods was null"); + + var knownMethods = configuredKnownMethods.ToDictionary(x => x, x => x, StringComparer.OrdinalIgnoreCase); + +#if NET8_0_OR_GREATER + return FrozenDictionary.ToFrozenDictionary(knownMethods, StringComparer.OrdinalIgnoreCase); +#else + return knownMethods.ToDictionary(x => x.Key, x => x.Value, StringComparer.OrdinalIgnoreCase); +#endif } } diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs index 2d3f6d2c21a..df7400c575c 100644 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs @@ -8,11 +8,13 @@ using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Testing; using Microsoft.AspNetCore.TestHost; +using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; using OpenTelemetry.Context.Propagation; using OpenTelemetry.Instrumentation.AspNetCore.Implementation; +using OpenTelemetry.Internal; using OpenTelemetry.Tests; using OpenTelemetry.Trace; using TestApp.AspNetCore; @@ -1024,6 +1026,74 @@ public async Task DiagnosticSourceExceptionCallBackIsNotReceivedForExceptionsHan Assert.True(exceptionHandled); } + [Theory] + [InlineData("GET", null)] + public async Task KnownHttpMethodsAreBeingRespected_Defaults(string expectedMethod, string expectedOriginalMethod) + { + var exportedItems = new List(); + + using var client = this.factory + .WithWebHostBuilder(builder => + { + builder.ConfigureLogging(loggingBuilder => loggingBuilder.ClearProviders()); + }) + .CreateClient(); + + this.tracerProvider = Sdk.CreateTracerProviderBuilder() + .AddAspNetCoreInstrumentation() + .AddInMemoryExporter(exportedItems) + .Build(); + + using var request = new HttpRequestMessage(HttpMethod.Get, "/api/values"); + using var response = await client.SendAsync(request); + + Assert.Single(exportedItems); + var activity = exportedItems[0]; + + Assert.Equal(expectedMethod, activity.GetTagValue(SemanticConventions.AttributeHttpRequestMethod) as string); + Assert.Equal(expectedOriginalMethod, activity.GetTagValue(SemanticConventions.AttributeHttpRequestMethodOriginal) as string); + } + + [Theory] + [InlineData("GET,POST,PUT", "GET", null)] + [InlineData("get,post,put", "get", null)] + [InlineData("POST,PUT", "_OTHER", "GET")] + [InlineData("post,put", "_OTHER", "GET")] + [InlineData("fooBar", "_OTHER", "GET")] + [InlineData("fooBar,POST", "_OTHER", "GET")] + [InlineData(",", "_OTHER", "GET")] + public async Task KnownHttpMethodsAreBeingRespected_EnvVar(string knownMethods, string expectedMethod, string expectedOriginalMethod) + { + var config = new KeyValuePair[] { new("OTEL_INSTRUMENTATION_HTTP_KNOWN_METHODS", knownMethods) }; + var configuration = new ConfigurationBuilder() + .AddInMemoryCollection(config) + .Build(); + + var exportedItems = new List(); + + this.tracerProvider = Sdk.CreateTracerProviderBuilder() + .AddAspNetCoreInstrumentation() + .ConfigureServices(services => services.AddSingleton(configuration)) + .AddInMemoryExporter(exportedItems) + .Build(); + + using var client = this.factory + .WithWebHostBuilder(builder => + { + builder.ConfigureLogging(loggingBuilder => loggingBuilder.ClearProviders()); + }) + .CreateClient(); + + using var request = new HttpRequestMessage(HttpMethod.Get, "/api/values"); + using var response = await client.SendAsync(request); + + Assert.Single(exportedItems); + var activity = exportedItems[0]; + + Assert.Equal(expectedMethod, activity.GetTagValue(SemanticConventions.AttributeHttpRequestMethod) as string); + Assert.Equal(expectedOriginalMethod, activity.GetTagValue(SemanticConventions.AttributeHttpRequestMethodOriginal) as string); + } + public void Dispose() { this.tracerProvider?.Dispose(); @@ -1119,7 +1189,8 @@ public override SamplingResult ShouldSample(in SamplingParameters samplingParame } } - private class TestHttpInListener(AspNetCoreTraceInstrumentationOptions options) : HttpInListener(options) + private class TestHttpInListener(AspNetCoreTraceInstrumentationOptions options) + : HttpInListener(options, new RequestMethodHelper(new ConfigurationBuilder().Build())) { public Action OnEventWrittenCallback; diff --git a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.cs b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.cs index ba758f25103..c0be57fcd4c 100644 --- a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.cs +++ b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.cs @@ -5,6 +5,7 @@ #if NETFRAMEWORK using System.Net.Http; #endif +using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using OpenTelemetry.Context.Propagation; using OpenTelemetry.Instrumentation.Http.Implementation; @@ -732,6 +733,64 @@ public async Task CustomPropagatorCalled(bool sample, bool createParentActivity) #endif } + [Theory] + [InlineData("GET", null)] + public async Task KnownHttpMethodsAreBeingRespected_Defaults(string expectedMethod, string expectedOriginalMethod) + { + var exportedItems = new List(); + + var tracerProvider = Sdk.CreateTracerProviderBuilder() + .AddHttpClientInstrumentation() + .AddInMemoryExporter(exportedItems) + .Build(); + + using var c = new HttpClient(); + await c.GetAsync(this.url); + + tracerProvider.Dispose(); + + Assert.Single(exportedItems); + var activity = exportedItems[0]; + + Assert.Equal(expectedMethod, activity.GetTagValue(SemanticConventions.AttributeHttpRequestMethod) as string); + Assert.Equal(expectedOriginalMethod, activity.GetTagValue(SemanticConventions.AttributeHttpRequestMethodOriginal) as string); + } + + [Theory] + [InlineData("GET,POST,PUT", "GET", null)] + [InlineData("get,post,put", "get", null)] + [InlineData("POST,PUT", "_OTHER", "GET")] + [InlineData("post,put", "_OTHER", "GET")] + [InlineData("fooBar", "_OTHER", "GET")] + [InlineData("fooBar,POST", "_OTHER", "GET")] + [InlineData(",", "_OTHER", "GET")] + public async Task KnownHttpMethodsAreBeingRespected_EnvVar(string knownMethods, string expectedMethod, string expectedOriginalMethod) + { + var config = new KeyValuePair[] { new("OTEL_INSTRUMENTATION_HTTP_KNOWN_METHODS", knownMethods) }; + var configuration = new ConfigurationBuilder() + .AddInMemoryCollection(config) + .Build(); + + var exportedItems = new List(); + + var tracerProvider = Sdk.CreateTracerProviderBuilder() + .AddHttpClientInstrumentation() + .ConfigureServices(services => services.AddSingleton(configuration)) + .AddInMemoryExporter(exportedItems) + .Build(); + + using var c = new HttpClient(); + await c.GetAsync(this.url); + + tracerProvider.Dispose(); + + Assert.Single(exportedItems); + var activity = exportedItems[0]; + + Assert.Equal(expectedMethod, activity.GetTagValue(SemanticConventions.AttributeHttpRequestMethod) as string); + Assert.Equal(expectedOriginalMethod, activity.GetTagValue(SemanticConventions.AttributeHttpRequestMethodOriginal) as string); + } + public void Dispose() { this.serverLifeTime?.Dispose();