diff --git a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/.publicApi/PublicAPI.Unshipped.txt b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/.publicApi/PublicAPI.Unshipped.txt index 9269618c42..84ef8ac2cb 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/.publicApi/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/.publicApi/PublicAPI.Unshipped.txt @@ -5,7 +5,7 @@ OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.TelemetryHttpModule() - OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModuleOptions OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModuleOptions.OnExceptionCallback.get -> System.Action? OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModuleOptions.OnExceptionCallback.set -> void -OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModuleOptions.OnRequestStartedCallback.get -> System.Action? +OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModuleOptions.OnRequestStartedCallback.get -> System.Func? OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModuleOptions.OnRequestStartedCallback.set -> void OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModuleOptions.OnRequestStoppedCallback.get -> System.Action? OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModuleOptions.OnRequestStoppedCallback.set -> void diff --git a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/ActivityHelper.cs b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/ActivityHelper.cs index d60f37c8e1..dddce54315 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/ActivityHelper.cs +++ b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/ActivityHelper.cs @@ -6,8 +6,6 @@ using System.Web; using OpenTelemetry.Context; using OpenTelemetry.Context.Propagation; -using OpenTelemetry.Internal; -using OpenTelemetry.Trace; namespace OpenTelemetry.Instrumentation.AspNet; @@ -16,31 +14,15 @@ namespace OpenTelemetry.Instrumentation.AspNet; /// internal static class ActivityHelper { - /// - /// for OpenTelemetry.Instrumentation.AspNet created objects. - /// - internal const string AspNetActivityName = "Microsoft.AspNet.HttpReqIn"; - /// /// Key to store the state in HttpContext. /// internal const string ContextKey = "__AspnetOpenTelemetryInstrumentationContext__"; - /// - /// OpenTelemetry.Instrumentation.AspNet name. - /// - internal const string AspNetSourceName = "OpenTelemetry.Instrumentation.AspNet"; - internal static readonly object StartedButNotSampledObj = new(); private const string BaggageSlotName = "otel.baggage"; private static readonly Func> HttpRequestHeaderValuesGetter = (request, name) => request.Headers.GetValues(name); - private static readonly ActivitySource AspNetSource = new( - AspNetSourceName, - typeof(ActivityHelper).Assembly.GetPackageVersion()); - - [ThreadStatic] - private static KeyValuePair[]? cachedTagsStorage; /// /// Try to get the started for the running /// . /// . - /// Callback action. + /// Function creating activity. /// New root activity. - public static Activity? StartAspNetActivity(TextMapPropagator textMapPropagator, HttpContextBase context, Action? onRequestStartedCallback) + public static Activity? StartAspNetActivity(TextMapPropagator textMapPropagator, HttpContextBase context, Func? onRequestStartedCallback) { var propagationContext = textMapPropagator.Extract(default, context.Request, HttpRequestHeaderValuesGetter); - KeyValuePair[]? tags; - if (context.Request?.Unvalidated?.Path is string path) - { - tags = cachedTagsStorage ??= new KeyValuePair[1]; - - tags[0] = new KeyValuePair(SemanticConventions.AttributeUrlPath, path); - } - else + Activity? activity = null; + try { - tags = null; + activity = onRequestStartedCallback?.Invoke(context, propagationContext.ActivityContext); } - - var activity = AspNetSource.StartActivity(AspNetActivityName, ActivityKind.Server, propagationContext.ActivityContext, tags); - - if (tags is not null) + catch (Exception callbackEx) { - tags[0] = default; + AspNetTelemetryEventSource.Log.CallbackException(activity, "Start", callbackEx); } if (activity != null) @@ -107,20 +80,10 @@ public static bool HasStarted(HttpContextBase context, out Activity? aspNetActiv context.Items[ContextKey] = new ContextHolder(activity); } - try - { - onRequestStartedCallback?.Invoke(activity, context); - } - catch (Exception callbackEx) - { - AspNetTelemetryEventSource.Log.CallbackException(activity, "OnStarted", callbackEx); - } - AspNetTelemetryEventSource.Log.ActivityStarted(activity); } else { - onRequestStartedCallback?.Invoke(activity, context); context.Items[ContextKey] = StartedButNotSampledObj; } diff --git a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/CHANGELOG.md index d763c36901..09aee59426 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/CHANGELOG.md @@ -5,6 +5,12 @@ * Updated OpenTelemetry core component version(s) to `1.13.0`. ([#3158](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/3158)) +* **Breaking Change**: This module is no longer responsible for creating activities. + The contract of `TelemetryHttpModuleOptions.OnRequestStartedCallback` was changed + to `Func?`. The consumer is now + responsible for providing function returning `Activity`. + ([#3151](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/3151)) + ## 1.12.0-beta.2 Released 2025-Sep-18 diff --git a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.csproj b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.csproj index a9281667fa..3d6554f219 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.csproj +++ b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.csproj @@ -19,6 +19,7 @@ + diff --git a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/TelemetryHttpModuleOptions.cs b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/TelemetryHttpModuleOptions.cs index e39c1d2ec1..2a19166f38 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/TelemetryHttpModuleOptions.cs +++ b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/TelemetryHttpModuleOptions.cs @@ -35,9 +35,10 @@ public TextMapPropagator TextMapPropagator } /// - /// Gets or sets a callback action to be fired when a request is started. + /// Gets or sets a callback function to be fired when a request is started. + /// This function should return the created to represent the request. /// - public Action? OnRequestStartedCallback { get; set; } + public Func? OnRequestStartedCallback { get; set; } /// /// Gets or sets a callback action to be fired when a request is stopped. diff --git a/src/OpenTelemetry.Instrumentation.AspNet/AspNetInstrumentation.cs b/src/OpenTelemetry.Instrumentation.AspNet/AspNetInstrumentation.cs index a60369a5b1..f12f4dfcba 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet/AspNetInstrumentation.cs +++ b/src/OpenTelemetry.Instrumentation.AspNet/AspNetInstrumentation.cs @@ -1,6 +1,7 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 +using System.Diagnostics; using System.Diagnostics.Metrics; using System.Reflection; using OpenTelemetry.Instrumentation.AspNet.Implementation; @@ -18,7 +19,9 @@ internal sealed class AspNetInstrumentation : IDisposable public static readonly Assembly Assembly = typeof(HttpInListener).Assembly; public static readonly AssemblyName AssemblyName = Assembly.GetName(); public static readonly string MeterName = AssemblyName.Name!; + public static readonly string ActivitySourceName = AssemblyName.Name; public static readonly Meter Meter = new(MeterName, Assembly.GetPackageVersion()); + public static readonly ActivitySource ActivitySource = new(ActivitySourceName, Assembly.GetPackageVersion()); public static readonly Histogram HttpServerDuration = Meter.CreateHistogram( "http.server.request.duration", unit: "s", diff --git a/src/OpenTelemetry.Instrumentation.AspNet/AspNetInstrumentationTracerProviderBuilderExtensions.cs b/src/OpenTelemetry.Instrumentation.AspNet/AspNetInstrumentationTracerProviderBuilderExtensions.cs index bf02cb529b..cdc4466222 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet/AspNetInstrumentationTracerProviderBuilderExtensions.cs +++ b/src/OpenTelemetry.Instrumentation.AspNet/AspNetInstrumentationTracerProviderBuilderExtensions.cs @@ -52,7 +52,7 @@ public static TracerProviderBuilder AddAspNetInstrumentation( { return AspNetInstrumentation.Instance.HandleManager.AddTracingHandle(); }); - tracerProviderBuilder.AddSource("OpenTelemetry.Instrumentation.AspNet"); + tracerProviderBuilder.AddSource(AspNetInstrumentation.ActivitySourceName); }); }); } diff --git a/src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md index ee169b2df3..8b88dd242f 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md @@ -5,6 +5,16 @@ * Updated OpenTelemetry core component version(s) to `1.13.0`. ([#3158](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/3158)) +* Following attributes are available while sampling: + * `http.request.method`, + * `server.address`, + * `server.port`, + * `url.path`, + * `url.query`, + * `url.scheme`, + * `user_agent.original`. + ([#3151](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/3151)) + ## 1.12.0-beta.2 Released 2025-Sep-18 diff --git a/src/OpenTelemetry.Instrumentation.AspNet/Implementation/HttpInListener.cs b/src/OpenTelemetry.Instrumentation.AspNet/Implementation/HttpInListener.cs index b6b0166169..70ee5464dd 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet/Implementation/HttpInListener.cs +++ b/src/OpenTelemetry.Instrumentation.AspNet/Implementation/HttpInListener.cs @@ -20,14 +20,14 @@ public HttpInListener() { TelemetryHttpModule.Options.TextMapPropagator = Propagators.DefaultTextMapPropagator; - TelemetryHttpModule.Options.OnRequestStartedCallback += this.OnStartActivity; + TelemetryHttpModule.Options.OnRequestStartedCallback += this.StartActivity; TelemetryHttpModule.Options.OnRequestStoppedCallback += this.OnStopActivity; TelemetryHttpModule.Options.OnExceptionCallback += this.OnException; } public void Dispose() { - TelemetryHttpModule.Options.OnRequestStartedCallback -= this.OnStartActivity; + TelemetryHttpModule.Options.OnRequestStartedCallback -= this.StartActivity; TelemetryHttpModule.Options.OnRequestStoppedCallback -= this.OnStopActivity; TelemetryHttpModule.Options.OnExceptionCallback -= this.OnException; } @@ -111,7 +111,7 @@ private void RecordDuration(Activity? activity, HttpContextBase context, Excepti AspNetInstrumentation.HttpServerDuration.Record(duration, tags); } - private void OnStartActivity(Activity? activity, HttpContextBase context) + private Activity? StartActivity(HttpContextBase context, ActivityContext activityContext) { if (AspNetInstrumentation.Instance.HandleManager.TracingHandles == 0) { @@ -122,16 +122,47 @@ private void OnStartActivity(Activity? activity, HttpContextBase context) this.beginTimestamp.Value = Stopwatch.GetTimestamp(); } - return; + return null; } - if (activity == null) + TagList tags = default; + + var request = context.Request; + var originalHttpMethod = request.HttpMethod; + var activityName = this.requestDataHelper.GetActivityDisplayName(originalHttpMethod); + this.requestDataHelper.SetHttpMethodTag(ref tags, originalHttpMethod); + + var url = request.Url; + tags.Add(SemanticConventions.AttributeServerAddress, url.Host); + tags.Add(SemanticConventions.AttributeServerPort, url.Port); + tags.Add(SemanticConventions.AttributeUrlScheme, url.Scheme); + + if (context.Request.Unvalidated?.Path is string path) { - AspNetInstrumentationEventSource.Log.NullActivity(nameof(this.OnStartActivity)); - return; + tags.Add(SemanticConventions.AttributeUrlPath, path); } + var userAgent = request.UserAgent; + if (!string.IsNullOrEmpty(userAgent)) + { + tags.Add(SemanticConventions.AttributeUserAgentOriginal, userAgent); + } + + var query = url.Query; var options = AspNetInstrumentation.Instance.TraceOptions; + if (!string.IsNullOrEmpty(query)) + { + var queryString = query.StartsWith("?", StringComparison.Ordinal) ? query.Substring(1) : query; + tags.Add(SemanticConventions.AttributeUrlQuery, options.DisableUrlQueryRedaction ? queryString : RedactionHelper.GetRedactedQueryString(queryString)); + } + + var activity = AspNetInstrumentation.ActivitySource.StartActivity(activityName, ActivityKind.Server, activityContext, tags); + + if (activity == null) + { + AspNetInstrumentationEventSource.Log.NullActivity(nameof(this.StartActivity)); + return null; + } if (activity.IsAllDataRequested) { @@ -148,7 +179,7 @@ private void OnStartActivity(Activity? activity, HttpContextBase context) AspNetInstrumentationEventSource.Log.RequestIsFilteredOut(activity.OperationName); activity.IsAllDataRequested = false; activity.ActivityTraceFlags &= ~ActivityTraceFlags.Recorded; - return; + return activity; } } catch (Exception ex) @@ -156,41 +187,15 @@ private void OnStartActivity(Activity? activity, HttpContextBase context) AspNetInstrumentationEventSource.Log.RequestFilterException(activity.OperationName, ex); activity.IsAllDataRequested = false; activity.ActivityTraceFlags &= ~ActivityTraceFlags.Recorded; - return; + return activity; } - var request = context.Request; - - // see the spec https://github.com/open-telemetry/semantic-conventions/blob/v1.24.0/docs/http/http-spans.md - var originalHttpMethod = request.HttpMethod; - this.requestDataHelper.SetActivityDisplayName(activity, originalHttpMethod); - - var url = request.Url; - activity.SetTag(SemanticConventions.AttributeServerAddress, url.Host); - activity.SetTag(SemanticConventions.AttributeServerPort, url.Port); - activity.SetTag(SemanticConventions.AttributeUrlScheme, url.Scheme); - - this.requestDataHelper.SetHttpMethodTag(activity, originalHttpMethod); - var protocolVersion = RequestDataHelperExtensions.GetHttpProtocolVersion(request); if (!string.IsNullOrEmpty(protocolVersion)) { activity.SetTag(SemanticConventions.AttributeNetworkProtocolVersion, protocolVersion); } - var query = url.Query; - if (!string.IsNullOrEmpty(query)) - { - var queryString = query.StartsWith("?", StringComparison.Ordinal) ? query.Substring(1) : query; - activity.SetTag(SemanticConventions.AttributeUrlQuery, options.DisableUrlQueryRedaction ? queryString : RedactionHelper.GetRedactedQueryString(queryString)); - } - - var userAgent = request.UserAgent; - if (!string.IsNullOrEmpty(userAgent)) - { - activity.SetTag(SemanticConventions.AttributeUserAgentOriginal, userAgent); - } - try { options.EnrichWithHttpRequest?.Invoke(activity, request); @@ -200,6 +205,8 @@ private void OnStartActivity(Activity? activity, HttpContextBase context) AspNetInstrumentationEventSource.Log.EnrichmentException("OnStartActivity", ex); } } + + return activity; } private void OnStopActivity(Activity? activity, HttpContextBase context) diff --git a/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlActivitySourceHelper.cs b/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlActivitySourceHelper.cs index f4d2f8b94d..ad6e3b3f69 100644 --- a/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlActivitySourceHelper.cs +++ b/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlActivitySourceHelper.cs @@ -52,7 +52,7 @@ public static TagList GetTagListFromConnectionInfo(string? dataSource, string? d ? MicrosoftSqlServerDbSystemName : MicrosoftSqlServerDbSystem; - var tags = new TagList { }; + TagList tags = default; if (options.EmitOldAttributes) { diff --git a/src/OpenTelemetry.Instrumentation.Wcf/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.Wcf/CHANGELOG.md index 6bab3815f0..0953a7ce0a 100644 --- a/src/OpenTelemetry.Instrumentation.Wcf/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.Wcf/CHANGELOG.md @@ -8,6 +8,11 @@ * Updated OpenTelemetry core component version(s) to `1.13.0`. ([#3158](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/3158)) +* **Breaking changes** Adjust to breaking changes from + `OpenTelemetry.Instrumentation.AspNet` version `1.12.0-beta.3`. + Fixing span hierarchy when hosted in ASP.NET. + ([#3151](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/3151)) + ## 1.12.0-beta.1 Released 2025-May-06 diff --git a/src/OpenTelemetry.Instrumentation.Wcf/Implementation/AspNetParentSpanCorrector.cs b/src/OpenTelemetry.Instrumentation.Wcf/Implementation/AspNetParentSpanCorrector.cs index 421fde3eac..cb661bc5c7 100644 --- a/src/OpenTelemetry.Instrumentation.Wcf/Implementation/AspNetParentSpanCorrector.cs +++ b/src/OpenTelemetry.Instrumentation.Wcf/Implementation/AspNetParentSpanCorrector.cs @@ -101,25 +101,74 @@ private static Func GenerateGetPropagatorLambda() private static Func GenerateSubscribeLambda() { // this method effectively generates this lambda: - // () => TelemetryHttpModule.Options.OnRequestStartedCallback += OnRequestStarted - // technically it generates this: - // () => TelemetryHttpModule.Options.OnRequestStartedCallback = - // (Action)addOurCallbackToDelegate(TelemetryHttpModule.Options.OnRequestStartedCallback); + // () => TelemetryHttpModule.Options.OnRequestStartedCallback = CreateCombinedCallback(TelemetryHttpModule.Options.OnRequestStartedCallback) + // The callback signature is Func var telemetryHttpModuleType = Type.GetType(TelemetryHttpModuleTypeName, true); var telemetryHttpModuleOptionsType = Type.GetType(TelemetryHttpModuleOptionsTypeName, true); - var onRequestStartedProp = telemetryHttpModuleOptionsType.GetProperty("OnRequestStartedCallback") ?? throw new NotSupportedException("TelemetryHttpModuleOptions.OnRequestStartedCallback property not found"); - Func addOurCallbackToDelegate = (existingCallback) => - { - var myCallback = OnRequestStarted; - var myCallbackProperlyTyped = Delegate.CreateDelegate(onRequestStartedProp.PropertyType, myCallback.Target, myCallback.Method); - return Delegate.Combine(existingCallback, myCallbackProperlyTyped); - }; + var onRequestStartedProp = telemetryHttpModuleOptionsType?.GetProperty("OnRequestStartedCallback") ?? throw new NotSupportedException("TelemetryHttpModuleOptions.OnRequestStartedCallback property not found"); + + // Get the parameter types from the callback property type itself to avoid hardcoded type loading + var callbackType = onRequestStartedProp.PropertyType; + var invokeMethod = callbackType.GetMethod("Invoke"); + var parameterTypes = invokeMethod!.GetParameters().Select(p => p.ParameterType).ToArray(); + var returnType = invokeMethod.ReturnType; + // Parameters for the new combined callback (use actual parameter types from the callback) + var httpContextParam = Expression.Parameter(parameterTypes[0], "httpContext"); + var activityContextParam = Expression.Parameter(parameterTypes[1], "activityContext"); + + // Get the existing callback value var options = Expression.Property(null, telemetryHttpModuleType, "Options"); + var existingCallback = Expression.Property(options, onRequestStartedProp); + + // Create conditional logic: if existingCallback != null, call it, otherwise return null + var nullCheck = Expression.NotEqual(existingCallback, Expression.Constant(null, existingCallback.Type)); + + // Call existing callback if it exists + var callExistingCallback = Expression.Call( + existingCallback, + invokeMethod, + httpContextParam, + activityContextParam); + + // If no existing callback, return null + var nullActivity = Expression.Constant(null, returnType); + + // Choose between calling existing callback or returning null + var activityResult = Expression.Condition(nullCheck, callExistingCallback, nullActivity); + + // Store the activity result in a variable + var activityVariable = Expression.Variable(returnType, "activity"); + var assignActivity = Expression.Assign(activityVariable, activityResult); + + // Check if activity is not null before calling OnRequestStarted + var activityNotNull = Expression.NotEqual(activityVariable, Expression.Constant(null, returnType)); + + // Call OnRequestStarted method - convert HttpContextBase to object for compatibility + var onRequestStartedMethod = typeof(AspNetParentSpanCorrector).GetMethod(nameof(OnRequestStarted), BindingFlags.Static | BindingFlags.NonPublic)!; + var callOnRequestStarted = Expression.Call(onRequestStartedMethod, activityVariable, Expression.Convert(httpContextParam, typeof(object))); + + // Conditional call to OnRequestStarted + var conditionalCall = Expression.IfThen(activityNotNull, callOnRequestStarted); + + // Return the activity + var returnActivity = activityVariable; + + // Create the method body + var methodBody = Expression.Block( + [activityVariable], + assignActivity, + conditionalCall, + returnActivity); + + // Create the combined callback lambda + var combinedCallbackLambda = Expression.Lambda(callbackType, methodBody, httpContextParam, activityContextParam); + + // Create assignment: Options.OnRequestStartedCallback = combinedCallback var callbackProperty = Expression.Property(options, onRequestStartedProp); - var combinedDelegate = Expression.Call(Expression.Constant(addOurCallbackToDelegate.Target), addOurCallbackToDelegate.Method, callbackProperty); - var subscribeExpression = Expression.Assign(callbackProperty, Expression.Convert(combinedDelegate, onRequestStartedProp.PropertyType)); + var subscribeExpression = Expression.Assign(callbackProperty, combinedCallbackLambda); + return (Func)Expression.Lambda(subscribeExpression).Compile(); } diff --git a/src/Shared/RequestDataHelper.cs b/src/Shared/RequestDataHelper.cs index edd9532569..6a56702a47 100644 --- a/src/Shared/RequestDataHelper.cs +++ b/src/Shared/RequestDataHelper.cs @@ -62,6 +62,17 @@ public void SetHttpMethodTag(Activity activity, string originalHttpMethod) } } + public void SetHttpMethodTag(ref TagList tags, string originalHttpMethod) + { + var normalizedHttpMethod = this.GetNormalizedHttpMethod(originalHttpMethod); + tags.Add(SemanticConventions.AttributeHttpRequestMethod, normalizedHttpMethod); + + if (originalHttpMethod != normalizedHttpMethod) + { + tags.Add(SemanticConventions.AttributeHttpRequestMethodOriginal, originalHttpMethod); + } + } + public string GetNormalizedHttpMethod(string method) { return this.knownHttpMethods.TryGetValue(method, out var normalizedMethod) @@ -70,13 +81,18 @@ public string GetNormalizedHttpMethod(string method) } public void SetActivityDisplayName(Activity activity, string originalHttpMethod, string? httpRoute = null) + { + activity.DisplayName = this.GetActivityDisplayName(originalHttpMethod, httpRoute); + } + + public string GetActivityDisplayName(string originalHttpMethod, string? httpRoute = null) { // 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; - activity.DisplayName = string.IsNullOrEmpty(httpRoute) ? namePrefix : $"{namePrefix} {httpRoute}"; + return string.IsNullOrEmpty(httpRoute) ? namePrefix : $"{namePrefix} {httpRoute}"; } internal static string GetHttpProtocolVersion(Version httpVersion) diff --git a/test/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.Tests/ActivityHelperTest.cs b/test/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.Tests/ActivityHelperTest.cs index 0b563ea823..73d8725f99 100644 --- a/test/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.Tests/ActivityHelperTest.cs +++ b/test/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.Tests/ActivityHelperTest.cs @@ -17,12 +17,16 @@ public class ActivityHelperTest : IDisposable private const string BaggageHeaderName = "baggage"; private const string BaggageInHeader = "TestKey1=123,TestKey2=456,TestKey1=789"; private const string TestActivityName = "Activity.Test"; + private const string TestActivitySourceName = "TestActivitySource"; + + private readonly ActivitySource testActivitySource = new(TestActivitySourceName); private readonly TextMapPropagator noopTextMapPropagator = new NoopTextMapPropagator(); private ActivityListener? activitySourceListener; public void Dispose() { this.activitySourceListener?.Dispose(); + this.testActivitySource?.Dispose(); } [Fact] @@ -57,7 +61,7 @@ public async Task Can_Restore_Activity() { this.EnableListener(); var context = HttpContextHelper.GetFakeHttpContextBase(); - using var rootActivity = ActivityHelper.StartAspNetActivity(this.noopTextMapPropagator, context, null)!; + using var rootActivity = ActivityHelper.StartAspNetActivity(this.noopTextMapPropagator, context, this.StartTestActivity)!; rootActivity.AddTag("k1", "v1"); rootActivity.AddTag("k2", "v2"); @@ -126,10 +130,10 @@ public void Can_Stop_Lost_Activity() { Assert.NotNull(Activity.Current); Assert.Equal(Activity.Current, a); - Assert.Equal(ActivityHelper.AspNetActivityName, Activity.Current.OperationName); + Assert.Equal("StartTestActivity", Activity.Current.OperationName); }); var context = HttpContextHelper.GetFakeHttpContextBase(); - using var rootActivity = ActivityHelper.StartAspNetActivity(this.noopTextMapPropagator, context, null)!; + using var rootActivity = ActivityHelper.StartAspNetActivity(this.noopTextMapPropagator, context, this.StartTestActivity)!; rootActivity.AddTag("k1", "v1"); rootActivity.AddTag("k2", "v2"); @@ -200,7 +204,7 @@ public void Can_Stop_Root_Activity_With_All_Children() { this.EnableListener(); var context = HttpContextHelper.GetFakeHttpContextBase(); - using var rootActivity = ActivityHelper.StartAspNetActivity(this.noopTextMapPropagator, context, null)!; + using var rootActivity = ActivityHelper.StartAspNetActivity(this.noopTextMapPropagator, context, this.StartTestActivity)!; var child = new Activity("child").Start(); new Activity("grandchild").Start(); @@ -234,7 +238,7 @@ public async Task Can_Stop_Root_Activity_If_It_Is_Broken() { this.EnableListener(); var context = HttpContextHelper.GetFakeHttpContextBase(); - using var root = ActivityHelper.StartAspNetActivity(this.noopTextMapPropagator, context, null)!; + using var root = ActivityHelper.StartAspNetActivity(this.noopTextMapPropagator, context, this.StartTestActivity)!; new Activity("child").Start(); for (var i = 0; i < 2; i++) @@ -264,7 +268,7 @@ public void Stop_Root_Activity_With_129_Nesting_Depth() { this.EnableListener(); var context = HttpContextHelper.GetFakeHttpContextBase(); - using var root = ActivityHelper.StartAspNetActivity(this.noopTextMapPropagator, context, null)!; + using var root = ActivityHelper.StartAspNetActivity(this.noopTextMapPropagator, context, this.StartTestActivity)!; for (var i = 0; i < 129; i++) { @@ -316,7 +320,7 @@ public void Can_Create_RootActivity_From_W3C_Traceparent() }; var context = HttpContextHelper.GetFakeHttpContextBase(headers: requestHeaders); - using var rootActivity = ActivityHelper.StartAspNetActivity(new TraceContextPropagator(), context, null); + using var rootActivity = ActivityHelper.StartAspNetActivity(new TraceContextPropagator(), context, this.StartTestActivity); Assert.NotNull(rootActivity); Assert.Equal(ActivityIdFormat.W3C, rootActivity.IdFormat); @@ -342,7 +346,7 @@ public void Can_Create_RootActivityWithTraceState_From_W3C_TraceContext() }; var context = HttpContextHelper.GetFakeHttpContextBase(headers: requestHeaders); - using var rootActivity = ActivityHelper.StartAspNetActivity(new TraceContextPropagator(), context, null); + using var rootActivity = ActivityHelper.StartAspNetActivity(new TraceContextPropagator(), context, this.StartTestActivity); Assert.NotNull(rootActivity); Assert.Equal(ActivityIdFormat.W3C, rootActivity.IdFormat); @@ -368,7 +372,7 @@ public void Can_Create_RootActivity_From_W3C_Traceparent_With_Baggage() }; var context = HttpContextHelper.GetFakeHttpContextBase(headers: requestHeaders); - using var rootActivity = ActivityHelper.StartAspNetActivity(new CompositeTextMapPropagator([new TraceContextPropagator(), new BaggagePropagator()]), context, null); + using var rootActivity = ActivityHelper.StartAspNetActivity(new CompositeTextMapPropagator([new TraceContextPropagator(), new BaggagePropagator()]), context, this.StartTestActivity); Assert.NotNull(rootActivity); Assert.Equal(ActivityIdFormat.W3C, rootActivity.IdFormat); @@ -394,7 +398,7 @@ public void Can_Create_RootActivity_And_Start_Activity() { this.EnableListener(); var context = HttpContextHelper.GetFakeHttpContextBase(); - using var rootActivity = ActivityHelper.StartAspNetActivity(this.noopTextMapPropagator, context, null); + using var rootActivity = ActivityHelper.StartAspNetActivity(this.noopTextMapPropagator, context, this.StartTestActivity); Assert.NotNull(rootActivity); Assert.False(string.IsNullOrEmpty(rootActivity.Id)); @@ -405,7 +409,7 @@ public void Can_Create_RootActivity_And_Saved_In_HttContext() { this.EnableListener(); var context = HttpContextHelper.GetFakeHttpContextBase(); - using var rootActivity = ActivityHelper.StartAspNetActivity(this.noopTextMapPropagator, context, null); + using var rootActivity = ActivityHelper.StartAspNetActivity(this.noopTextMapPropagator, context, this.StartTestActivity); Assert.NotNull(rootActivity); Assert.Same(rootActivity, ((ActivityHelper.ContextHolder)context.Items[ActivityHelper.ContextKey])?.Activity); @@ -435,23 +439,27 @@ public void Fire_Exception_Events() public void Should_Handle_Activity_Events_In_Correct_Order() { var eventOrder = new List(); - const string ActivityOnStarted = "ActivityOnStarted"; - const string ActivityOnStopped = "ActivityOnStarted"; - const string OnStartCallback = "OnStartCallback"; - const string OnStopCallback = "OnStopCallback"; + const string activityOnStarted = nameof(activityOnStarted); + const string activityOnStopped = nameof(activityOnStopped); + const string onStartCallback = nameof(onStartCallback); + const string onStopCallback = nameof(onStopCallback); - this.EnableListener(_ => eventOrder.Add(ActivityOnStarted), _ => eventOrder.Add(ActivityOnStopped)); + this.EnableListener(_ => eventOrder.Add(activityOnStarted), _ => eventOrder.Add(activityOnStopped)); var context = HttpContextHelper.GetFakeHttpContextBase(); - using var rootActivity = ActivityHelper.StartAspNetActivity(this.noopTextMapPropagator, context, (_, _) => eventOrder.Add(OnStartCallback)); - ActivityHelper.StopAspNetActivity(this.noopTextMapPropagator, rootActivity, context, (_, _) => eventOrder.Add(OnStopCallback)); + using var rootActivity = ActivityHelper.StartAspNetActivity(this.noopTextMapPropagator, context, (httpContext, activityContext) => + { + eventOrder.Add(onStartCallback); + return this.StartTestActivity(httpContext, activityContext); + }); + ActivityHelper.StopAspNetActivity(this.noopTextMapPropagator, rootActivity, context, (_, _) => eventOrder.Add(onStopCallback)); - var expectedOrder = new List() + var expectedOrder = new List { - ActivityOnStarted, - OnStartCallback, - OnStopCallback, - ActivityOnStopped, + onStartCallback, + activityOnStarted, + onStopCallback, + activityOnStopped, }; Assert.Equal(expectedOrder, eventOrder); @@ -464,7 +472,12 @@ public void Should_Not_Pass_Stopped_Activity_To_Callbacks() var wasStopped = false; var context = HttpContextHelper.GetFakeHttpContextBase(); - using var rootActivity = ActivityHelper.StartAspNetActivity(this.noopTextMapPropagator, context, (activity, _) => wasStopped = (activity?.IsStopped ?? false) || wasStopped); + using var rootActivity = ActivityHelper.StartAspNetActivity(this.noopTextMapPropagator, context, (_, _) => + { + var activity = this.testActivitySource.StartActivity(); + wasStopped = (activity?.IsStopped ?? false) || wasStopped; + return activity; + }); ActivityHelper.StopAspNetActivity(this.noopTextMapPropagator, rootActivity, context, (activity, _) => wasStopped = (activity?.IsStopped ?? false) || wasStopped); Assert.False(wasStopped); @@ -476,7 +489,7 @@ private void EnableListener(Action? onStarted = null, Action this.activitySourceListener = new ActivityListener { - ShouldListenTo = (activitySource) => activitySource.Name == ActivityHelper.AspNetSourceName, + ShouldListenTo = (activitySource) => activitySource.Name == TestActivitySourceName, ActivityStarted = (a) => onStarted?.Invoke(a), ActivityStopped = (a) => onStopped?.Invoke(a), Sample = (ref ActivityCreationOptions options) => @@ -488,6 +501,11 @@ private void EnableListener(Action? onStarted = null, Action ActivitySource.AddActivityListener(this.activitySourceListener); } + private Activity? StartTestActivity(HttpContextBase httpContext, ActivityContext activityContext) + { + return this.testActivitySource.StartActivity(ActivityKind.Server, activityContext); + } + private class TestHttpRequest : HttpRequestBase { private readonly NameValueCollection headers = []; diff --git a/test/OpenTelemetry.Instrumentation.AspNet.Tests/HttpInListenerTests.cs b/test/OpenTelemetry.Instrumentation.AspNet.Tests/HttpInListenerTests.cs index 03d78e875f..232ada1189 100644 --- a/test/OpenTelemetry.Instrumentation.AspNet.Tests/HttpInListenerTests.cs +++ b/test/OpenTelemetry.Instrumentation.AspNet.Tests/HttpInListenerTests.cs @@ -26,25 +26,25 @@ public enum QueryRedactionDisableBehavior } [Theory] - [InlineData("http://localhost/", "http", "/", null, null, "localhost", 80, "GET", "GET", null, 0, null, "GET")] - [InlineData("http://localhost/?foo=bar&baz=test", "http", "/", "foo=bar&baz=test", QueryRedactionDisableBehavior.DisableViaEnvVar, "localhost", 80, "POST", "POST", null, 0, null, "POST", true)] - [InlineData("http://localhost/?foo=bar&baz=test", "http", "/", "foo=bar&baz=test", QueryRedactionDisableBehavior.DisableViaIConfiguration, "localhost", 80, "POST", "POST", null, 0, null, "POST", true)] - [InlineData("http://localhost/?foo=bar&baz=test", "http", "/", "foo=Redacted&baz=Redacted", null, "localhost", 80, "POST", "POST", null, 0, null, "POST", true)] - [InlineData("https://localhost/", "https", "/", null, null, "localhost", 443, "NonStandard", "_OTHER", "NonStandard", 0, null, "HTTP")] - [InlineData("https://user:pass@localhost/", "https", "/", null, null, "localhost", 443, "GeT", "GET", "GeT", 0, null, "GET")] // Test URL sanitization - [InlineData("http://localhost:443/", "http", "/", null, null, "localhost", 443, "GET", "GET", null, 0, null, "GET")] // Test http over 443 - [InlineData("https://localhost:80/", "https", "/", null, null, "localhost", 80, "GET", "GET", null, 0, null, "GET")] // Test https over 80 - [InlineData("https://localhost:80/Home/Index.htm?q1=v1&q2=v2#FragmentName", "https", "/Home/Index.htm", "q1=v1&q2=v2", QueryRedactionDisableBehavior.DisableViaEnvVar, "localhost", 80, "GET", "GET", null, 0, null, "GET")] // Test complex URL - [InlineData("https://localhost:80/Home/Index.htm?q1=v1&q2=v2#FragmentName", "https", "/Home/Index.htm", "q1=Redacted&q2=Redacted", null, "localhost", 80, "GET", "GET", null, 0, null, "GET")] // Test complex URL - [InlineData("https://user:password@localhost:80/Home/Index.htm?q1=v1&q2=v2#FragmentName", "https", "/Home/Index.htm", "q1=v1&q2=v2", QueryRedactionDisableBehavior.DisableViaIConfiguration, "localhost", 80, "GET", "GET", null, 0, null, "GET")] // Test complex URL sanitization - [InlineData("http://localhost:80/Index", "http", "/Index", null, null, "localhost", 80, "GET", "GET", null, 1, "{controller}/{action}/{id}", "GET {controller}/{action}/{id}")] - [InlineData("https://localhost:443/about_attr_route/10", "https", "/about_attr_route/10", null, null, "localhost", 443, "HEAD", "HEAD", null, 2, "about_attr_route/{customerId}", "HEAD about_attr_route/{customerId}")] - [InlineData("http://localhost:1880/api/weatherforecast", "http", "/api/weatherforecast", null, null, "localhost", 1880, "GET", "GET", null, 3, "api/{controller}/{id}", "GET api/{controller}/{id}")] - [InlineData("https://localhost:1843/subroute/10", "https", "/subroute/10", null, null, "localhost", 1843, "GET", "GET", null, 4, "subroute/{customerId}", "GET subroute/{customerId}")] - [InlineData("https://localhost:1843/subroute/10", "https", "/subroute/10", null, null, "localhost", 1843, "GET", "GET", null, 5, "subroute/{customerId}", "GET subroute/{customerId}")] - [InlineData("http://localhost/api/value", "http", "/api/value", null, null, "localhost", 80, "GET", "GET", null, 0, null, "GET", false, "/api/value")] // Request will be filtered - [InlineData("http://localhost/api/value", "http", "/api/value", null, null, "localhost", 80, "GET", "GET", null, 0, null, "GET", false, "{ThrowException}")] // Filter user code will throw an exception - [InlineData("http://localhost/", "http", "/", null, null, "localhost", 80, "GET", "GET", null, 0, null, "GET", false, null, true, "System.InvalidOperationException")] // Test RecordException option + [InlineData("http://localhost/", "http", "/", null, null, "localhost", 80, "GET", "GET", null, 0, null, "GET", "GET")] + [InlineData("http://localhost/?foo=bar&baz=test", "http", "/", "foo=bar&baz=test", QueryRedactionDisableBehavior.DisableViaEnvVar, "localhost", 80, "POST", "POST", null, 0, null, "POST", "POST", true)] + [InlineData("http://localhost/?foo=bar&baz=test", "http", "/", "foo=bar&baz=test", QueryRedactionDisableBehavior.DisableViaIConfiguration, "localhost", 80, "POST", "POST", null, 0, null, "POST", "POST", true)] + [InlineData("http://localhost/?foo=bar&baz=test", "http", "/", "foo=Redacted&baz=Redacted", null, "localhost", 80, "POST", "POST", null, 0, null, "POST", "POST", true)] + [InlineData("https://localhost/", "https", "/", null, null, "localhost", 443, "NonStandard", "_OTHER", "NonStandard", 0, null, "HTTP", "HTTP")] + [InlineData("https://user:pass@localhost/", "https", "/", null, null, "localhost", 443, "GeT", "GET", "GeT", 0, null, "GET", "GET")] // Test URL sanitization + [InlineData("http://localhost:443/", "http", "/", null, null, "localhost", 443, "GET", "GET", null, 0, null, "GET", "GET")] // Test http over 443 + [InlineData("https://localhost:80/", "https", "/", null, null, "localhost", 80, "GET", "GET", null, 0, null, "GET", "GET")] // Test https over 80 + [InlineData("https://localhost:80/Home/Index.htm?q1=v1&q2=v2#FragmentName", "https", "/Home/Index.htm", "q1=v1&q2=v2", QueryRedactionDisableBehavior.DisableViaEnvVar, "localhost", 80, "GET", "GET", null, 0, null, "GET", "GET")] // Test complex URL + [InlineData("https://localhost:80/Home/Index.htm?q1=v1&q2=v2#FragmentName", "https", "/Home/Index.htm", "q1=Redacted&q2=Redacted", null, "localhost", 80, "GET", "GET", null, 0, null, "GET", "GET")] // Test complex URL + [InlineData("https://user:password@localhost:80/Home/Index.htm?q1=v1&q2=v2#FragmentName", "https", "/Home/Index.htm", "q1=v1&q2=v2", QueryRedactionDisableBehavior.DisableViaIConfiguration, "localhost", 80, "GET", "GET", null, 0, null, "GET", "GET")] // Test complex URL sanitization + [InlineData("http://localhost:80/Index", "http", "/Index", null, null, "localhost", 80, "GET", "GET", null, 1, "{controller}/{action}/{id}", "GET", "GET {controller}/{action}/{id}")] + [InlineData("https://localhost:443/about_attr_route/10", "https", "/about_attr_route/10", null, null, "localhost", 443, "HEAD", "HEAD", null, 2, "about_attr_route/{customerId}", "HEAD", "HEAD about_attr_route/{customerId}")] + [InlineData("http://localhost:1880/api/weatherforecast", "http", "/api/weatherforecast", null, null, "localhost", 1880, "GET", "GET", null, 3, "api/{controller}/{id}", "GET", "GET api/{controller}/{id}")] + [InlineData("https://localhost:1843/subroute/10", "https", "/subroute/10", null, null, "localhost", 1843, "GET", "GET", null, 4, "subroute/{customerId}", "GET", "GET subroute/{customerId}")] + [InlineData("https://localhost:1843/subroute/10", "https", "/subroute/10", null, null, "localhost", 1843, "GET", "GET", null, 5, "subroute/{customerId}", "GET", "GET subroute/{customerId}")] + [InlineData("http://localhost/api/value", "http", "/api/value", null, null, "localhost", 80, "GET", "GET", null, 0, null, "GET", "GET", false, "/api/value")] // Request will be filtered + [InlineData("http://localhost/api/value", "http", "/api/value", null, null, "localhost", 80, "GET", "GET", null, 0, null, "GET", "GET", false, "{ThrowException}")] // Filter user code will throw an exception + [InlineData("http://localhost/", "http", "/", null, null, "localhost", 80, "GET", "GET", null, 0, null, "GET", "GET", false, null, true, "System.InvalidOperationException")] // Test RecordException option public void AspNetRequestsAreCollectedSuccessfully( string url, string expectedUrlScheme, @@ -58,7 +58,8 @@ public void AspNetRequestsAreCollectedSuccessfully( string? expectedOriginalRequestMethod, int routeType, string? routeTemplate, - string expectedName, + string expectedNameAfterStart, + string expectedNameAfterStop, bool setStatusToErrorInEnrich = false, string? filter = null, bool recordException = false, @@ -159,7 +160,7 @@ public void AspNetRequestsAreCollectedSuccessfully( Assert.Single(inMemoryEventListener.Events, e => e.EventId == 2); } - Assert.Equal(ActivityHelper.AspNetActivityName, Activity.Current!.OperationName); + Assert.Equal(expectedNameAfterStart, Activity.Current!.DisplayName); if (recordException) { @@ -179,10 +180,9 @@ public void AspNetRequestsAreCollectedSuccessfully( var span = exportedItems[0]; - Assert.Equal(ActivityHelper.AspNetActivityName, span.OperationName); Assert.NotEqual(TimeSpan.Zero, span.Duration); - Assert.Equal(expectedName, span.DisplayName); + Assert.Equal(expectedNameAfterStop, span.DisplayName); Assert.Equal(ActivityKind.Server, span.Kind); Assert.True(span.Duration != TimeSpan.Zero);