diff --git a/src/Hosting/Hosting/src/Internal/HostingApplication.cs b/src/Hosting/Hosting/src/Internal/HostingApplication.cs index b610173be531..37996905bdf1 100644 --- a/src/Hosting/Hosting/src/Internal/HostingApplication.cs +++ b/src/Hosting/Hosting/src/Internal/HostingApplication.cs @@ -17,6 +17,13 @@ internal sealed class HostingApplication : IHttpApplication _diagnostics.SuppressActivityOpenTelemetryData; + set => _diagnostics.SuppressActivityOpenTelemetryData = value; + } + public HostingApplication( RequestDelegate application, ILogger logger, diff --git a/src/Hosting/Hosting/src/Internal/HostingApplicationDiagnostics.cs b/src/Hosting/Hosting/src/Internal/HostingApplicationDiagnostics.cs index 1a344203727c..67f584462e6e 100644 --- a/src/Hosting/Hosting/src/Internal/HostingApplicationDiagnostics.cs +++ b/src/Hosting/Hosting/src/Internal/HostingApplicationDiagnostics.cs @@ -34,6 +34,9 @@ internal sealed class HostingApplicationDiagnostics private readonly HostingMetrics _metrics; private readonly ILogger _logger; + // Internal for testing purposes only + internal bool SuppressActivityOpenTelemetryData { get; set; } + public HostingApplicationDiagnostics( ILogger logger, DiagnosticListener diagnosticListener, @@ -48,6 +51,19 @@ public HostingApplicationDiagnostics( _propagator = propagator; _eventSource = eventSource; _metrics = metrics; + + SuppressActivityOpenTelemetryData = GetSuppressActivityOpenTelemetryData(); + } + + private static bool GetSuppressActivityOpenTelemetryData() + { + // Default to true if the switch isn't set. + if (!AppContext.TryGetSwitch("Microsoft.AspNetCore.Hosting.SuppressActivityOpenTelemetryData", out var enabled)) + { + return true; + } + + return enabled; } [MethodImpl(MethodImplOptions.AggressiveInlining)] @@ -88,9 +104,9 @@ public void BeginRequest(HttpContext httpContext, HostingApplication.Context con var diagnosticListenerActivityCreationEnabled = (diagnosticListenerEnabled && _diagnosticListener.IsEnabled(ActivityName, httpContext)); var loggingEnabled = _logger.IsEnabled(LogLevel.Critical); - if (loggingEnabled || diagnosticListenerActivityCreationEnabled || _activitySource.HasListeners()) + if (ActivityCreator.IsActivityCreated(_activitySource, loggingEnabled || diagnosticListenerActivityCreationEnabled)) { - context.Activity = StartActivity(httpContext, loggingEnabled, diagnosticListenerActivityCreationEnabled, out var hasDiagnosticListener); + context.Activity = StartActivity(httpContext, loggingEnabled || diagnosticListenerActivityCreationEnabled, out var hasDiagnosticListener); context.HasDiagnosticListener = hasDiagnosticListener; if (context.Activity != null) @@ -385,10 +401,18 @@ private void RecordRequestStartMetrics(HttpContext httpContext) } [MethodImpl(MethodImplOptions.NoInlining)] - private Activity? StartActivity(HttpContext httpContext, bool loggingEnabled, bool diagnosticListenerActivityCreationEnabled, out bool hasDiagnosticListener) + private Activity? StartActivity(HttpContext httpContext, bool diagnosticsOrLoggingEnabled, out bool hasDiagnosticListener) { + // StartActivity is only called if an Activity is already verified to be created. + Debug.Assert(ActivityCreator.IsActivityCreated(_activitySource, diagnosticsOrLoggingEnabled), + "Activity should only be created if diagnostics or logging is enabled."); + hasDiagnosticListener = false; + var initializeTags = !SuppressActivityOpenTelemetryData + ? CreateInitializeActivityTags(httpContext) + : (TagList?)null; + var headers = httpContext.Request.Headers; var activity = ActivityCreator.CreateFromRemote( _activitySource, @@ -402,9 +426,9 @@ private void RecordRequestStartMetrics(HttpContext httpContext) }, ActivityName, ActivityKind.Server, - tags: null, + tags: initializeTags, links: null, - loggingEnabled || diagnosticListenerActivityCreationEnabled); + diagnosticsOrLoggingEnabled); if (activity is null) { return null; @@ -425,6 +449,47 @@ private void RecordRequestStartMetrics(HttpContext httpContext) return activity; } + private static TagList CreateInitializeActivityTags(HttpContext httpContext) + { + // The tags here are set when the activity is created. They can be used in sampling decisions. + // Most values in semantic conventions that are present at creation are specified: + // https://github.com/open-telemetry/semantic-conventions/blob/27735ccca3746d7bb7fa061dfb73d93bcbae2b6e/docs/http/http-spans.md#L581-L592 + // Missing values recommended by the spec are: + // - url.query (need configuration around redaction to do properly) + // - http.request.header. + + var request = httpContext.Request; + var creationTags = new TagList(); + + if (request.Host.HasValue) + { + creationTags.Add(HostingTelemetryHelpers.AttributeServerAddress, request.Host.Host); + + if (HostingTelemetryHelpers.TryGetServerPort(request.Host, request.Scheme, out var port)) + { + creationTags.Add(HostingTelemetryHelpers.AttributeServerPort, port); + } + } + + HostingTelemetryHelpers.SetActivityHttpMethodTags(ref creationTags, request.Method); + + if (request.Headers.TryGetValue("User-Agent", out var values)) + { + var userAgent = values.Count > 0 ? values[0] : null; + if (!string.IsNullOrEmpty(userAgent)) + { + creationTags.Add(HostingTelemetryHelpers.AttributeUserAgentOriginal, userAgent); + } + } + + creationTags.Add(HostingTelemetryHelpers.AttributeUrlScheme, request.Scheme); + + var path = (request.PathBase.HasValue || request.Path.HasValue) ? (request.PathBase + request.Path).ToString() : "/"; + creationTags.Add(HostingTelemetryHelpers.AttributeUrlPath, path); + + return creationTags; + } + [MethodImpl(MethodImplOptions.NoInlining)] private void StopActivity(HttpContext httpContext, Activity activity, bool hasDiagnosticListener) { diff --git a/src/Hosting/Hosting/src/Internal/HostingMetrics.cs b/src/Hosting/Hosting/src/Internal/HostingMetrics.cs index 129542fec15a..583c4d6f3348 100644 --- a/src/Hosting/Hosting/src/Internal/HostingMetrics.cs +++ b/src/Hosting/Hosting/src/Internal/HostingMetrics.cs @@ -1,9 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System.Collections.Frozen; using System.Diagnostics; -using System.Diagnostics.CodeAnalysis; using System.Diagnostics.Metrics; using Microsoft.AspNetCore.Http; @@ -55,7 +53,7 @@ public void RequestEnd(string protocol, string scheme, string method, string? ro if (!disableHttpRequestDurationMetric && _requestDuration.Enabled) { - if (TryGetHttpVersion(protocol, out var httpVersion)) + if (HostingTelemetryHelpers.TryGetHttpVersion(protocol, out var httpVersion)) { tags.Add("network.protocol.version", httpVersion); } @@ -65,7 +63,7 @@ public void RequestEnd(string protocol, string scheme, string method, string? ro } // Add information gathered during request. - tags.Add("http.response.status_code", GetBoxedStatusCode(statusCode)); + tags.Add("http.response.status_code", HostingTelemetryHelpers.GetBoxedStatusCode(statusCode)); if (route != null) { tags.Add("http.route", route); @@ -104,73 +102,6 @@ public void Dispose() private static void InitializeRequestTags(ref TagList tags, string scheme, string method) { tags.Add("url.scheme", scheme); - tags.Add("http.request.method", ResolveHttpMethod(method)); - } - - private static readonly object[] BoxedStatusCodes = new object[512]; - - private static object GetBoxedStatusCode(int statusCode) - { - object[] boxes = BoxedStatusCodes; - return (uint)statusCode < (uint)boxes.Length - ? boxes[statusCode] ??= statusCode - : statusCode; - } - - private static readonly FrozenDictionary KnownMethods = FrozenDictionary.ToFrozenDictionary(new[] - { - KeyValuePair.Create(HttpMethods.Connect, HttpMethods.Connect), - KeyValuePair.Create(HttpMethods.Delete, HttpMethods.Delete), - KeyValuePair.Create(HttpMethods.Get, HttpMethods.Get), - KeyValuePair.Create(HttpMethods.Head, HttpMethods.Head), - KeyValuePair.Create(HttpMethods.Options, HttpMethods.Options), - KeyValuePair.Create(HttpMethods.Patch, HttpMethods.Patch), - KeyValuePair.Create(HttpMethods.Post, HttpMethods.Post), - KeyValuePair.Create(HttpMethods.Put, HttpMethods.Put), - KeyValuePair.Create(HttpMethods.Trace, HttpMethods.Trace) - }, StringComparer.OrdinalIgnoreCase); - - private static string ResolveHttpMethod(string method) - { - // TODO: Support configuration for configuring known methods - if (KnownMethods.TryGetValue(method, out var result)) - { - // KnownMethods ignores case. Use the value returned by the dictionary to have a consistent case. - return result; - } - return "_OTHER"; - } - - private static bool TryGetHttpVersion(string protocol, [NotNullWhen(true)] out string? version) - { - if (HttpProtocol.IsHttp11(protocol)) - { - version = "1.1"; - return true; - } - if (HttpProtocol.IsHttp2(protocol)) - { - // HTTP/2 only has one version. - version = "2"; - return true; - } - if (HttpProtocol.IsHttp3(protocol)) - { - // HTTP/3 only has one version. - version = "3"; - return true; - } - if (HttpProtocol.IsHttp10(protocol)) - { - version = "1.0"; - return true; - } - if (HttpProtocol.IsHttp09(protocol)) - { - version = "0.9"; - return true; - } - version = null; - return false; + tags.Add("http.request.method", HostingTelemetryHelpers.GetNormalizedHttpMethod(method)); } } diff --git a/src/Hosting/Hosting/src/Internal/HostingTelemetryHelpers.cs b/src/Hosting/Hosting/src/Internal/HostingTelemetryHelpers.cs new file mode 100644 index 000000000000..e415869e9cff --- /dev/null +++ b/src/Hosting/Hosting/src/Internal/HostingTelemetryHelpers.cs @@ -0,0 +1,132 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Frozen; +using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; +using Microsoft.AspNetCore.Http; + +namespace Microsoft.AspNetCore.Hosting; + +internal static class HostingTelemetryHelpers +{ + // Semantic Conventions for HTTP. + // Note: Not all telemetry code is using these const attribute names yet. + public const string AttributeHttpRequestMethod = "http.request.method"; + public const string AttributeHttpRequestMethodOriginal = "http.request.method_original"; + public const string AttributeUrlScheme = "url.scheme"; + public const string AttributeUrlPath = "url.path"; + public const string AttributeServerAddress = "server.address"; + public const string AttributeServerPort = "server.port"; + public const string AttributeUserAgentOriginal = "user_agent.original"; + + // The value "_OTHER" is used for non-standard HTTP methods. + // https://github.com/open-telemetry/semantic-conventions/blob/v1.23.0/docs/http/http-spans.md#common-attributes + private const string OtherHttpMethod = "_OTHER"; + + private static readonly object[] BoxedStatusCodes = new object[512]; + + private static readonly FrozenDictionary KnownHttpMethods = FrozenDictionary.ToFrozenDictionary([ + KeyValuePair.Create(HttpMethods.Connect, HttpMethods.Connect), + KeyValuePair.Create(HttpMethods.Delete, HttpMethods.Delete), + KeyValuePair.Create(HttpMethods.Get, HttpMethods.Get), + KeyValuePair.Create(HttpMethods.Head, HttpMethods.Head), + KeyValuePair.Create(HttpMethods.Options, HttpMethods.Options), + KeyValuePair.Create(HttpMethods.Patch, HttpMethods.Patch), + KeyValuePair.Create(HttpMethods.Post, HttpMethods.Post), + KeyValuePair.Create(HttpMethods.Put, HttpMethods.Put), + KeyValuePair.Create(HttpMethods.Trace, HttpMethods.Trace) + ], StringComparer.OrdinalIgnoreCase); + + // Boxed port values for HTTP and HTTPS. + private static readonly object HttpPort = 80; + private static readonly object HttpsPort = 443; + + public static bool TryGetServerPort(HostString host, string scheme, [NotNullWhen(true)] out object? port) + { + if (host.Port.HasValue) + { + port = host.Port.Value; + return true; + } + + // If the port is not specified, use the default port for the scheme. + if (string.Equals(scheme, "http", StringComparison.OrdinalIgnoreCase)) + { + port = HttpPort; + return true; + } + else if (string.Equals(scheme, "https", StringComparison.OrdinalIgnoreCase)) + { + port = HttpsPort; + return true; + } + + // Unknown scheme, no default port. + port = null; + return false; + } + + public static object GetBoxedStatusCode(int statusCode) + { + object[] boxes = BoxedStatusCodes; + return (uint)statusCode < (uint)boxes.Length + ? boxes[statusCode] ??= statusCode + : statusCode; + } + + public static string GetNormalizedHttpMethod(string method) + { + // TODO: Support configuration for configuring known methods + if (method != null && KnownHttpMethods.TryGetValue(method, out var result)) + { + // KnownHttpMethods ignores case. Use the value returned by the dictionary to have a consistent case. + return result; + } + return OtherHttpMethod; + } + + public static bool TryGetHttpVersion(string protocol, [NotNullWhen(true)] out string? version) + { + if (HttpProtocol.IsHttp11(protocol)) + { + version = "1.1"; + return true; + } + if (HttpProtocol.IsHttp2(protocol)) + { + // HTTP/2 only has one version. + version = "2"; + return true; + } + if (HttpProtocol.IsHttp3(protocol)) + { + // HTTP/3 only has one version. + version = "3"; + return true; + } + if (HttpProtocol.IsHttp10(protocol)) + { + version = "1.0"; + return true; + } + if (HttpProtocol.IsHttp09(protocol)) + { + version = "0.9"; + return true; + } + version = null; + return false; + } + + public static void SetActivityHttpMethodTags(ref TagList tags, string originalHttpMethod) + { + var normalizedHttpMethod = GetNormalizedHttpMethod(originalHttpMethod); + tags.Add(AttributeHttpRequestMethod, normalizedHttpMethod); + + if (originalHttpMethod != normalizedHttpMethod) + { + tags.Add(AttributeHttpRequestMethodOriginal, originalHttpMethod); + } + } +} diff --git a/src/Hosting/Hosting/test/HostingApplicationDiagnosticsTests.cs b/src/Hosting/Hosting/test/HostingApplicationDiagnosticsTests.cs index ff6735d1b5e3..d9878ad03b3e 100644 --- a/src/Hosting/Hosting/test/HostingApplicationDiagnosticsTests.cs +++ b/src/Hosting/Hosting/test/HostingApplicationDiagnosticsTests.cs @@ -1021,6 +1021,7 @@ public void ActivityListenersAreCalled() var testSource = new ActivitySource(Path.GetRandomFileName()); var hostingApplication = CreateApplication(out var features, activitySource: testSource); var parentSpanId = ""; + var tags = new List>(); using var listener = new ActivityListener { ShouldListenTo = activitySource => ReferenceEquals(activitySource, testSource), @@ -1028,6 +1029,7 @@ public void ActivityListenersAreCalled() ActivityStarted = activity => { parentSpanId = Activity.Current.ParentSpanId.ToHexString(); + tags = Activity.Current.TagObjects.OrderBy(t => t.Key).ToList(); } }; @@ -1039,12 +1041,112 @@ public void ActivityListenersAreCalled() { {"traceparent", "00-0123456789abcdef0123456789abcdef-0123456789abcdef-01"}, {"tracestate", "TraceState1"}, - {"baggage", "Key1=value1, Key2=value2"} + {"baggage", "Key1=value1, Key2=value2"}, + {"host", "localhost:8080" } + }, + PathBase = "/path_base", + Path = "/path", + Scheme = "http", + Method = "CUSTOM_METHOD", + Protocol = "HTTP/1.1" + }); + + hostingApplication.CreateContext(features); + Assert.Equal("0123456789abcdef", parentSpanId); + + Assert.Empty(tags); + } + + [Fact] + public void ActivityListeners_DontSuppressActivityTags_TagsAdded() + { + var testSource = new ActivitySource(Path.GetRandomFileName()); + var hostingApplication = CreateApplication(out var features, activitySource: testSource, suppressActivityOpenTelemetryData: false); + var parentSpanId = ""; + var tags = new List>(); + using var listener = new ActivityListener + { + ShouldListenTo = activitySource => ReferenceEquals(activitySource, testSource), + Sample = (ref ActivityCreationOptions _) => ActivitySamplingResult.AllData, + ActivityStarted = activity => + { + parentSpanId = Activity.Current.ParentSpanId.ToHexString(); + tags = Activity.Current.TagObjects.OrderBy(t => t.Key).ToList(); } + }; + + ActivitySource.AddActivityListener(listener); + + features.Set(new HttpRequestFeature() + { + Headers = new HeaderDictionary() + { + {"traceparent", "00-0123456789abcdef0123456789abcdef-0123456789abcdef-01"}, + {"tracestate", "TraceState1"}, + {"baggage", "Key1=value1, Key2=value2"}, + {"host", "localhost:8080" } + }, + PathBase = "/path_base", + Path = "/path", + Scheme = "http", + Method = "CUSTOM_METHOD", + Protocol = "HTTP/1.1" }); hostingApplication.CreateContext(features); Assert.Equal("0123456789abcdef", parentSpanId); + + Assert.Collection(tags, + kvp => AssertKeyValuePair(kvp, "http.request.method", "_OTHER"), + kvp => AssertKeyValuePair(kvp, "http.request.method_original", "CUSTOM_METHOD"), + kvp => AssertKeyValuePair(kvp, "server.address", "localhost"), + kvp => AssertKeyValuePair(kvp, "server.port", 8080), + kvp => AssertKeyValuePair(kvp, "url.path", "/path_base/path"), + kvp => AssertKeyValuePair(kvp, "url.scheme", "http")); + + static void AssertKeyValuePair(KeyValuePair pair, string key, T value) + { + Assert.Equal(key, pair.Key); + Assert.Equal(value, pair.Value); + } + } + + [Theory] + [InlineData("http", 80)] + [InlineData("HTTP", 80)] + [InlineData("https", 443)] + [InlineData("HTTPS", 443)] + [InlineData("other", null)] + public void ActivityListeners_DefaultPorts(string scheme, int? expectedPort) + { + var testSource = new ActivitySource(Path.GetRandomFileName()); + var hostingApplication = CreateApplication(out var features, activitySource: testSource, suppressActivityOpenTelemetryData: false); + var tags = new Dictionary(); + using var listener = new ActivityListener + { + ShouldListenTo = activitySource => ReferenceEquals(activitySource, testSource), + Sample = (ref ActivityCreationOptions _) => ActivitySamplingResult.AllData, + ActivityStarted = activity => + { + tags = Activity.Current.TagObjects.ToDictionary(); + } + }; + + ActivitySource.AddActivityListener(listener); + + features.Set(new HttpRequestFeature() + { + Headers = new HeaderDictionary() + { + {"host", "localhost" } + }, + Scheme = scheme, + }); + + hostingApplication.CreateContext(features); + + Assert.Equal(expectedPort != null, tags.TryGetValue("server.port", out var actualPort)); + Assert.Equal(expectedPort, (int?)actualPort); } [Fact] @@ -1092,7 +1194,8 @@ private static void AssertProperty(object o, string name) private static HostingApplication CreateApplication(out FeatureCollection features, DiagnosticListener diagnosticListener = null, ActivitySource activitySource = null, ILogger logger = null, - Action configure = null, HostingEventSource eventSource = null, IMeterFactory meterFactory = null) + Action configure = null, HostingEventSource eventSource = null, IMeterFactory meterFactory = null, + bool? suppressActivityOpenTelemetryData = null) { var httpContextFactory = new Mock(); @@ -1114,6 +1217,11 @@ private static HostingApplication CreateApplication(out FeatureCollection featur eventSource ?? HostingEventSource.Log, new HostingMetrics(meterFactory ?? new TestMeterFactory())); + if (suppressActivityOpenTelemetryData is { } suppress) + { + hostingApplication.SuppressActivityOpenTelemetryData = suppress; + } + return hostingApplication; } diff --git a/src/Shared/Diagnostics/ActivityCreator.cs b/src/Shared/Diagnostics/ActivityCreator.cs index 170e9cff267d..9960ff91be84 100644 --- a/src/Shared/Diagnostics/ActivityCreator.cs +++ b/src/Shared/Diagnostics/ActivityCreator.cs @@ -7,6 +7,11 @@ namespace Microsoft.AspNetCore.Shared; internal static class ActivityCreator { + public static bool IsActivityCreated(ActivitySource activitySource, bool diagnosticsOrLoggingEnabled) + { + return activitySource.HasListeners() || diagnosticsOrLoggingEnabled; + } + /// /// Create an activity with details received from a remote source. ///