From c7fa446d38acb31f884fe8d4be929ceb7ce9ea6d Mon Sep 17 00:00:00 2001
From: Anton Firszov <Anton.Firszov@microsoft.com>
Date: Wed, 10 Jul 2024 02:40:06 +0200
Subject: [PATCH] Add standard tags to HttpClient native trace instrumentation
 (#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 #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.
---
 .../src/System.Net.Http.csproj                |   1 +
 .../src/System/Net/Http/DiagnosticsHandler.cs |  58 ++++-
 .../src/System/Net/Http/DiagnosticsHelper.cs  | 121 +++++++++++
 .../src/System/Net/Http/GlobalHttpSettings.cs |   5 +
 .../System/Net/Http/Metrics/MetricsHandler.cs |  92 +-------
 .../Metrics/SocketsHttpHandlerMetrics.cs      |   2 +-
 .../tests/FunctionalTests/DiagnosticsTests.cs | 203 +++++++++++++++++-
 .../tests/FunctionalTests/MetricsTest.cs      |  59 ++---
 .../tests/UnitTests/DiagnosticsHelperTest.cs  | 116 ++++++++++
 .../System.Net.Http.Unit.Tests.csproj         |   3 +
 10 files changed, 535 insertions(+), 125 deletions(-)
 create mode 100644 src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHelper.cs
 create mode 100644 src/libraries/System.Net.Http/tests/UnitTests/DiagnosticsHelperTest.cs

diff --git a/src/libraries/System.Net.Http/src/System.Net.Http.csproj b/src/libraries/System.Net.Http/src/System.Net.Http.csproj
index fbaf1d950a196..477cb08f059a4 100644
--- a/src/libraries/System.Net.Http/src/System.Net.Http.csproj
+++ b/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" />
diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHandler.cs b/src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHandler.cs
index 6436c1595b46e..ed82d0b37d299 100644
--- a/src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHandler.cs
+++ b/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))
                     {
diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHelper.cs b/src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHelper.cs
new file mode 100644
index 0000000000000..3622edfee0cb8
--- /dev/null
+++ b/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();
+        }
+    }
+}
diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/GlobalHttpSettings.cs b/src/libraries/System.Net.Http/src/System/Net/Http/GlobalHttpSettings.cs
index afeac67e38a5d..72c9a4a3fc52e 100644
--- a/src/libraries/System.Net.Http/src/System/Net/Http/GlobalHttpSettings.cs
+++ b/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
diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Metrics/MetricsHandler.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Metrics/MetricsHandler.cs
index 942cd6ceaed36..152ad063a5b8a 100644
--- a/src/libraries/System.Net.Http/src/System/Net/Http/Metrics/MetricsHandler.cs
+++ b/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();
diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Metrics/SocketsHttpHandlerMetrics.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Metrics/SocketsHttpHandlerMetrics.cs
index bd37b3f66b805..f654136a1001c 100644
--- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Metrics/SocketsHttpHandlerMetrics.cs
+++ b/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);
             }
diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/DiagnosticsTests.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/DiagnosticsTests.cs
index a46d376157fc1..4a067b778fb89 100644
--- a/src/libraries/System.Net.Http/tests/FunctionalTests/DiagnosticsTests.cs
+++ b/src/libraries/System.Net.Http/tests/FunctionalTests/DiagnosticsTests.cs
@@ -18,7 +18,7 @@
 
 namespace System.Net.Http.Functional.Tests
 {
-    public abstract class DiagnosticsTest : HttpClientHandlerTestBase
+    public abstract class DiagnosticsTest : DiagnosticsTestBase
     {
         private const string EnableActivityPropagationEnvironmentVariableSettingName = "DOTNET_SYSTEM_NET_HTTP_ENABLEACTIVITYPROPAGATION";
         private const string EnableActivityPropagationAppCtxSettingName = "System.Net.Http.EnableActivityPropagation";
@@ -377,6 +377,206 @@ await GetFactoryForVersion(useVersion).CreateClientAndServerAsync(
             }, UseVersion.ToString(), TestAsync.ToString(), idFormat.ToString()).DisposeAsync();
         }
 
+        [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
+        [InlineData(200, "GET")]
+        [InlineData(404, "GET")]
+        [InlineData(200, "CUSTOM")]
+        public async Task SendAsync_DiagnosticListener_ExpectedTagsRecorded(int statusCode, string method)
+        {
+            await RemoteExecutor.Invoke(static async (useVersion, testAsync, statusCodeStr, method) =>
+            {
+                TaskCompletionSource activityStopTcs = new(TaskCreationOptions.RunContinuationsAsynchronously);
+
+                Activity parentActivity = new Activity("parent");
+                parentActivity.Start();
+
+                Uri? currentUri = null;
+                string? expectedUriFull = null;
+
+                var diagnosticListenerObserver = new FakeDiagnosticListenerObserver(kvp =>
+                {
+                    Assert.NotNull(currentUri);
+
+                    if (!kvp.Key.StartsWith("System.Net.Http.HttpRequestOut"))
+                    {
+                        return;
+                    }
+                    Activity activity = Activity.Current;
+                    Assert.NotNull(activity);
+                    Assert.Equal(parentActivity, Activity.Current.Parent);
+                    IEnumerable<KeyValuePair<string, object?>> tags = activity.TagObjects;
+                    Assert.Equal(method is "CUSTOM" ? "HTTP" : method, activity.DisplayName);
+                    VerifyRequestTags(tags, currentUri, expectedUriFull, expectedMethodTag: method is "CUSTOM" ? "_OTHER" : method);
+                    VerifyTag(tags, "http.request.method_original", method is "CUSTOM" ? method : null);
+
+                    if (kvp.Key.EndsWith(".Stop"))
+                    {
+                        VerifyTag(tags, "network.protocol.version", GetVersionString(Version.Parse(useVersion)));
+                        VerifyTag(tags, "http.response.status_code", int.Parse(statusCodeStr));
+
+                        if (statusCodeStr != "200")
+                        {
+                            string errorType = (string)tags.Single(t => t.Key == "error.type").Value;
+                            Assert.Equal(ActivityStatusCode.Error, activity.Status);
+                            Assert.Equal(statusCodeStr, errorType);
+                        }
+                        else
+                        {
+                            Assert.Equal(ActivityStatusCode.Unset, activity.Status);
+                            Assert.DoesNotContain(tags, t => t.Key == "error.type");
+                        }
+
+                        activityStopTcs.SetResult();
+                    }
+                });
+
+                using (DiagnosticListener.AllListeners.Subscribe(diagnosticListenerObserver))
+                {
+                    diagnosticListenerObserver.Enable(s => s.Contains("HttpRequestOut"));
+
+                    await GetFactoryForVersion(useVersion).CreateClientAndServerAsync(
+                        async uri =>
+                        {
+                            uri = new Uri($"{uri.Scheme}://user:pass@{uri.Authority}/1/2/?a=1&b=2");
+                            expectedUriFull = $"{uri.Scheme}://{uri.Authority}{uri.AbsolutePath}?*";
+                            currentUri = uri;
+
+                            using HttpClient client = new(CreateHttpClientHandler(allowAllCertificates: true));
+                            using HttpRequestMessage request = CreateRequest(HttpMethod.Parse(method), uri, Version.Parse(useVersion), exactVersion: true);
+                            await client.SendAsync(bool.Parse(testAsync), request);
+                            await activityStopTcs.Task;
+                        },
+                        async server =>
+                        {
+                            HttpRequestData requestData = await server.AcceptConnectionSendResponseAndCloseAsync(
+                                statusCode: (HttpStatusCode)int.Parse(statusCodeStr));
+                            AssertHeadersAreInjected(requestData, parentActivity);
+                        });
+                }
+            }, UseVersion.ToString(), TestAsync.ToString(), statusCode.ToString(), method).DisposeAsync();
+        }
+
+
+        [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
+        [InlineData(200, "GET")]
+        [InlineData(404, "GET")]
+        [InlineData(200, "CUSTOM")]
+        public async Task SendAsync_ActivitySource_ExpectedTagsRecorded(int statusCode, string method)
+        {
+            await RemoteExecutor.Invoke(static async (useVersion, testAsync, statusCodeStr, method) =>
+            {
+                TaskCompletionSource activityStopTcs = new(TaskCreationOptions.RunContinuationsAsynchronously);
+                Activity? activity = null;
+                Uri? currentUri = null;
+                string? expectedUriFull = null;
+
+                using ActivityListener listener = new ActivityListener()
+                {
+                    ShouldListenTo = s => s.Name is "System.Net.Http",
+                    Sample = (ref ActivityCreationOptions<ActivityContext> _) => ActivitySamplingResult.AllData,
+                    ActivityStopped = a =>
+                    {
+                        activity = a;
+                        var tags = activity.TagObjects;
+                        VerifyRequestTags(a.TagObjects, currentUri, expectedUriFull, expectedMethodTag: method is "CUSTOM" ? "_OTHER" : method);
+                        VerifyTag(tags, "http.request.method_original", method is "CUSTOM" ? method : null);
+                        VerifyTag(tags, "network.protocol.version", GetVersionString(Version.Parse(useVersion)));
+                        VerifyTag(tags, "http.response.status_code", int.Parse(statusCodeStr));
+
+                        if (statusCodeStr != "200")
+                        {
+                            string errorType = (string)tags.Single(t => t.Key == "error.type").Value;
+                            Assert.Equal(ActivityStatusCode.Error, activity.Status);
+                            Assert.Equal(statusCodeStr, errorType);
+                        }
+                        else
+                        {
+                            Assert.Equal(ActivityStatusCode.Unset, activity.Status);
+                            Assert.DoesNotContain(tags, t => t.Key == "error.type");
+                        }
+
+                        activityStopTcs.SetResult();
+                    }
+                };
+                ActivitySource.AddActivityListener(listener);
+
+                await GetFactoryForVersion(useVersion).CreateClientAndServerAsync(
+                        async uri =>
+                        {
+                            uri = new Uri($"{uri.Scheme}://user:pass@{uri.Authority}/1/2/?a=1&b=2");
+                            expectedUriFull = $"{uri.Scheme}://{uri.Authority}{uri.AbsolutePath}?*";
+                            currentUri = uri;
+
+                            using HttpClient client = new(CreateHttpClientHandler(allowAllCertificates: true));
+                            using HttpRequestMessage request = CreateRequest(HttpMethod.Parse(method), uri, Version.Parse(useVersion), exactVersion: true);
+                            await client.SendAsync(bool.Parse(testAsync), request);
+                            await activityStopTcs.Task;
+                        },
+                        async server =>
+                        {
+                            await server.AcceptConnectionSendResponseAndCloseAsync(statusCode: (HttpStatusCode)int.Parse(statusCodeStr));
+                        });
+
+                Assert.NotNull(activity);
+            }, UseVersion.ToString(), TestAsync.ToString(), statusCode.ToString(), method).DisposeAsync();
+        }
+
+        [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
+        public async Task SendAsync_DoNotSampleAllData_NoTagsRecorded()
+        {
+            await RemoteExecutor.Invoke(static async (useVersion, testAsync) =>
+            {
+                TaskCompletionSource activityStopTcs = new(TaskCreationOptions.RunContinuationsAsynchronously);
+                Activity? activity = null;
+                using ActivityListener listener = new ActivityListener()
+                {
+                    ShouldListenTo = s => s.Name is "System.Net.Http",
+                    Sample = (ref ActivityCreationOptions<ActivityContext> options) => ActivitySamplingResult.PropagationData,
+                    ActivityStopped = a =>
+                    {
+                        activity = a;
+                        activityStopTcs.SetResult();
+                    }
+                };
+                ActivitySource.AddActivityListener(listener);
+
+                await GetFactoryForVersion(useVersion).CreateClientAndServerAsync(
+                            async uri =>
+                            {
+
+                                await GetAsync(useVersion, testAsync, uri);
+                                await activityStopTcs.Task;
+                            },
+                            async server =>
+                            {
+                                _ = await server.AcceptConnectionSendResponseAndCloseAsync();
+                            });
+
+                Assert.NotNull(activity);
+                Assert.Empty(activity.TagObjects);
+            }, UseVersion.ToString(), TestAsync.ToString()).DisposeAsync();
+        }
+
+        protected static void VerifyRequestTags(IEnumerable<KeyValuePair<string, object?>> tags, Uri uri, string expectedUriFull, string expectedMethodTag = "GET")
+        {
+            VerifyTag(tags, "server.address", uri.Host);
+            VerifyTag(tags, "server.port", uri.Port);
+            VerifyTag(tags, "http.request.method", expectedMethodTag);
+            VerifyTag(tags, "url.full", expectedUriFull);
+        }
+
+        protected static void VerifyTag<T>(KeyValuePair<string, object?>[] tags, string name, T value)
+        {
+            if (value is null)
+            {
+                Assert.DoesNotContain(tags, t => t.Key == name);
+            }
+            else
+            {
+                Assert.Equal(value, (T)tags.Single(t => t.Key == name).Value);
+            }
+        }
+
         [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
         public async Task SendAsync_ExpectedDiagnosticSourceActivityLogging_InvalidBaggage()
         {
@@ -988,7 +1188,6 @@ public static IEnumerable<object[]> SocketsHttpHandler_ActivityCreation_MemberDa
         public async Task SendAsync_ActivityIsCreatedIfRequested(bool currentActivitySet, bool? diagnosticListenerActivityEnabled, bool? activitySourceCreatesActivity)
         {
             string parameters = $"{currentActivitySet},{diagnosticListenerActivityEnabled},{activitySourceCreatesActivity}";
-
             await RemoteExecutor.Invoke(async (useVersion, testAsync, parametersString) =>
             {
                 bool?[] parameters = parametersString.Split(',').Select(p => p.Length == 0 ? (bool?)null : bool.Parse(p)).ToArray();
diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs
index 773fa142a1c7d..b8dc01e63fdb7 100644
--- a/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs
+++ b/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs
@@ -19,23 +19,13 @@
 
 namespace System.Net.Http.Functional.Tests
 {
-    public abstract class HttpMetricsTestBase : HttpClientHandlerTestBase
+    public abstract class DiagnosticsTestBase : HttpClientHandlerTestBase
     {
-        protected static class InstrumentNames
-        {
-            public const string RequestDuration = "http.client.request.duration";
-            public const string ActiveRequests = "http.client.active_requests";
-            public const string OpenConnections = "http.client.open_connections";
-            public const string IdleConnections = "http-client-current-idle-connections";
-            public const string ConnectionDuration = "http.client.connection.duration";
-            public const string TimeInQueue = "http.client.request.time_in_queue";
-        }
-
-        protected HttpMetricsTestBase(ITestOutputHelper output) : base(output)
+        protected DiagnosticsTestBase(ITestOutputHelper output) : base(output)
         {
         }
 
-        protected static void VerifyTag<T>(KeyValuePair<string, object?>[] tags, string name, T value)
+        protected static void VerifyTag<T>(IEnumerable<KeyValuePair<string, object?>> tags, string name, T value)
         {
             if (value is null)
             {
@@ -43,32 +33,53 @@ protected static void VerifyTag<T>(KeyValuePair<string, object?>[] tags, string
             }
             else
             {
-                Assert.Equal(value, (T)tags.Single(t => t.Key == name).Value);
+                object? actualValue = tags.Single(t => t.Key == name).Value;
+                Assert.Equal(value, (T)actualValue);
             }
         }
 
-        private static void VerifyPeerAddress(KeyValuePair<string, object?>[] tags)
-        {
-            string ipString = (string)tags.Single(t => t.Key == "network.peer.address").Value;
-            IPAddress ip = IPAddress.Parse(ipString);
-            Assert.True(ip.Equals(IPAddress.Loopback.MapToIPv6()) ||
-                    ip.Equals(IPAddress.Loopback) ||
-                    ip.Equals(IPAddress.IPv6Loopback));
-        }
 
-        private static void VerifySchemeHostPortTags(KeyValuePair<string, object?>[] tags, Uri uri)
+        protected static void VerifySchemeHostPortTags(IEnumerable<KeyValuePair<string, object?>> tags, Uri uri)
         {
             VerifyTag(tags, "url.scheme", uri.Scheme);
             VerifyTag(tags, "server.address", uri.Host);
             VerifyTag(tags, "server.port", uri.Port);
         }
 
-        private static string? GetVersionString(Version? version) => version == null ? null : version.Major switch
+        protected static string? GetVersionString(Version? version) => version == null ? null : version.Major switch
         {
             1 => "1.1",
             2 => "2",
             _ => "3"
         };
+    }
+
+    public abstract class HttpMetricsTestBase : DiagnosticsTestBase
+    {
+        protected static class InstrumentNames
+        {
+            public const string RequestDuration = "http.client.request.duration";
+            public const string ActiveRequests = "http.client.active_requests";
+            public const string OpenConnections = "http.client.open_connections";
+            public const string IdleConnections = "http-client-current-idle-connections";
+            public const string ConnectionDuration = "http.client.connection.duration";
+            public const string TimeInQueue = "http.client.request.time_in_queue";
+        }
+
+        protected HttpMetricsTestBase(ITestOutputHelper output) : base(output)
+        {
+        }
+
+
+        private static void VerifyPeerAddress(KeyValuePair<string, object?>[] tags)
+        {
+            string ipString = (string)tags.Single(t => t.Key == "network.peer.address").Value;
+            IPAddress ip = IPAddress.Parse(ipString);
+            Assert.True(ip.Equals(IPAddress.Loopback.MapToIPv6()) ||
+                    ip.Equals(IPAddress.Loopback) ||
+                    ip.Equals(IPAddress.IPv6Loopback));
+        }
+
 
         protected static void VerifyRequestDuration(Measurement<double> measurement,
             Uri uri,
diff --git a/src/libraries/System.Net.Http/tests/UnitTests/DiagnosticsHelperTest.cs b/src/libraries/System.Net.Http/tests/UnitTests/DiagnosticsHelperTest.cs
new file mode 100644
index 0000000000000..24567e47cd9d3
--- /dev/null
+++ b/src/libraries/System.Net.Http/tests/UnitTests/DiagnosticsHelperTest.cs
@@ -0,0 +1,116 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+using Microsoft.Diagnostics.Runtime.Utilities;
+using Microsoft.DotNet.RemoteExecutor;
+using Xunit;
+
+namespace System.Net.Http.Tests
+{
+    public class DiagnosticsHelperTest
+    {
+        public static readonly TheoryData<string, string> GetRedactedUriString_Data = new TheoryData<string, string>()
+        {
+            { "http://q.app/foo", "http://q.app/foo" },
+            { "http://q.app:123/foo", "http://q.app:123/foo" },
+            { "http://user:xxx@q.app/foo", "http://q.app/foo" }, // has user info
+            { "http://q.app/foo?", "http://q.app/foo?" },
+            { "http://q.app/foo?XXX", "http://q.app/foo?*" },
+            { "http://q.app/a/b/c?a=b%20c&x=1", "http://q.app/a/b/c?*" },
+            { "http://q.app:4242/a/b/c?a=b%20c&x=1", "http://q.app:4242/a/b/c?*" },
+        };
+
+        [Theory]
+        [MemberData(nameof(GetRedactedUriString_Data))]
+        public void GetRedactedUriString_RedactsUriByDefault(string original, string expected)
+        {
+            string redacted = DiagnosticsHelper.GetRedactedUriString(new Uri(original));
+            Assert.Equal(expected, redacted);
+        }
+
+        [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
+        public async Task GetRedactedUriString_DisableUriRedaction_DoesNotRedactUri()
+        {
+            await RemoteExecutor.Invoke(() =>
+            {
+                AppContext.SetSwitch("System.Net.Http.DisableUriRedaction", true);
+
+                Uri[] uris = GetRedactedUriString_Data.Select(a => a[0] == null ? null : new Uri((string)a[0], UriKind.RelativeOrAbsolute)).ToArray();
+
+                foreach (Uri uri in uris)
+                {
+                    string actual = DiagnosticsHelper.GetRedactedUriString(uri);
+                    Assert.Equal(uri.AbsoluteUri, actual);
+                }
+            }).DisposeAsync();
+        }
+
+        [Fact]
+        public void TryGetErrorType_NoError_ReturnsFalse()
+        {
+            HttpResponseMessage response = new(HttpStatusCode.OK);
+            Assert.False(DiagnosticsHelper.TryGetErrorType(response, null, out _));
+        }
+
+        [Theory]
+        [InlineData(HttpStatusCode.NotFound)]
+        [InlineData(HttpStatusCode.InternalServerError)]
+        public void TryGetErrorType_ErrorStatus_OutputsStatusCodeString(HttpStatusCode statusCode)
+        {
+            HttpResponseMessage response = new(statusCode);
+            Assert.True(DiagnosticsHelper.TryGetErrorType(response, null, out string errorType));
+        }
+
+
+        public static TheoryData<HttpRequestError> TryGetErrorType_HttpRequestError_Data()
+        {
+            TheoryData<HttpRequestError> result = new();
+            foreach (var e in Enum.GetValues(typeof(HttpRequestError)))
+            {
+                result.Add((HttpRequestError)e);
+            }
+            return result;
+        }
+
+        [Theory]
+        [MemberData(nameof(TryGetErrorType_HttpRequestError_Data))]
+        public void TryGetErrorType_HttpRequestError(HttpRequestError error)
+        {
+            HttpRequestException exception = new(error);
+
+            Assert.True(DiagnosticsHelper.TryGetErrorType(null, exception, out string errorType));
+            Assert.Equal(GetExpectedErrorType(error), errorType);
+
+            Assert.True(DiagnosticsHelper.TryGetErrorType(new HttpResponseMessage(HttpStatusCode.OK), exception, out errorType));
+            Assert.Equal(GetExpectedErrorType(error), errorType);
+
+            static string GetExpectedErrorType(HttpRequestError error)
+            {
+                if (error == HttpRequestError.Unknown)
+                {
+                    return typeof(HttpRequestException).FullName;
+                }
+
+                string s = error.ToString();
+                StringBuilder bld = new();
+                bld.Append(char.ToLower(s[0]));
+                for (int i = 1; i < s.Length; i++)
+                {
+                    bld.Append(char.IsUpper(s[i]) ? $"_{char.ToLower(s[i])}" : s[i]);
+                }
+                return bld.ToString();
+            }
+        }
+
+        [Fact]
+        public void TryGetErrorType_OperationCanceledException()
+        {
+            OperationCanceledException ex = new();
+            Assert.True(DiagnosticsHelper.TryGetErrorType(null, ex, out string errorType));
+            Assert.Equal(ex.GetType().FullName, errorType);
+        }
+    }
+}
diff --git a/src/libraries/System.Net.Http/tests/UnitTests/System.Net.Http.Unit.Tests.csproj b/src/libraries/System.Net.Http/tests/UnitTests/System.Net.Http.Unit.Tests.csproj
index f0f23f16ccb3c..4483fe2bf0e47 100755
--- a/src/libraries/System.Net.Http/tests/UnitTests/System.Net.Http.Unit.Tests.csproj
+++ b/src/libraries/System.Net.Http/tests/UnitTests/System.Net.Http.Unit.Tests.csproj
@@ -183,6 +183,8 @@
              Link="ProductionCode\System\Net\Http\Headers\ViaHeaderValue.cs" />
     <Compile Include="..\..\src\System\Net\Http\Headers\WarningHeaderValue.cs"
              Link="ProductionCode\System\Net\Http\Headers\WarningHeaderValue.cs" />
+    <Compile Include="..\..\src\System\Net\Http\DiagnosticsHelper.cs"
+             Link="ProductionCode\System\Net\Http\DiagnosticsHelper.cs" />
     <Compile Include="..\..\src\System\Net\Http\HttpClient.cs"
              Link="ProductionCode\System\Net\Http\HttpClient.cs" />
     <Compile Include="..\..\src\System\Net\Http\HttpHandlerDefaults.cs"
@@ -253,6 +255,7 @@
              Link="ProductionCode\System\Net\Http\SocketsHttpHandler\SystemProxyInfo.Windows.cs" />
     <Compile Include="$(CommonPath)System\Net\Http\HttpHandlerDefaults.cs"
              Link="ProductionCode\System\Net\Http\HttpHandlerDefaults.cs" />
+    <Compile Include="DiagnosticsHelperTest.cs" />
     <Compile Include="DigestAuthenticationTests.cs" />
     <Compile Include="Fakes\HttpClientHandler.cs" />
     <Compile Include="Fakes\HttpTelemetry.cs" />