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 6678c5be1de78..4ff5b316d725e 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 @@ -200,7 +200,7 @@ await _innerHandler.SendAsync(request, cancellationToken).ConfigureAwait(false) // 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("http.response.status_code", DiagnosticsHelper.GetBoxedInt32((int)response.StatusCode)); activity.SetTag("network.protocol.version", DiagnosticsHelper.GetProtocolVersionString(response.Version)); } @@ -219,7 +219,7 @@ await _innerHandler.SendAsync(request, cancellationToken).ConfigureAwait(false) // 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("http.response.status_code", DiagnosticsHelper.GetBoxedInt32((int)response.StatusCode)); activity.SetTag("network.protocol.version", DiagnosticsHelper.GetProtocolVersionString(response.Version)); } 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 ca80bda721c40..d47ecde26a14e 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 @@ -106,13 +106,15 @@ public static bool TryGetErrorType(HttpResponseMessage? response, Exception? exc private static string[]? s_statusCodeStrings; #pragma warning disable CA1859 // we explictly box here - public static object GetBoxedStatusCode(int statusCode) + // Returns a pooled object if 'value' is between 0-512, + // saving allocations for standard HTTP status codes and small port tag values. + public static object GetBoxedInt32(int value) { object[] boxes = LazyInitializer.EnsureInitialized(ref s_boxedStatusCodes, static () => new object[512]); - return (uint)statusCode < (uint)boxes.Length - ? boxes[statusCode] ??= statusCode - : statusCode; + return (uint)value < (uint)boxes.Length + ? boxes[value] ??= value + : value; } #pragma warning restore 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 d08f2c1751f56..7a381483099d4 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 @@ -110,7 +110,7 @@ private void RequestStop(HttpRequestMessage request, HttpResponseMessage? respon if (response is not null) { - tags.Add("http.response.status_code", DiagnosticsHelper.GetBoxedStatusCode((int)response.StatusCode)); + tags.Add("http.response.status_code", DiagnosticsHelper.GetBoxedInt32((int)response.StatusCode)); tags.Add("network.protocol.version", DiagnosticsHelper.GetProtocolVersionString(response.Version)); } @@ -140,11 +140,7 @@ private static TagList InitializeCommonTags(HttpRequestMessage request) { tags.Add("url.scheme", requestUri.Scheme); tags.Add("server.address", requestUri.Host); - // Add port tag when not the default value for the current scheme - if (!requestUri.IsDefaultPort) - { - tags.Add("server.port", requestUri.Port); - } + 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/HttpConnectionBase.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionBase.cs index c5e578f45b2bc..d018d72c5fc34 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 @@ -72,7 +72,7 @@ protected void MarkConnectionAsEstablished(Activity? connectionSetupActivity, IP protocol, _pool.IsSecure ? "https" : "http", _pool.OriginAuthority.HostValue, - _pool.IsDefaultPort ? null : _pool.OriginAuthority.Port, + _pool.OriginAuthority.Port, remoteEndPoint?.Address?.ToString()); _connectionMetrics.ConnectionEstablished(); diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Metrics/ConnectionMetrics.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Metrics/ConnectionMetrics.cs index 12dbdface7398..160592493caa8 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Metrics/ConnectionMetrics.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Metrics/ConnectionMetrics.cs @@ -13,18 +13,18 @@ internal sealed class ConnectionMetrics private readonly object _protocolVersionTag; private readonly object _schemeTag; private readonly object _hostTag; - private readonly object? _portTag; + private readonly object _portTag; private readonly object? _peerAddressTag; private bool _currentlyIdle; - public ConnectionMetrics(SocketsHttpHandlerMetrics metrics, string protocolVersion, string scheme, string host, int? port, string? peerAddress) + public ConnectionMetrics(SocketsHttpHandlerMetrics metrics, string protocolVersion, string scheme, string host, int port, string? peerAddress) { _metrics = metrics; _openConnectionsEnabled = _metrics.OpenConnections.Enabled; _protocolVersionTag = protocolVersion; _schemeTag = scheme; _hostTag = host; - _portTag = port; + _portTag = DiagnosticsHelper.GetBoxedInt32(port); _peerAddressTag = peerAddress; } @@ -36,11 +36,7 @@ private TagList GetTags() tags.Add("network.protocol.version", _protocolVersionTag); tags.Add("url.scheme", _schemeTag); tags.Add("server.address", _hostTag); - - if (_portTag is not null) - { - tags.Add("server.port", _portTag); - } + tags.Add("server.port", _portTag); if (_peerAddressTag is not null) { diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs index 53a716cd92045..19003fddfff51 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs @@ -14,6 +14,7 @@ using System.Threading; using System.Threading.Tasks; using Microsoft.DotNet.RemoteExecutor; +using Microsoft.DotNet.XUnitExtensions; using Xunit; using Xunit.Abstractions; @@ -71,13 +72,12 @@ protected HttpMetricsTestBase(ITestOutputHelper output) : base(output) } - private static void VerifyPeerAddress(KeyValuePair[] tags) + private static void VerifyPeerAddress(KeyValuePair[] tags, IPAddress[] validPeerAddresses = null) { string ipString = (string)tags.Single(t => t.Key == "network.peer.address").Value; + validPeerAddresses ??= [IPAddress.Loopback.MapToIPv6(), IPAddress.Loopback, IPAddress.IPv6Loopback]; IPAddress ip = IPAddress.Parse(ipString); - Assert.True(ip.Equals(IPAddress.Loopback.MapToIPv6()) || - ip.Equals(IPAddress.Loopback) || - ip.Equals(IPAddress.IPv6Loopback)); + Assert.Contains(ip, validPeerAddresses); } @@ -126,24 +126,29 @@ protected static void VerifyActiveRequests(string instrumentName, long measureme Assert.Equal(method, tags.Single(t => t.Key == "http.request.method").Value); } - protected static void VerifyOpenConnections(string actualName, object measurement, KeyValuePair[] tags, long expectedValue, Uri uri, Version? protocolVersion, string state) + protected static void VerifyOpenConnections(string actualName, object measurement, KeyValuePair[] tags, long expectedValue, Uri uri, Version? protocolVersion, string state, IPAddress[] validPeerAddresses = null) { Assert.Equal(InstrumentNames.OpenConnections, actualName); Assert.Equal(expectedValue, Assert.IsType(measurement)); VerifySchemeHostPortTags(tags, uri); VerifyTag(tags, "network.protocol.version", GetVersionString(protocolVersion)); VerifyTag(tags, "http.connection.state", state); - VerifyPeerAddress(tags); + VerifyPeerAddress(tags, validPeerAddresses); } - protected static void VerifyConnectionDuration(string instrumentName, object measurement, KeyValuePair[] tags, Uri uri, Version? protocolVersion) + protected static void VerifyConnectionDuration(string instrumentName, object measurement, KeyValuePair[] tags, Uri uri, Version? protocolVersion, IPAddress[] validPeerAddresses = null) { Assert.Equal(InstrumentNames.ConnectionDuration, instrumentName); double value = Assert.IsType(measurement); - Assert.InRange(value, double.Epsilon, 60); + + // This flakes for remote requests on CI. + if (validPeerAddresses is null) + { + Assert.InRange(value, double.Epsilon, 60); + } VerifySchemeHostPortTags(tags, uri); VerifyTag(tags, "network.protocol.version", GetVersionString(protocolVersion)); - VerifyPeerAddress(tags); + VerifyPeerAddress(tags, validPeerAddresses); } protected static void VerifyTimeInQueue(string instrumentName, object measurement, KeyValuePair[] tags, Uri uri, Version? protocolVersion, string method = "GET") @@ -362,6 +367,41 @@ public Task RequestDuration_Success_Recorded(string method, HttpStatusCode statu }); } + [OuterLoop("Uses external server.")] + [ConditionalFact] + public async Task ExternalServer_DurationMetrics_Recorded() + { + if (UseVersion == HttpVersion.Version30) + { + throw new SkipTestException("No remote HTTP/3 server available for testing."); + } + + using InstrumentRecorder requestDurationRecorder = SetupInstrumentRecorder(InstrumentNames.RequestDuration); + using InstrumentRecorder connectionDurationRecorder = SetupInstrumentRecorder(InstrumentNames.ConnectionDuration); + using InstrumentRecorder openConnectionsRecorder = SetupInstrumentRecorder(InstrumentNames.OpenConnections); + + Uri uri = UseVersion == HttpVersion.Version11 + ? Test.Common.Configuration.Http.RemoteHttp11Server.EchoUri + : Test.Common.Configuration.Http.RemoteHttp2Server.EchoUri; + IPAddress[] addresses = await Dns.GetHostAddressesAsync(uri.Host); + addresses = addresses.Union(addresses.Select(a => a.MapToIPv6())).ToArray(); + + using (HttpMessageInvoker client = CreateHttpMessageInvoker()) + { + using HttpRequestMessage request = new(HttpMethod.Get, uri) { Version = UseVersion }; + request.Headers.ConnectionClose = true; + using HttpResponseMessage response = await SendAsync(client, request); + await response.Content.LoadIntoBufferAsync(); + await WaitForEnvironmentTicksToAdvance(); + } + + VerifyRequestDuration(Assert.Single(requestDurationRecorder.GetMeasurements()), uri, UseVersion, 200, "GET"); + Measurement cd = Assert.Single(connectionDurationRecorder.GetMeasurements()); + VerifyConnectionDuration(InstrumentNames.ConnectionDuration, cd.Value, cd.Tags.ToArray(), uri, UseVersion, addresses); + Measurement oc = openConnectionsRecorder.GetMeasurements().First(); + VerifyOpenConnections(InstrumentNames.OpenConnections, oc.Value, oc.Tags.ToArray(), 1, uri, UseVersion, "idle", addresses); + } + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] public async Task RequestDuration_HttpTracingEnabled_RecordedWhileRequestActivityRunning() {