From 77f21a07cc8fa7043feb5b41272061898b816ab8 Mon Sep 17 00:00:00 2001 From: antonfirsov Date: Fri, 11 Jul 2025 15:24:06 +0200 Subject: [PATCH 1/5] prioritize Host header when emitting server.address --- .../src/System.Net.Http.csproj | 3 +- .../src/System/Net/Http/DiagnosticsHandler.cs | 8 +- .../src/System/Net/Http/DiagnosticsHelper.cs | 14 +++ .../Net/Http/HttpClientHandler.AnyMobile.cs | 5 +- .../src/System/Net/Http/HttpClientHandler.cs | 4 +- .../src/System/Net/Http/HttpUtilities.cs | 37 ++++++ .../System/Net/Http/Metrics/MetricsHandler.cs | 8 +- .../ConnectionSetupDistributedTracing.cs | 8 +- .../HttpConnectionPool.Http3.cs | 2 +- .../ConnectionPool/HttpConnectionPool.cs | 9 +- .../SocketsHttpHandler/HttpConnectionBase.cs | 2 +- .../HttpConnectionPoolManager.cs | 55 ++++----- ...cs => HttpUtilities.SocketsHttpHandler.cs} | 2 +- .../Metrics/SocketsHttpHandlerMetrics.cs | 2 +- .../SocketsHttpHandler/SocketsHttpHandler.cs | 4 +- .../tests/FunctionalTests/DiagnosticsTests.cs | 116 ++++++++++++++++++ .../tests/FunctionalTests/MetricsTest.cs | 98 +++++++++++++++ 17 files changed, 324 insertions(+), 53 deletions(-) create mode 100644 src/libraries/System.Net.Http/src/System/Net/Http/HttpUtilities.cs rename src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/{HttpUtilities.cs => HttpUtilities.SocketsHttpHandler.cs} (97%) 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 4fafeccae048ac..6789d578ffd9f4 100644 --- a/src/libraries/System.Net.Http/src/System.Net.Http.csproj +++ b/src/libraries/System.Net.Http/src/System.Net.Http.csproj @@ -64,6 +64,7 @@ + @@ -209,7 +210,7 @@ - + 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 72f1f88e1e1c3d..4ebcb43752d2e1 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 @@ -21,14 +21,16 @@ internal sealed class DiagnosticsHandler : HttpMessageHandlerStage private readonly HttpMessageHandler _innerHandler; private readonly DistributedContextPropagator _propagator; private readonly HeaderDescriptor[]? _propagatorFields; + private readonly IWebProxy? _proxy; - public DiagnosticsHandler(HttpMessageHandler innerHandler, DistributedContextPropagator propagator, bool autoRedirect = false) + public DiagnosticsHandler(HttpMessageHandler innerHandler, DistributedContextPropagator propagator, IWebProxy? proxy, bool autoRedirect = false) { Debug.Assert(GlobalHttpSettings.DiagnosticsHandler.EnableActivityPropagation); Debug.Assert(innerHandler is not null && propagator is not null); _innerHandler = innerHandler; _propagator = propagator; + _proxy = proxy; // Prepare HeaderDescriptors for fields we need to clear when following redirects if (autoRedirect && _propagator.Fields is IReadOnlyCollection fields && fields.Count > 0) @@ -43,6 +45,8 @@ public DiagnosticsHandler(HttpMessageHandler innerHandler, DistributedContextPro } _propagatorFields = fieldDescriptors.ToArray(); } + + _proxy = proxy; } private static bool IsEnabled() @@ -125,7 +129,7 @@ private async ValueTask SendAsyncCore(HttpRequestMessage re if (request.RequestUri is Uri requestUri && requestUri.IsAbsoluteUri) { - activity.SetTag("server.address", requestUri.Host); + activity.SetTag("server.address", DiagnosticsHelper.GetServerAddress(request, _proxy)); activity.SetTag("server.port", requestUri.Port); activity.SetTag("url.full", UriRedactionHelper.GetRedactedUriString(requestUri)); } 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 index 5b22e671420cd4..e8ec9e7ef3ed1d 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHelper.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHelper.cs @@ -35,6 +35,20 @@ internal static class DiagnosticsHelper _ => httpVersion.ToString() }; + // Picks the value of the 'server.address' tag following rules specified in + // https://github.com/open-telemetry/semantic-conventions/blob/728e5d1/docs/http/http-spans.md#http-client-span + // When there is no proxy, we need to prioritize the contents of the Host header. + public static string GetServerAddress(HttpRequestMessage request, IWebProxy? proxy) + { + Debug.Assert(request.RequestUri is not null); + if ((proxy is null || proxy.IsBypassed(request.RequestUri)) && request.HasHeaders && request.Headers.Host is string hostHeader) + { + return HttpUtilities.ParseHostNameFromHeader(hostHeader); + } + + return request.RequestUri.Host; + } + public static bool TryGetErrorType(HttpResponseMessage? response, Exception? exception, out string? errorType) { if (response is not null) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/HttpClientHandler.AnyMobile.cs b/src/libraries/System.Net.Http/src/System/Net/Http/HttpClientHandler.AnyMobile.cs index da2eb54de9b8d9..ab39fe08a965da 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/HttpClientHandler.AnyMobile.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/HttpClientHandler.AnyMobile.cs @@ -43,13 +43,14 @@ private HttpMessageHandler Handler // MetricsHandler should be descendant of DiagnosticsHandler in the handler chain to make sure the 'http.request.duration' // metric is recorded before stopping the request Activity. This is needed to make sure that our telemetry supports Exemplars. + // Since HttpClientHandler.Proxy is unsupported on many platforms, don't bother passing it. if (GlobalHttpSettings.MetricsHandler.IsGloballyEnabled) { - handler = new MetricsHandler(handler, _nativeMeterFactory, out _); + handler = new MetricsHandler(handler, _nativeMeterFactory, proxy: null, out _); } if (GlobalHttpSettings.DiagnosticsHandler.EnableActivityPropagation) { - handler = new DiagnosticsHandler(handler, DistributedContextPropagator.Current); + handler = new DiagnosticsHandler(handler, DistributedContextPropagator.Current, proxy: null); } // Ensure a single handler is used for all requests. diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/HttpClientHandler.cs b/src/libraries/System.Net.Http/src/System/Net/Http/HttpClientHandler.cs index 9af5dba293170b..a7a817dd9cc7fc 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/HttpClientHandler.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/HttpClientHandler.cs @@ -47,11 +47,11 @@ private HttpMessageHandler Handler // metric is recorded before stopping the request Activity. This is needed to make sure that our telemetry supports Exemplars. if (GlobalHttpSettings.MetricsHandler.IsGloballyEnabled) { - handler = new MetricsHandler(handler, _meterFactory, out _); + handler = new MetricsHandler(handler, _meterFactory, proxy: null, out _); } if (GlobalHttpSettings.DiagnosticsHandler.EnableActivityPropagation) { - handler = new DiagnosticsHandler(handler, DistributedContextPropagator.Current); + handler = new DiagnosticsHandler(handler, DistributedContextPropagator.Current, proxy: null); } // Ensure a single handler is used for all requests. diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/HttpUtilities.cs b/src/libraries/System.Net.Http/src/System/Net/Http/HttpUtilities.cs new file mode 100644 index 00000000000000..19dc5cc54dd0b0 --- /dev/null +++ b/src/libraries/System.Net.Http/src/System/Net/Http/HttpUtilities.cs @@ -0,0 +1,37 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace System.Net.Http +{ + internal static partial class HttpUtilities + { + public static string ParseHostNameFromHeader(string hostHeader) + { + // See if we need to trim off a port. + int colonPos = hostHeader.IndexOf(':'); + if (colonPos >= 0) + { + // There is colon, which could either be a port separator or a separator in + // an IPv6 address. See if this is an IPv6 address; if it's not, use everything + // before the colon as the host name, and if it is, use everything before the last + // colon iff the last colon is after the end of the IPv6 address (otherwise it's a + // part of the address). + int ipV6AddressEnd = hostHeader.IndexOf(']'); + if (ipV6AddressEnd == -1) + { + return hostHeader.Substring(0, colonPos); + } + else + { + colonPos = hostHeader.LastIndexOf(':'); + if (colonPos > ipV6AddressEnd) + { + return hostHeader.Substring(0, colonPos); + } + } + } + + return hostHeader; + } + } +} 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 9b18f93b7d8e1d..4e6f6a05e69ce2 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 @@ -15,12 +15,14 @@ internal sealed class MetricsHandler : HttpMessageHandlerStage private readonly HttpMessageHandler _innerHandler; private readonly UpDownCounter _activeRequests; private readonly Histogram _requestsDuration; + private readonly IWebProxy? _proxy; - public MetricsHandler(HttpMessageHandler innerHandler, IMeterFactory? meterFactory, out Meter meter) + public MetricsHandler(HttpMessageHandler innerHandler, IMeterFactory? meterFactory, IWebProxy? proxy, out Meter meter) { Debug.Assert(GlobalHttpSettings.MetricsHandler.IsGloballyEnabled); _innerHandler = innerHandler; + _proxy = proxy; meter = meterFactory?.Create("System.Net.Http") ?? SharedMeter.Instance; @@ -137,14 +139,14 @@ private void RequestStop(HttpRequestMessage request, HttpResponseMessage? respon } } - private static TagList InitializeCommonTags(HttpRequestMessage request) + private TagList InitializeCommonTags(HttpRequestMessage request) { TagList tags = default; if (request.RequestUri is Uri requestUri && requestUri.IsAbsoluteUri) { tags.Add("url.scheme", requestUri.Scheme); - tags.Add("server.address", requestUri.Host); + tags.Add("server.address", DiagnosticsHelper.GetServerAddress(request, _proxy)); tags.Add("server.port", DiagnosticsHelper.GetBoxedInt32(requestUri.Port)); } tags.Add(DiagnosticsHelper.GetMethodTag(request.Method, out _)); diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/ConnectionSetupDistributedTracing.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/ConnectionSetupDistributedTracing.cs index 522a57f76e61aa..2efd68d7d6b59c 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/ConnectionSetupDistributedTracing.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/ConnectionSetupDistributedTracing.cs @@ -12,7 +12,7 @@ internal static class ConnectionSetupDistributedTracing { private static readonly ActivitySource s_connectionsActivitySource = new ActivitySource(DiagnosticsHandlerLoggingStrings.ConnectionsNamespace); - public static Activity? StartConnectionSetupActivity(bool isSecure, HttpAuthority authority) + public static Activity? StartConnectionSetupActivity(bool isSecure, string host, int port) { Activity? activity = null; if (s_connectionsActivitySource.HasListeners()) @@ -25,11 +25,11 @@ internal static class ConnectionSetupDistributedTracing if (activity is not null) { - activity.DisplayName = $"HTTP connection_setup {authority.HostValue}:{authority.Port}"; + activity.DisplayName = $"HTTP connection_setup {host}:{port}"; if (activity.IsAllDataRequested) { - activity.SetTag("server.address", authority.HostValue); - activity.SetTag("server.port", authority.Port); + activity.SetTag("server.address", host); + activity.SetTag("server.port", port); activity.SetTag("url.scheme", isSecure ? "https" : "http"); } } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs index c2545cd4d7dc36..22bc7aa5f488d0 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.Http3.cs @@ -263,7 +263,7 @@ private async Task InjectNewHttp3ConnectionAsync(RequestQueue. { if (TryGetHttp3Authority(queueItem.Request, out authority, out Exception? reasonException)) { - connectionSetupActivity = ConnectionSetupDistributedTracing.StartConnectionSetupActivity(isSecure: true, authority); + connectionSetupActivity = ConnectionSetupDistributedTracing.StartConnectionSetupActivity(isSecure: true, _telemetryServerAddress, authority.Port); // If the authority was sent as an option through alt-svc then include alt-used header. connection = new Http3Connection(this, authority, includeAltUsedHeader: _http3Authority == authority); QuicConnection quicConnection = await ConnectHelper.ConnectQuicAsync(queueItem.Request, new DnsEndPoint(authority.IdnHost, authority.Port), _poolManager.Settings._pooledConnectionIdleTimeout, _sslOptionsHttp3!, connection.StreamCapacityCallback, cts.Token).ConfigureAwait(false); diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.cs index 2b6ae06651122b..b02ee2decfebe0 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.cs @@ -35,6 +35,7 @@ internal sealed partial class HttpConnectionPool : IDisposable private readonly HttpConnectionPoolManager _poolManager; private readonly HttpConnectionKind _kind; private readonly Uri? _proxyUri; + private readonly string _telemetryServerAddress; /// The origin authority used to construct the . private readonly HttpAuthority _originAuthority; @@ -72,12 +73,14 @@ internal sealed partial class HttpConnectionPool : IDisposable /// The port with which this pool is associated. /// The SSL host with which this pool is associated. /// The proxy this pool targets (optional). - public HttpConnectionPool(HttpConnectionPoolManager poolManager, HttpConnectionKind kind, string? host, int port, string? sslHostName, Uri? proxyUri) + /// The value of the 'server.address' tag to be emitted by Metrics and Distributed Tracing. + public HttpConnectionPool(HttpConnectionPoolManager poolManager, HttpConnectionKind kind, string? host, int port, string? sslHostName, Uri? proxyUri, string telemetryServerAddress) { _poolManager = poolManager; _kind = kind; _proxyUri = proxyUri; _maxHttp11Connections = Settings._maxConnectionsPerServer; + _telemetryServerAddress = telemetryServerAddress; // The only case where 'host' will not be set is if this is a Proxy connection pool. Debug.Assert(host is not null || (kind == HttpConnectionKind.Proxy && proxyUri is not null)); @@ -256,6 +259,7 @@ public HttpConnectionPool(HttpConnectionPoolManager poolManager, HttpConnectionK } if (NetEventSource.Log.IsEnabled()) Trace($"{this}"); + _telemetryServerAddress = telemetryServerAddress; } private static SslClientAuthenticationOptions ConstructSslOptions(HttpConnectionPoolManager poolManager, string sslHostName) @@ -279,6 +283,7 @@ private static SslClientAuthenticationOptions ConstructSslOptions(HttpConnection return sslOptions; } + public string TelemetryServerAddress => _telemetryServerAddress; public HttpAuthority OriginAuthority => _originAuthority; public HttpConnectionSettings Settings => _poolManager.Settings; public HttpConnectionKind Kind => _kind; @@ -557,7 +562,7 @@ public async ValueTask SendWithVersionDetectionAndRetryAsyn Exception? exception = null; TransportContext? transportContext = null; - Activity? activity = ConnectionSetupDistributedTracing.StartConnectionSetupActivity(IsSecure, OriginAuthority); + Activity? activity = ConnectionSetupDistributedTracing.StartConnectionSetupActivity(IsSecure, _telemetryServerAddress, OriginAuthority.Port); try { diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionBase.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionBase.cs index 786aff42b94a76..94a82df383379a 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionBase.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionBase.cs @@ -73,7 +73,7 @@ protected void MarkConnectionAsEstablished(Activity? connectionSetupActivity, IP metrics, protocol, _pool.IsSecure ? "https" : "http", - _pool.OriginAuthority.HostValue, + _pool.TelemetryServerAddress, _pool.OriginAuthority.Port, remoteEndPoint?.Address?.ToString()); diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPoolManager.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPoolManager.cs index 8424d1b26cebe2..52a4e55b22e9a3 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPoolManager.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPoolManager.cs @@ -227,35 +227,6 @@ public void Dispose() public HttpConnectionSettings Settings => _settings; public ICredentials? ProxyCredentials => _proxyCredentials; - private static string ParseHostNameFromHeader(string hostHeader) - { - // See if we need to trim off a port. - int colonPos = hostHeader.IndexOf(':'); - if (colonPos >= 0) - { - // There is colon, which could either be a port separator or a separator in - // an IPv6 address. See if this is an IPv6 address; if it's not, use everything - // before the colon as the host name, and if it is, use everything before the last - // colon iff the last colon is after the end of the IPv6 address (otherwise it's a - // part of the address). - int ipV6AddressEnd = hostHeader.IndexOf(']'); - if (ipV6AddressEnd == -1) - { - return hostHeader.Substring(0, colonPos); - } - else - { - colonPos = hostHeader.LastIndexOf(':'); - if (colonPos > ipV6AddressEnd) - { - return hostHeader.Substring(0, colonPos); - } - } - } - - return hostHeader; - } - private HttpConnectionKey GetConnectionKey(HttpRequestMessage request, Uri? proxyUri, bool isProxyConnect) { Uri? uri = request.RequestUri; @@ -273,7 +244,7 @@ private HttpConnectionKey GetConnectionKey(HttpRequestMessage request, Uri? prox string? hostHeader = request.Headers.Host; if (hostHeader != null) { - sslHostName = ParseHostNameFromHeader(hostHeader); + sslHostName = HttpUtilities.ParseHostNameFromHeader(hostHeader); } else { @@ -330,6 +301,28 @@ private HttpConnectionKey GetConnectionKey(HttpRequestMessage request, Uri? prox } } + // Picks the value of the 'server.address' tag following rules specified in + // https://github.com/open-telemetry/semantic-conventions/blob/728e5d1/docs/http/http-spans.md#http-client-span + // When there is no proxy, we need to prioritize the contents of the Host header. + private static string GetTelemetryServerAddress(HttpRequestMessage request, HttpConnectionKey key) + { + Uri? uri = request.RequestUri; + Debug.Assert(uri is not null); + + if (key.ProxyUri is not null) + { + return uri.IdnHost; + } + + if (key.SslHostName is not null) + { + return key.SslHostName; + } + + string? hostHeader = request.Headers.Host; + return hostHeader is null ? uri.IdnHost : HttpUtilities.ParseHostNameFromHeader(hostHeader); + } + public ValueTask SendAsyncCore(HttpRequestMessage request, Uri? proxyUri, bool async, bool doRequestAuth, bool isProxyConnect, CancellationToken cancellationToken) { HttpConnectionKey key = GetConnectionKey(request, proxyUri, isProxyConnect); @@ -337,7 +330,7 @@ public ValueTask SendAsyncCore(HttpRequestMessage request, HttpConnectionPool? pool; while (!_pools.TryGetValue(key, out pool)) { - pool = new HttpConnectionPool(this, key.Kind, key.Host, key.Port, key.SslHostName, key.ProxyUri); + pool = new HttpConnectionPool(this, key.Kind, key.Host, key.Port, key.SslHostName, key.ProxyUri, GetTelemetryServerAddress(request, key)); if (_cleaningTimer == null) { diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpUtilities.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpUtilities.SocketsHttpHandler.cs similarity index 97% rename from src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpUtilities.cs rename to src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpUtilities.SocketsHttpHandler.cs index be47d560ea77e1..ed5f1c14459716 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpUtilities.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpUtilities.SocketsHttpHandler.cs @@ -3,7 +3,7 @@ namespace System.Net.Http { - internal static class HttpUtilities + internal static partial class HttpUtilities { internal static bool IsSupportedScheme(string scheme) => IsSupportedNonSecureScheme(scheme) || 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 8933aa5c3fd23b..b3886cb8d66f09 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 @@ -46,7 +46,7 @@ public void RequestLeftQueue(HttpRequestMessage request, HttpConnectionPool pool }); tags.Add("url.scheme", pool.IsSecure ? "https" : "http"); - tags.Add("server.address", pool.OriginAuthority.HostValue); + tags.Add("server.address", pool.TelemetryServerAddress); if (!pool.IsDefaultPort) { diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/SocketsHttpHandler.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/SocketsHttpHandler.cs index 19f4197ccd26d6..f4b5dba31dd724 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/SocketsHttpHandler.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/SocketsHttpHandler.cs @@ -527,14 +527,14 @@ private HttpMessageHandlerStage SetupHandlerChain() // metric is recorded before stopping the request Activity. This is needed to make sure that our telemetry supports Exemplars. if (GlobalHttpSettings.MetricsHandler.IsGloballyEnabled) { - handler = new MetricsHandler(handler, settings._meterFactory, out Meter meter); + handler = new MetricsHandler(handler, settings._meterFactory, settings._proxy, out Meter meter); settings._metrics = new SocketsHttpHandlerMetrics(meter); } // DiagnosticsHandler is inserted before RedirectHandler so that trace propagation is done on redirects as well if (GlobalHttpSettings.DiagnosticsHandler.EnableActivityPropagation && settings._activityHeadersPropagator is DistributedContextPropagator propagator) { - handler = new DiagnosticsHandler(handler, propagator, settings._allowAutoRedirect); + handler = new DiagnosticsHandler(handler, propagator, settings._proxy, settings._allowAutoRedirect); } if (settings._allowAutoRedirect) diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/DiagnosticsTests.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/DiagnosticsTests.cs index ffc8cc7fdd93ce..ba5db2585f86a7 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/DiagnosticsTests.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/DiagnosticsTests.cs @@ -1665,6 +1665,122 @@ static async Task RunTest() } } + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [InlineData(false)] + [InlineData(true)] + public async Task UseIPAddressInTargetUri_NoProxy_RecordsHostHeaderAsServerAddress(bool useTls) + { + if (UseVersion == HttpVersion30 && !useTls) return; + + await RemoteExecutor.Invoke(RunTest, UseVersion.ToString(), TestAsync.ToString(), useTls.ToString()).DisposeAsync(); + //await RunTest(UseVersion.ToString(), TestAsync.ToString(), useTls.ToString()); + static async Task RunTest(string useVersion, string testAsync, string useTlsString) + { + bool useTls = bool.Parse(useTlsString); + + Activity parentActivity = new Activity("parent").Start(); + + using ActivityRecorder requestRecorder = new("System.Net.Http", "System.Net.Http.HttpRequestOut") + { + ExpectedParent = parentActivity + }; + + using ActivityRecorder connectionSetupRecorder = new("Experimental.System.Net.Http.Connections", "Experimental.System.Net.Http.Connections.ConnectionSetup"); + + await GetFactoryForVersion(useVersion).CreateClientAndServerAsync( + async uri => + { + string hostName = uri.Host; + uri = new Uri($"{uri.Scheme}://{IPAddress.Loopback}:{uri.Port}"); + + Version version = Version.Parse(useVersion); + + using HttpClient client = new HttpClient(CreateHttpClientHandler(allowAllCertificates: true)); + + using HttpRequestMessage request = CreateRequest(HttpMethod.Get, uri, version, exactVersion: true); + request.Headers.Host = hostName; + + await client.SendAsync(bool.Parse(testAsync), request); + + Activity requestActivity = requestRecorder.VerifyActivityRecordedOnce(); + ActivityAssert.HasTag(requestActivity, "server.address", hostName); + + Activity connectionActivity = connectionSetupRecorder.VerifyActivityRecordedOnce(); + //Assert.Equal($"HTTP connection_setup {hostName}:{uri.Port}", conn.DisplayName); + //ActivityAssert.HasTag(conn, "network.peer.address", + // (string a) => a == IPAddress.Loopback.ToString() || + // a == IPAddress.Loopback.MapToIPv6().ToString() || + // a == IPAddress.IPv6Loopback.ToString()); + //ActivityAssert.HasTag(conn, "server.address", hostName); + }, + async server => + { + await server.AcceptConnectionSendResponseAndCloseAsync(); + }, options: new GenericLoopbackOptions() + { + UseSsl = useTls, + }); + } + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public async Task UseIPAddressInUri_UseProxy_RecordsUriHostAsServerAddress() + { + if (UseVersion != HttpVersion.Version11) + { + throw new SkipTestException("Test only for HTTP/1.1"); + } + + await RemoteExecutor.Invoke(RunTest, UseVersion.ToString(), TestAsync.ToString()).DisposeAsync(); + //await RunTest(UseVersion.ToString(), TestAsync.ToString(), useTls.ToString()); + static async Task RunTest(string useVersion, string testAsync) + { + const string IpString = "1.2.3.4"; + Activity parentActivity = new Activity("parent").Start(); + + using ActivityRecorder requestRecorder = new("System.Net.Http", "System.Net.Http.HttpRequestOut") + { + ExpectedParent = parentActivity + }; + + using ActivityRecorder connectionSetupRecorder = new("Experimental.System.Net.Http.Connections", "Experimental.System.Net.Http.Connections.ConnectionSetup"); + + await GetFactoryForVersion(useVersion).CreateClientAndServerAsync( + async uri => + { + Version version = Version.Parse(useVersion); + + HttpClientHandler handler = CreateHttpClientHandler(allowAllCertificates: true); + handler.Proxy = new UseSpecifiedUriWebProxy(uri); + using HttpClient client = new HttpClient(handler); + + uri = new Uri($"{uri.Scheme}://{IpString}:4242"); + using HttpRequestMessage request = CreateRequest(HttpMethod.Get, uri, version, exactVersion: true); + request.Headers.Host = "localhost"; + + await client.SendAsync(bool.Parse(testAsync), request); + + Activity req = requestRecorder.VerifyActivityRecordedOnce(); + Activity conn = connectionSetupRecorder.VerifyActivityRecordedOnce(); + ActivityAssert.HasTag(req, "server.address", IpString); + + //Assert.Equal($"HTTP connection_setup {hostName}:{uri.Port}", conn.DisplayName); + //ActivityAssert.HasTag(conn, "network.peer.address", + // (string a) => a == IPAddress.Loopback.ToString() || + // a == IPAddress.Loopback.MapToIPv6().ToString() || + // a == IPAddress.IPv6Loopback.ToString()); + //ActivityAssert.HasTag(conn, "server.address", hostName); + }, + async server => + { + await server.AcceptConnectionSendResponseAndCloseAsync(); + }, options: new GenericLoopbackOptions() + { + UseSsl = false, + }); + } + } + private static T GetProperty(object obj, string propertyName) { Type t = obj.GetType(); diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs index c1cbd3540a823d..29db97971ac444 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs @@ -920,6 +920,57 @@ await server.AcceptConnectionAsync(async conn => }); } + [ConditionalTheory(typeof(SocketsHttpHandler), nameof(SocketsHttpHandler.IsSupported))] + [InlineData(false)] + [InlineData(true)] + public Task UseIPAddressInTargetUri_NoProxy_RecordsHostHeaderAsServerAddress(bool useTls) + { + if (UseVersion == HttpVersion30 && !useTls) + { + throw new SkipTestException("No insecure connections with HTTP/3."); + } + + return LoopbackServerFactory.CreateClientAndServerAsync(async uri => + { + using HttpMessageInvoker client = CreateHttpMessageInvoker(); + uri = new Uri($"{uri.Scheme}://{IPAddress.Loopback}:{uri.Port}"); + + using InstrumentRecorder activeRequestsRecorder = SetupInstrumentRecorder(InstrumentNames.ActiveRequests); + using InstrumentRecorder requestDurationRecorder = SetupInstrumentRecorder(InstrumentNames.RequestDuration); + using InstrumentRecorder openConnectionsRecorder = SetupInstrumentRecorder(InstrumentNames.OpenConnections); + using InstrumentRecorder connectionDurationRecorder = SetupInstrumentRecorder(InstrumentNames.ConnectionDuration); + using InstrumentRecorder timeInQueueRecorder = SetupInstrumentRecorder(InstrumentNames.TimeInQueue); + + using HttpRequestMessage request = new(HttpMethod.Get, uri) { Version = UseVersion, VersionPolicy = HttpVersionPolicy.RequestVersionExact }; + request.Headers.Host = "localhost"; + HttpResponseMessage response = await SendAsync(client, request); + response.Dispose(); // Make sure disposal doesn't interfere with recording by enforcing early disposal. + + // Request metrics: + VerifyHostName(activeRequestsRecorder); + VerifyHostName(requestDurationRecorder); + + // Connection metrics: + VerifyHostName(openConnectionsRecorder); + VerifyHostName(connectionDurationRecorder); + VerifyHostName(timeInQueueRecorder); + }, async server => + { + await server.AcceptConnectionSendResponseAndCloseAsync(); + }, options: new GenericLoopbackOptions() + { + UseSsl = useTls, + }); + + static void VerifyHostName(InstrumentRecorder recorder) where T : struct + { + foreach (Measurement m in recorder.GetMeasurements()) + { + VerifyTag(m.Tags.ToArray(), "server.address", "localhost"); + } + } + } + protected override void Dispose(bool disposing) { if (disposing) @@ -1158,6 +1209,53 @@ await server.AcceptConnectionAsync(async connection => }, new LoopbackServer.Options() { UseSsl = true }); } + + [ConditionalFact(typeof(SocketsHttpHandler), nameof(SocketsHttpHandler.IsSupported))] + public Task UseIPAddressInUri_Proxy_RecordsUriHostAsServerAddress() + { + const string IpString = "1.2.3.4"; + + return LoopbackServerFactory.CreateClientAndServerAsync(async uri => + { + Handler.Proxy = new UseSpecifiedUriWebProxy(uri); + using HttpMessageInvoker client = CreateHttpMessageInvoker(); + uri = new Uri($"{uri.Scheme}://{IpString}:4242"); + + using InstrumentRecorder activeRequestsRecorder = SetupInstrumentRecorder(InstrumentNames.ActiveRequests); + using InstrumentRecorder requestDurationRecorder = SetupInstrumentRecorder(InstrumentNames.RequestDuration); + using InstrumentRecorder openConnectionsRecorder = SetupInstrumentRecorder(InstrumentNames.OpenConnections); + using InstrumentRecorder connectionDurationRecorder = SetupInstrumentRecorder(InstrumentNames.ConnectionDuration); + using InstrumentRecorder timeInQueueRecorder = SetupInstrumentRecorder(InstrumentNames.TimeInQueue); + + using HttpRequestMessage request = new(HttpMethod.Get, uri) { Version = UseVersion, VersionPolicy = HttpVersionPolicy.RequestVersionExact }; + request.Headers.Host = "localhost"; + HttpResponseMessage response = await SendAsync(client, request); + response.Dispose(); // Make sure disposal doesn't interfere with recording by enforcing early disposal. + + // Request metrics: + VerifyHostName(activeRequestsRecorder); + VerifyHostName(requestDurationRecorder); + + // Connection metrics: + VerifyHostName(openConnectionsRecorder); + VerifyHostName(connectionDurationRecorder); + VerifyHostName(timeInQueueRecorder); + }, async server => + { + await server.AcceptConnectionSendResponseAndCloseAsync(); + }, options: new GenericLoopbackOptions() + { + UseSsl = false, + }); + + static void VerifyHostName(InstrumentRecorder recorder) where T : struct + { + foreach (Measurement m in recorder.GetMeasurements()) + { + VerifyTag(m.Tags.ToArray(), "server.address", IpString); + } + } + } } [ConditionalClass(typeof(PlatformDetection), nameof(PlatformDetection.IsNotMobile))] From b04c78cfe72e113b8218145dc453d14942be60be Mon Sep 17 00:00:00 2001 From: antonfirsov Date: Fri, 11 Jul 2025 16:52:58 +0200 Subject: [PATCH 2/5] fix overlooks --- .../src/System/Net/Http/DiagnosticsHandler.cs | 2 -- .../src/System/Net/Http/DiagnosticsHelper.cs | 2 +- .../tests/FunctionalTests/DiagnosticsTests.cs | 25 +++++-------------- .../tests/FunctionalTests/MetricsTest.cs | 2 +- .../System.Net.Http.Unit.Tests.csproj | 8 +++--- 5 files changed, 13 insertions(+), 26 deletions(-) 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 4ebcb43752d2e1..aa704ae4f74e12 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 @@ -45,8 +45,6 @@ public DiagnosticsHandler(HttpMessageHandler innerHandler, DistributedContextPro } _propagatorFields = fieldDescriptors.ToArray(); } - - _proxy = proxy; } private static bool IsEnabled() 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 index e8ec9e7ef3ed1d..d577c97b2abf39 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHelper.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHelper.cs @@ -46,7 +46,7 @@ public static string GetServerAddress(HttpRequestMessage request, IWebProxy? pro return HttpUtilities.ParseHostNameFromHeader(hostHeader); } - return request.RequestUri.Host; + return request.RequestUri.IdnHost; } public static bool TryGetErrorType(HttpResponseMessage? response, Exception? exception, out string? errorType) diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/DiagnosticsTests.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/DiagnosticsTests.cs index ba5db2585f86a7..5230508fd69ad3 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/DiagnosticsTests.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/DiagnosticsTests.cs @@ -1673,7 +1673,6 @@ public async Task UseIPAddressInTargetUri_NoProxy_RecordsHostHeaderAsServerAddre if (UseVersion == HttpVersion30 && !useTls) return; await RemoteExecutor.Invoke(RunTest, UseVersion.ToString(), TestAsync.ToString(), useTls.ToString()).DisposeAsync(); - //await RunTest(UseVersion.ToString(), TestAsync.ToString(), useTls.ToString()); static async Task RunTest(string useVersion, string testAsync, string useTlsString) { bool useTls = bool.Parse(useTlsString); @@ -1702,16 +1701,11 @@ await GetFactoryForVersion(useVersion).CreateClientAndServerAsync( await client.SendAsync(bool.Parse(testAsync), request); - Activity requestActivity = requestRecorder.VerifyActivityRecordedOnce(); - ActivityAssert.HasTag(requestActivity, "server.address", hostName); + Activity req = requestRecorder.VerifyActivityRecordedOnce(); + ActivityAssert.HasTag(req, "server.address", hostName); - Activity connectionActivity = connectionSetupRecorder.VerifyActivityRecordedOnce(); - //Assert.Equal($"HTTP connection_setup {hostName}:{uri.Port}", conn.DisplayName); - //ActivityAssert.HasTag(conn, "network.peer.address", - // (string a) => a == IPAddress.Loopback.ToString() || - // a == IPAddress.Loopback.MapToIPv6().ToString() || - // a == IPAddress.IPv6Loopback.ToString()); - //ActivityAssert.HasTag(conn, "server.address", hostName); + Activity conn = connectionSetupRecorder.VerifyActivityRecordedOnce(); + ActivityAssert.HasTag(conn, "server.address", hostName); }, async server => { @@ -1724,7 +1718,7 @@ await GetFactoryForVersion(useVersion).CreateClientAndServerAsync( } [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public async Task UseIPAddressInUri_UseProxy_RecordsUriHostAsServerAddress() + public async Task UseIPAddressInTargetUri_UseProxy_RecordsUriHostAsServerAddress() { if (UseVersion != HttpVersion.Version11) { @@ -1732,7 +1726,6 @@ public async Task UseIPAddressInUri_UseProxy_RecordsUriHostAsServerAddress() } await RemoteExecutor.Invoke(RunTest, UseVersion.ToString(), TestAsync.ToString()).DisposeAsync(); - //await RunTest(UseVersion.ToString(), TestAsync.ToString(), useTls.ToString()); static async Task RunTest(string useVersion, string testAsync) { const string IpString = "1.2.3.4"; @@ -1763,13 +1756,7 @@ await GetFactoryForVersion(useVersion).CreateClientAndServerAsync( Activity req = requestRecorder.VerifyActivityRecordedOnce(); Activity conn = connectionSetupRecorder.VerifyActivityRecordedOnce(); ActivityAssert.HasTag(req, "server.address", IpString); - - //Assert.Equal($"HTTP connection_setup {hostName}:{uri.Port}", conn.DisplayName); - //ActivityAssert.HasTag(conn, "network.peer.address", - // (string a) => a == IPAddress.Loopback.ToString() || - // a == IPAddress.Loopback.MapToIPv6().ToString() || - // a == IPAddress.IPv6Loopback.ToString()); - //ActivityAssert.HasTag(conn, "server.address", hostName); + ActivityAssert.HasTag(conn, "server.address", IpString); }, async server => { diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs index 29db97971ac444..d9328d37427a22 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs @@ -1211,7 +1211,7 @@ await server.AcceptConnectionAsync(async connection => } [ConditionalFact(typeof(SocketsHttpHandler), nameof(SocketsHttpHandler.IsSupported))] - public Task UseIPAddressInUri_Proxy_RecordsUriHostAsServerAddress() + public Task UseIPAddressInTargetUri_UseProxy_RecordsUriHostAsServerAddress() { const string IpString = "1.2.3.4"; 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 40eacbfd8e6e08..be119816ef4532 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 @@ -1,4 +1,4 @@ - + ../../src/Resources/Strings.resx true @@ -77,6 +77,8 @@ Link="ProductionCode\System\Net\Http\GlobalHttpSettings.cs"/> + - + Date: Fri, 11 Jul 2025 16:56:06 +0200 Subject: [PATCH 3/5] fix another dup --- .../Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.cs index b02ee2decfebe0..3808ffd305944f 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.cs @@ -259,7 +259,6 @@ public HttpConnectionPool(HttpConnectionPoolManager poolManager, HttpConnectionK } if (NetEventSource.Log.IsEnabled()) Trace($"{this}"); - _telemetryServerAddress = telemetryServerAddress; } private static SslClientAuthenticationOptions ConstructSslOptions(HttpConnectionPoolManager poolManager, string sslHostName) From 0b23873530b1d0c51122379292e093832f0bc339 Mon Sep 17 00:00:00 2001 From: antonfirsov Date: Fri, 11 Jul 2025 17:26:04 +0200 Subject: [PATCH 4/5] do not fetch TelemetryServerAddress when telemetry is disabled --- .../Net/Http/HttpClientHandler.AnyMobile.cs | 2 +- .../src/System/Net/Http/HttpClientHandler.cs | 1 + .../ConnectionSetupDistributedTracing.cs | 7 +++-- .../ConnectionPool/HttpConnectionPool.cs | 6 ++-- .../SocketsHttpHandler/HttpConnectionBase.cs | 1 + .../HttpConnectionPoolManager.cs | 29 +++++++++++-------- .../Metrics/SocketsHttpHandlerMetrics.cs | 2 ++ 7 files changed, 29 insertions(+), 19 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/HttpClientHandler.AnyMobile.cs b/src/libraries/System.Net.Http/src/System/Net/Http/HttpClientHandler.AnyMobile.cs index ab39fe08a965da..960db9edc0d461 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/HttpClientHandler.AnyMobile.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/HttpClientHandler.AnyMobile.cs @@ -43,7 +43,7 @@ private HttpMessageHandler Handler // MetricsHandler should be descendant of DiagnosticsHandler in the handler chain to make sure the 'http.request.duration' // metric is recorded before stopping the request Activity. This is needed to make sure that our telemetry supports Exemplars. - // Since HttpClientHandler.Proxy is unsupported on many platforms, don't bother passing it. + // Since HttpClientHandler.Proxy is unsupported on most platforms, don't bother passing it to telemetry handlers. if (GlobalHttpSettings.MetricsHandler.IsGloballyEnabled) { handler = new MetricsHandler(handler, _nativeMeterFactory, proxy: null, out _); diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/HttpClientHandler.cs b/src/libraries/System.Net.Http/src/System/Net/Http/HttpClientHandler.cs index a7a817dd9cc7fc..481eb0212524f6 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/HttpClientHandler.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/HttpClientHandler.cs @@ -45,6 +45,7 @@ private HttpMessageHandler Handler // MetricsHandler should be descendant of DiagnosticsHandler in the handler chain to make sure the 'http.request.duration' // metric is recorded before stopping the request Activity. This is needed to make sure that our telemetry supports Exemplars. + // Since HttpClientHandler.Proxy is unsupported on most platforms, don't bother passing it to telemetry handlers. if (GlobalHttpSettings.MetricsHandler.IsGloballyEnabled) { handler = new MetricsHandler(handler, _meterFactory, proxy: null, out _); diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/ConnectionSetupDistributedTracing.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/ConnectionSetupDistributedTracing.cs index 2efd68d7d6b59c..3a65389d20e045 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/ConnectionSetupDistributedTracing.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/ConnectionSetupDistributedTracing.cs @@ -12,7 +12,7 @@ internal static class ConnectionSetupDistributedTracing { private static readonly ActivitySource s_connectionsActivitySource = new ActivitySource(DiagnosticsHandlerLoggingStrings.ConnectionsNamespace); - public static Activity? StartConnectionSetupActivity(bool isSecure, string host, int port) + public static Activity? StartConnectionSetupActivity(bool isSecure, string? serverAddress, int port) { Activity? activity = null; if (s_connectionsActivitySource.HasListeners()) @@ -25,10 +25,11 @@ internal static class ConnectionSetupDistributedTracing if (activity is not null) { - activity.DisplayName = $"HTTP connection_setup {host}:{port}"; + Debug.Assert(serverAddress is not null, "serverAddress should not be null when System.Net.Http.EnableActivityPropagation is true."); + activity.DisplayName = $"HTTP connection_setup {serverAddress}:{port}"; if (activity.IsAllDataRequested) { - activity.SetTag("server.address", host); + activity.SetTag("server.address", serverAddress); activity.SetTag("server.port", port); activity.SetTag("url.scheme", isSecure ? "https" : "http"); } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.cs index 3808ffd305944f..e356c0306aba42 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.cs @@ -35,7 +35,7 @@ internal sealed partial class HttpConnectionPool : IDisposable private readonly HttpConnectionPoolManager _poolManager; private readonly HttpConnectionKind _kind; private readonly Uri? _proxyUri; - private readonly string _telemetryServerAddress; + private readonly string? _telemetryServerAddress; /// The origin authority used to construct the . private readonly HttpAuthority _originAuthority; @@ -74,7 +74,7 @@ internal sealed partial class HttpConnectionPool : IDisposable /// The SSL host with which this pool is associated. /// The proxy this pool targets (optional). /// The value of the 'server.address' tag to be emitted by Metrics and Distributed Tracing. - public HttpConnectionPool(HttpConnectionPoolManager poolManager, HttpConnectionKind kind, string? host, int port, string? sslHostName, Uri? proxyUri, string telemetryServerAddress) + public HttpConnectionPool(HttpConnectionPoolManager poolManager, HttpConnectionKind kind, string? host, int port, string? sslHostName, Uri? proxyUri, string? telemetryServerAddress) { _poolManager = poolManager; _kind = kind; @@ -282,7 +282,7 @@ private static SslClientAuthenticationOptions ConstructSslOptions(HttpConnection return sslOptions; } - public string TelemetryServerAddress => _telemetryServerAddress; + public string? TelemetryServerAddress => _telemetryServerAddress; public HttpAuthority OriginAuthority => _originAuthority; public HttpConnectionSettings Settings => _poolManager.Settings; public HttpConnectionKind Kind => _kind; diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionBase.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionBase.cs index 94a82df383379a..d92131e892a0a4 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionBase.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionBase.cs @@ -69,6 +69,7 @@ protected void MarkConnectionAsEstablished(Activity? connectionSetupActivity, IP this is Http2Connection ? "2" : "3"; + Debug.Assert(_pool.TelemetryServerAddress is not null, "TelemetryServerAddress should not be null when System.Diagnostics.Metrics.Meter.IsSupported is true."); _connectionMetrics = new ConnectionMetrics( metrics, protocol, diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPoolManager.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPoolManager.cs index 52a4e55b22e9a3..863d3eda08cf86 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPoolManager.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPoolManager.cs @@ -304,23 +304,28 @@ private HttpConnectionKey GetConnectionKey(HttpRequestMessage request, Uri? prox // Picks the value of the 'server.address' tag following rules specified in // https://github.com/open-telemetry/semantic-conventions/blob/728e5d1/docs/http/http-spans.md#http-client-span // When there is no proxy, we need to prioritize the contents of the Host header. - private static string GetTelemetryServerAddress(HttpRequestMessage request, HttpConnectionKey key) + private static string? GetTelemetryServerAddress(HttpRequestMessage request, HttpConnectionKey key) { - Uri? uri = request.RequestUri; - Debug.Assert(uri is not null); - - if (key.ProxyUri is not null) + if (GlobalHttpSettings.MetricsHandler.IsGloballyEnabled || GlobalHttpSettings.DiagnosticsHandler.EnableActivityPropagation) { - return uri.IdnHost; - } + Uri? uri = request.RequestUri; + Debug.Assert(uri is not null); - if (key.SslHostName is not null) - { - return key.SslHostName; + if (key.ProxyUri is not null) + { + return uri.IdnHost; + } + + if (key.SslHostName is not null) + { + return key.SslHostName; + } + + string? hostHeader = request.Headers.Host; + return hostHeader is null ? uri.IdnHost : HttpUtilities.ParseHostNameFromHeader(hostHeader); } - string? hostHeader = request.Headers.Host; - return hostHeader is null ? uri.IdnHost : HttpUtilities.ParseHostNameFromHeader(hostHeader); + return null; } public ValueTask SendAsyncCore(HttpRequestMessage request, Uri? proxyUri, bool async, bool doRequestAuth, bool isProxyConnect, CancellationToken cancellationToken) 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 b3886cb8d66f09..eabd5db88de69f 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 @@ -46,6 +46,8 @@ public void RequestLeftQueue(HttpRequestMessage request, HttpConnectionPool pool }); tags.Add("url.scheme", pool.IsSecure ? "https" : "http"); + + Debug.Assert(pool.TelemetryServerAddress is not null, "TelemetryServerAddress should not be null when System.Diagnostics.Metrics.Meter.IsSupported is true."); tags.Add("server.address", pool.TelemetryServerAddress); if (!pool.IsDefaultPort) From 812b44c287ba3db759cfcd160eeb8e004409fd3b Mon Sep 17 00:00:00 2001 From: antonfirsov Date: Thu, 17 Jul 2025 19:04:21 +0200 Subject: [PATCH 5/5] emit proxy Uri host for untunneled proxy connections --- .../src/System/Net/Http/DiagnosticsHelper.cs | 1 + .../HttpConnectionPoolManager.cs | 9 +-- .../tests/FunctionalTests/DiagnosticsTests.cs | 21 ++++--- .../tests/FunctionalTests/MetricsTest.cs | 60 +++++++++---------- 4 files changed, 45 insertions(+), 46 deletions(-) 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 index d577c97b2abf39..95d1e923bcec90 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHelper.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHelper.cs @@ -38,6 +38,7 @@ internal static class DiagnosticsHelper // Picks the value of the 'server.address' tag following rules specified in // https://github.com/open-telemetry/semantic-conventions/blob/728e5d1/docs/http/http-spans.md#http-client-span // When there is no proxy, we need to prioritize the contents of the Host header. + // Note that this is a best-effort guess, e.g. we are not checking if proxy.GetProxy(uri) returns null. public static string GetServerAddress(HttpRequestMessage request, IWebProxy? proxy) { Debug.Assert(request.RequestUri is not null); diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPoolManager.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPoolManager.cs index 863d3eda08cf86..a696e153ca5c89 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPoolManager.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPoolManager.cs @@ -311,14 +311,15 @@ private HttpConnectionKey GetConnectionKey(HttpRequestMessage request, Uri? prox Uri? uri = request.RequestUri; Debug.Assert(uri is not null); - if (key.ProxyUri is not null) + if (key.SslHostName is not null) { - return uri.IdnHost; + return key.SslHostName; } - if (key.SslHostName is not null) + if (key.ProxyUri is not null && key.Kind == HttpConnectionKind.Proxy) { - return key.SslHostName; + // In case there is no tunnel, return the proxy address since the connection is shared. + return key.ProxyUri.IdnHost; } string? hostHeader = request.Headers.Host; diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/DiagnosticsTests.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/DiagnosticsTests.cs index 5230508fd69ad3..44d8488ad5adef 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/DiagnosticsTests.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/DiagnosticsTests.cs @@ -1718,7 +1718,7 @@ await GetFactoryForVersion(useVersion).CreateClientAndServerAsync( } [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public async Task UseIPAddressInTargetUri_UseProxy_RecordsUriHostAsServerAddress() + public async Task UseIPAddressInTargetUri_ProxyTunnel() { if (UseVersion != HttpVersion.Version11) { @@ -1728,7 +1728,6 @@ public async Task UseIPAddressInTargetUri_UseProxy_RecordsUriHostAsServerAddress await RemoteExecutor.Invoke(RunTest, UseVersion.ToString(), TestAsync.ToString()).DisposeAsync(); static async Task RunTest(string useVersion, string testAsync) { - const string IpString = "1.2.3.4"; Activity parentActivity = new Activity("parent").Start(); using ActivityRecorder requestRecorder = new("System.Net.Http", "System.Net.Http.HttpRequestOut") @@ -1738,32 +1737,36 @@ static async Task RunTest(string useVersion, string testAsync) using ActivityRecorder connectionSetupRecorder = new("Experimental.System.Net.Http.Connections", "Experimental.System.Net.Http.Connections.ConnectionSetup"); + using LoopbackProxyServer proxyServer = LoopbackProxyServer.Create(); await GetFactoryForVersion(useVersion).CreateClientAndServerAsync( async uri => { + uri = new Uri($"{uri.Scheme}://{IPAddress.Loopback}:{uri.Port}"); + Version version = Version.Parse(useVersion); HttpClientHandler handler = CreateHttpClientHandler(allowAllCertificates: true); - handler.Proxy = new UseSpecifiedUriWebProxy(uri); + handler.Proxy = new WebProxy(proxyServer.Uri); using HttpClient client = new HttpClient(handler); - uri = new Uri($"{uri.Scheme}://{IpString}:4242"); using HttpRequestMessage request = CreateRequest(HttpMethod.Get, uri, version, exactVersion: true); request.Headers.Host = "localhost"; await client.SendAsync(bool.Parse(testAsync), request); - Activity req = requestRecorder.VerifyActivityRecordedOnce(); - Activity conn = connectionSetupRecorder.VerifyActivityRecordedOnce(); - ActivityAssert.HasTag(req, "server.address", IpString); - ActivityAssert.HasTag(conn, "server.address", IpString); + // There should be only one Activity for the request. Server address should match the Uri host. + // https://github.com/open-telemetry/semantic-conventions/blob/728e5d1/docs/http/http-spans.md#http-client-span + ActivityAssert.HasTag(requestRecorder.FinishedActivities.Single(), "server.address", IPAddress.Loopback.ToString()); + + // Check the SslProxyTunnel connection only, it should use the host header. + Assert.Contains(connectionSetupRecorder.FinishedActivities, a => a.TagObjects.Any(t => t.Key == "server.port" && t.Value.Equals(uri.Port))); }, async server => { await server.AcceptConnectionSendResponseAndCloseAsync(); }, options: new GenericLoopbackOptions() { - UseSsl = false, + UseSsl = true, }); } } diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs index d9328d37427a22..46a294180e9813 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs @@ -1211,48 +1211,42 @@ await server.AcceptConnectionAsync(async connection => } [ConditionalFact(typeof(SocketsHttpHandler), nameof(SocketsHttpHandler.IsSupported))] - public Task UseIPAddressInTargetUri_UseProxy_RecordsUriHostAsServerAddress() + public async Task UseIPAddressInTargetUri_ProxyTunnel_RequestMetricsRecordUriHostAsServerAddress() { - const string IpString = "1.2.3.4"; - - return LoopbackServerFactory.CreateClientAndServerAsync(async uri => - { - Handler.Proxy = new UseSpecifiedUriWebProxy(uri); - using HttpMessageInvoker client = CreateHttpMessageInvoker(); - uri = new Uri($"{uri.Scheme}://{IpString}:4242"); + using LoopbackProxyServer proxyServer = LoopbackProxyServer.Create(); + await LoopbackServerFactory.CreateClientAndServerAsync( + async uri => + { + uri = new Uri($"{uri.Scheme}://{IPAddress.Loopback}:{uri.Port}"); - using InstrumentRecorder activeRequestsRecorder = SetupInstrumentRecorder(InstrumentNames.ActiveRequests); - using InstrumentRecorder requestDurationRecorder = SetupInstrumentRecorder(InstrumentNames.RequestDuration); - using InstrumentRecorder openConnectionsRecorder = SetupInstrumentRecorder(InstrumentNames.OpenConnections); - using InstrumentRecorder connectionDurationRecorder = SetupInstrumentRecorder(InstrumentNames.ConnectionDuration); - using InstrumentRecorder timeInQueueRecorder = SetupInstrumentRecorder(InstrumentNames.TimeInQueue); + //HttpClientHandler handler = CreateHttpClientHandler(allowAllCertificates: true); + Handler.Proxy = new WebProxy(proxyServer.Uri); + using HttpMessageInvoker client = CreateHttpMessageInvoker(); + using HttpRequestMessage request = CreateRequest(HttpMethod.Get, uri, UseVersion, exactVersion: true); + request.Headers.Host = "localhost"; - using HttpRequestMessage request = new(HttpMethod.Get, uri) { Version = UseVersion, VersionPolicy = HttpVersionPolicy.RequestVersionExact }; - request.Headers.Host = "localhost"; - HttpResponseMessage response = await SendAsync(client, request); - response.Dispose(); // Make sure disposal doesn't interfere with recording by enforcing early disposal. + using InstrumentRecorder activeRequestsRecorder = SetupInstrumentRecorder(InstrumentNames.ActiveRequests); + using InstrumentRecorder requestDurationRecorder = SetupInstrumentRecorder(InstrumentNames.RequestDuration); - // Request metrics: - VerifyHostName(activeRequestsRecorder); - VerifyHostName(requestDurationRecorder); + (await SendAsync(client, request)).Dispose(); - // Connection metrics: - VerifyHostName(openConnectionsRecorder); - VerifyHostName(connectionDurationRecorder); - VerifyHostName(timeInQueueRecorder); - }, async server => - { - await server.AcceptConnectionSendResponseAndCloseAsync(); - }, options: new GenericLoopbackOptions() - { - UseSsl = false, - }); + // Request metrics: + VerifyHostName(activeRequestsRecorder, uri.Host); + VerifyHostName(requestDurationRecorder, uri.Host); + }, + async server => + { + await server.AcceptConnectionSendResponseAndCloseAsync(); + }, options: new GenericLoopbackOptions() + { + UseSsl = true, + }); - static void VerifyHostName(InstrumentRecorder recorder) where T : struct + void VerifyHostName(InstrumentRecorder recorder, string hostName) where T : struct { foreach (Measurement m in recorder.GetMeasurements()) { - VerifyTag(m.Tags.ToArray(), "server.address", IpString); + VerifyTag(m.Tags.ToArray(), "server.address", hostName); } } }