Add standard tags to HttpClient native trace instrumentation (dotnet#104251)

Add the following standard tags to the HTTP Request Activities started in DelegatingHandler:

http.request.method
http.request.method_original
server.address
server.port
url.full

error.type
http.response.status_code
network.protocol.version

Just like in dotnet#103769, url.full is being redacted by removing UserInfo and the query string, while exposing a System.Net.Http.DisableQueryRedaction switch for opting-out from the latter.

Author: antonfirsov

src/libraries/System.Net.Http/src/System.Net.Http.csproj

@@ -38,6 +38,7 @@
     <Compile Include="System\Net\Http\DelegatingHandler.cs" />
     <Compile Include="System\Net\Http\DiagnosticsHandler.cs" />
     <Compile Include="System\Net\Http\DiagnosticsHandlerLoggingStrings.cs" />
+    <Compile Include="System\Net\Http\DiagnosticsHelper.cs" />
     <Compile Include="System\Net\Http\EmptyContent.cs" />
     <Compile Include="System\Net\Http\EmptyReadStream.cs" />
     <Compile Include="System\Net\Http\FormUrlEncodedContent.cs" />

src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHandler.cs

@@ -53,20 +53,19 @@ private static bool IsEnabled()
                    s_diagnosticListener.IsEnabled();
         }
 
-        private static Activity? CreateActivity(HttpRequestMessage requestMessage)
+        private static Activity? StartActivity(HttpRequestMessage request)
         {
             Activity? activity = null;
             if (s_activitySource.HasListeners())
             {
-                activity = s_activitySource.CreateActivity(DiagnosticsHandlerLoggingStrings.ActivityName, ActivityKind.Client);
+                activity = s_activitySource.StartActivity(DiagnosticsHandlerLoggingStrings.ActivityName, ActivityKind.Client);
             }
 
-            if (activity is null)
+            if (activity is null &&
+                (Activity.Current is not null ||
+                s_diagnosticListener.IsEnabled(DiagnosticsHandlerLoggingStrings.ActivityName, request)))
             {
-                if (Activity.Current is not null || s_diagnosticListener.IsEnabled(DiagnosticsHandlerLoggingStrings.ActivityName, requestMessage))
-                {
-                    activity = new Activity(DiagnosticsHandlerLoggingStrings.ActivityName);
-                }
+                activity = new Activity(DiagnosticsHandlerLoggingStrings.ActivityName).Start();
             }
 
             return activity;
@@ -109,12 +108,30 @@ private async ValueTask<HttpResponseMessage> SendAsyncCore(HttpRequestMessage re
             DiagnosticListener diagnosticListener = s_diagnosticListener;
 
             Guid loggingRequestId = Guid.Empty;
-            Activity? activity = CreateActivity(request);
+            Activity? activity = StartActivity(request);
 
-            // Start activity anyway if it was created.
             if (activity is not null)
             {
-                activity.Start();
+                // https://github.com/open-telemetry/semantic-conventions/blob/release/v1.23.x/docs/http/http-spans.md#name
+                activity.DisplayName = HttpMethod.GetKnownMethod(request.Method.Method)?.Method ?? "HTTP";
+
+                if (activity.IsAllDataRequested)
+                {
+                    // Add standard tags known before sending the request.
+                    KeyValuePair<string, object?> methodTag = DiagnosticsHelper.GetMethodTag(request.Method, out bool isUnknownMethod);
+                    activity.SetTag(methodTag.Key, methodTag.Value);
+                    if (isUnknownMethod)
+                    {
+                        activity.SetTag("http.request.method_original", request.Method.Method);
+                    }
+
+                    if (request.RequestUri is Uri requestUri && requestUri.IsAbsoluteUri)
+                    {
+                        activity.SetTag("server.address", requestUri.Host);
+                        activity.SetTag("server.port", requestUri.Port);
+                        activity.SetTag("url.full", DiagnosticsHelper.GetRedactedUriString(requestUri));
+                    }
+                }
 
                 // Only send start event to users who subscribed for it.
                 if (diagnosticListener.IsEnabled(DiagnosticsHandlerLoggingStrings.ActivityStartName))
@@ -141,6 +158,7 @@ private async ValueTask<HttpResponseMessage> SendAsyncCore(HttpRequestMessage re
             }
 
             HttpResponseMessage? response = null;
+            Exception? exception = null;
             TaskStatus taskStatus = TaskStatus.RanToCompletion;
             try
             {
@@ -159,6 +177,7 @@ await _innerHandler.SendAsync(request, cancellationToken).ConfigureAwait(false)
             catch (Exception ex)
             {
                 taskStatus = TaskStatus.Faulted;
+                exception = ex;
 
                 if (diagnosticListener.IsEnabled(DiagnosticsHandlerLoggingStrings.ExceptionEventName))
                 {
@@ -176,6 +195,25 @@ await _innerHandler.SendAsync(request, cancellationToken).ConfigureAwait(false)
                 {
                     activity.SetEndTime(DateTime.UtcNow);
 
+                    if (activity.IsAllDataRequested)
+                    {
+                        // Add standard tags known at request completion.
+                        if (response is not null)
+                        {
+                            activity.SetTag("http.response.status_code", DiagnosticsHelper.GetBoxedStatusCode((int)response.StatusCode));
+                            activity.SetTag("network.protocol.version", DiagnosticsHelper.GetProtocolVersionString(response.Version));
+                        }
+
+                        if (DiagnosticsHelper.TryGetErrorType(response, exception, out string? errorType))
+                        {
+                            activity.SetTag("error.type", errorType);
+
+                            // The presence of error.type indicates that the conditions for setting Error status are also met.
+                            // https://github.com/open-telemetry/semantic-conventions/blob/v1.26.0/docs/http/http-spans.md#status
+                            activity.SetStatus(ActivityStatusCode.Error);
+                        }
+                    }
+
                     // Only send stop event to users who subscribed for it.
                     if (diagnosticListener.IsEnabled(DiagnosticsHandlerLoggingStrings.ActivityStopName))
                     {

src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHelper.cs

@@ -0,0 +1,121 @@
+// 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.Generic;
+using System.Diagnostics;
+using System.Threading;
+
+namespace System.Net.Http
+{
+    internal static class DiagnosticsHelper
+    {
+        internal static string GetRedactedUriString(Uri uri)
+        {
+            Debug.Assert(uri.IsAbsoluteUri);
+
+            if (GlobalHttpSettings.DiagnosticsHandler.DisableUriRedaction)
+            {
+                return uri.AbsoluteUri;
+            }
+
+            string pathAndQuery = uri.PathAndQuery;
+            int queryIndex = pathAndQuery.IndexOf('?');
+
+            bool redactQuery = queryIndex >= 0 && // Query is present.
+                queryIndex < pathAndQuery.Length - 1; // Query is not empty.
+
+            return (redactQuery, uri.IsDefaultPort) switch
+            {
+                (true, true) => $"{uri.Scheme}://{uri.Host}{pathAndQuery.AsSpan(0, queryIndex + 1)}*",
+                (true, false) => $"{uri.Scheme}://{uri.Host}:{uri.Port}{pathAndQuery.AsSpan(0, queryIndex + 1)}*",
+                (false, true) => $"{uri.Scheme}://{uri.Host}{pathAndQuery}",
+                (false, false) => $"{uri.Scheme}://{uri.Host}:{uri.Port}{pathAndQuery}"
+            };
+        }
+
+        internal static KeyValuePair<string, object?> GetMethodTag(HttpMethod method, out bool isUnknownMethod)
+        {
+            // Return canonical names for known methods and "_OTHER" for unknown ones.
+            HttpMethod? known = HttpMethod.GetKnownMethod(method.Method);
+            isUnknownMethod = known is null;
+            return new KeyValuePair<string, object?>("http.request.method", isUnknownMethod ? "_OTHER" : known!.Method);
+        }
+
+        internal static string GetProtocolVersionString(Version httpVersion) => (httpVersion.Major, httpVersion.Minor) switch
+        {
+            (1, 0) => "1.0",
+            (1, 1) => "1.1",
+            (2, 0) => "2",
+            (3, 0) => "3",
+            _ => httpVersion.ToString()
+        };
+
+        public static bool TryGetErrorType(HttpResponseMessage? response, Exception? exception, out string? errorType)
+        {
+            if (response is not null)
+            {
+                int statusCode = (int)response.StatusCode;
+
+                // In case the status code indicates a client or a server error, return the string representation of the status code.
+                // See the paragraph Status and the definition of 'error.type' in
+                // https://github.com/open-telemetry/semantic-conventions/blob/2bad9afad58fbd6b33cc683d1ad1f006e35e4a5d/docs/http/http-spans.md
+                if (statusCode >= 400 && statusCode <= 599)
+                {
+                    errorType = GetErrorStatusCodeString(statusCode);
+                    return true;
+                }
+            }
+
+            if (exception is null)
+            {
+                errorType = null;
+                return false;
+            }
+
+            Debug.Assert(Enum.GetValues<HttpRequestError>().Length == 12, "We need to extend the mapping in case new values are added to HttpRequestError.");
+            errorType = (exception as HttpRequestException)?.HttpRequestError switch
+            {
+                HttpRequestError.NameResolutionError => "name_resolution_error",
+                HttpRequestError.ConnectionError => "connection_error",
+                HttpRequestError.SecureConnectionError => "secure_connection_error",
+                HttpRequestError.HttpProtocolError => "http_protocol_error",
+                HttpRequestError.ExtendedConnectNotSupported => "extended_connect_not_supported",
+                HttpRequestError.VersionNegotiationError => "version_negotiation_error",
+                HttpRequestError.UserAuthenticationError => "user_authentication_error",
+                HttpRequestError.ProxyTunnelError => "proxy_tunnel_error",
+                HttpRequestError.InvalidResponse => "invalid_response",
+                HttpRequestError.ResponseEnded => "response_ended",
+                HttpRequestError.ConfigurationLimitExceeded => "configuration_limit_exceeded",
+
+                // Fall back to the exception type name in case of HttpRequestError.Unknown or when exception is not an HttpRequestException.
+                _ => exception.GetType().FullName!
+            };
+            return true;
+        }
+
+        private static object[]? s_boxedStatusCodes;
+        private static string[]? s_statusCodeStrings;
+
+#pragma warning disable CA1859 // we explictly box here
+        public static object GetBoxedStatusCode(int statusCode)
+        {
+            object[] boxes = LazyInitializer.EnsureInitialized(ref s_boxedStatusCodes, static () => new object[512]);
+
+            return (uint)statusCode < (uint)boxes.Length
+                ? boxes[statusCode] ??= statusCode
+                : statusCode;
+        }
+#pragma warning restore
+
+        private static string GetErrorStatusCodeString(int statusCode)
+        {
+            Debug.Assert(statusCode >= 400 && statusCode <= 599);
+
+            string[] strings = LazyInitializer.EnsureInitialized(ref s_statusCodeStrings, static () => new string[200]);
+            int index = statusCode - 400;
+            return (uint)index < (uint)strings.Length
+                ? strings[index] ??= statusCode.ToString()
+                : statusCode.ToString();
+        }
+    }
+}

src/libraries/System.Net.Http/src/System/Net/Http/GlobalHttpSettings.cs

@@ -14,6 +14,11 @@ internal static class DiagnosticsHandler
                 "System.Net.Http.EnableActivityPropagation",
                 "DOTNET_SYSTEM_NET_HTTP_ENABLEACTIVITYPROPAGATION",
                 true);
+
+            public static bool DisableUriRedaction { get; } = RuntimeSettingParser.QueryRuntimeSettingSwitch(
+                "System.Net.Http.DisableUriRedaction",
+                "DOTNET_SYSTEM_NET_HTTP_DISABLEURIREDACTION",
+                false);
         }
 
         internal static class SocketsHttpHandler

src/libraries/System.Net.Http/src/System/Net/Http/Metrics/MetricsHandler.cs

@@ -109,11 +109,11 @@ private void RequestStop(HttpRequestMessage request, HttpResponseMessage? respon
 
             if (response is not null)
             {
-                tags.Add("http.response.status_code", GetBoxedStatusCode((int)response.StatusCode));
-                tags.Add("network.protocol.version", GetProtocolVersionString(response.Version));
+                tags.Add("http.response.status_code", DiagnosticsHelper.GetBoxedStatusCode((int)response.StatusCode));
+                tags.Add("network.protocol.version", DiagnosticsHelper.GetProtocolVersionString(response.Version));
             }
 
-            if (TryGetErrorType(response, exception, out string? errorType))
+            if (DiagnosticsHelper.TryGetErrorType(response, exception, out string? errorType))
             {
                 tags.Add("error.type", errorType);
             }
@@ -131,58 +131,6 @@ private void RequestStop(HttpRequestMessage request, HttpResponseMessage? respon
             }
         }
 
-        private static bool TryGetErrorType(HttpResponseMessage? response, Exception? exception, out string? errorType)
-        {
-            if (response is not null)
-            {
-                int statusCode = (int)response.StatusCode;
-
-                // In case the status code indicates a client or a server error, return the string representation of the status code.
-                // See the paragraph Status and the definition of 'error.type' in
-                // https://github.com/open-telemetry/semantic-conventions/blob/2bad9afad58fbd6b33cc683d1ad1f006e35e4a5d/docs/http/http-spans.md
-                if (statusCode >= 400 && statusCode <= 599)
-                {
-                    errorType = GetErrorStatusCodeString(statusCode);
-                    return true;
-                }
-            }
-
-            if (exception is null)
-            {
-                errorType = null;
-                return false;
-            }
-
-            Debug.Assert(Enum.GetValues<HttpRequestError>().Length == 12, "We need to extend the mapping in case new values are added to HttpRequestError.");
-            errorType = (exception as HttpRequestException)?.HttpRequestError switch
-            {
-                HttpRequestError.NameResolutionError => "name_resolution_error",
-                HttpRequestError.ConnectionError => "connection_error",
-                HttpRequestError.SecureConnectionError => "secure_connection_error",
-                HttpRequestError.HttpProtocolError => "http_protocol_error",
-                HttpRequestError.ExtendedConnectNotSupported => "extended_connect_not_supported",
-                HttpRequestError.VersionNegotiationError => "version_negotiation_error",
-                HttpRequestError.UserAuthenticationError => "user_authentication_error",
-                HttpRequestError.ProxyTunnelError => "proxy_tunnel_error",
-                HttpRequestError.InvalidResponse => "invalid_response",
-                HttpRequestError.ResponseEnded => "response_ended",
-                HttpRequestError.ConfigurationLimitExceeded => "configuration_limit_exceeded",
-
-                // Fall back to the exception type name in case of HttpRequestError.Unknown or when exception is not an HttpRequestException.
-                _ => exception.GetType().FullName!
-            };
-            return true;
-        }
-
-        private static string GetProtocolVersionString(Version httpVersion) => (httpVersion.Major, httpVersion.Minor) switch
-        {
-            (1, 0) => "1.0",
-            (1, 1) => "1.1",
-            (2, 0) => "2",
-            (3, 0) => "3",
-            _ => httpVersion.ToString()
-        };
-
         private static TagList InitializeCommonTags(HttpRequestMessage request)
         {
             TagList tags = default;
@@ -197,43 +145,11 @@ private static TagList InitializeCommonTags(HttpRequestMessage request)
                     tags.Add("server.port", requestUri.Port);
                 }
             }
-            tags.Add(GetMethodTag(request.Method));
+            tags.Add(DiagnosticsHelper.GetMethodTag(request.Method, out _));
 
             return tags;
         }
 
-        internal static KeyValuePair<string, object?> GetMethodTag(HttpMethod method)
-        {
-            // Return canonical names for known methods and "_OTHER" for unknown ones.
-            HttpMethod? known = HttpMethod.GetKnownMethod(method.Method);
-            return new KeyValuePair<string, object?>("http.request.method", known?.Method ?? "_OTHER");
-        }
-
-        private static object[]? s_boxedStatusCodes;
-        private static string[]? s_statusCodeStrings;
-
-#pragma warning disable CA1859 // we explictly box here
-        private static object GetBoxedStatusCode(int statusCode)
-        {
-            object[] boxes = LazyInitializer.EnsureInitialized(ref s_boxedStatusCodes, static () => new object[512]);
-
-            return (uint)statusCode < (uint)boxes.Length
-                ? boxes[statusCode] ??= statusCode
-                : statusCode;
-        }
-#pragma warning restore
-
-        private static string GetErrorStatusCodeString(int statusCode)
-        {
-            Debug.Assert(statusCode >= 400 && statusCode <= 599);
-
-            string[] strings = LazyInitializer.EnsureInitialized(ref s_statusCodeStrings, static () => new string[200]);
-            int index = statusCode - 400;
-            return (uint)index < (uint)strings.Length
-                ? strings[index] ??= statusCode.ToString()
-                : statusCode.ToString();
-        }
-
         private sealed class SharedMeter : Meter
         {
             public static Meter Instance { get; } = new SharedMeter();

src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Metrics/SocketsHttpHandlerMetrics.cs

@@ -47,7 +47,7 @@ public void RequestLeftQueue(HttpRequestMessage request, HttpConnectionPool pool
                     tags.Add("server.port", pool.OriginAuthority.Port);
                 }
 
-                tags.Add(MetricsHandler.GetMethodTag(request.Method));
+                tags.Add(DiagnosticsHelper.GetMethodTag(request.Method, out _));
 
                 RequestsQueueDuration.Record(duration.TotalSeconds, tags);
             }