From 5d44a87408fba02210b2ac92f97e9f213cc10c2e Mon Sep 17 00:00:00 2001 From: ManickaP Date: Wed, 22 Jul 2026 13:54:37 +0200 Subject: [PATCH 01/11] Move to observable counterpart for high cardinality counters --- .../System/Net/Http/Metrics/MetricsHandler.cs | 145 ++++++++++++++- .../Metrics/ConnectionMetrics.cs | 31 ++-- .../Metrics/SocketsHttpHandlerMetrics.cs | 173 ++++++++++++++++-- .../tests/FunctionalTests/MetricsTest.cs | 69 +++---- 4 files changed, 334 insertions(+), 84 deletions(-) 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 4e6f6a05e69ce2..58a029905d834b 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 @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Collections.Concurrent; using System.Collections.Generic; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; @@ -10,10 +11,124 @@ namespace System.Net.Http.Metrics { + /// + /// Represents a unique combination of tags for tracking active requests. + /// + internal readonly struct ActiveRequestsTagKey : IEquatable + { + public readonly string? Scheme; + public readonly string? Host; + public readonly int Port; + public readonly bool HasUriTags; + public readonly string Method; + private readonly int _hashCode; + + public ActiveRequestsTagKey(string? scheme, string? host, int port, bool hasUriTags, string method) + { + Scheme = scheme; + Host = host; + Port = port; + HasUriTags = hasUriTags; + Method = method; + _hashCode = HashCode.Combine(scheme, host, port, hasUriTags, method); + } + + public bool Equals(ActiveRequestsTagKey other) => + Scheme == other.Scheme && + Host == other.Host && + Port == other.Port && + HasUriTags == other.HasUriTags && + Method == other.Method; + + public override bool Equals(object? obj) => obj is ActiveRequestsTagKey other && Equals(other); + + public override int GetHashCode() => _hashCode; + + public TagList ToTagList() + { + TagList tags = default; + if (HasUriTags) + { + tags.Add("url.scheme", Scheme); + tags.Add("server.address", Host); + tags.Add("server.port", DiagnosticsHelper.GetBoxedInt32(Port)); + } + tags.Add("http.request.method", Method); + return tags; + } + } + + /// + /// Thread-safe tracker for active request counts by tag combination. + /// + internal sealed class ActiveRequestsTracker + { + private readonly ConcurrentDictionary _counts = new(); + + /// + /// Increments the count for the specified tag combination. + /// + public void Increment(in ActiveRequestsTagKey key) + { + _counts.AddOrUpdate(key, 1, static (_, currentValue) => currentValue + 1); + } + + /// + /// Decrements the count for the specified tag combination. + /// Removes the entry if the count reaches zero. + /// + public void Decrement(in ActiveRequestsTagKey key) + { + // We need to atomically decrement and remove if zero. + // Use a spin loop with TryGetValue/TryUpdate/TryRemove to handle this safely. + while (true) + { + if (!_counts.TryGetValue(key, out long currentValue)) + { + // Key doesn't exist, nothing to decrement. + // This shouldn't happen in normal operation but we handle it gracefully. + return; + } + + if (currentValue <= 1) + { + // Try to remove the entry since it will become zero. + // Use the overload that checks the current value to ensure atomicity. + if (_counts.TryRemove(new KeyValuePair(key, currentValue))) + { + return; + } + // Another thread modified the value, retry. + } + else + { + // Try to decrement the value. + if (_counts.TryUpdate(key, currentValue - 1, currentValue)) + { + return; + } + // Another thread modified the value, retry. + } + } + } + + /// + /// Returns measurements for all tag combinations with non-zero counts. + /// + public IEnumerable> GetMeasurements() + { + foreach (KeyValuePair entry in _counts) + { + yield return new Measurement(entry.Value, entry.Key.ToTagList()); + } + } + } + internal sealed class MetricsHandler : HttpMessageHandlerStage { private readonly HttpMessageHandler _innerHandler; - private readonly UpDownCounter _activeRequests; + private readonly ActiveRequestsTracker _activeRequestsTracker = new(); + private readonly ObservableUpDownCounter _activeRequests; private readonly Histogram _requestsDuration; private readonly IWebProxy? _proxy; @@ -27,8 +142,9 @@ public MetricsHandler(HttpMessageHandler innerHandler, IMeterFactory? meterFacto meter = meterFactory?.Create("System.Net.Http") ?? SharedMeter.Instance; // Meter has a cache for the instruments it owns - _activeRequests = meter.CreateUpDownCounter( + _activeRequests = meter.CreateObservableUpDownCounter( "http.client.active_requests", + observeValues: _activeRequestsTracker.GetMeasurements, unit: "{request}", description: "Number of outbound HTTP requests that are currently active on the client."); _requestsDuration = meter.CreateHistogram( @@ -94,8 +210,7 @@ protected override void Dispose(bool disposing) if (recordCurrentRequests) { - TagList tags = InitializeCommonTags(request); - _activeRequests.Add(1, tags); + _activeRequestsTracker.Increment(CreateActiveRequestsTagKey(request)); } return (startTimestamp, recordCurrentRequests); @@ -107,7 +222,7 @@ private void RequestStop(HttpRequestMessage request, HttpResponseMessage? respon if (recordCurrentRequests) { - _activeRequests.Add(-1, tags); + _activeRequestsTracker.Decrement(CreateActiveRequestsTagKey(request)); } if (!_requestsDuration.Enabled) @@ -154,6 +269,26 @@ private TagList InitializeCommonTags(HttpRequestMessage request) return tags; } + private ActiveRequestsTagKey CreateActiveRequestsTagKey(HttpRequestMessage request) + { + string? scheme = null; + string? host = null; + int port = 0; + bool hasUriTags = false; + + if (request.RequestUri is Uri requestUri && requestUri.IsAbsoluteUri) + { + scheme = requestUri.Scheme; + host = DiagnosticsHelper.GetServerAddress(request, _proxy); + port = requestUri.Port; + hasUriTags = true; + } + + string method = (string)DiagnosticsHelper.GetMethodTag(request.Method, out _).Value!; + + return new ActiveRequestsTagKey(scheme, host, port, hasUriTags, method); + } + 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/ConnectionMetrics.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Metrics/ConnectionMetrics.cs index 160592493caa85..b864bc586ea257 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 @@ -10,11 +10,11 @@ internal sealed class ConnectionMetrics { private readonly SocketsHttpHandlerMetrics _metrics; private readonly bool _openConnectionsEnabled; - private readonly object _protocolVersionTag; - private readonly object _schemeTag; - private readonly object _hostTag; - private readonly object _portTag; - private readonly object? _peerAddressTag; + private readonly string _protocolVersionTag; + private readonly string _schemeTag; + private readonly string _hostTag; + private readonly int _portTag; + private readonly string? _peerAddressTag; private bool _currentlyIdle; public ConnectionMetrics(SocketsHttpHandlerMetrics metrics, string protocolVersion, string scheme, string host, int port, string? peerAddress) @@ -24,7 +24,7 @@ public ConnectionMetrics(SocketsHttpHandlerMetrics metrics, string protocolVersi _protocolVersionTag = protocolVersion; _schemeTag = scheme; _hostTag = host; - _portTag = DiagnosticsHelper.GetBoxedInt32(port); + _portTag = port; _peerAddressTag = peerAddress; } @@ -36,7 +36,7 @@ private TagList GetTags() tags.Add("network.protocol.version", _protocolVersionTag); tags.Add("url.scheme", _schemeTag); tags.Add("server.address", _hostTag); - tags.Add("server.port", _portTag); + tags.Add("server.port", DiagnosticsHelper.GetBoxedInt32(_portTag)); if (_peerAddressTag is not null) { @@ -46,16 +46,15 @@ private TagList GetTags() return tags; } - private static KeyValuePair GetStateTag(bool idle) => new KeyValuePair("http.connection.state", idle ? "idle" : "active"); + private OpenConnectionsTagKey CreateTagKey(bool idle) => + new OpenConnectionsTagKey(_protocolVersionTag, _schemeTag, _hostTag, _portTag, idle, _peerAddressTag); public void ConnectionEstablished() { if (_openConnectionsEnabled) { _currentlyIdle = true; - TagList tags = GetTags(); - tags.Add(GetStateTag(idle: true)); - _metrics.OpenConnections.Add(1, tags); + _metrics.OpenConnectionsTracker.Increment(CreateTagKey(idle: true)); } } @@ -70,8 +69,7 @@ public void ConnectionClosed(long durationMs) if (_openConnectionsEnabled) { - tags.Add(GetStateTag(idle: _currentlyIdle)); - _metrics.OpenConnections.Add(-1, tags); + _metrics.OpenConnectionsTracker.Decrement(CreateTagKey(idle: _currentlyIdle)); } } @@ -80,11 +78,8 @@ public void IdleStateChanged(bool idle) if (_openConnectionsEnabled && _currentlyIdle != idle) { _currentlyIdle = idle; - TagList tags = GetTags(); - tags.Add(GetStateTag(idle: !idle)); - _metrics.OpenConnections.Add(-1, tags); - tags[tags.Count - 1] = GetStateTag(idle: idle); - _metrics.OpenConnections.Add(1, tags); + _metrics.OpenConnectionsTracker.Decrement(CreateTagKey(idle: !idle)); + _metrics.OpenConnectionsTracker.Increment(CreateTagKey(idle: idle)); } } } 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 eabd5db88de69f..15de0c6cf279bf 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 @@ -1,33 +1,166 @@ // 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.Concurrent; +using System.Collections.Generic; using System.Diagnostics; using System.Diagnostics.Metrics; +using System.Threading; namespace System.Net.Http.Metrics { - internal sealed class SocketsHttpHandlerMetrics(Meter meter) + /// + /// Represents a unique combination of tags for tracking open connections. + /// + internal readonly struct OpenConnectionsTagKey : IEquatable { - public readonly UpDownCounter OpenConnections = meter.CreateUpDownCounter( - name: "http.client.open_connections", - unit: "{connection}", - description: "Number of outbound HTTP connections that are currently active or idle on the client."); - - public readonly Histogram ConnectionDuration = meter.CreateHistogram( - name: "http.client.connection.duration", - unit: "s", - description: "The duration of successfully established outbound HTTP connections.", - advice: new InstrumentAdvice() + public readonly string ProtocolVersion; + public readonly string Scheme; + public readonly string Host; + public readonly int Port; + public readonly bool IsIdle; + public readonly string? PeerAddress; + private readonly int _hashCode; + + public OpenConnectionsTagKey(string protocolVersion, string scheme, string host, int port, bool isIdle, string? peerAddress) + { + ProtocolVersion = protocolVersion; + Scheme = scheme; + Host = host; + Port = port; + IsIdle = isIdle; + PeerAddress = peerAddress; + _hashCode = HashCode.Combine(protocolVersion, scheme, host, port, isIdle); + } + + public bool Equals(OpenConnectionsTagKey other) => + ProtocolVersion == other.ProtocolVersion && + Scheme == other.Scheme && + Host == other.Host && + Port == other.Port && + IsIdle == other.IsIdle && + PeerAddress == other.PeerAddress; + + public override bool Equals(object? obj) => obj is OpenConnectionsTagKey other && Equals(other); + + public override int GetHashCode() => _hashCode; + + public TagList ToTagList() + { + TagList tags = default; + tags.Add("network.protocol.version", ProtocolVersion); + tags.Add("url.scheme", Scheme); + tags.Add("server.address", Host); + tags.Add("server.port", DiagnosticsHelper.GetBoxedInt32(Port)); + tags.Add("http.connection.state", IsIdle ? "idle" : "active"); + if (PeerAddress is not null) + { + tags.Add("network.peer.address", PeerAddress); + } + return tags; + } + } + + /// + /// Thread-safe tracker for open connection counts by tag combination. + /// + internal sealed class OpenConnectionsTracker + { + private readonly ConcurrentDictionary _counts = new(); + + /// + /// Increments the count for the specified tag combination. + /// + public void Increment(in OpenConnectionsTagKey key) + { + _counts.AddOrUpdate(key, 1, static (_, currentValue) => currentValue + 1); + } + + /// + /// Decrements the count for the specified tag combination. + /// Removes the entry if the count reaches zero. + /// + public void Decrement(in OpenConnectionsTagKey key) + { + // We need to atomically decrement and remove if zero. + // Use a spin loop with TryGetValue/TryUpdate/TryRemove to handle this safely. + while (true) + { + if (!_counts.TryGetValue(key, out long currentValue)) + { + // Key doesn't exist, nothing to decrement. + // This shouldn't happen in normal operation but we handle it gracefully. + return; + } + + if (currentValue <= 1) + { + // Try to remove the entry since it will become zero. + // Use the overload that checks the current value to ensure atomicity. + if (_counts.TryRemove(new KeyValuePair(key, currentValue))) + { + return; + } + // Another thread modified the value, retry. + } + else + { + // Try to decrement the value. + if (_counts.TryUpdate(key, currentValue - 1, currentValue)) + { + return; + } + // Another thread modified the value, retry. + } + } + } + + /// + /// Returns measurements for all tag combinations with non-zero counts. + /// + public IEnumerable> GetMeasurements() + { + foreach (KeyValuePair entry in _counts) { - // These values are not based on a standard and may change in the future. - HistogramBucketBoundaries = [0.01, 0.02, 0.05, 0.1, 0.2, 0.5, 1, 2, 5, 10, 30, 60, 120, 300] - }); - - public readonly Histogram RequestsQueueDuration = meter.CreateHistogram( - name: "http.client.request.time_in_queue", - unit: "s", - description: "The amount of time requests spent on a queue waiting for an available connection.", - advice: DiagnosticsHelper.ShortHistogramAdvice); + yield return new Measurement(entry.Value, entry.Key.ToTagList()); + } + } + } + + internal sealed class SocketsHttpHandlerMetrics + { + public readonly OpenConnectionsTracker OpenConnectionsTracker = new(); + + public readonly ObservableUpDownCounter OpenConnections; + + public readonly Histogram ConnectionDuration; + + public readonly Histogram RequestsQueueDuration; + + public SocketsHttpHandlerMetrics(Meter meter) + { + OpenConnections = meter.CreateObservableUpDownCounter( + name: "http.client.open_connections", + observeValues: OpenConnectionsTracker.GetMeasurements, + unit: "{connection}", + description: "Number of outbound HTTP connections that are currently active or idle on the client."); + + ConnectionDuration = meter.CreateHistogram( + name: "http.client.connection.duration", + unit: "s", + description: "The duration of successfully established outbound HTTP connections.", + advice: new InstrumentAdvice() + { + // These values are not based on a standard and may change in the future. + HistogramBucketBoundaries = [0.01, 0.02, 0.05, 0.1, 0.2, 0.5, 1, 2, 5, 10, 30, 60, 120, 300] + }); + + RequestsQueueDuration = meter.CreateHistogram( + name: "http.client.request.time_in_queue", + unit: "s", + description: "The amount of time requests spent on a queue waiting for an available connection.", + advice: DiagnosticsHelper.ShortHistogramAdvice); + } public void RequestLeftQueue(HttpRequestMessage request, HttpConnectionPool pool, TimeSpan duration, int versionMajor) { diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs index dd830324d6723a..13e9b6375d6168 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs @@ -221,6 +221,7 @@ private void OnMeasurementRecorded(Instrument instrument, T measurement, ReadOnl } } + public void RecordObservableInstruments() => _meterListener.RecordObservableInstruments(); public IReadOnlyList> GetMeasurements() => _values.ToArray(); public void Dispose() => _meterListener.Dispose(); } @@ -272,6 +273,7 @@ private MultiInstrumentRecorder(Meter? meter) _meterListener.Start(); } + public void RecordObservableInstruments() => _meterListener.RecordObservableInstruments(); public IReadOnlyList GetMeasurements() => _values.ToArray(); public void Dispose() => _meterListener.Dispose(); } @@ -752,6 +754,7 @@ await LoopbackServerFactory.CreateClientAndServerAsync(async uri => public async Task AllSocketsHttpHandlerCounters_Success_Recorded() { TaskCompletionSource clientWaitingTcs = new(TaskCreationOptions.RunContinuationsAsynchronously); + TaskCompletionSource serverWaitingTcs = new(TaskCreationOptions.RunContinuationsAsynchronously); TaskCompletionSource clientDisposedTcs = new(TaskCreationOptions.RunContinuationsAsynchronously); await LoopbackServerFactory.CreateClientAndServerAsync(async uri => @@ -764,33 +767,20 @@ await LoopbackServerFactory.CreateClientAndServerAsync(async uri => using HttpRequestMessage request = new(HttpMethod.Get, uri) { Version = UseVersion }; Task sendAsyncTask = SendAsync(invoker, request); + await serverWaitingTcs.Task.WaitAsync(TestHelper.PassingTestTimeout); + recorder.RecordObservableInstruments(); clientWaitingTcs.SetResult(); - using HttpResponseMessage response = await sendAsyncTask; + recorder.RecordObservableInstruments(); + await serverWaitingTcs.Task.WaitAsync(TestHelper.PassingTestTimeout); + HttpResponseMessage response = await sendAsyncTask; + response.Dispose(); await WaitForEnvironmentTicksToAdvance(); + recorder.RecordObservableInstruments(); } clientDisposedTcs.SetResult(); - Action requestsQueueDuration = m => - VerifyTimeInQueue(m.InstrumentName, m.Value, m.Tags, uri, UseVersion); - Action connectionNoLongerIdle = m => - VerifyOpenConnections(m.InstrumentName, m.Value, m.Tags, -1, uri, UseVersion, "idle"); - Action connectionIsActive = m => - VerifyOpenConnections(m.InstrumentName, m.Value, m.Tags, 1, uri, UseVersion, "active"); - - Action check1 = requestsQueueDuration; - Action check2 = connectionNoLongerIdle; - Action check3 = connectionIsActive; - - if (UseVersion.Major > 2) - { - // With HTTP/3, the idle state change is emitted before RequestsQueueDuration. - check1 = connectionNoLongerIdle; - check2 = connectionIsActive; - check3 = requestsQueueDuration; - } - IReadOnlyList measurements = recorder.GetMeasurements(); foreach (RecordedCounter m in measurements) { @@ -799,26 +789,22 @@ await LoopbackServerFactory.CreateClientAndServerAsync(async uri => Assert.Collection(measurements, m => VerifyActiveRequests(m.InstrumentName, (long)m.Value, m.Tags, 1, uri), - m => VerifyOpenConnections(m.InstrumentName, m.Value, m.Tags, 1, uri, UseVersion, "idle"), - check1, // requestsQueueDuration, connectionNoLongerIdle, connectionIsActive in the appropriate order. - check2, - check3, - m => VerifyOpenConnections(m.InstrumentName, m.Value, m.Tags, -1, uri, UseVersion, "active"), - m => VerifyOpenConnections(m.InstrumentName, m.Value, m.Tags, 1, uri, UseVersion, "idle"), - + m => VerifyTimeInQueue(m.InstrumentName, m.Value, m.Tags, uri, UseVersion), + m => VerifyOpenConnections(m.InstrumentName, m.Value, m.Tags, 1, uri, UseVersion, "active"), m => VerifyActiveRequests(m.InstrumentName, (long)m.Value, m.Tags, -1, uri), m => VerifyRequestDuration(m.InstrumentName, (double)m.Value, m.Tags, uri, UseVersion, 200), - m => VerifyConnectionDuration(m.InstrumentName, m.Value, m.Tags, uri, UseVersion), - m => VerifyOpenConnections(m.InstrumentName, m.Value, m.Tags, -1, uri, UseVersion, "idle")); + m => VerifyOpenConnections(m.InstrumentName, m.Value, m.Tags, 1, uri, UseVersion, "idle"), + m => VerifyConnectionDuration(m.InstrumentName, m.Value, m.Tags, uri, UseVersion)); }, async server => { - await clientWaitingTcs.Task.WaitAsync(TestHelper.PassingTestTimeout); - await server.AcceptConnectionAsync(async connection => { await connection.ReadRequestDataAsync(); - await connection.SendResponseAsync(); + await clientWaitingTcs.Task.WaitAsync(TestHelper.PassingTestTimeout); + serverWaitingTcs.SetResult(); + await clientWaitingTcs.Task.WaitAsync(TestHelper.PassingTestTimeout); + await connection.SendResponseAsync(content: new string('a', 10*1024)); await clientDisposedTcs.Task.WaitAsync(TestHelper.PassingTestTimeout); }); }); @@ -1528,6 +1514,7 @@ public async Task AllSocketsHttpHandlerCounters_Success_Recorded() await RemoteExecutor.Invoke(static async Task () => { TaskCompletionSource clientWaitingTcs = new(TaskCreationOptions.RunContinuationsAsynchronously); + TaskCompletionSource serverWaitingTcs = new(TaskCreationOptions.RunContinuationsAsynchronously); using HttpMetricsTest_DefaultMeter test = new(null); await test.LoopbackServerFactory.CreateClientAndServerAsync(async uri => @@ -1538,27 +1525,25 @@ await test.LoopbackServerFactory.CreateClientAndServerAsync(async uri => { using HttpRequestMessage request = new(HttpMethod.Get, uri) { Version = test.UseVersion }; Task sendAsyncTask = client.SendAsync(request); + await serverWaitingTcs.Task.WaitAsync(TestHelper.PassingTestTimeout); + recorder.RecordObservableInstruments(); clientWaitingTcs.SetResult(); - using HttpResponseMessage response = await sendAsyncTask; + HttpResponseMessage response = await sendAsyncTask; + response.Dispose(); await WaitForEnvironmentTicksToAdvance(); + recorder.RecordObservableInstruments(); } Version version = HttpVersion.Version11; Assert.Collection(recorder.GetMeasurements(), m => VerifyActiveRequests(m.InstrumentName, (long)m.Value, m.Tags, 1, uri), - m => VerifyOpenConnections(m.InstrumentName, m.Value, m.Tags, 1, uri, version, "idle"), m => VerifyTimeInQueue(m.InstrumentName, m.Value, m.Tags, uri, version), - - m => VerifyOpenConnections(m.InstrumentName, m.Value, m.Tags, -1, uri, version, "idle"), m => VerifyOpenConnections(m.InstrumentName, m.Value, m.Tags, 1, uri, version, "active"), - m => VerifyOpenConnections(m.InstrumentName, m.Value, m.Tags, -1, uri, version, "active"), - m => VerifyOpenConnections(m.InstrumentName, m.Value, m.Tags, 1, uri, version, "idle"), - m => VerifyActiveRequests(m.InstrumentName, (long)m.Value, m.Tags, -1, uri), m => VerifyRequestDuration(m.InstrumentName, (double)m.Value, m.Tags, uri, version, 200), - m => VerifyConnectionDuration(m.InstrumentName, m.Value, m.Tags, uri, version), - m => VerifyOpenConnections(m.InstrumentName, m.Value, m.Tags, -1, uri, version, "idle")); + m => VerifyOpenConnections(m.InstrumentName, m.Value, m.Tags, 1, uri, version, "idle"), + m => VerifyConnectionDuration(m.InstrumentName, m.Value, m.Tags, uri, version)); }, async server => { @@ -1567,6 +1552,8 @@ await test.LoopbackServerFactory.CreateClientAndServerAsync(async uri => await server.AcceptConnectionAsync(async connection => { await connection.ReadRequestDataAsync(); + serverWaitingTcs.SetResult(); + await clientWaitingTcs.Task.WaitAsync(TestHelper.PassingTestTimeout); await connection.SendResponseAsync(isFinal: false); await connection.WaitForCloseAsync(CancellationToken.None); }); From eea896c1fd1b80e412ca2bbe7e6bbc9e5894caee Mon Sep 17 00:00:00 2001 From: ManickaP Date: Thu, 23 Jul 2026 14:02:44 +0200 Subject: [PATCH 02/11] Fixed ActiveRequests_Success_Recorded test --- .../tests/FunctionalTests/MetricsTest.cs | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs index 13e9b6375d6168..98860ceb2f0385 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs @@ -294,21 +294,33 @@ public HttpMetricsTest(ITestOutputHelper output) : base(output) [ActiveIssue("https://github.com/dotnet/runtime/issues/129223", TestPlatforms.Wasi)] public Task ActiveRequests_Success_Recorded() { + var serverTcs = new TaskCompletionSource(); + var clientTcs = new TaskCompletionSource(); + return LoopbackServerFactory.CreateClientAndServerAsync(async uri => { using HttpMessageInvoker client = CreateHttpMessageInvoker(); using InstrumentRecorder recorder = SetupInstrumentRecorder(InstrumentNames.ActiveRequests); using HttpRequestMessage request = new(HttpMethod.Get, uri) { Version = UseVersion }; - HttpResponseMessage response = await SendAsync(client, request); + var requestTask = SendAsync(client, request); + await serverTcs.Task.WaitAsync(TestHelper.PassingTestTimeout); + recorder.RecordObservableInstruments(); + clientTcs.SetResult(); + var response = await requestTask; response.Dispose(); // Make sure disposal doesn't interfere with recording by enforcing early disposal. Assert.Collection(recorder.GetMeasurements(), - m => VerifyActiveRequests(m, 1, uri), - m => VerifyActiveRequests(m, -1, uri)); + m => VerifyActiveRequests(m, 1, uri)); }, async server => { - await server.AcceptConnectionSendResponseAndCloseAsync(); + await server.AcceptConnectionAsync(async connection => + { + var requestData = await connection.ReadRequestDataAsync(); + serverTcs.SetResult(); + await clientTcs.Task.WaitAsync(TestHelper.PassingTestTimeout); + await connection.SendResponseAsync(); + }); }); } From 0c6d6115639e641dc3e44cf97d2700d0082be2f4 Mon Sep 17 00:00:00 2001 From: ManickaP Date: Thu, 23 Jul 2026 16:45:53 +0200 Subject: [PATCH 03/11] Fixed ActiveRequests_Redirect_RecordedForEachHttpSpan test --- .../tests/FunctionalTests/MetricsTest.cs | 32 +++++++++++++++---- 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs index 98860ceb2f0385..b974da3c5ec558 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs @@ -687,6 +687,11 @@ public Task ActiveRequests_Redirect_RecordedForEachHttpSpan(int credentialsMode) Handler.Credentials = credentialsMode == 1 ? new CredentialCache() : new CustomCredentials(); } + var originalServerTcs = new TaskCompletionSource(); + var redirectServerTcs = new TaskCompletionSource(); + var clientTcs1 = new TaskCompletionSource(); + var clientTcs2 = new TaskCompletionSource(); + return LoopbackServerFactory.CreateServerAsync((originalServer, originalUri) => { return LoopbackServerFactory.CreateServerAsync(async (redirectServer, redirectUri) => @@ -696,21 +701,36 @@ public Task ActiveRequests_Redirect_RecordedForEachHttpSpan(int credentialsMode) using HttpRequestMessage request = new(HttpMethod.Get, originalUri) { Version = UseVersion }; Task clientTask = SendAsync(client, request); - Task serverTask = originalServer.HandleRequestAsync(HttpStatusCode.Redirect, new[] { new HttpHeaderData("Location", redirectUri.AbsoluteUri) }); - + var serverTask = originalServer.AcceptConnectionAsync(async connection => + { + var requestData = await connection.ReadRequestDataAsync(); + originalServerTcs.SetResult(); + await clientTcs1.Task.WaitAsync(TestHelper.PassingTestTimeout); + await connection.SendResponseAsync(HttpStatusCode.Redirect, new[] { new HttpHeaderData("Location", redirectUri.AbsoluteUri) }); + }); + await originalServerTcs.Task.WaitAsync(TestHelper.PassingTestTimeout); + recorder.RecordObservableInstruments(); + clientTcs1.SetResult(); await Task.WhenAny(clientTask, serverTask); Assert.False(clientTask.IsCompleted, $"{clientTask.Status}: {clientTask.Exception}"); await serverTask; - serverTask = redirectServer.HandleRequestAsync(); + serverTask = redirectServer.AcceptConnectionAsync(async connection => + { + var requestData = await connection.ReadRequestDataAsync(); + redirectServerTcs.SetResult(); + await clientTcs2.Task.WaitAsync(TestHelper.PassingTestTimeout); + await connection.SendResponseAsync(); + }); + await redirectServerTcs.Task.WaitAsync(TestHelper.PassingTestTimeout); + recorder.RecordObservableInstruments(); + clientTcs2.SetResult(); await TestHelper.WhenAllCompletedOrAnyFailed(clientTask, serverTask); await clientTask; Assert.Collection(recorder.GetMeasurements(), m => VerifyActiveRequests(m, 1, originalUri), - m => VerifyActiveRequests(m, -1, originalUri), - m => VerifyActiveRequests(m, 1, redirectUri), - m => VerifyActiveRequests(m, -1, redirectUri)); + m => VerifyActiveRequests(m, 1, redirectUri)); }); }); } From 7b3a27de5f5c6498ba980b49f07cf18de4e74310 Mon Sep 17 00:00:00 2001 From: ManickaP Date: Thu, 23 Jul 2026 17:01:22 +0200 Subject: [PATCH 04/11] Fixed AllSocketsHttpHandlerCounters_Success_Recorded test --- .../tests/FunctionalTests/MetricsTest.cs | 70 +++++++++---------- 1 file changed, 35 insertions(+), 35 deletions(-) diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs index b974da3c5ec558..6d7d198ca69853 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs @@ -785,8 +785,8 @@ await LoopbackServerFactory.CreateClientAndServerAsync(async uri => [ConditionalFact(typeof(SocketsHttpHandler), nameof(SocketsHttpHandler.IsSupported))] public async Task AllSocketsHttpHandlerCounters_Success_Recorded() { - TaskCompletionSource clientWaitingTcs = new(TaskCreationOptions.RunContinuationsAsynchronously); - TaskCompletionSource serverWaitingTcs = new(TaskCreationOptions.RunContinuationsAsynchronously); + TaskCompletionSource clientTcs = new(TaskCreationOptions.RunContinuationsAsynchronously); + TaskCompletionSource serverTcs = new(TaskCreationOptions.RunContinuationsAsynchronously); TaskCompletionSource clientDisposedTcs = new(TaskCreationOptions.RunContinuationsAsynchronously); await LoopbackServerFactory.CreateClientAndServerAsync(async uri => @@ -799,31 +799,21 @@ await LoopbackServerFactory.CreateClientAndServerAsync(async uri => using HttpRequestMessage request = new(HttpMethod.Get, uri) { Version = UseVersion }; Task sendAsyncTask = SendAsync(invoker, request); - await serverWaitingTcs.Task.WaitAsync(TestHelper.PassingTestTimeout); + await serverTcs.Task.WaitAsync(TestHelper.PassingTestTimeout); recorder.RecordObservableInstruments(); - clientWaitingTcs.SetResult(); - recorder.RecordObservableInstruments(); - await serverWaitingTcs.Task.WaitAsync(TestHelper.PassingTestTimeout); + clientTcs.SetResult(); HttpResponseMessage response = await sendAsyncTask; response.Dispose(); await WaitForEnvironmentTicksToAdvance(); recorder.RecordObservableInstruments(); } - clientDisposedTcs.SetResult(); - IReadOnlyList measurements = recorder.GetMeasurements(); - foreach (RecordedCounter m in measurements) - { - _output.WriteLine(m.ToString()); - } - - Assert.Collection(measurements, - m => VerifyActiveRequests(m.InstrumentName, (long)m.Value, m.Tags, 1, uri), + Assert.Collection(recorder.GetMeasurements(), m => VerifyTimeInQueue(m.InstrumentName, m.Value, m.Tags, uri, UseVersion), + m => VerifyActiveRequests(m.InstrumentName, (long)m.Value, m.Tags, 1, uri), m => VerifyOpenConnections(m.InstrumentName, m.Value, m.Tags, 1, uri, UseVersion, "active"), - m => VerifyActiveRequests(m.InstrumentName, (long)m.Value, m.Tags, -1, uri), m => VerifyRequestDuration(m.InstrumentName, (double)m.Value, m.Tags, uri, UseVersion, 200), m => VerifyOpenConnections(m.InstrumentName, m.Value, m.Tags, 1, uri, UseVersion, "idle"), m => VerifyConnectionDuration(m.InstrumentName, m.Value, m.Tags, uri, UseVersion)); @@ -833,10 +823,9 @@ await LoopbackServerFactory.CreateClientAndServerAsync(async uri => await server.AcceptConnectionAsync(async connection => { await connection.ReadRequestDataAsync(); - await clientWaitingTcs.Task.WaitAsync(TestHelper.PassingTestTimeout); - serverWaitingTcs.SetResult(); - await clientWaitingTcs.Task.WaitAsync(TestHelper.PassingTestTimeout); - await connection.SendResponseAsync(content: new string('a', 10*1024)); + serverTcs.SetResult(); + await clientTcs.Task.WaitAsync(TestHelper.PassingTestTimeout); + await connection.SendResponseAsync(isFinal: true); await clientDisposedTcs.Task.WaitAsync(TestHelper.PassingTestTimeout); }); }); @@ -1518,6 +1507,9 @@ public async Task ActiveRequests_Success_Recorded() { await RemoteExecutor.Invoke(static async Task () => { + var serverTcs = new TaskCompletionSource(); + var clientTcs = new TaskCompletionSource(); + using HttpMetricsTest_DefaultMeter test = new(null); await test.LoopbackServerFactory.CreateClientAndServerAsync(async uri => { @@ -1525,15 +1517,24 @@ await test.LoopbackServerFactory.CreateClientAndServerAsync(async uri => using InstrumentRecorder recorder = new InstrumentRecorder(InstrumentNames.ActiveRequests); using HttpRequestMessage request = new(HttpMethod.Get, uri) { Version = test.UseVersion }; - HttpResponseMessage response = await client.SendAsync(request); + var requestTask = client.SendAsync(request); + await serverTcs.Task.WaitAsync(TestHelper.PassingTestTimeout); + recorder.RecordObservableInstruments(); + clientTcs.SetResult(); + var response = await requestTask; response.Dispose(); // Make sure disposal doesn't interfere with recording by enforcing early disposal. Assert.Collection(recorder.GetMeasurements(), - m => VerifyActiveRequests(m, 1, uri), - m => VerifyActiveRequests(m, -1, uri)); + m => VerifyActiveRequests(m, 1, uri)); }, async server => { - await server.AcceptConnectionSendResponseAndCloseAsync(); + await server.AcceptConnectionAsync(async connection => + { + var requestData = await connection.ReadRequestDataAsync(); + serverTcs.SetResult(); + await clientTcs.Task.WaitAsync(TestHelper.PassingTestTimeout); + await connection.SendResponseAsync(); + }); }); }).DisposeAsync(); } @@ -1545,8 +1546,9 @@ public async Task AllSocketsHttpHandlerCounters_Success_Recorded() { await RemoteExecutor.Invoke(static async Task () => { - TaskCompletionSource clientWaitingTcs = new(TaskCreationOptions.RunContinuationsAsynchronously); - TaskCompletionSource serverWaitingTcs = new(TaskCreationOptions.RunContinuationsAsynchronously); + TaskCompletionSource clientTcs = new(TaskCreationOptions.RunContinuationsAsynchronously); + TaskCompletionSource serverTcs = new(TaskCreationOptions.RunContinuationsAsynchronously); + TaskCompletionSource clientDisposedTcs = new(TaskCreationOptions.RunContinuationsAsynchronously); using HttpMetricsTest_DefaultMeter test = new(null); await test.LoopbackServerFactory.CreateClientAndServerAsync(async uri => @@ -1557,37 +1559,35 @@ await test.LoopbackServerFactory.CreateClientAndServerAsync(async uri => { using HttpRequestMessage request = new(HttpMethod.Get, uri) { Version = test.UseVersion }; Task sendAsyncTask = client.SendAsync(request); - await serverWaitingTcs.Task.WaitAsync(TestHelper.PassingTestTimeout); + await serverTcs.Task.WaitAsync(TestHelper.PassingTestTimeout); recorder.RecordObservableInstruments(); - clientWaitingTcs.SetResult(); + clientTcs.SetResult(); HttpResponseMessage response = await sendAsyncTask; response.Dispose(); await WaitForEnvironmentTicksToAdvance(); recorder.RecordObservableInstruments(); } + clientDisposedTcs.SetResult(); Version version = HttpVersion.Version11; Assert.Collection(recorder.GetMeasurements(), - m => VerifyActiveRequests(m.InstrumentName, (long)m.Value, m.Tags, 1, uri), m => VerifyTimeInQueue(m.InstrumentName, m.Value, m.Tags, uri, version), + m => VerifyActiveRequests(m.InstrumentName, (long)m.Value, m.Tags, 1, uri), m => VerifyOpenConnections(m.InstrumentName, m.Value, m.Tags, 1, uri, version, "active"), - m => VerifyActiveRequests(m.InstrumentName, (long)m.Value, m.Tags, -1, uri), m => VerifyRequestDuration(m.InstrumentName, (double)m.Value, m.Tags, uri, version, 200), m => VerifyOpenConnections(m.InstrumentName, m.Value, m.Tags, 1, uri, version, "idle"), m => VerifyConnectionDuration(m.InstrumentName, m.Value, m.Tags, uri, version)); }, async server => { - await clientWaitingTcs.Task.WaitAsync(TestHelper.PassingTestTimeout); - await server.AcceptConnectionAsync(async connection => { await connection.ReadRequestDataAsync(); - serverWaitingTcs.SetResult(); - await clientWaitingTcs.Task.WaitAsync(TestHelper.PassingTestTimeout); + serverTcs.SetResult(); + await clientTcs.Task.WaitAsync(TestHelper.PassingTestTimeout); await connection.SendResponseAsync(isFinal: false); - await connection.WaitForCloseAsync(CancellationToken.None); + await clientDisposedTcs.Task.WaitAsync(TestHelper.PassingTestTimeout); }); }); }).DisposeAsync(); From 5624f759536648737ccc35a74ff091c59e13ccb9 Mon Sep 17 00:00:00 2001 From: ManickaP Date: Thu, 23 Jul 2026 17:49:48 +0200 Subject: [PATCH 05/11] Fixed ExternalServer_DurationMetrics_Recorded test --- .../System.Net.Http/tests/FunctionalTests/MetricsTest.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs index 6d7d198ca69853..43a1525c31db19 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs @@ -407,9 +407,9 @@ public async Task ExternalServer_DurationMetrics_Recorded() 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(); + openConnectionsRecorder.RecordObservableInstruments(); await WaitForEnvironmentTicksToAdvance(); } From 877451e17cdf069abb24f77188949f3cd1c7810c Mon Sep 17 00:00:00 2001 From: ManickaP Date: Fri, 24 Jul 2026 09:28:54 +0200 Subject: [PATCH 06/11] Fix missing peerAddress in hash code, removed unused namespace. --- .../SocketsHttpHandler/Metrics/SocketsHttpHandlerMetrics.cs | 3 +-- .../System.Net.Http/tests/FunctionalTests/MetricsTest.cs | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) 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 15de0c6cf279bf..d068997fabe68a 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 @@ -5,7 +5,6 @@ using System.Collections.Generic; using System.Diagnostics; using System.Diagnostics.Metrics; -using System.Threading; namespace System.Net.Http.Metrics { @@ -30,7 +29,7 @@ public OpenConnectionsTagKey(string protocolVersion, string scheme, string host, Port = port; IsIdle = isIdle; PeerAddress = peerAddress; - _hashCode = HashCode.Combine(protocolVersion, scheme, host, port, isIdle); + _hashCode = HashCode.Combine(protocolVersion, scheme, host, port, isIdle, peerAddress); } public bool Equals(OpenConnectionsTagKey other) => diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs index 43a1525c31db19..9e459ba88ef954 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs @@ -316,7 +316,7 @@ public Task ActiveRequests_Success_Recorded() { await server.AcceptConnectionAsync(async connection => { - var requestData = await connection.ReadRequestDataAsync(); + await connection.ReadRequestDataAsync(); serverTcs.SetResult(); await clientTcs.Task.WaitAsync(TestHelper.PassingTestTimeout); await connection.SendResponseAsync(); From 69f45024964ab076ee75b53bfd5474a11bdbf1db Mon Sep 17 00:00:00 2001 From: ManickaP Date: Fri, 24 Jul 2026 13:46:32 +0200 Subject: [PATCH 07/11] Feedback --- .../System/Net/Http/Metrics/MetricsHandler.cs | 20 +++++++-------- .../Metrics/ConnectionMetrics.cs | 25 ++++++++++++------- .../Metrics/SocketsHttpHandlerMetrics.cs | 4 +++ 3 files changed, 29 insertions(+), 20 deletions(-) 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 58a029905d834b..347632851c9694 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 @@ -19,25 +19,22 @@ namespace System.Net.Http.Metrics public readonly string? Scheme; public readonly string? Host; public readonly int Port; - public readonly bool HasUriTags; public readonly string Method; private readonly int _hashCode; - public ActiveRequestsTagKey(string? scheme, string? host, int port, bool hasUriTags, string method) + public ActiveRequestsTagKey(string? scheme, string? host, int port, string method) { Scheme = scheme; Host = host; Port = port; - HasUriTags = hasUriTags; Method = method; - _hashCode = HashCode.Combine(scheme, host, port, hasUriTags, method); + _hashCode = HashCode.Combine(scheme, host, port, method); } public bool Equals(ActiveRequestsTagKey other) => Scheme == other.Scheme && Host == other.Host && Port == other.Port && - HasUriTags == other.HasUriTags && Method == other.Method; public override bool Equals(object? obj) => obj is ActiveRequestsTagKey other && Equals(other); @@ -47,7 +44,7 @@ public bool Equals(ActiveRequestsTagKey other) => public TagList ToTagList() { TagList tags = default; - if (HasUriTags) + if (Scheme is not null) { tags.Add("url.scheme", Scheme); tags.Add("server.address", Host); @@ -56,6 +53,9 @@ public TagList ToTagList() tags.Add("http.request.method", Method); return tags; } + + public override string ToString() => + $"{Method}{(Scheme is not null ? $" {Scheme}://{Host}:{Port}" : "")}"; } /// @@ -87,6 +87,7 @@ public void Decrement(in ActiveRequestsTagKey key) { // Key doesn't exist, nothing to decrement. // This shouldn't happen in normal operation but we handle it gracefully. + Debug.Fail($"Decrement for non-existing request {key}"); return; } @@ -218,8 +219,6 @@ protected override void Dispose(bool disposing) private void RequestStop(HttpRequestMessage request, HttpResponseMessage? response, Exception? exception, long startTimestamp, bool recordCurrentRequests) { - TagList tags = InitializeCommonTags(request); - if (recordCurrentRequests) { _activeRequestsTracker.Decrement(CreateActiveRequestsTagKey(request)); @@ -230,6 +229,7 @@ private void RequestStop(HttpRequestMessage request, HttpResponseMessage? respon return; } + TagList tags = InitializeCommonTags(request); if (response is not null) { tags.Add("http.response.status_code", DiagnosticsHelper.GetBoxedInt32((int)response.StatusCode)); @@ -274,19 +274,17 @@ private ActiveRequestsTagKey CreateActiveRequestsTagKey(HttpRequestMessage reque string? scheme = null; string? host = null; int port = 0; - bool hasUriTags = false; if (request.RequestUri is Uri requestUri && requestUri.IsAbsoluteUri) { scheme = requestUri.Scheme; host = DiagnosticsHelper.GetServerAddress(request, _proxy); port = requestUri.Port; - hasUriTags = true; } string method = (string)DiagnosticsHelper.GetMethodTag(request.Method, out _).Value!; - return new ActiveRequestsTagKey(scheme, host, port, hasUriTags, method); + return new ActiveRequestsTagKey(scheme, host, port, method); } private sealed class SharedMeter : Meter 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 b864bc586ea257..d8022e621ae147 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 @@ -53,23 +53,27 @@ public void ConnectionEstablished() { if (_openConnectionsEnabled) { - _currentlyIdle = true; - _metrics.OpenConnectionsTracker.Increment(CreateTagKey(idle: true)); + lock (this) + { + _currentlyIdle = true; + _metrics.OpenConnectionsTracker.Increment(CreateTagKey(idle: true)); + } } } public void ConnectionClosed(long durationMs) { - TagList tags = GetTags(); - if (_metrics.ConnectionDuration.Enabled) { - _metrics.ConnectionDuration.Record(durationMs / 1000d, tags); + _metrics.ConnectionDuration.Record(durationMs / 1000d, GetTags()); } if (_openConnectionsEnabled) { - _metrics.OpenConnectionsTracker.Decrement(CreateTagKey(idle: _currentlyIdle)); + lock (this) + { + _metrics.OpenConnectionsTracker.Decrement(CreateTagKey(idle: _currentlyIdle)); + } } } @@ -77,9 +81,12 @@ public void IdleStateChanged(bool idle) { if (_openConnectionsEnabled && _currentlyIdle != idle) { - _currentlyIdle = idle; - _metrics.OpenConnectionsTracker.Decrement(CreateTagKey(idle: !idle)); - _metrics.OpenConnectionsTracker.Increment(CreateTagKey(idle: idle)); + lock (this) + { + _currentlyIdle = idle; + _metrics.OpenConnectionsTracker.Decrement(CreateTagKey(idle: !idle)); + _metrics.OpenConnectionsTracker.Increment(CreateTagKey(idle: idle)); + } } } } 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 d068997fabe68a..7d38f6fca17d49 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 @@ -58,6 +58,9 @@ public TagList ToTagList() } return tags; } + + public override string ToString() => + $"HTTP/{ProtocolVersion} {Scheme}://{Host}:{Port} {(IsIdle ? "idle" : "active")}{(PeerAddress is not null ? $" {PeerAddress}" : "")}"; } /// @@ -89,6 +92,7 @@ public void Decrement(in OpenConnectionsTagKey key) { // Key doesn't exist, nothing to decrement. // This shouldn't happen in normal operation but we handle it gracefully. + Debug.Fail($"Decrement for non-existing connection {key}"); return; } From 46ae1218852332fee06549347b52a8d94d241b9d Mon Sep 17 00:00:00 2001 From: ManickaP Date: Fri, 24 Jul 2026 14:03:38 +0200 Subject: [PATCH 08/11] Copilot feedback numero 146 --- .../tests/FunctionalTests/MetricsTest.cs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs index 9e459ba88ef954..c21f0a4d5a834d 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs @@ -294,8 +294,8 @@ public HttpMetricsTest(ITestOutputHelper output) : base(output) [ActiveIssue("https://github.com/dotnet/runtime/issues/129223", TestPlatforms.Wasi)] public Task ActiveRequests_Success_Recorded() { - var serverTcs = new TaskCompletionSource(); - var clientTcs = new TaskCompletionSource(); + var serverTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var clientTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); return LoopbackServerFactory.CreateClientAndServerAsync(async uri => { @@ -687,10 +687,10 @@ public Task ActiveRequests_Redirect_RecordedForEachHttpSpan(int credentialsMode) Handler.Credentials = credentialsMode == 1 ? new CredentialCache() : new CustomCredentials(); } - var originalServerTcs = new TaskCompletionSource(); - var redirectServerTcs = new TaskCompletionSource(); - var clientTcs1 = new TaskCompletionSource(); - var clientTcs2 = new TaskCompletionSource(); + var originalServerTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var redirectServerTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var clientTcs1 = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var clientTcs2 = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); return LoopbackServerFactory.CreateServerAsync((originalServer, originalUri) => { @@ -1507,8 +1507,8 @@ public async Task ActiveRequests_Success_Recorded() { await RemoteExecutor.Invoke(static async Task () => { - var serverTcs = new TaskCompletionSource(); - var clientTcs = new TaskCompletionSource(); + var serverTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var clientTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); using HttpMetricsTest_DefaultMeter test = new(null); await test.LoopbackServerFactory.CreateClientAndServerAsync(async uri => From ab871f714aca4d8dd825326b7b043d57d2d44fc5 Mon Sep 17 00:00:00 2001 From: ManickaP Date: Mon, 27 Jul 2026 11:49:50 +0200 Subject: [PATCH 09/11] Fix H/3 connection counter --- .../System/Net/Http/SocketsHttpHandler/Http3Connection.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs index ebe52071a870bd..0610fa8a49b8d9 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs @@ -457,14 +457,14 @@ public void RemoveStream(QuicStream stream) { if (_activeRequests.Remove(stream)) { - if (ShuttingDown) + if (_activeRequests.Count == 0) { - CheckForShutdown(); + MarkConnectionAsIdle(); } - if (_activeRequests.Count == 0) + if (ShuttingDown) { - MarkConnectionAsIdle(); + CheckForShutdown(); } } } From 086969439955548abd3a086b120ffa457fbf66d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marie=20P=C3=ADchov=C3=A1?= <11718369+ManickaP@users.noreply.github.com> Date: Tue, 28 Jul 2026 10:06:10 +0200 Subject: [PATCH 10/11] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../System.Net.Http/tests/FunctionalTests/MetricsTest.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs index c21f0a4d5a834d..5ba543931fcd45 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs @@ -717,7 +717,7 @@ public Task ActiveRequests_Redirect_RecordedForEachHttpSpan(int credentialsMode) serverTask = redirectServer.AcceptConnectionAsync(async connection => { - var requestData = await connection.ReadRequestDataAsync(); + await connection.ReadRequestDataAsync(); redirectServerTcs.SetResult(); await clientTcs2.Task.WaitAsync(TestHelper.PassingTestTimeout); await connection.SendResponseAsync(); From 9344a7e01204d39a670721e93b9f05b25b5f5a49 Mon Sep 17 00:00:00 2001 From: ManickaP Date: Wed, 29 Jul 2026 16:28:54 +0200 Subject: [PATCH 11/11] Feedback --- .../System/Net/Http/Metrics/MetricsHandler.cs | 32 ++++++------------- 1 file changed, 9 insertions(+), 23 deletions(-) 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 347632851c9694..9b53197bb735d2 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 @@ -173,7 +173,7 @@ private async ValueTask SendAsyncWithMetrics(HttpRequestMes { Debug.Assert(GlobalHttpSettings.MetricsHandler.IsGloballyEnabled); - (long startTimestamp, bool recordCurrentRequests) = RequestStart(request); + (long startTimestamp, bool recordCurrentRequests, ActiveRequestsTagKey requestTagKey) = RequestStart(request); HttpResponseMessage? response = null; Exception? exception = null; try @@ -190,7 +190,7 @@ await _innerHandler.SendAsync(request, cancellationToken).ConfigureAwait(false) } finally { - RequestStop(request, response, exception, startTimestamp, recordCurrentRequests); + RequestStop(request, response, exception, startTimestamp, recordCurrentRequests, requestTagKey); } } @@ -204,24 +204,25 @@ protected override void Dispose(bool disposing) base.Dispose(disposing); } - private (long StartTimestamp, bool RecordCurrentRequests) RequestStart(HttpRequestMessage request) + private (long StartTimestamp, bool RecordCurrentRequests, ActiveRequestsTagKey RequestTagKey) RequestStart(HttpRequestMessage request) { bool recordCurrentRequests = _activeRequests.Enabled; long startTimestamp = Stopwatch.GetTimestamp(); + ActiveRequestsTagKey requestTagKey = CreateActiveRequestsTagKey(request); if (recordCurrentRequests) { - _activeRequestsTracker.Increment(CreateActiveRequestsTagKey(request)); + _activeRequestsTracker.Increment(requestTagKey); } - return (startTimestamp, recordCurrentRequests); + return (startTimestamp, recordCurrentRequests, requestTagKey); } - private void RequestStop(HttpRequestMessage request, HttpResponseMessage? response, Exception? exception, long startTimestamp, bool recordCurrentRequests) + private void RequestStop(HttpRequestMessage request, HttpResponseMessage? response, Exception? exception, long startTimestamp, bool recordCurrentRequests, ActiveRequestsTagKey requestTagKey) { if (recordCurrentRequests) { - _activeRequestsTracker.Decrement(CreateActiveRequestsTagKey(request)); + _activeRequestsTracker.Decrement(requestTagKey); } if (!_requestsDuration.Enabled) @@ -229,7 +230,7 @@ private void RequestStop(HttpRequestMessage request, HttpResponseMessage? respon return; } - TagList tags = InitializeCommonTags(request); + TagList tags = requestTagKey.ToTagList(); if (response is not null) { tags.Add("http.response.status_code", DiagnosticsHelper.GetBoxedInt32((int)response.StatusCode)); @@ -254,21 +255,6 @@ private void RequestStop(HttpRequestMessage request, HttpResponseMessage? respon } } - 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", DiagnosticsHelper.GetServerAddress(request, _proxy)); - tags.Add("server.port", DiagnosticsHelper.GetBoxedInt32(requestUri.Port)); - } - tags.Add(DiagnosticsHelper.GetMethodTag(request.Method, out _)); - - return tags; - } - private ActiveRequestsTagKey CreateActiveRequestsTagKey(HttpRequestMessage request) { string? scheme = null;