From 295d18475375e417650b1b7c85b38042ae1ea18f Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Wed, 3 May 2023 12:26:43 +0800 Subject: [PATCH 1/9] Skip counter measurement when start listening while processing --- .../Hosting/test/HostingApplicationTests.cs | 122 ----------- .../Hosting/test/HostingMetricsTests.cs | 204 ++++++++++++++++++ .../Microsoft.AspNetCore.Hosting.Tests.csproj | 1 + 3 files changed, 205 insertions(+), 122 deletions(-) create mode 100644 src/Hosting/Hosting/test/HostingMetricsTests.cs diff --git a/src/Hosting/Hosting/test/HostingApplicationTests.cs b/src/Hosting/Hosting/test/HostingApplicationTests.cs index d3e9e2623b09..3c5246b6f7a8 100644 --- a/src/Hosting/Hosting/test/HostingApplicationTests.cs +++ b/src/Hosting/Hosting/test/HostingApplicationTests.cs @@ -20,128 +20,6 @@ namespace Microsoft.AspNetCore.Hosting.Tests; public class HostingApplicationTests { - [Fact] - public void Metrics() - { - // Arrange - var meterFactory = new TestMeterFactory(); - var meterRegistry = new TestMeterRegistry(meterFactory.Meters); - var hostingApplication = CreateApplication(meterFactory: meterFactory); - var httpContext = new DefaultHttpContext(); - var meter = meterFactory.Meters.Single(); - - using var requestDurationRecorder = new InstrumentRecorder(meterRegistry, HostingMetrics.MeterName, "request-duration"); - using var currentRequestsRecorder = new InstrumentRecorder(meterRegistry, HostingMetrics.MeterName, "current-requests"); - - // Act/Assert - Assert.Equal(HostingMetrics.MeterName, meter.Name); - Assert.Null(meter.Version); - - // Request 1 (after success) - httpContext.Request.Protocol = HttpProtocol.Http11; - var context1 = hostingApplication.CreateContext(httpContext.Features); - context1.HttpContext.Response.StatusCode = StatusCodes.Status200OK; - hostingApplication.DisposeContext(context1, null); - - Assert.Collection(currentRequestsRecorder.GetMeasurements(), - m => Assert.Equal(1, m.Value), - m => Assert.Equal(-1, m.Value)); - Assert.Collection(requestDurationRecorder.GetMeasurements(), - m => AssertRequestDuration(m, HttpProtocol.Http11, StatusCodes.Status200OK)); - - // Request 2 (after failure) - httpContext.Request.Protocol = HttpProtocol.Http2; - var context2 = hostingApplication.CreateContext(httpContext.Features); - context2.HttpContext.Response.StatusCode = StatusCodes.Status500InternalServerError; - hostingApplication.DisposeContext(context2, new InvalidOperationException("Test error")); - - Assert.Collection(currentRequestsRecorder.GetMeasurements(), - m => Assert.Equal(1, m.Value), - m => Assert.Equal(-1, m.Value), - m => Assert.Equal(1, m.Value), - m => Assert.Equal(-1, m.Value)); - Assert.Collection(requestDurationRecorder.GetMeasurements(), - m => AssertRequestDuration(m, HttpProtocol.Http11, StatusCodes.Status200OK), - m => AssertRequestDuration(m, HttpProtocol.Http2, StatusCodes.Status500InternalServerError, exceptionName: "System.InvalidOperationException")); - - // Request 3 - httpContext.Request.Protocol = HttpProtocol.Http3; - var context3 = hostingApplication.CreateContext(httpContext.Features); - context3.HttpContext.Response.StatusCode = StatusCodes.Status200OK; - - Assert.Collection(currentRequestsRecorder.GetMeasurements(), - m => Assert.Equal(1, m.Value), - m => Assert.Equal(-1, m.Value), - m => Assert.Equal(1, m.Value), - m => Assert.Equal(-1, m.Value), - m => Assert.Equal(1, m.Value)); - Assert.Collection(requestDurationRecorder.GetMeasurements(), - m => AssertRequestDuration(m, HttpProtocol.Http11, StatusCodes.Status200OK), - m => AssertRequestDuration(m, HttpProtocol.Http2, StatusCodes.Status500InternalServerError, exceptionName: "System.InvalidOperationException")); - - hostingApplication.DisposeContext(context3, null); - - Assert.Collection(currentRequestsRecorder.GetMeasurements(), - m => Assert.Equal(1, m.Value), - m => Assert.Equal(-1, m.Value), - m => Assert.Equal(1, m.Value), - m => Assert.Equal(-1, m.Value), - m => Assert.Equal(1, m.Value), - m => Assert.Equal(-1, m.Value)); - Assert.Collection(requestDurationRecorder.GetMeasurements(), - m => AssertRequestDuration(m, HttpProtocol.Http11, StatusCodes.Status200OK), - m => AssertRequestDuration(m, HttpProtocol.Http2, StatusCodes.Status500InternalServerError, exceptionName: "System.InvalidOperationException"), - m => AssertRequestDuration(m, HttpProtocol.Http3, StatusCodes.Status200OK)); - - static void AssertRequestDuration(Measurement measurement, string protocol, int statusCode, string exceptionName = null) - { - Assert.True(measurement.Value > 0); - Assert.Equal(protocol, (string)measurement.Tags.ToArray().Single(t => t.Key == "protocol").Value); - Assert.Equal(statusCode, (int)measurement.Tags.ToArray().Single(t => t.Key == "status-code").Value); - if (exceptionName == null) - { - Assert.DoesNotContain(measurement.Tags.ToArray(), t => t.Key == "exception-name"); - } - else - { - Assert.Equal(exceptionName, (string)measurement.Tags.ToArray().Single(t => t.Key == "exception-name").Value); - } - } - } - - [Fact] - public void IHttpMetricsTagsFeatureNotUsedFromFeatureCollection() - { - // Arrange - var meterFactory = new TestMeterFactory(); - var meterRegistry = new TestMeterRegistry(meterFactory.Meters); - var hostingApplication = CreateApplication(meterFactory: meterFactory); - var httpContext = new DefaultHttpContext(); - var meter = meterFactory.Meters.Single(); - - using var requestDurationRecorder = new InstrumentRecorder(meterRegistry, HostingMetrics.MeterName, "request-duration"); - using var currentRequestsRecorder = new InstrumentRecorder(meterRegistry, HostingMetrics.MeterName, "current-requests"); - - // Act/Assert - Assert.Equal(HostingMetrics.MeterName, meter.Name); - Assert.Null(meter.Version); - - // This feature will be overidden by hosting. Hosting is the owner of the feature and is resposible for setting it. - var overridenFeature = new TestHttpMetricsTagsFeature(); - httpContext.Features.Set(overridenFeature); - - var context = hostingApplication.CreateContext(httpContext.Features); - var contextFeature = httpContext.Features.Get(); - - Assert.NotNull(contextFeature); - Assert.NotEqual(overridenFeature, contextFeature); - } - - private sealed class TestHttpMetricsTagsFeature : IHttpMetricsTagsFeature - { - public ICollection> Tags { get; } = new Collection>(); - } - [Fact] public void DisposeContextDoesNotClearHttpContextIfDefaultHttpContextFactoryUsed() { diff --git a/src/Hosting/Hosting/test/HostingMetricsTests.cs b/src/Hosting/Hosting/test/HostingMetricsTests.cs new file mode 100644 index 000000000000..72877540bce7 --- /dev/null +++ b/src/Hosting/Hosting/test/HostingMetricsTests.cs @@ -0,0 +1,204 @@ +// 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.ObjectModel; +using System.Diagnostics; +using System.Diagnostics.Metrics; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http.Features; +using Microsoft.AspNetCore.Internal; +using Microsoft.AspNetCore.Testing; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Metrics; + +namespace Microsoft.AspNetCore.Hosting.Tests; + +public class HostingMetricsTests +{ + [Fact] + public void MultipleRequests() + { + // Arrange + var meterFactory = new TestMeterFactory(); + var meterRegistry = new TestMeterRegistry(meterFactory.Meters); + var hostingApplication = CreateApplication(meterFactory: meterFactory); + var httpContext = new DefaultHttpContext(); + var meter = meterFactory.Meters.Single(); + + using var requestDurationRecorder = new InstrumentRecorder(meterRegistry, HostingMetrics.MeterName, "request-duration"); + using var currentRequestsRecorder = new InstrumentRecorder(meterRegistry, HostingMetrics.MeterName, "current-requests"); + + // Act/Assert + Assert.Equal(HostingMetrics.MeterName, meter.Name); + Assert.Null(meter.Version); + + // Request 1 (after success) + httpContext.Request.Protocol = HttpProtocol.Http11; + var context1 = hostingApplication.CreateContext(httpContext.Features); + context1.HttpContext.Response.StatusCode = StatusCodes.Status200OK; + hostingApplication.DisposeContext(context1, null); + + Assert.Collection(currentRequestsRecorder.GetMeasurements(), + m => Assert.Equal(1, m.Value), + m => Assert.Equal(-1, m.Value)); + Assert.Collection(requestDurationRecorder.GetMeasurements(), + m => AssertRequestDuration(m, HttpProtocol.Http11, StatusCodes.Status200OK)); + + // Request 2 (after failure) + httpContext.Request.Protocol = HttpProtocol.Http2; + var context2 = hostingApplication.CreateContext(httpContext.Features); + context2.HttpContext.Response.StatusCode = StatusCodes.Status500InternalServerError; + hostingApplication.DisposeContext(context2, new InvalidOperationException("Test error")); + + Assert.Collection(currentRequestsRecorder.GetMeasurements(), + m => Assert.Equal(1, m.Value), + m => Assert.Equal(-1, m.Value), + m => Assert.Equal(1, m.Value), + m => Assert.Equal(-1, m.Value)); + Assert.Collection(requestDurationRecorder.GetMeasurements(), + m => AssertRequestDuration(m, HttpProtocol.Http11, StatusCodes.Status200OK), + m => AssertRequestDuration(m, HttpProtocol.Http2, StatusCodes.Status500InternalServerError, exceptionName: "System.InvalidOperationException")); + + // Request 3 + httpContext.Request.Protocol = HttpProtocol.Http3; + var context3 = hostingApplication.CreateContext(httpContext.Features); + context3.HttpContext.Response.StatusCode = StatusCodes.Status200OK; + + Assert.Collection(currentRequestsRecorder.GetMeasurements(), + m => Assert.Equal(1, m.Value), + m => Assert.Equal(-1, m.Value), + m => Assert.Equal(1, m.Value), + m => Assert.Equal(-1, m.Value), + m => Assert.Equal(1, m.Value)); + Assert.Collection(requestDurationRecorder.GetMeasurements(), + m => AssertRequestDuration(m, HttpProtocol.Http11, StatusCodes.Status200OK), + m => AssertRequestDuration(m, HttpProtocol.Http2, StatusCodes.Status500InternalServerError, exceptionName: "System.InvalidOperationException")); + + hostingApplication.DisposeContext(context3, null); + + Assert.Collection(currentRequestsRecorder.GetMeasurements(), + m => Assert.Equal(1, m.Value), + m => Assert.Equal(-1, m.Value), + m => Assert.Equal(1, m.Value), + m => Assert.Equal(-1, m.Value), + m => Assert.Equal(1, m.Value), + m => Assert.Equal(-1, m.Value)); + Assert.Collection(requestDurationRecorder.GetMeasurements(), + m => AssertRequestDuration(m, HttpProtocol.Http11, StatusCodes.Status200OK), + m => AssertRequestDuration(m, HttpProtocol.Http2, StatusCodes.Status500InternalServerError, exceptionName: "System.InvalidOperationException"), + m => AssertRequestDuration(m, HttpProtocol.Http3, StatusCodes.Status200OK)); + + static void AssertRequestDuration(Measurement measurement, string protocol, int statusCode, string exceptionName = null) + { + Assert.True(measurement.Value > 0); + Assert.Equal(protocol, (string)measurement.Tags.ToArray().Single(t => t.Key == "protocol").Value); + Assert.Equal(statusCode, (int)measurement.Tags.ToArray().Single(t => t.Key == "status-code").Value); + if (exceptionName == null) + { + Assert.DoesNotContain(measurement.Tags.ToArray(), t => t.Key == "exception-name"); + } + else + { + Assert.Equal(exceptionName, (string)measurement.Tags.ToArray().Single(t => t.Key == "exception-name").Value); + } + } + } + + [Fact] + public async Task StartListeningDuringRequest_NotMeasured() + { + // Arrange + var syncPoint = new SyncPoint(); + var meterFactory = new TestMeterFactory(); + var meterRegistry = new TestMeterRegistry(meterFactory.Meters); + var hostingApplication = CreateApplication(meterFactory: meterFactory, requestDelegate: async ctx => + { + await syncPoint.WaitToContinue(); + }); + var httpContext = new DefaultHttpContext(); + var meter = meterFactory.Meters.Single(); + + // Act/Assert + Assert.Equal(HostingMetrics.MeterName, meter.Name); + Assert.Null(meter.Version); + + // Request 1 (after success) + httpContext.Request.Protocol = HttpProtocol.Http11; + var context1 = hostingApplication.CreateContext(httpContext.Features); + var processRequestTask = hostingApplication.ProcessRequestAsync(context1); + + await syncPoint.WaitForSyncPoint().DefaultTimeout(); + + using var requestDurationRecorder = new InstrumentRecorder(meterRegistry, HostingMetrics.MeterName, "request-duration"); + using var currentRequestsRecorder = new InstrumentRecorder(meterRegistry, HostingMetrics.MeterName, "current-requests"); + context1.HttpContext.Response.StatusCode = StatusCodes.Status200OK; + + syncPoint.Continue(); + await processRequestTask.DefaultTimeout(); + + hostingApplication.DisposeContext(context1, null); + + Assert.Empty(currentRequestsRecorder.GetMeasurements()); + Assert.Empty(requestDurationRecorder.GetMeasurements()); + } + + [Fact] + public void IHttpMetricsTagsFeatureNotUsedFromFeatureCollection() + { + // Arrange + var meterFactory = new TestMeterFactory(); + var meterRegistry = new TestMeterRegistry(meterFactory.Meters); + var hostingApplication = CreateApplication(meterFactory: meterFactory); + var httpContext = new DefaultHttpContext(); + var meter = meterFactory.Meters.Single(); + + using var requestDurationRecorder = new InstrumentRecorder(meterRegistry, HostingMetrics.MeterName, "request-duration"); + using var currentRequestsRecorder = new InstrumentRecorder(meterRegistry, HostingMetrics.MeterName, "current-requests"); + + // Act/Assert + Assert.Equal(HostingMetrics.MeterName, meter.Name); + Assert.Null(meter.Version); + + // This feature will be overidden by hosting. Hosting is the owner of the feature and is resposible for setting it. + var overridenFeature = new TestHttpMetricsTagsFeature(); + httpContext.Features.Set(overridenFeature); + + var context = hostingApplication.CreateContext(httpContext.Features); + var contextFeature = httpContext.Features.Get(); + + Assert.NotNull(contextFeature); + Assert.NotEqual(overridenFeature, contextFeature); + } + + private sealed class TestHttpMetricsTagsFeature : IHttpMetricsTagsFeature + { + public ICollection> Tags { get; } = new Collection>(); + } + + private static HostingApplication CreateApplication(IHttpContextFactory httpContextFactory = null, bool useHttpContextAccessor = false, + ActivitySource activitySource = null, IMeterFactory meterFactory = null, RequestDelegate requestDelegate = null) + { + var services = new ServiceCollection(); + services.AddOptions(); + if (useHttpContextAccessor) + { + services.AddHttpContextAccessor(); + } + + httpContextFactory ??= new DefaultHttpContextFactory(services.BuildServiceProvider()); + requestDelegate ??= ctx => Task.CompletedTask; + + var hostingApplication = new HostingApplication( + requestDelegate, + NullLogger.Instance, + new DiagnosticListener("Microsoft.AspNetCore"), + activitySource ?? new ActivitySource("Microsoft.AspNetCore"), + DistributedContextPropagator.CreateDefaultPropagator(), + httpContextFactory, + HostingEventSource.Log, + new HostingMetrics(meterFactory ?? new TestMeterFactory())); + + return hostingApplication; + } +} diff --git a/src/Hosting/Hosting/test/Microsoft.AspNetCore.Hosting.Tests.csproj b/src/Hosting/Hosting/test/Microsoft.AspNetCore.Hosting.Tests.csproj index cf25a1547daa..25a94cfc96a3 100644 --- a/src/Hosting/Hosting/test/Microsoft.AspNetCore.Hosting.Tests.csproj +++ b/src/Hosting/Hosting/test/Microsoft.AspNetCore.Hosting.Tests.csproj @@ -7,6 +7,7 @@ + From 09c4e74ac44d94a4baf05e26bbec400374c53274 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Wed, 3 May 2023 20:33:20 +0800 Subject: [PATCH 2/9] Kestrel metrics doesn't under count when start listening during connection --- .../Core/src/Internal/ConnectionDispatcher.cs | 8 +- .../Core/src/Internal/Http/Http1Connection.cs | 2 +- .../Http/HttpProtocol.FeatureCollection.cs | 2 +- .../src/Internal/Http2/Http2Connection.cs | 7 +- .../Core/src/Internal/Http2/Http2Stream.cs | 1 + .../src/Internal/Http2/Http2StreamContext.cs | 3 +- .../Core/src/Internal/Http2/Http2StreamOfT.cs | 2 +- .../src/Internal/Http3/Http3Connection.cs | 4 +- .../Core/src/Internal/Http3/Http3Stream.cs | 1 + .../src/Internal/Http3/Http3StreamContext.cs | 2 +- .../Core/src/Internal/Http3/Http3StreamOfT.cs | 2 +- .../src/Internal/HttpConnectionContext.cs | 5 +- .../Infrastructure/KestrelConnection.cs | 8 +- .../Infrastructure/KestrelConnectionOfT.cs | 47 +++++- .../Internal/Infrastructure/KestrelMetrics.cs | 159 ++++++++++-------- .../Middleware/ConnectionLimitMiddleware.cs | 3 +- .../Middleware/HttpConnectionMiddleware.cs | 5 +- .../Middleware/HttpsConnectionMiddleware.cs | 25 +-- .../Core/test/ConnectionDispatcherTests.cs | 14 +- ...Http3HttpProtocolFeatureCollectionTests.cs | 5 +- .../Core/test/HttpConnectionManagerTests.cs | 2 +- .../Core/test/PooledStreamStackTests.cs | 19 +-- .../shared/test/Http3/Http3InMemory.cs | 4 +- .../Kestrel/shared/test/TestContextFactory.cs | 18 +- .../Http2/Http2TestBase.cs | 9 +- .../WebTransport/WebTransportTestUtilities.cs | 2 + .../KestrelMetricsTests.cs | 58 +++++++ 27 files changed, 278 insertions(+), 139 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/ConnectionDispatcher.cs b/src/Servers/Kestrel/Core/src/Internal/ConnectionDispatcher.cs index c9407633064b..cefa8fdeba30 100644 --- a/src/Servers/Kestrel/Core/src/Internal/ConnectionDispatcher.cs +++ b/src/Servers/Kestrel/Core/src/Internal/ConnectionDispatcher.cs @@ -22,6 +22,7 @@ public ConnectionDispatcher(ServiceContext serviceContext, Func connect } private KestrelTrace Log => _serviceContext.Log; + private KestrelMetrics Metrics => _serviceContext.Metrics; public Task StartAcceptingConnections(IConnectionListener listener) { @@ -50,14 +51,17 @@ async Task AcceptConnectionsAsync() // Add the connection to the connection manager before we queue it for execution var id = _transportConnectionManager.GetNewConnectionId(); + var metricsContext = new ConnectionMetricsContext(connection, + Metrics.CurrentConnectionsCounterEnabled, Metrics.ConnectionDurationEnabled, Metrics.QueuedConnectionsCounterEnabled, + Metrics.QueuedRequestsCounterEnabled, Metrics.CurrentUpgradedRequestsCounterEnabled, Metrics.CurrentTlsHandshakesCounterEnabled); var kestrelConnection = new KestrelConnection( - id, _serviceContext, _transportConnectionManager, _connectionDelegate, connection, Log); + id, _serviceContext, _transportConnectionManager, _connectionDelegate, connection, Log, metricsContext); _transportConnectionManager.AddConnection(id, kestrelConnection); Log.ConnectionAccepted(connection.ConnectionId); KestrelEventSource.Log.ConnectionQueuedStart(connection); - _serviceContext.Metrics.ConnectionQueuedStart(connection); + Metrics.ConnectionQueuedStart(metricsContext); ThreadPool.UnsafeQueueUserWorkItem(kestrelConnection, preferLocal: false); } diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs b/src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs index 4e7b87c3f9ff..373c8f5b8774 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs @@ -86,7 +86,7 @@ protected override void OnRequestProcessingEnded() if (IsUpgraded) { KestrelEventSource.Log.RequestUpgradedStop(this); - ServiceContext.Metrics.RequestUpgradedStop(_context.ConnectionContext); + ServiceContext.Metrics.RequestUpgradedStop(_context.MetricsContext); ServiceContext.ConnectionManager.UpgradedConnectionCount.ReleaseOne(); } diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.FeatureCollection.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.FeatureCollection.cs index 713a8ad97209..d6d070bd7de1 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.FeatureCollection.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.FeatureCollection.cs @@ -276,7 +276,7 @@ async Task IHttpUpgradeFeature.UpgradeAsync() IsUpgraded = true; KestrelEventSource.Log.RequestUpgradedStart(this); - ServiceContext.Metrics.RequestUpgradedStart(_context.ConnectionContext); + ServiceContext.Metrics.RequestUpgradedStart(_context.MetricsContext); ConnectionFeatures.Get()?.ReleaseConnection(); diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs index 846e751870ca..a7ee8515a09d 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs @@ -47,6 +47,7 @@ internal sealed partial class Http2Connection : IHttp2StreamLifetimeHandler, IHt private const long StreamPoolExpiryTicks = TimeSpan.TicksPerSecond * 5; private readonly HttpConnectionContext _context; + private readonly ConnectionMetricsContext _metricsContext; private readonly Http2FrameWriter _frameWriter; private readonly Pipe _input; private readonly Task _inputTask; @@ -93,6 +94,7 @@ public Http2Connection(HttpConnectionContext context) _context = context; _streamLifetimeHandler = this; + _metricsContext = context.ConnectionFeatures.GetRequiredFeature().MetricsContext; // Capture the ExecutionContext before dispatching HTTP/2 middleware. Will be restored by streams when processing request _context.InitialExecutionContext = ExecutionContext.Capture(); @@ -772,7 +774,8 @@ private Http2StreamContext CreateHttp2StreamContext() _clientSettings, _serverSettings, _frameWriter, - _inputFlowControl); + _inputFlowControl, + _metricsContext); streamContext.TimeoutControl = _context.TimeoutControl; streamContext.InitialExecutionContext = _context.InitialExecutionContext; @@ -1189,7 +1192,7 @@ private void StartStream() } KestrelEventSource.Log.RequestQueuedStart(_currentHeadersStream, AspNetCore.Http.HttpProtocol.Http2); - _context.ServiceContext.Metrics.RequestQueuedStart(_context.ConnectionContext, AspNetCore.Http.HttpProtocol.Http2); + _context.ServiceContext.Metrics.RequestQueuedStart(_metricsContext, AspNetCore.Http.HttpProtocol.Http2); // _scheduleInline is only true in tests if (!_scheduleInline) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs index 1ffa495f2f3d..9a10e29d58d7 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs @@ -83,6 +83,7 @@ public void InitializeWithExistingContext(int streamId) public int StreamId => _context.StreamId; public BaseConnectionContext ConnectionContext => _context.ConnectionContext; + public ConnectionMetricsContext MetricsContext => _context.MetricsContext; public long? InputRemaining { get; internal set; } diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2StreamContext.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2StreamContext.cs index 154b916886ef..9537d30b55fa 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2StreamContext.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2StreamContext.cs @@ -27,7 +27,8 @@ public Http2StreamContext( Http2PeerSettings clientPeerSettings, Http2PeerSettings serverPeerSettings, Http2FrameWriter frameWriter, - InputFlowControl connectionInputFlowControl) : base(connectionId, protocols, altSvcHeader, connectionContext, serviceContext, connectionFeatures, memoryPool, localEndPoint, remoteEndPoint) + InputFlowControl connectionInputFlowControl, + ConnectionMetricsContext metricsContext) : base(connectionId, protocols, altSvcHeader, connectionContext, serviceContext, connectionFeatures, memoryPool, localEndPoint, remoteEndPoint, metricsContext) { StreamId = streamId; StreamLifetimeHandler = streamLifetimeHandler; diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2StreamOfT.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2StreamOfT.cs index 03e957eaf56e..ac0600ea0731 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2StreamOfT.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2StreamOfT.cs @@ -20,7 +20,7 @@ public Http2Stream(IHttpApplication application, Http2StreamContext co public override void Execute() { KestrelEventSource.Log.RequestQueuedStop(this, AspNetCore.Http.HttpProtocol.Http2); - ServiceContext.Metrics.RequestQueuedStop(ConnectionContext, AspNetCore.Http.HttpProtocol.Http2); + ServiceContext.Metrics.RequestQueuedStop(MetricsContext, AspNetCore.Http.HttpProtocol.Http2); // REVIEW: Should we store this in a field for easy debugging? _ = ProcessRequestsAsync(_application); diff --git a/src/Servers/Kestrel/Core/src/Internal/Http3/Http3Connection.cs b/src/Servers/Kestrel/Core/src/Internal/Http3/Http3Connection.cs index d6e43c6644f8..8597cadfd7df 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http3/Http3Connection.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http3/Http3Connection.cs @@ -55,6 +55,7 @@ public Http3Connection(HttpMultiplexedConnectionContext context) _multiplexedContext = (MultiplexedConnectionContext)context.ConnectionContext; _context = context; _streamLifetimeHandler = this; + MetricsContext = context.ConnectionFeatures.GetRequiredFeature().MetricsContext; _errorCodeFeature = context.ConnectionFeatures.GetRequiredFeature(); @@ -91,6 +92,7 @@ private void UpdateHighestOpenedRequestStreamId(long streamId) private long GetCurrentGoAwayStreamId() => Interlocked.Read(ref _highestOpenedRequestStreamId) + 4; private KestrelTrace Log => _context.ServiceContext.Log; + public ConnectionMetricsContext MetricsContext { get; } public KestrelServerLimits Limits => _context.ServiceContext.ServerOptions.Limits; public Http3ControlStream? OutboundControlStream { get; set; } public Http3ControlStream? ControlStream { get; set; } @@ -594,7 +596,7 @@ private async Task CreateHttp3Stream(ConnectionContext streamContext, _streamLifetimeHandler.OnStreamCreated(stream); KestrelEventSource.Log.RequestQueuedStart(stream, AspNetCore.Http.HttpProtocol.Http3); - _context.ServiceContext.Metrics.RequestQueuedStart(_multiplexedContext, AspNetCore.Http.HttpProtocol.Http3); + _context.ServiceContext.Metrics.RequestQueuedStart(MetricsContext, AspNetCore.Http.HttpProtocol.Http3); ThreadPool.UnsafeQueueUserWorkItem(stream, preferLocal: false); } diff --git a/src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs b/src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs index 87a99b21304c..7e032302d5ce 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs @@ -78,6 +78,7 @@ internal abstract partial class Http3Stream : HttpProtocol, IHttp3Stream, IHttpS public bool IsDraining => _appCompletedTaskSource.GetStatus() != ValueTaskSourceStatus.Pending; // Draining starts once app is complete public bool IsRequestStream => true; public BaseConnectionContext ConnectionContext => _context.ConnectionContext; + public ConnectionMetricsContext MetricsContext => _context.MetricsContext; public void Initialize(Http3StreamContext context) { diff --git a/src/Servers/Kestrel/Core/src/Internal/Http3/Http3StreamContext.cs b/src/Servers/Kestrel/Core/src/Internal/Http3/Http3StreamContext.cs index 1b59bf076351..50c406b3d06c 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http3/Http3StreamContext.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http3/Http3StreamContext.cs @@ -23,7 +23,7 @@ public Http3StreamContext( IPEndPoint? localEndPoint, IPEndPoint? remoteEndPoint, ConnectionContext streamContext, - Http3Connection connection) : base(connectionId, protocols, altSvcHeader, connectionContext, serviceContext, connectionFeatures, memoryPool, localEndPoint, remoteEndPoint) + Http3Connection connection) : base(connectionId, protocols, altSvcHeader, connectionContext, serviceContext, connectionFeatures, memoryPool, localEndPoint, remoteEndPoint, connection.MetricsContext) { StreamLifetimeHandler = connection._streamLifetimeHandler; StreamContext = streamContext; diff --git a/src/Servers/Kestrel/Core/src/Internal/Http3/Http3StreamOfT.cs b/src/Servers/Kestrel/Core/src/Internal/Http3/Http3StreamOfT.cs index 3daca086d764..671d64c5eb53 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http3/Http3StreamOfT.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http3/Http3StreamOfT.cs @@ -20,7 +20,7 @@ public Http3Stream(IHttpApplication application, Http3StreamContext co public override void Execute() { KestrelEventSource.Log.RequestQueuedStop(this, AspNetCore.Http.HttpProtocol.Http3); - ServiceContext.Metrics.RequestQueuedStop(ConnectionContext, AspNetCore.Http.HttpProtocol.Http3); + ServiceContext.Metrics.RequestQueuedStop(MetricsContext, AspNetCore.Http.HttpProtocol.Http3); if (_requestHeaderParsingState == RequestHeaderParsingState.Ready) { diff --git a/src/Servers/Kestrel/Core/src/Internal/HttpConnectionContext.cs b/src/Servers/Kestrel/Core/src/Internal/HttpConnectionContext.cs index 98f84784bc84..92c6ad1a0583 100644 --- a/src/Servers/Kestrel/Core/src/Internal/HttpConnectionContext.cs +++ b/src/Servers/Kestrel/Core/src/Internal/HttpConnectionContext.cs @@ -21,9 +21,12 @@ public HttpConnectionContext( IFeatureCollection connectionFeatures, MemoryPool memoryPool, IPEndPoint? localEndPoint, - IPEndPoint? remoteEndPoint) : base(connectionId, protocols, altSvcHeader, connectionContext, serviceContext, connectionFeatures, memoryPool, localEndPoint, remoteEndPoint) + IPEndPoint? remoteEndPoint, + ConnectionMetricsContext metricsContext) : base(connectionId, protocols, altSvcHeader, connectionContext, serviceContext, connectionFeatures, memoryPool, localEndPoint, remoteEndPoint) { + MetricsContext = metricsContext; } public IDuplexPipe Transport { get; set; } = default!; + public ConnectionMetricsContext MetricsContext { get; } } diff --git a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/KestrelConnection.cs b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/KestrelConnection.cs index d46f95919a6c..045a4e2e0505 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/KestrelConnection.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/KestrelConnection.cs @@ -7,7 +7,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; -internal abstract class KestrelConnection : IConnectionHeartbeatFeature, IConnectionCompleteFeature, IConnectionLifetimeNotificationFeature +internal abstract class KestrelConnection : IConnectionHeartbeatFeature, IConnectionCompleteFeature, IConnectionLifetimeNotificationFeature, IConnectionMetricsContextFeature { private List<(Action handler, object state)>? _heartbeatHandlers; private readonly object _heartbeatLock = new object(); @@ -24,18 +24,20 @@ internal abstract class KestrelConnection : IConnectionHeartbeatFeature, IConnec public KestrelConnection(long id, ServiceContext serviceContext, TransportConnectionManager transportConnectionManager, - KestrelTrace logger) + KestrelTrace logger, + ConnectionMetricsContext connectionMetricsContext) { _id = id; _serviceContext = serviceContext; _transportConnectionManager = transportConnectionManager; Logger = logger; - + MetricsContext = connectionMetricsContext; ConnectionClosedRequested = _connectionClosingCts.Token; } protected KestrelTrace Logger { get; } + public ConnectionMetricsContext MetricsContext { get; set; } public CancellationToken ConnectionClosedRequested { get; set; } public Task ExecutionTask => _completionTcs.Task; diff --git a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/KestrelConnectionOfT.cs b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/KestrelConnectionOfT.cs index 2cb095ce3258..b605d716792a 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/KestrelConnectionOfT.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/KestrelConnectionOfT.cs @@ -8,6 +8,35 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; +internal interface IConnectionMetricsContextFeature +{ + ConnectionMetricsContext MetricsContext { get; } +} + +internal readonly struct ConnectionMetricsContext +{ + public BaseConnectionContext ConnectionContext { get; } + public bool CurrentConnectionsCounterEnabled { get; } + public bool ConnectionDurationEnabled { get; } + public bool QueuedConnectionsCounterEnabled { get; } + public bool QueuedRequestsCounterEnabled { get; } + public bool CurrentUpgradedRequestsCounterEnabled { get; } + public bool CurrentTlsHandshakesCounterEnabled { get; } + + public ConnectionMetricsContext(BaseConnectionContext connectionContext, bool currentConnectionsCounterEnabled, + bool connectionDurationEnabled, bool queuedConnectionsCounterEnabled, bool queuedRequestsCounterEnabled, + bool currentUpgradedRequestsCounterEnabled, bool currentTlsHandshakesCounterEnabled) + { + ConnectionContext = connectionContext; + CurrentConnectionsCounterEnabled = currentConnectionsCounterEnabled; + ConnectionDurationEnabled = connectionDurationEnabled; + QueuedConnectionsCounterEnabled = queuedConnectionsCounterEnabled; + QueuedRequestsCounterEnabled = queuedRequestsCounterEnabled; + CurrentUpgradedRequestsCounterEnabled = currentUpgradedRequestsCounterEnabled; + CurrentTlsHandshakesCounterEnabled = currentTlsHandshakesCounterEnabled; + } +} + internal sealed class KestrelConnection : KestrelConnection, IThreadPoolWorkItem where T : BaseConnectionContext { private readonly Func _connectionDelegate; @@ -18,16 +47,19 @@ public KestrelConnection(long id, TransportConnectionManager transportConnectionManager, Func connectionDelegate, T connectionContext, - KestrelTrace logger) - : base(id, serviceContext, transportConnectionManager, logger) + KestrelTrace logger, + ConnectionMetricsContext connectionMetricsContext) + : base(id, serviceContext, transportConnectionManager, logger, connectionMetricsContext) { _connectionDelegate = connectionDelegate; _transportConnection = connectionContext; connectionContext.Features.Set(this); connectionContext.Features.Set(this); connectionContext.Features.Set(this); + connectionContext.Features.Set(this); } + private KestrelMetrics Metrics => _serviceContext.Metrics; public override BaseConnectionContext TransportConnection => _transportConnection; void IThreadPoolWorkItem.Execute() @@ -38,12 +70,11 @@ void IThreadPoolWorkItem.Execute() internal async Task ExecuteAsync() { var connectionContext = _transportConnection; - var metricsConnectionDurationEnabled = _serviceContext.Metrics.IsConnectionDurationEnabled(); var startTimestamp = 0L; ConnectionMetricsTagsFeature? metricsTagsFeature = null; Exception? unhandledException = null; - if (metricsConnectionDurationEnabled) + if (MetricsContext.ConnectionDurationEnabled) { metricsTagsFeature = new ConnectionMetricsTagsFeature(); connectionContext.Features.Set(metricsTagsFeature); @@ -54,11 +85,11 @@ internal async Task ExecuteAsync() try { KestrelEventSource.Log.ConnectionQueuedStop(connectionContext); - _serviceContext.Metrics.ConnectionQueuedStop(connectionContext); + Metrics.ConnectionQueuedStop(MetricsContext); Logger.ConnectionStart(connectionContext.ConnectionId); KestrelEventSource.Log.ConnectionStart(connectionContext); - _serviceContext.Metrics.ConnectionStart(connectionContext); + Metrics.ConnectionStart(MetricsContext); using (BeginConnectionScope(connectionContext)) { @@ -78,14 +109,14 @@ internal async Task ExecuteAsync() await FireOnCompletedAsync(); var currentTimestamp = 0L; - if (metricsConnectionDurationEnabled) + if (MetricsContext.ConnectionDurationEnabled) { currentTimestamp = Stopwatch.GetTimestamp(); } Logger.ConnectionStop(connectionContext.ConnectionId); KestrelEventSource.Log.ConnectionStop(connectionContext); - _serviceContext.Metrics.ConnectionStop(connectionContext, unhandledException, metricsTagsFeature?.TagsList, startTimestamp, currentTimestamp); + Metrics.ConnectionStop(MetricsContext, unhandledException, metricsTagsFeature?.TagsList, startTimestamp, currentTimestamp); // Dispose the transport connection, this needs to happen before removing it from the // connection manager so that we only signal completion of this connection after the transport diff --git a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/KestrelMetrics.cs b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/KestrelMetrics.cs index 9bd1ab2a5969..be8eb1a72ebd 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/KestrelMetrics.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/KestrelMetrics.cs @@ -1,7 +1,6 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using Microsoft.AspNetCore.Connections; using Microsoft.Extensions.Metrics; using System.Diagnostics; using System.Diagnostics.Metrics; @@ -25,6 +24,13 @@ internal sealed class KestrelMetrics private readonly Histogram _tlsHandshakeDuration; private readonly UpDownCounter _currentTlsHandshakesCounter; + public bool CurrentConnectionsCounterEnabled => _currentConnectionsCounter.Enabled; + public bool ConnectionDurationEnabled => _connectionDuration.Enabled; + public bool QueuedConnectionsCounterEnabled => _queuedConnectionsCounter.Enabled; + public bool QueuedRequestsCounterEnabled => _queuedRequestsCounter.Enabled; + public bool CurrentUpgradedRequestsCounterEnabled => _currentUpgradedRequestsCounter.Enabled; + public bool CurrentTlsHandshakesCounterEnabled => _currentTlsHandshakesCounter.Enabled; + public KestrelMetrics(IMeterFactory meterFactory) { _meter = meterFactory.CreateMeter(MeterName); @@ -64,204 +70,213 @@ public KestrelMetrics(IMeterFactory meterFactory) description: "Number of TLS handshakes that are currently in progress on the server."); } - public void ConnectionStart(BaseConnectionContext connection) + public void ConnectionStart(in ConnectionMetricsContext metricsContext) { - if (_currentConnectionsCounter.Enabled) + if (metricsContext.CurrentConnectionsCounterEnabled) { - ConnectionStartCore(connection); + ConnectionStartCore(metricsContext); } } [MethodImpl(MethodImplOptions.NoInlining)] - private void ConnectionStartCore(BaseConnectionContext connection) + private void ConnectionStartCore(in ConnectionMetricsContext metricsContext) { var tags = new TagList(); - InitializeConnectionTags(ref tags, connection); + InitializeConnectionTags(ref tags, metricsContext); _currentConnectionsCounter.Add(1, tags); } - public void ConnectionStop(BaseConnectionContext connection, Exception? exception, List>? customTags, long startTimestamp, long currentTimestamp) + public void ConnectionStop(in ConnectionMetricsContext metricsContext, Exception? exception, List>? customTags, long startTimestamp, long currentTimestamp) { - if (_currentConnectionsCounter.Enabled || _connectionDuration.Enabled) + if (metricsContext.CurrentConnectionsCounterEnabled || metricsContext.ConnectionDurationEnabled) { - ConnectionStopCore(connection, exception, customTags, startTimestamp, currentTimestamp); + ConnectionStopCore(metricsContext, exception, customTags, startTimestamp, currentTimestamp); } } [MethodImpl(MethodImplOptions.NoInlining)] - private void ConnectionStopCore(BaseConnectionContext connection, Exception? exception, List>? customTags, long startTimestamp, long currentTimestamp) + private void ConnectionStopCore(in ConnectionMetricsContext metricsContext, Exception? exception, List>? customTags, long startTimestamp, long currentTimestamp) { var tags = new TagList(); - InitializeConnectionTags(ref tags, connection); - - // Decrease in connections counter must match tags from increase. No custom tags. - _currentConnectionsCounter.Add(-1, tags); + InitializeConnectionTags(ref tags, metricsContext); - if (exception != null) + if (metricsContext.CurrentConnectionsCounterEnabled) { - tags.Add("exception-name", exception.GetType().FullName); + // Decrease in connections counter must match tags from increase. No custom tags. + _currentConnectionsCounter.Add(-1, tags); } - // Add custom tags for duration. - if (customTags != null) + if (metricsContext.ConnectionDurationEnabled) { - for (var i = 0; i < customTags.Count; i++) + if (exception != null) { - tags.Add(customTags[i]); + tags.Add("exception-name", exception.GetType().FullName); } - } - var duration = Stopwatch.GetElapsedTime(startTimestamp, currentTimestamp); - _connectionDuration.Record(duration.TotalSeconds, tags); + // Add custom tags for duration. + if (customTags != null) + { + for (var i = 0; i < customTags.Count; i++) + { + tags.Add(customTags[i]); + } + } + + var duration = Stopwatch.GetElapsedTime(startTimestamp, currentTimestamp); + _connectionDuration.Record(duration.TotalSeconds, tags); + } } - public void ConnectionRejected(BaseConnectionContext connection) + public void ConnectionRejected(in ConnectionMetricsContext metricsContext) { if (_rejectedConnectionsCounter.Enabled) { - ConnectionRejectedCore(connection); + ConnectionRejectedCore(metricsContext); } } [MethodImpl(MethodImplOptions.NoInlining)] - private void ConnectionRejectedCore(BaseConnectionContext connection) + private void ConnectionRejectedCore(in ConnectionMetricsContext metricsContext) { var tags = new TagList(); - InitializeConnectionTags(ref tags, connection); + InitializeConnectionTags(ref tags, metricsContext); _rejectedConnectionsCounter.Add(1, tags); } - public void ConnectionQueuedStart(BaseConnectionContext connection) + public void ConnectionQueuedStart(in ConnectionMetricsContext metricsContext) { - if (_queuedConnectionsCounter.Enabled) + if (metricsContext.QueuedConnectionsCounterEnabled) { - ConnectionQueuedStartCore(connection); + ConnectionQueuedStartCore(metricsContext); } } [MethodImpl(MethodImplOptions.NoInlining)] - private void ConnectionQueuedStartCore(BaseConnectionContext connection) + private void ConnectionQueuedStartCore(in ConnectionMetricsContext metricsContext) { var tags = new TagList(); - InitializeConnectionTags(ref tags, connection); + InitializeConnectionTags(ref tags, metricsContext); _queuedConnectionsCounter.Add(1, tags); } - public void ConnectionQueuedStop(BaseConnectionContext connection) + public void ConnectionQueuedStop(in ConnectionMetricsContext metricsContext) { - if (_queuedConnectionsCounter.Enabled) + if (metricsContext.QueuedConnectionsCounterEnabled) { - ConnectionQueuedStopCore(connection); + ConnectionQueuedStopCore(metricsContext); } } [MethodImpl(MethodImplOptions.NoInlining)] - private void ConnectionQueuedStopCore(BaseConnectionContext connection) + private void ConnectionQueuedStopCore(in ConnectionMetricsContext metricsContext) { var tags = new TagList(); - InitializeConnectionTags(ref tags, connection); + InitializeConnectionTags(ref tags, metricsContext); _queuedConnectionsCounter.Add(-1, tags); } - public void RequestQueuedStart(BaseConnectionContext connection, string httpVersion) + public void RequestQueuedStart(in ConnectionMetricsContext metricsContext, string httpVersion) { - if (_queuedRequestsCounter.Enabled) + if (metricsContext.QueuedRequestsCounterEnabled) { - RequestQueuedStartCore(connection, httpVersion); + RequestQueuedStartCore(metricsContext, httpVersion); } } [MethodImpl(MethodImplOptions.NoInlining)] - private void RequestQueuedStartCore(BaseConnectionContext connection, string httpVersion) + private void RequestQueuedStartCore(in ConnectionMetricsContext metricsContext, string httpVersion) { var tags = new TagList(); - InitializeConnectionTags(ref tags, connection); + InitializeConnectionTags(ref tags, metricsContext); tags.Add("version", httpVersion); _queuedRequestsCounter.Add(1, tags); } - public void RequestQueuedStop(BaseConnectionContext connection, string httpVersion) + public void RequestQueuedStop(in ConnectionMetricsContext metricsContext, string httpVersion) { - if (_queuedRequestsCounter.Enabled) + if (metricsContext.QueuedRequestsCounterEnabled) { - RequestQueuedStopCore(connection, httpVersion); + RequestQueuedStopCore(metricsContext, httpVersion); } } [MethodImpl(MethodImplOptions.NoInlining)] - private void RequestQueuedStopCore(BaseConnectionContext connection, string httpVersion) + private void RequestQueuedStopCore(in ConnectionMetricsContext metricsContext, string httpVersion) { var tags = new TagList(); - InitializeConnectionTags(ref tags, connection); + InitializeConnectionTags(ref tags, metricsContext); tags.Add("version", httpVersion); _queuedRequestsCounter.Add(-1, tags); } - public void RequestUpgradedStart(BaseConnectionContext connection) + public void RequestUpgradedStart(in ConnectionMetricsContext metricsContext) { - if (_currentUpgradedRequestsCounter.Enabled) + if (metricsContext.CurrentUpgradedRequestsCounterEnabled) { - RequestUpgradedStartCore(connection); + RequestUpgradedStartCore(metricsContext); } } [MethodImpl(MethodImplOptions.NoInlining)] - private void RequestUpgradedStartCore(BaseConnectionContext connection) + private void RequestUpgradedStartCore(in ConnectionMetricsContext metricsContext) { var tags = new TagList(); - InitializeConnectionTags(ref tags, connection); + InitializeConnectionTags(ref tags, metricsContext); _currentUpgradedRequestsCounter.Add(1, tags); } - public void RequestUpgradedStop(BaseConnectionContext connection) + public void RequestUpgradedStop(in ConnectionMetricsContext metricsContext) { - if (_currentUpgradedRequestsCounter.Enabled) + if (metricsContext.CurrentUpgradedRequestsCounterEnabled) { - RequestUpgradedStopCore(connection); + RequestUpgradedStopCore(metricsContext); } } [MethodImpl(MethodImplOptions.NoInlining)] - private void RequestUpgradedStopCore(BaseConnectionContext connection) + private void RequestUpgradedStopCore(in ConnectionMetricsContext metricsContext) { var tags = new TagList(); - InitializeConnectionTags(ref tags, connection); + InitializeConnectionTags(ref tags, metricsContext); _currentUpgradedRequestsCounter.Add(-1, tags); } - public void TlsHandshakeStart(BaseConnectionContext connection) + public void TlsHandshakeStart(in ConnectionMetricsContext metricsContext) { - if (_currentTlsHandshakesCounter.Enabled) + if (metricsContext.CurrentTlsHandshakesCounterEnabled) { - TlsHandshakeStartCore(connection); + TlsHandshakeStartCore(metricsContext); } } [MethodImpl(MethodImplOptions.NoInlining)] - private void TlsHandshakeStartCore(BaseConnectionContext connection) + private void TlsHandshakeStartCore(in ConnectionMetricsContext metricsContext) { // Tags must match TLS handshake end. var tags = new TagList(); - InitializeConnectionTags(ref tags, connection); + InitializeConnectionTags(ref tags, metricsContext); _currentTlsHandshakesCounter.Add(1, tags); } - public void TlsHandshakeStop(BaseConnectionContext connection, long startTimestamp, long currentTimestamp, SslProtocols? protocol = null, Exception? exception = null) + public void TlsHandshakeStop(in ConnectionMetricsContext metricsContext, long startTimestamp, long currentTimestamp, SslProtocols? protocol = null, Exception? exception = null) { - if (_currentTlsHandshakesCounter.Enabled || _tlsHandshakeDuration.Enabled) + if (metricsContext.CurrentTlsHandshakesCounterEnabled || _tlsHandshakeDuration.Enabled) { - TlsHandshakeStopCore(connection, startTimestamp, currentTimestamp, protocol, exception); + TlsHandshakeStopCore(metricsContext, startTimestamp, currentTimestamp, protocol, exception); } } [MethodImpl(MethodImplOptions.NoInlining)] - private void TlsHandshakeStopCore(BaseConnectionContext connection, long startTimestamp, long currentTimestamp, SslProtocols? protocol = null, Exception? exception = null) + private void TlsHandshakeStopCore(in ConnectionMetricsContext metricsContext, long startTimestamp, long currentTimestamp, SslProtocols? protocol = null, Exception? exception = null) { var tags = new TagList(); - InitializeConnectionTags(ref tags, connection); + InitializeConnectionTags(ref tags, metricsContext); - // Tags must match TLS handshake start. - _currentTlsHandshakesCounter.Add(-1, tags); + if (metricsContext.CurrentTlsHandshakesCounterEnabled) + { + // Tags must match TLS handshake start. + _currentTlsHandshakesCounter.Add(-1, tags); + } if (protocol != null) { @@ -276,9 +291,9 @@ private void TlsHandshakeStopCore(BaseConnectionContext connection, long startTi _tlsHandshakeDuration.Record(duration.TotalSeconds, tags); } - private static void InitializeConnectionTags(ref TagList tags, BaseConnectionContext connection) + private static void InitializeConnectionTags(ref TagList tags, in ConnectionMetricsContext metricsContext) { - if (connection.LocalEndPoint is { } localEndpoint) + if (metricsContext.ConnectionContext.LocalEndPoint is { } localEndpoint) { // TODO: Improve getting string allocation for endpoint. Currently allocates. // Possible solution is to cache in the endpoint: https://github.com/dotnet/runtime/issues/84515 @@ -286,6 +301,4 @@ private static void InitializeConnectionTags(ref TagList tags, BaseConnectionCon tags.Add("endpoint", localEndpoint.ToString()); } } - - public bool IsConnectionDurationEnabled() => _connectionDuration.Enabled; } diff --git a/src/Servers/Kestrel/Core/src/Middleware/ConnectionLimitMiddleware.cs b/src/Servers/Kestrel/Core/src/Middleware/ConnectionLimitMiddleware.cs index 674eb8a10bb0..84e3c41a65e0 100644 --- a/src/Servers/Kestrel/Core/src/Middleware/ConnectionLimitMiddleware.cs +++ b/src/Servers/Kestrel/Core/src/Middleware/ConnectionLimitMiddleware.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using Microsoft.AspNetCore.Connections; +using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Server.Kestrel.Core.Features; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; @@ -34,7 +35,7 @@ public async Task OnConnectionAsync(T connection) { KestrelEventSource.Log.ConnectionRejected(connection.ConnectionId); _trace.ConnectionRejected(connection.ConnectionId); - _metrics.ConnectionRejected(connection); + _metrics.ConnectionRejected(connection.Features.GetRequiredFeature().MetricsContext); await connection.DisposeAsync(); return; } diff --git a/src/Servers/Kestrel/Core/src/Middleware/HttpConnectionMiddleware.cs b/src/Servers/Kestrel/Core/src/Middleware/HttpConnectionMiddleware.cs index 877027e9ce0d..1f3903e549a8 100644 --- a/src/Servers/Kestrel/Core/src/Middleware/HttpConnectionMiddleware.cs +++ b/src/Servers/Kestrel/Core/src/Middleware/HttpConnectionMiddleware.cs @@ -5,6 +5,7 @@ using Microsoft.AspNetCore.Connections; using Microsoft.AspNetCore.Connections.Features; using Microsoft.AspNetCore.Hosting.Server; +using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal; @@ -28,6 +29,7 @@ public Task OnConnectionAsync(ConnectionContext connectionContext) { var memoryPoolFeature = connectionContext.Features.Get(); var protocols = connectionContext.Features.Get()?.HttpProtocols ?? _endpointDefaultProtocols; + var metricContext = connectionContext.Features.GetRequiredFeature().MetricsContext; var localEndPoint = connectionContext.LocalEndPoint as IPEndPoint; var altSvcHeader = _addAltSvcHeader && localEndPoint != null ? HttpUtilities.GetEndpointAltSvc(localEndPoint, protocols) : null; @@ -40,7 +42,8 @@ public Task OnConnectionAsync(ConnectionContext connectionContext) connectionContext.Features, memoryPoolFeature?.MemoryPool ?? System.Buffers.MemoryPool.Shared, localEndPoint, - connectionContext.RemoteEndPoint as IPEndPoint); + connectionContext.RemoteEndPoint as IPEndPoint, + metricContext); httpConnectionContext.Transport = connectionContext.Transport; var connection = new HttpConnection(httpConnectionContext); diff --git a/src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs b/src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs index 3488024f2294..d6fb771e5f08 100644 --- a/src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs +++ b/src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs @@ -152,6 +152,7 @@ public async Task OnConnectionAsync(ConnectionContext context) context.Features.Set(feature); context.Features.Set(sslStream); // Anti-pattern, but retain for back compat + var metricsContext = context.Features.GetRequiredFeature().MetricsContext; var startTimestamp = Stopwatch.GetTimestamp(); try { @@ -164,13 +165,13 @@ public async Task OnConnectionAsync(ConnectionContext context) } else { - var state = (this, context, feature); + var state = (this, context, feature, metricsContext); await sslStream.AuthenticateAsServerAsync(ServerOptionsCallback, state, cancellationTokenSource.Token); } } catch (OperationCanceledException ex) { - RecordHandshakeFailed(_metrics, startTimestamp, Stopwatch.GetTimestamp(), context, ex); + RecordHandshakeFailed(_metrics, startTimestamp, Stopwatch.GetTimestamp(), metricsContext, ex); _logger.AuthenticationTimedOut(); await sslStream.DisposeAsync(); @@ -178,7 +179,7 @@ public async Task OnConnectionAsync(ConnectionContext context) } catch (IOException ex) { - RecordHandshakeFailed(_metrics, startTimestamp, Stopwatch.GetTimestamp(), context, ex); + RecordHandshakeFailed(_metrics, startTimestamp, Stopwatch.GetTimestamp(), metricsContext, ex); _logger.AuthenticationFailed(ex); await sslStream.DisposeAsync(); @@ -186,7 +187,7 @@ public async Task OnConnectionAsync(ConnectionContext context) } catch (AuthenticationException ex) { - RecordHandshakeFailed(_metrics, startTimestamp, Stopwatch.GetTimestamp(), context, ex); + RecordHandshakeFailed(_metrics, startTimestamp, Stopwatch.GetTimestamp(), metricsContext, ex); _logger.AuthenticationFailed(ex); @@ -195,7 +196,7 @@ public async Task OnConnectionAsync(ConnectionContext context) } KestrelEventSource.Log.TlsHandshakeStop(context, feature); - _metrics.TlsHandshakeStop(context, startTimestamp, Stopwatch.GetTimestamp(), protocol: sslStream.SslProtocol); + _metrics.TlsHandshakeStop(metricsContext, startTimestamp, Stopwatch.GetTimestamp(), protocol: sslStream.SslProtocol); _logger.HttpsConnectionEstablished(context.ConnectionId, sslStream.SslProtocol); @@ -220,11 +221,11 @@ public async Task OnConnectionAsync(ConnectionContext context) context.Transport = originalTransport; } - static void RecordHandshakeFailed(KestrelMetrics metrics, long startTimestamp, long currentTimestamp, ConnectionContext context, Exception ex) + static void RecordHandshakeFailed(KestrelMetrics metrics, long startTimestamp, long currentTimestamp, ConnectionMetricsContext metricsContext, Exception ex) { - KestrelEventSource.Log.TlsHandshakeFailed(context.ConnectionId); - KestrelEventSource.Log.TlsHandshakeStop(context, null); - metrics.TlsHandshakeStop(context, startTimestamp, currentTimestamp, exception: ex); + KestrelEventSource.Log.TlsHandshakeFailed(metricsContext.ConnectionContext.ConnectionId); + KestrelEventSource.Log.TlsHandshakeStop(metricsContext.ConnectionContext, null); + metrics.TlsHandshakeStop(metricsContext, startTimestamp, currentTimestamp, exception: ex); } } @@ -336,7 +337,7 @@ private Task DoOptionsBasedHandshakeAsync(ConnectionContext context, SslStream s _options.OnAuthenticate?.Invoke(context, sslOptions); KestrelEventSource.Log.TlsHandshakeStart(context, sslOptions); - _metrics.TlsHandshakeStart(context); + _metrics.TlsHandshakeStart(context.Features.GetRequiredFeature().MetricsContext); return sslStream.AuthenticateAsServerAsync(sslOptions, cancellationToken); } @@ -425,7 +426,7 @@ private SslDuplexPipe CreateSslDuplexPipe(IDuplexPipe transport, MemoryPool ServerOptionsCallback(SslStream sslStream, SslClientHelloInfo clientHelloInfo, object? state, CancellationToken cancellationToken) { - var (middleware, context, feature) = (ValueTuple)state!; + var (middleware, context, feature, metricsContext) = (ValueTuple)state!; feature.HostName = clientHelloInfo.ServerName; context.Features.Set(sslStream); @@ -447,7 +448,7 @@ private static async ValueTask ServerOptionsCall } KestrelEventSource.Log.TlsHandshakeStart(context, sslOptions); - middleware._metrics.TlsHandshakeStart(context); + middleware._metrics.TlsHandshakeStart(metricsContext); return sslOptions; } diff --git a/src/Servers/Kestrel/Core/test/ConnectionDispatcherTests.cs b/src/Servers/Kestrel/Core/test/ConnectionDispatcherTests.cs index 7c52d490efed..0062cd0fbf4c 100644 --- a/src/Servers/Kestrel/Core/test/ConnectionDispatcherTests.cs +++ b/src/Servers/Kestrel/Core/test/ConnectionDispatcherTests.cs @@ -33,7 +33,7 @@ public async Task OnConnectionCreatesLogScopeWithConnectionId() var connection = new Mock { CallBase = true }.Object; connection.ConnectionClosed = new CancellationToken(canceled: true); var transportConnectionManager = new TransportConnectionManager(serviceContext.ConnectionManager); - var kestrelConnection = new KestrelConnection(0, serviceContext, transportConnectionManager, _ => tcs.Task, connection, serviceContext.Log); + var kestrelConnection = CreateKestrelConnection(serviceContext, connection, transportConnectionManager, connectionDelegate: _ => tcs.Task); transportConnectionManager.AddConnection(0, kestrelConnection); var task = kestrelConnection.ExecuteAsync(); @@ -77,7 +77,7 @@ public async Task OnConnectionFiresOnCompleted() var connection = new Mock { CallBase = true }.Object; connection.ConnectionClosed = new CancellationToken(canceled: true); var transportConnectionManager = new TransportConnectionManager(serviceContext.ConnectionManager); - var kestrelConnection = new KestrelConnection(0, serviceContext, transportConnectionManager, _ => Task.CompletedTask, connection, serviceContext.Log); + var kestrelConnection = CreateKestrelConnection(serviceContext, connection, transportConnectionManager); transportConnectionManager.AddConnection(0, kestrelConnection); var completeFeature = kestrelConnection.TransportConnection.Features.Get(); @@ -98,7 +98,7 @@ public async Task OnConnectionOnCompletedExceptionCaught() var connection = new Mock { CallBase = true }.Object; connection.ConnectionClosed = new CancellationToken(canceled: true); var transportConnectionManager = new TransportConnectionManager(serviceContext.ConnectionManager); - var kestrelConnection = new KestrelConnection(0, serviceContext, transportConnectionManager, _ => Task.CompletedTask, connection, serviceContext.Log); + var kestrelConnection = CreateKestrelConnection(serviceContext, connection, transportConnectionManager); transportConnectionManager.AddConnection(0, kestrelConnection); var completeFeature = kestrelConnection.TransportConnection.Features.Get(); @@ -115,6 +115,14 @@ public async Task OnConnectionOnCompletedExceptionCaught() Assert.Equal("An error occurred running an IConnectionCompleteFeature.OnCompleted callback.", errors[0].Message); } + private static KestrelConnection CreateKestrelConnection(TestServiceContext serviceContext, DefaultConnectionContext connection, TransportConnectionManager transportConnectionManager, Func connectionDelegate = null) + { + connectionDelegate ??= _ => Task.CompletedTask; + + return new KestrelConnection( + id: 0, serviceContext, transportConnectionManager, connectionDelegate, connection, serviceContext.Log, TestContextFactory.CreateMetricsContext(connection)); + } + private class ThrowingListener : IConnectionListener { public EndPoint EndPoint { get; set; } diff --git a/src/Servers/Kestrel/Core/test/Http3/Http3HttpProtocolFeatureCollectionTests.cs b/src/Servers/Kestrel/Core/test/Http3/Http3HttpProtocolFeatureCollectionTests.cs index ba9d460474ef..1b722dbaaf1a 100644 --- a/src/Servers/Kestrel/Core/test/Http3/Http3HttpProtocolFeatureCollectionTests.cs +++ b/src/Servers/Kestrel/Core/test/Http3/Http3HttpProtocolFeatureCollectionTests.cs @@ -7,6 +7,7 @@ using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Server.Kestrel.Core.Features; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http3; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; using Microsoft.AspNetCore.Testing; namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests; @@ -67,7 +68,7 @@ public override void Execute() } } - private class TestConnectionFeatures : IProtocolErrorCodeFeature, IStreamIdFeature, IStreamAbortFeature, IStreamClosedFeature + private class TestConnectionFeatures : IProtocolErrorCodeFeature, IStreamIdFeature, IStreamAbortFeature, IStreamClosedFeature, IConnectionMetricsContextFeature { public TestConnectionFeatures() { @@ -76,11 +77,13 @@ public TestConnectionFeatures() featureCollection.Set(this); featureCollection.Set(this); featureCollection.Set(this); + featureCollection.Set(this); FeatureCollection = featureCollection; } public IFeatureCollection FeatureCollection { get; } + public ConnectionMetricsContext MetricsContext { get; } long IProtocolErrorCodeFeature.Error { get; set; } long IStreamIdFeature.StreamId { get; } diff --git a/src/Servers/Kestrel/Core/test/HttpConnectionManagerTests.cs b/src/Servers/Kestrel/Core/test/HttpConnectionManagerTests.cs index b1e1bfbfa710..11668bc40334 100644 --- a/src/Servers/Kestrel/Core/test/HttpConnectionManagerTests.cs +++ b/src/Servers/Kestrel/Core/test/HttpConnectionManagerTests.cs @@ -46,7 +46,7 @@ private void UnrootedConnectionsGetRemovedFromHeartbeatInnerScope( var mock = new Mock() { CallBase = true }; mock.Setup(m => m.ConnectionId).Returns(connectionId); var transportConnectionManager = new TransportConnectionManager(httpConnectionManager); - var httpConnection = new KestrelConnection(0, serviceContext, transportConnectionManager, _ => Task.CompletedTask, mock.Object, trace); + var httpConnection = new KestrelConnection(0, serviceContext, transportConnectionManager, _ => Task.CompletedTask, mock.Object, trace, TestContextFactory.CreateMetricsContext(mock.Object)); transportConnectionManager.AddConnection(0, httpConnection); var connectionCount = 0; diff --git a/src/Servers/Kestrel/Core/test/PooledStreamStackTests.cs b/src/Servers/Kestrel/Core/test/PooledStreamStackTests.cs index 8616f35f19d6..087c4c8c9b88 100644 --- a/src/Servers/Kestrel/Core/test/PooledStreamStackTests.cs +++ b/src/Servers/Kestrel/Core/test/PooledStreamStackTests.cs @@ -96,24 +96,7 @@ public void RemoveExpired_AllExpired_ExpiredStreamRemoved() private static Http2Stream CreateStream(int streamId, long expirationTicks) { - var context = new Http2StreamContext - ( - connectionId: "TestConnectionId", - protocols: HttpProtocols.Http2, - altSvcHeader: null, - connectionContext: new DefaultConnectionContext(), - serviceContext: TestContextFactory.CreateServiceContext(serverOptions: new KestrelServerOptions()), - connectionFeatures: new FeatureCollection(), - memoryPool: MemoryPool.Shared, - localEndPoint: null, - remoteEndPoint: null, - streamId: streamId, - streamLifetimeHandler: null!, - clientPeerSettings: new Http2PeerSettings(), - serverPeerSettings: new Http2PeerSettings(), - frameWriter: null!, - connectionInputFlowControl: null! - ); + var context = TestContextFactory.CreateHttp2StreamContext(connectionId: "TestConnectionId", streamId: streamId); return new Http2Stream(new DummyApplication(), context) { diff --git a/src/Servers/Kestrel/shared/test/Http3/Http3InMemory.cs b/src/Servers/Kestrel/shared/test/Http3/Http3InMemory.cs index f4ecc6a6a598..fb688bcd5a51 100644 --- a/src/Servers/Kestrel/shared/test/Http3/Http3InMemory.cs +++ b/src/Servers/Kestrel/shared/test/Http3/Http3InMemory.cs @@ -982,7 +982,7 @@ internal void VerifyGoAway(Http3FrameWithPayload frame, long expectedLastStreamI } } -internal class TestMultiplexedConnectionContext : MultiplexedConnectionContext, IConnectionLifetimeNotificationFeature, IConnectionLifetimeFeature, IConnectionHeartbeatFeature, IProtocolErrorCodeFeature +internal class TestMultiplexedConnectionContext : MultiplexedConnectionContext, IConnectionLifetimeNotificationFeature, IConnectionLifetimeFeature, IConnectionHeartbeatFeature, IProtocolErrorCodeFeature, IConnectionMetricsContextFeature { public readonly Channel ToServerAcceptQueue = Channel.CreateUnbounded(new UnboundedChannelOptions { @@ -1006,6 +1006,7 @@ public TestMultiplexedConnectionContext(Http3InMemory testBase) Features.Set(this); Features.Set(this); Features.Set(this); + Features.Set(this); ConnectionClosedRequested = ConnectionClosingCts.Token; } @@ -1024,6 +1025,7 @@ public long Error get => _error ?? -1; set => _error = value; } + public ConnectionMetricsContext MetricsContext { get; } public override void Abort() { diff --git a/src/Servers/Kestrel/shared/test/TestContextFactory.cs b/src/Servers/Kestrel/shared/test/TestContextFactory.cs index 6a9caffff83a..e72deb676129 100644 --- a/src/Servers/Kestrel/shared/test/TestContextFactory.cs +++ b/src/Servers/Kestrel/shared/test/TestContextFactory.cs @@ -69,7 +69,8 @@ public static HttpConnectionContext CreateHttpConnectionContext( connectionFeatures, memoryPool ?? MemoryPool.Shared, localEndPoint, - remoteEndPoint); + remoteEndPoint, + CreateMetricsContext(connectionContext)); context.TimeoutControl = timeoutControl; context.Transport = transport; @@ -147,12 +148,13 @@ public static Http2StreamContext CreateHttp2StreamContext( InputFlowControl connectionInputFlowControl = null, ITimeoutControl timeoutControl = null) { + var connectionContext = new DefaultConnectionContext(); var context = new Http2StreamContext ( connectionId: connectionId ?? "TestConnectionId", protocols: HttpProtocols.Http2, altSvcHeader: null, - connectionContext: new DefaultConnectionContext(), + connectionContext: connectionContext, serviceContext: serviceContext ?? CreateServiceContext(new KestrelServerOptions()), connectionFeatures: connectionFeatures ?? new FeatureCollection(), memoryPool: memoryPool ?? MemoryPool.Shared, @@ -163,7 +165,8 @@ public static Http2StreamContext CreateHttp2StreamContext( clientPeerSettings: clientPeerSettings ?? new Http2PeerSettings(), serverPeerSettings: serverPeerSettings ?? new Http2PeerSettings(), frameWriter: frameWriter, - connectionInputFlowControl: connectionInputFlowControl + connectionInputFlowControl: connectionInputFlowControl, + metricsContext: CreateMetricsContext(connectionContext) ); context.TimeoutControl = timeoutControl; @@ -183,7 +186,7 @@ public static Http3StreamContext CreateHttp3StreamContext( IHttp3StreamLifetimeHandler streamLifetimeHandler = null) { var http3ConnectionContext = CreateHttp3ConnectionContext( - null, + new TestMultiplexedConnectionContext(), serviceContext, connectionFeatures, memoryPool, @@ -216,6 +219,13 @@ public static Http3StreamContext CreateHttp3StreamContext( }; } + public static ConnectionMetricsContext CreateMetricsContext(ConnectionContext connectionContext) + { + return new ConnectionMetricsContext(connectionContext, + currentConnectionsCounterEnabled: false, connectionDurationEnabled: false, queuedConnectionsCounterEnabled: false, + queuedRequestsCounterEnabled: false, currentUpgradedRequestsCounterEnabled: false, currentTlsHandshakesCounterEnabled: false); + } + private class TestHttp2StreamLifetimeHandler : IHttp2StreamLifetimeHandler { public void DecrementActiveClientStreamCount() diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs index 3e980aa0ba1e..77239526401a 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs @@ -455,6 +455,7 @@ protected void CreateConnection() _pair = DuplexPipe.CreateConnectionPair(inputPipeOptions, outputPipeOptions); var features = new FeatureCollection(); + features.Set(new TestConnectionMetricsContextFeature()); _mockConnectionContext.Setup(x => x.Features).Returns(features); var httpConnectionContext = TestContextFactory.CreateHttpConnectionContext( serviceContext: _serviceContext, @@ -475,6 +476,11 @@ protected void CreateConnection() _timeoutControl.Initialize(_serviceContext.SystemClock.UtcNow.Ticks); } + private class TestConnectionMetricsContextFeature : IConnectionMetricsContextFeature + { + public ConnectionMetricsContext MetricsContext { get; } + } + private class LifetimeHandlerInterceptor : IHttp2StreamLifetimeHandler { private readonly IHttp2StreamLifetimeHandler _inner; @@ -570,7 +576,8 @@ protected void AddKestrelConnection() new TransportConnectionManager(_serviceContext.ConnectionManager), _ => throw new NotImplementedException($"{nameof(_connection.ProcessRequestsAsync)} should invoked instead - hence transport connection manager does not have the connection registered."), _mockConnectionContext.Object, - new KestrelTrace(_serviceContext.LoggerFactory)); + new KestrelTrace(_serviceContext.LoggerFactory), + TestContextFactory.CreateMetricsContext(_mockConnectionContext.Object)); } protected Task StartStreamAsync(int streamId, IEnumerable> headers, bool endStream, bool flushFrame = true) diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/WebTransport/WebTransportTestUtilities.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/WebTransport/WebTransportTestUtilities.cs index 9f4695710b92..96538fea57ee 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/WebTransport/WebTransportTestUtilities.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/WebTransport/WebTransportTestUtilities.cs @@ -8,6 +8,7 @@ using Microsoft.AspNetCore.Server.Kestrel.Core.Features; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http3; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.WebTransport; using Microsoft.AspNetCore.Server.Kestrel.Core.WebTransport; using Microsoft.AspNetCore.Testing; @@ -78,6 +79,7 @@ public static WebTransportStream CreateStream(WebTransportStreamType type, Memor features.Set(new DefaultStreamDirectionFeature(type != WebTransportStreamType.Output, type != WebTransportStreamType.Input)); features.Set(Mock.Of()); features.Set(Mock.Of()); + features.Set(Mock.Of()); var writer = new HttpResponsePipeWriter(new StreamWriterControl(memory)); writer.StartAcceptingWrites(); diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/KestrelMetricsTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/KestrelMetricsTests.cs index b2b29966c1a9..c0e8349a3bbb 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/KestrelMetricsTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/KestrelMetricsTests.cs @@ -88,6 +88,64 @@ await connection.ReceiveEnd( Assert.Collection(queuedConnections.GetMeasurements(), m => AssertCount(m, 1, "127.0.0.1:0"), m => AssertCount(m, -1, "127.0.0.1:0")); } + [Fact] + public async Task Http1Connection_BeginListeningAfterConnectionStarted() + { + var sync = new SyncPoint(); + bool? hasConnectionMetricsTagsFeature = null; + + var listenOptions = new ListenOptions(new IPEndPoint(IPAddress.Loopback, 0)); + listenOptions.Use(next => + { + return async connectionContext => + { + hasConnectionMetricsTagsFeature = connectionContext.Features.Get() != null; + + // Wait for the test to verify the connection has started. + await sync.WaitToContinue(); + + await next(connectionContext); + }; + }); + + var testMeterFactory = new TestMeterFactory(); + var serviceContext = new TestServiceContext(LoggerFactory, metrics: new KestrelMetrics(testMeterFactory)); + + var sendString = "POST / HTTP/1.0\r\nContent-Length: 12\r\n\r\nHello World?"; + + await using var server = new TestServer(EchoApp, serviceContext, listenOptions); + + using (var connection = server.CreateConnection()) + { + await connection.Send(sendString); + + // Wait for connection to start on the server. + await sync.WaitForSyncPoint(); + + using var connectionDuration = new InstrumentRecorder(new TestMeterRegistry(testMeterFactory.Meters), "Microsoft.AspNetCore.Server.Kestrel", "connection-duration"); + using var currentConnections = new InstrumentRecorder(new TestMeterRegistry(testMeterFactory.Meters), "Microsoft.AspNetCore.Server.Kestrel", "current-connections"); + using var queuedConnections = new InstrumentRecorder(new TestMeterRegistry(testMeterFactory.Meters), "Microsoft.AspNetCore.Server.Kestrel", "queued-connections"); + + // Signal that connection can continue. + sync.Continue(); + + await connection.ReceiveEnd( + "HTTP/1.1 200 OK", + "Connection: close", + $"Date: {serviceContext.DateHeaderValue}", + "", + "Hello World?"); + + await connection.WaitForConnectionClose(); + + Assert.Empty(connectionDuration.GetMeasurements()); + Assert.Empty(currentConnections.GetMeasurements()); + Assert.Empty(queuedConnections.GetMeasurements()); + + Assert.False(hasConnectionMetricsTagsFeature); + } + } + [Fact] public async Task Http1Connection_IHttpConnectionTagsFeatureIgnoreFeatureSetOnTransport() { From 7746eee9349c6950a4de1008eff6c64f9a479536 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Wed, 3 May 2023 20:50:24 +0800 Subject: [PATCH 3/9] Clean up --- .../ConnectionMetricsContext.cs | 30 +++++++++++++++++++ .../IConnectionMetricsContextFeature.cs | 9 ++++++ .../Infrastructure/KestrelConnectionOfT.cs | 29 ------------------ 3 files changed, 39 insertions(+), 29 deletions(-) create mode 100644 src/Servers/Kestrel/Core/src/Internal/Infrastructure/ConnectionMetricsContext.cs create mode 100644 src/Servers/Kestrel/Core/src/Internal/Infrastructure/IConnectionMetricsContextFeature.cs diff --git a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/ConnectionMetricsContext.cs b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/ConnectionMetricsContext.cs new file mode 100644 index 000000000000..f561bda2437e --- /dev/null +++ b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/ConnectionMetricsContext.cs @@ -0,0 +1,30 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.AspNetCore.Connections; + +namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; + +internal readonly struct ConnectionMetricsContext +{ + public BaseConnectionContext ConnectionContext { get; } + public bool CurrentConnectionsCounterEnabled { get; } + public bool ConnectionDurationEnabled { get; } + public bool QueuedConnectionsCounterEnabled { get; } + public bool QueuedRequestsCounterEnabled { get; } + public bool CurrentUpgradedRequestsCounterEnabled { get; } + public bool CurrentTlsHandshakesCounterEnabled { get; } + + public ConnectionMetricsContext(BaseConnectionContext connectionContext, bool currentConnectionsCounterEnabled, + bool connectionDurationEnabled, bool queuedConnectionsCounterEnabled, bool queuedRequestsCounterEnabled, + bool currentUpgradedRequestsCounterEnabled, bool currentTlsHandshakesCounterEnabled) + { + ConnectionContext = connectionContext; + CurrentConnectionsCounterEnabled = currentConnectionsCounterEnabled; + ConnectionDurationEnabled = connectionDurationEnabled; + QueuedConnectionsCounterEnabled = queuedConnectionsCounterEnabled; + QueuedRequestsCounterEnabled = queuedRequestsCounterEnabled; + CurrentUpgradedRequestsCounterEnabled = currentUpgradedRequestsCounterEnabled; + CurrentTlsHandshakesCounterEnabled = currentTlsHandshakesCounterEnabled; + } +} diff --git a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/IConnectionMetricsContextFeature.cs b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/IConnectionMetricsContextFeature.cs new file mode 100644 index 000000000000..a1266549de3c --- /dev/null +++ b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/IConnectionMetricsContextFeature.cs @@ -0,0 +1,9 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; + +internal interface IConnectionMetricsContextFeature +{ + ConnectionMetricsContext MetricsContext { get; } +} diff --git a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/KestrelConnectionOfT.cs b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/KestrelConnectionOfT.cs index b605d716792a..700c879bae33 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/KestrelConnectionOfT.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/KestrelConnectionOfT.cs @@ -8,35 +8,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; -internal interface IConnectionMetricsContextFeature -{ - ConnectionMetricsContext MetricsContext { get; } -} - -internal readonly struct ConnectionMetricsContext -{ - public BaseConnectionContext ConnectionContext { get; } - public bool CurrentConnectionsCounterEnabled { get; } - public bool ConnectionDurationEnabled { get; } - public bool QueuedConnectionsCounterEnabled { get; } - public bool QueuedRequestsCounterEnabled { get; } - public bool CurrentUpgradedRequestsCounterEnabled { get; } - public bool CurrentTlsHandshakesCounterEnabled { get; } - - public ConnectionMetricsContext(BaseConnectionContext connectionContext, bool currentConnectionsCounterEnabled, - bool connectionDurationEnabled, bool queuedConnectionsCounterEnabled, bool queuedRequestsCounterEnabled, - bool currentUpgradedRequestsCounterEnabled, bool currentTlsHandshakesCounterEnabled) - { - ConnectionContext = connectionContext; - CurrentConnectionsCounterEnabled = currentConnectionsCounterEnabled; - ConnectionDurationEnabled = connectionDurationEnabled; - QueuedConnectionsCounterEnabled = queuedConnectionsCounterEnabled; - QueuedRequestsCounterEnabled = queuedRequestsCounterEnabled; - CurrentUpgradedRequestsCounterEnabled = currentUpgradedRequestsCounterEnabled; - CurrentTlsHandshakesCounterEnabled = currentTlsHandshakesCounterEnabled; - } -} - internal sealed class KestrelConnection : KestrelConnection, IThreadPoolWorkItem where T : BaseConnectionContext { private readonly Func _connectionDelegate; From 18b1870e2bdeb002af3fb133344831e99bad66aa Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Wed, 3 May 2023 20:52:17 +0800 Subject: [PATCH 4/9] Clean up --- src/Servers/Kestrel/Core/src/Internal/ConnectionDispatcher.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Servers/Kestrel/Core/src/Internal/ConnectionDispatcher.cs b/src/Servers/Kestrel/Core/src/Internal/ConnectionDispatcher.cs index cefa8fdeba30..90b3224e2b1c 100644 --- a/src/Servers/Kestrel/Core/src/Internal/ConnectionDispatcher.cs +++ b/src/Servers/Kestrel/Core/src/Internal/ConnectionDispatcher.cs @@ -51,9 +51,13 @@ async Task AcceptConnectionsAsync() // Add the connection to the connection manager before we queue it for execution var id = _transportConnectionManager.GetNewConnectionId(); + + // Cache counter enabled state at the start of the connection. + // This ensures that the state is consistent for the entire connection. var metricsContext = new ConnectionMetricsContext(connection, Metrics.CurrentConnectionsCounterEnabled, Metrics.ConnectionDurationEnabled, Metrics.QueuedConnectionsCounterEnabled, Metrics.QueuedRequestsCounterEnabled, Metrics.CurrentUpgradedRequestsCounterEnabled, Metrics.CurrentTlsHandshakesCounterEnabled); + var kestrelConnection = new KestrelConnection( id, _serviceContext, _transportConnectionManager, _connectionDelegate, connection, Log, metricsContext); From 3c438be6682110169e6e770787ed328de83db0d5 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Thu, 4 May 2023 12:25:15 +0800 Subject: [PATCH 5/9] Fix perf tests --- .../Http2/Http2ConnectionBenchmarkBase.cs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2ConnectionBenchmarkBase.cs b/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2ConnectionBenchmarkBase.cs index 010d7385417e..89d955a7db6b 100644 --- a/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2ConnectionBenchmarkBase.cs +++ b/src/Servers/Kestrel/perf/Microbenchmarks/Http2/Http2ConnectionBenchmarkBase.cs @@ -11,6 +11,7 @@ using Microsoft.AspNetCore.Server.Kestrel.Core; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; using Microsoft.AspNetCore.Testing; using Microsoft.Extensions.Primitives; using Microsoft.Net.Http.Headers; @@ -72,13 +73,15 @@ public virtual void GlobalSetup() systemClock: new MockSystemClock()); serviceContext.DateHeaderValueManager.OnHeartbeat(default); + var featureCollection = new FeatureCollection(); + featureCollection.Set(new TestConnectionMetricsContextFeature()); var connectionContext = TestContextFactory.CreateHttpConnectionContext( serviceContext: serviceContext, connectionContext: null, transport: _connectionPair.Transport, timeoutControl: new MockTimeoutControl(), memoryPool: _memoryPool, - connectionFeatures: new FeatureCollection()); + connectionFeatures: featureCollection); _connection = new Http2Connection(connectionContext); @@ -183,4 +186,9 @@ public async ValueTask DisposeAsync() await _requestProcessingTask; _memoryPool?.Dispose(); } + + private sealed class TestConnectionMetricsContextFeature : IConnectionMetricsContextFeature + { + public ConnectionMetricsContext MetricsContext { get; } + } } From 7bd1a958b7ad915e644a7da946fb21727147e3cb Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Wed, 10 May 2023 13:50:00 +0800 Subject: [PATCH 6/9] Refactor context creation --- .../Core/src/Internal/ConnectionDispatcher.cs | 4 +--- .../Internal/Infrastructure/KestrelMetrics.cs | 18 +++++++++--------- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/ConnectionDispatcher.cs b/src/Servers/Kestrel/Core/src/Internal/ConnectionDispatcher.cs index 90b3224e2b1c..aa8bb85ac35f 100644 --- a/src/Servers/Kestrel/Core/src/Internal/ConnectionDispatcher.cs +++ b/src/Servers/Kestrel/Core/src/Internal/ConnectionDispatcher.cs @@ -54,9 +54,7 @@ async Task AcceptConnectionsAsync() // Cache counter enabled state at the start of the connection. // This ensures that the state is consistent for the entire connection. - var metricsContext = new ConnectionMetricsContext(connection, - Metrics.CurrentConnectionsCounterEnabled, Metrics.ConnectionDurationEnabled, Metrics.QueuedConnectionsCounterEnabled, - Metrics.QueuedRequestsCounterEnabled, Metrics.CurrentUpgradedRequestsCounterEnabled, Metrics.CurrentTlsHandshakesCounterEnabled); + var metricsContext = Metrics.CreateContext(connection); var kestrelConnection = new KestrelConnection( id, _serviceContext, _transportConnectionManager, _connectionDelegate, connection, Log, metricsContext); diff --git a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/KestrelMetrics.cs b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/KestrelMetrics.cs index be8eb1a72ebd..6f0c62a7d299 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/KestrelMetrics.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/KestrelMetrics.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 Microsoft.AspNetCore.Connections; using Microsoft.Extensions.Metrics; using System.Diagnostics; using System.Diagnostics.Metrics; @@ -24,13 +25,6 @@ internal sealed class KestrelMetrics private readonly Histogram _tlsHandshakeDuration; private readonly UpDownCounter _currentTlsHandshakesCounter; - public bool CurrentConnectionsCounterEnabled => _currentConnectionsCounter.Enabled; - public bool ConnectionDurationEnabled => _connectionDuration.Enabled; - public bool QueuedConnectionsCounterEnabled => _queuedConnectionsCounter.Enabled; - public bool QueuedRequestsCounterEnabled => _queuedRequestsCounter.Enabled; - public bool CurrentUpgradedRequestsCounterEnabled => _currentUpgradedRequestsCounter.Enabled; - public bool CurrentTlsHandshakesCounterEnabled => _currentTlsHandshakesCounter.Enabled; - public KestrelMetrics(IMeterFactory meterFactory) { _meter = meterFactory.CreateMeter(MeterName); @@ -296,9 +290,15 @@ private static void InitializeConnectionTags(ref TagList tags, in ConnectionMetr if (metricsContext.ConnectionContext.LocalEndPoint is { } localEndpoint) { // TODO: Improve getting string allocation for endpoint. Currently allocates. - // Possible solution is to cache in the endpoint: https://github.com/dotnet/runtime/issues/84515 - // Alternatively, add cache to ConnectionContext. + // Considering adding a way to cache on ConnectionContext. tags.Add("endpoint", localEndpoint.ToString()); } } + + public ConnectionMetricsContext CreateContext(BaseConnectionContext connection) + { + return new ConnectionMetricsContext(connection, + _currentConnectionsCounter.Enabled, _connectionDuration.Enabled, _queuedConnectionsCounter.Enabled, + _queuedRequestsCounter.Enabled, _currentUpgradedRequestsCounter.Enabled, _currentTlsHandshakesCounter.Enabled); + } } From b5ef4ebdc331c02e093ef2e9ff6e52c9b56bdb22 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Wed, 10 May 2023 13:57:53 +0800 Subject: [PATCH 7/9] Update rate limiting --- .../RateLimiting/src/MetricsContext.cs | 12 +++++------ .../RateLimiting/src/RateLimitingMetrics.cs | 20 ++++++++++--------- .../src/RateLimitingMiddleware.cs | 5 ++--- 3 files changed, 19 insertions(+), 18 deletions(-) diff --git a/src/Middleware/RateLimiting/src/MetricsContext.cs b/src/Middleware/RateLimiting/src/MetricsContext.cs index 36995e5a53a2..5a77cd9c4cd3 100644 --- a/src/Middleware/RateLimiting/src/MetricsContext.cs +++ b/src/Middleware/RateLimiting/src/MetricsContext.cs @@ -1,4 +1,4 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. namespace Microsoft.AspNetCore.RateLimiting; @@ -8,15 +8,15 @@ internal readonly struct MetricsContext public readonly string? PolicyName; public readonly string? Method; public readonly string? Route; - public readonly bool CurrentLeaseRequestsCounterEnabled; - public readonly bool CurrentRequestsQueuedCounterEnabled; + public readonly bool CurrentLeasedRequestsCounterEnabled; + public readonly bool CurrentQueuedRequestsCounterEnabled; - public MetricsContext(string? policyName, string? method, string? route, bool currentLeaseRequestsCounterEnabled, bool currentRequestsQueuedCounterEnabled) + public MetricsContext(string? policyName, string? method, string? route, bool currentLeasedRequestsCounterEnabled, bool currentQueuedRequestsCounterEnabled) { PolicyName = policyName; Method = method; Route = route; - CurrentLeaseRequestsCounterEnabled = currentLeaseRequestsCounterEnabled; - CurrentRequestsQueuedCounterEnabled = currentRequestsQueuedCounterEnabled; + CurrentLeasedRequestsCounterEnabled = currentLeasedRequestsCounterEnabled; + CurrentQueuedRequestsCounterEnabled = currentQueuedRequestsCounterEnabled; } } diff --git a/src/Middleware/RateLimiting/src/RateLimitingMetrics.cs b/src/Middleware/RateLimiting/src/RateLimitingMetrics.cs index cf7bbb533e7a..96d69256eea3 100644 --- a/src/Middleware/RateLimiting/src/RateLimitingMetrics.cs +++ b/src/Middleware/RateLimiting/src/RateLimitingMetrics.cs @@ -46,9 +46,6 @@ public RateLimitingMetrics(IMeterFactory meterFactory) description: "Number of HTTP requests that failed to acquire a rate limiting lease. Requests could be rejected by global or endpoint rate limiting policies. Or the request could be canceled while waiting for the lease."); } - public bool CurrentLeasedRequestsCounterEnabled => _currentLeasedRequestsCounter.Enabled; - public bool CurrentQueuedRequestsCounterEnabled => _currentQueuedRequestsCounter.Enabled; - public void LeaseFailed(in MetricsContext metricsContext, RequestRejectionReason reason) { if (_leaseFailedRequestsCounter.Enabled) @@ -68,7 +65,7 @@ private void LeaseFailedCore(in MetricsContext metricsContext, RequestRejectionR public void LeaseStart(in MetricsContext metricsContext) { - if (metricsContext.CurrentLeaseRequestsCounterEnabled) + if (metricsContext.CurrentLeasedRequestsCounterEnabled) { LeaseStartCore(metricsContext); } @@ -84,7 +81,7 @@ public void LeaseStartCore(in MetricsContext metricsContext) public void LeaseEnd(in MetricsContext metricsContext, long startTimestamp, long currentTimestamp) { - if (metricsContext.CurrentLeaseRequestsCounterEnabled || _leasedRequestDurationCounter.Enabled) + if (metricsContext.CurrentLeasedRequestsCounterEnabled || _leasedRequestDurationCounter.Enabled) { LeaseEndCore(metricsContext, startTimestamp, currentTimestamp); } @@ -96,7 +93,7 @@ private void LeaseEndCore(in MetricsContext metricsContext, long startTimestamp, var tags = new TagList(); InitializeRateLimitingTags(ref tags, metricsContext); - if (metricsContext.CurrentLeaseRequestsCounterEnabled) + if (metricsContext.CurrentLeasedRequestsCounterEnabled) { _currentLeasedRequestsCounter.Add(-1, tags); } @@ -110,7 +107,7 @@ private void LeaseEndCore(in MetricsContext metricsContext, long startTimestamp, public void QueueStart(in MetricsContext metricsContext) { - if (metricsContext.CurrentRequestsQueuedCounterEnabled) + if (metricsContext.CurrentQueuedRequestsCounterEnabled) { QueueStartCore(metricsContext); } @@ -126,7 +123,7 @@ private void QueueStartCore(in MetricsContext metricsContext) public void QueueEnd(in MetricsContext metricsContext, RequestRejectionReason? reason, long startTimestamp, long currentTimestamp) { - if (metricsContext.CurrentRequestsQueuedCounterEnabled || _queuedRequestDurationCounter.Enabled) + if (metricsContext.CurrentQueuedRequestsCounterEnabled || _queuedRequestDurationCounter.Enabled) { QueueEndCore(metricsContext, reason, startTimestamp, currentTimestamp); } @@ -138,7 +135,7 @@ private void QueueEndCore(in MetricsContext metricsContext, RequestRejectionReas var tags = new TagList(); InitializeRateLimitingTags(ref tags, metricsContext); - if (metricsContext.CurrentRequestsQueuedCounterEnabled) + if (metricsContext.CurrentQueuedRequestsCounterEnabled) { _currentQueuedRequestsCounter.Add(-1, tags); } @@ -174,4 +171,9 @@ private static void InitializeRateLimitingTags(ref TagList tags, in MetricsConte tags.Add("route", metricsContext.Route); } } + + public MetricsContext CreateContext(string? policyName, string? method, string? route) + { + return new MetricsContext(policyName, method, route, _currentLeasedRequestsCounter.Enabled, _currentQueuedRequestsCounter.Enabled); + } } diff --git a/src/Middleware/RateLimiting/src/RateLimitingMiddleware.cs b/src/Middleware/RateLimiting/src/RateLimitingMiddleware.cs index 2e494b8c5449..18c1e9bf1bd4 100644 --- a/src/Middleware/RateLimiting/src/RateLimitingMiddleware.cs +++ b/src/Middleware/RateLimiting/src/RateLimitingMiddleware.cs @@ -94,15 +94,14 @@ private async Task InvokeInternal(HttpContext context, EnableRateLimitingAttribu // This ensures that the state is consistent for the entire request. // For example, if a meter listener starts after a request is queued, when the request exits the queue // the requests queued counter won't go into a negative value. - var metricsContext = new MetricsContext(policyName, method, route, - _metrics.CurrentLeasedRequestsCounterEnabled, _metrics.CurrentQueuedRequestsCounterEnabled); + var metricsContext = _metrics.CreateContext(policyName, method, route); using var leaseContext = await TryAcquireAsync(context, metricsContext); if (leaseContext.Lease?.IsAcquired == true) { var startTimestamp = Stopwatch.GetTimestamp(); - var currentLeaseStart = _metrics.CurrentLeasedRequestsCounterEnabled; + var currentLeaseStart = metricsContext.CurrentLeasedRequestsCounterEnabled; try { From 3620c936702a6bd6d035d14819e0df4200d088f0 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Wed, 10 May 2023 15:05:54 +0800 Subject: [PATCH 8/9] Add context to SignalR --- .../src/Internal/HttpConnectionContext.cs | 5 +- .../src/Internal/HttpConnectionDispatcher.cs | 2 +- .../src/Internal/HttpConnectionManager.cs | 12 ++-- .../src/Internal/HttpConnectionsMetrics.cs | 55 ++++++++++++++----- .../test/HttpConnectionManagerTests.cs | 23 ++++++++ .../Http.Connections/test/WebSocketsTests.cs | 23 +++++--- 6 files changed, 91 insertions(+), 29 deletions(-) diff --git a/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionContext.cs b/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionContext.cs index 5af5f316def0..a74f17d6aad7 100644 --- a/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionContext.cs +++ b/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionContext.cs @@ -57,7 +57,7 @@ internal sealed partial class HttpConnectionContext : ConnectionContext, /// Creates the DefaultConnectionContext without Pipes to avoid upfront allocations. /// The caller is expected to set the and pipes manually. /// - public HttpConnectionContext(string connectionId, string connectionToken, ILogger logger, IDuplexPipe transport, IDuplexPipe application, HttpConnectionDispatcherOptions options) + public HttpConnectionContext(string connectionId, string connectionToken, ILogger logger, MetricsContext metricsContext, IDuplexPipe transport, IDuplexPipe application, HttpConnectionDispatcherOptions options) { Transport = transport; _applicationStream = new PipeWriterStream(application.Output); @@ -73,6 +73,7 @@ public HttpConnectionContext(string connectionId, string connectionToken, ILogge ActiveFormat = TransferFormat.Text; _logger = logger ?? NullLogger.Instance; + MetricsContext = metricsContext; // PERF: This type could just implement IFeatureCollection Features = new FeatureCollection(); @@ -135,6 +136,8 @@ public long? LastSeenTicksIfInactive public override string ConnectionId { get; set; } + public MetricsContext MetricsContext { get; } + internal string ConnectionToken { get; set; } public override IFeatureCollection Features { get; } diff --git a/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionDispatcher.cs b/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionDispatcher.cs index 469a60666d45..de9fc73186df 100644 --- a/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionDispatcher.cs +++ b/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionDispatcher.cs @@ -528,7 +528,7 @@ private async Task EnsureConnectionStateAsync(HttpConnectionContext connec if (connection.TransportType == HttpTransportType.None) { connection.TransportType = transportType; - _metrics.TransportStart(transportType); + _metrics.TransportStart(connection.MetricsContext, transportType); } else if (connection.TransportType != transportType) { diff --git a/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionManager.cs b/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionManager.cs index 61a133ae0b31..3f1cce2edb6b 100644 --- a/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionManager.cs +++ b/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionManager.cs @@ -80,16 +80,18 @@ internal HttpConnectionContext CreateConnection(HttpConnectionDispatcherOptions connectionToken = id; } - var startTimestamp = HttpConnectionsEventSource.Log.IsEnabled() || _metrics.IsEnabled() + var metricsContext = _metrics.CreateContext(); + + var startTimestamp = HttpConnectionsEventSource.Log.IsEnabled() || metricsContext.ConnectionDurationEnabled ? Stopwatch.GetTimestamp() : default; Log.CreatedNewConnection(_logger, id); HttpConnectionsEventSource.Log.ConnectionStart(id); - _metrics.ConnectionStart(); + _metrics.ConnectionStart(metricsContext); var pair = DuplexPipe.CreateConnectionPair(options.TransportPipeOptions, options.AppPipeOptions); - var connection = new HttpConnectionContext(id, connectionToken, _connectionLogger, pair.Application, pair.Transport, options); + var connection = new HttpConnectionContext(id, connectionToken, _connectionLogger, metricsContext, pair.Application, pair.Transport, options); _connections.TryAdd(connectionToken, (connection, startTimestamp)); @@ -104,8 +106,8 @@ public void RemoveConnection(string id, HttpTransportType transportType, HttpCon var currentTimestamp = (pair.StartTimestamp > 0) ? Stopwatch.GetTimestamp() : default; HttpConnectionsEventSource.Log.ConnectionStop(id, pair.StartTimestamp, currentTimestamp); - _metrics.TransportStop(transportType); - _metrics.ConnectionStop(transportType, status, pair.StartTimestamp, currentTimestamp); + _metrics.TransportStop(pair.Connection.MetricsContext, transportType); + _metrics.ConnectionStop(pair.Connection.MetricsContext, transportType, status, pair.StartTimestamp, currentTimestamp); Log.RemovedConnection(_logger, id); } } diff --git a/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionsMetrics.cs b/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionsMetrics.cs index e55520538792..80807c5d0534 100644 --- a/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionsMetrics.cs +++ b/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionsMetrics.cs @@ -7,6 +7,20 @@ namespace Microsoft.AspNetCore.Http.Connections.Internal; +internal readonly struct MetricsContext +{ + public MetricsContext(bool currentConnectionsCounterEnabled, bool connectionDurationEnabled, bool currentTransportsCounterEnabled) + { + CurrentConnectionsCounterEnabled = currentConnectionsCounterEnabled; + ConnectionDurationEnabled = connectionDurationEnabled; + CurrentTransportsCounterEnabled = currentTransportsCounterEnabled; + } + + public bool CurrentConnectionsCounterEnabled { get; } + public bool ConnectionDurationEnabled { get; } + public bool CurrentTransportsCounterEnabled { get; } +} + internal sealed class HttpConnectionsMetrics : IDisposable { public const string MeterName = "Microsoft.AspNetCore.Http.Connections"; @@ -34,18 +48,24 @@ public HttpConnectionsMetrics(IMeterFactory meterFactory) description: "Number of negotiated transports that are currently active on the server."); } - public void ConnectionStart() + public void ConnectionStart(in MetricsContext metricsContext) { // Tags must match connection end. - _currentConnectionsCounter.Add(1); + if (metricsContext.CurrentConnectionsCounterEnabled) + { + _currentConnectionsCounter.Add(1); + } } - public void ConnectionStop(HttpTransportType transportType, HttpConnectionStopStatus status, long startTimestamp, long currentTimestamp) + public void ConnectionStop(in MetricsContext metricsContext, HttpTransportType transportType, HttpConnectionStopStatus status, long startTimestamp, long currentTimestamp) { // Tags must match connection start. - _currentConnectionsCounter.Add(-1); + if (metricsContext.CurrentConnectionsCounterEnabled) + { + _currentConnectionsCounter.Add(-1); + } - if (_connectionDuration.Enabled) + if (metricsContext.ConnectionDurationEnabled) { var duration = Stopwatch.GetElapsedTime(startTimestamp, currentTimestamp); _connectionDuration.Record(duration.TotalSeconds, @@ -54,21 +74,27 @@ public void ConnectionStop(HttpTransportType transportType, HttpConnectionStopSt } } - public void TransportStart(HttpTransportType transportType) + public void TransportStart(in MetricsContext metricsContext, HttpTransportType transportType) { Debug.Assert(transportType != HttpTransportType.None); // Tags must match transport end. - _currentTransportsCounter.Add(1, new KeyValuePair("transport", transportType.ToString())); + if (metricsContext.CurrentTransportsCounterEnabled) + { + _currentTransportsCounter.Add(1, new KeyValuePair("transport", transportType.ToString())); + } } - public void TransportStop(HttpTransportType transportType) + public void TransportStop(in MetricsContext metricsContext, HttpTransportType transportType) { - // Tags must match transport start. - // If the transport type is none then the transport was never started for this connection. - if (transportType != HttpTransportType.None) + if (metricsContext.CurrentTransportsCounterEnabled) { - _currentTransportsCounter.Add(-1, new KeyValuePair("transport", transportType.ToString())); + // Tags must match transport start. + // If the transport type is none then the transport was never started for this connection. + if (transportType != HttpTransportType.None) + { + _currentTransportsCounter.Add(-1, new KeyValuePair("transport", transportType.ToString())); + } } } @@ -77,5 +103,8 @@ public void Dispose() _meter.Dispose(); } - public bool IsEnabled() => _currentConnectionsCounter.Enabled || _connectionDuration.Enabled; + public MetricsContext CreateContext() + { + return new MetricsContext(_currentConnectionsCounter.Enabled, _connectionDuration.Enabled, _currentTransportsCounter.Enabled); + } } diff --git a/src/SignalR/common/Http.Connections/test/HttpConnectionManagerTests.cs b/src/SignalR/common/Http.Connections/test/HttpConnectionManagerTests.cs index 2447ed4f1747..ff067e567f4a 100644 --- a/src/SignalR/common/Http.Connections/test/HttpConnectionManagerTests.cs +++ b/src/SignalR/common/Http.Connections/test/HttpConnectionManagerTests.cs @@ -450,6 +450,29 @@ public void Metrics() } } + [Fact] + public void Metrics_ListenStartAfterConnection_Empty() + { + using (StartVerifiableLog()) + { + var testMeterFactory = new TestMeterFactory(); + var connectionManager = CreateConnectionManager(LoggerFactory, metrics: new HttpConnectionsMetrics(testMeterFactory)); + var connection = connectionManager.CreateConnection(); + + using var connectionDuration = new InstrumentRecorder(new TestMeterRegistry(testMeterFactory.Meters), HttpConnectionsMetrics.MeterName, "connection-duration"); + using var currentConnections = new InstrumentRecorder(new TestMeterRegistry(testMeterFactory.Meters), HttpConnectionsMetrics.MeterName, "current-connections"); + + Assert.NotNull(connection.ConnectionId); + + connection.TransportType = HttpTransportType.WebSockets; + + connectionManager.RemoveConnection(connection.ConnectionId, connection.TransportType, HttpConnectionStopStatus.NormalClosure); + + Assert.Empty(currentConnections.GetMeasurements()); + Assert.Empty(connectionDuration.GetMeasurements()); + } + } + private static void AssertDuration(Measurement measurement, HttpConnectionStopStatus status, HttpTransportType transportType) { Assert.True(measurement.Value > 0); diff --git a/src/SignalR/common/Http.Connections/test/WebSocketsTests.cs b/src/SignalR/common/Http.Connections/test/WebSocketsTests.cs index 77890b02d9e6..a3283f1b8a51 100644 --- a/src/SignalR/common/Http.Connections/test/WebSocketsTests.cs +++ b/src/SignalR/common/Http.Connections/test/WebSocketsTests.cs @@ -31,7 +31,7 @@ public async Task ReceivedFramesAreWrittenToChannel(string webSocketMessageType) using (StartVerifiableLog()) { var pair = DuplexPipe.CreateConnectionPair(PipeOptions.Default, PipeOptions.Default); - var connection = new HttpConnectionContext("foo", connectionToken: null, LoggerFactory.CreateLogger("HttpConnectionContext1"), pair.Transport, pair.Application, new()); + var connection = CreateHttpConnectionContext(pair, loggerName: "HttpConnectionContext1"); using (var feature = new TestWebSocketConnectionFeature()) { @@ -78,7 +78,7 @@ public async Task WebSocketTransportSetsMessageTypeBasedOnTransferFormatFeature( using (StartVerifiableLog()) { var pair = DuplexPipe.CreateConnectionPair(PipeOptions.Default, PipeOptions.Default); - var connection = new HttpConnectionContext("foo", connectionToken: null, LoggerFactory.CreateLogger("HttpConnectionContext1"), pair.Transport, pair.Application, new()); + var connection = CreateHttpConnectionContext(pair, loggerName: "HttpConnectionContext1"); using (var feature = new TestWebSocketConnectionFeature()) { @@ -108,13 +108,18 @@ public async Task WebSocketTransportSetsMessageTypeBasedOnTransferFormatFeature( } } + private HttpConnectionContext CreateHttpConnectionContext(DuplexPipe.DuplexPipePair pair, string loggerName = null) + { + return new HttpConnectionContext("foo", connectionToken: null, LoggerFactory.CreateLogger(loggerName ?? nameof(HttpConnectionContext)), metricsContext: default, pair.Transport, pair.Application, new()); + } + [Fact] public async Task TransportCommunicatesErrorToApplicationWhenClientDisconnectsAbnormally() { using (StartVerifiableLog()) { var pair = DuplexPipe.CreateConnectionPair(PipeOptions.Default, PipeOptions.Default); - var connection = new HttpConnectionContext("foo", connectionToken: null, LoggerFactory.CreateLogger("HttpConnectionContext1"), pair.Transport, pair.Application, new()); + var connection = CreateHttpConnectionContext(pair, loggerName: "HttpConnectionContext1"); using (var feature = new TestWebSocketConnectionFeature()) { @@ -166,7 +171,7 @@ public async Task ClientReceivesInternalServerErrorWhenTheApplicationFails() using (StartVerifiableLog()) { var pair = DuplexPipe.CreateConnectionPair(PipeOptions.Default, PipeOptions.Default); - var connection = new HttpConnectionContext("foo", connectionToken: null, LoggerFactory.CreateLogger(nameof(HttpConnectionContext)), pair.Transport, pair.Application, new()); + var connection = CreateHttpConnectionContext(pair); using (var feature = new TestWebSocketConnectionFeature()) { @@ -197,7 +202,7 @@ public async Task TransportClosesOnCloseTimeoutIfClientDoesNotSendCloseFrame() using (StartVerifiableLog()) { var pair = DuplexPipe.CreateConnectionPair(PipeOptions.Default, PipeOptions.Default); - var connection = new HttpConnectionContext("foo", connectionToken: null, LoggerFactory.CreateLogger(nameof(HttpConnectionContext)), pair.Transport, pair.Application, new()); + var connection = CreateHttpConnectionContext(pair); using (var feature = new TestWebSocketConnectionFeature()) { @@ -231,7 +236,7 @@ public async Task TransportFailsOnTimeoutWithErrorWhenApplicationFailsAndClientD using (StartVerifiableLog()) { var pair = DuplexPipe.CreateConnectionPair(PipeOptions.Default, PipeOptions.Default); - var connection = new HttpConnectionContext("foo", connectionToken: null, LoggerFactory.CreateLogger(nameof(HttpConnectionContext)), pair.Transport, pair.Application, new()); + var connection = CreateHttpConnectionContext(pair); using (var feature = new TestWebSocketConnectionFeature()) { @@ -265,7 +270,7 @@ public async Task ServerGracefullyClosesWhenApplicationEndsThenClientSendsCloseF using (StartVerifiableLog()) { var pair = DuplexPipe.CreateConnectionPair(PipeOptions.Default, PipeOptions.Default); - var connection = new HttpConnectionContext("foo", connectionToken: null, LoggerFactory.CreateLogger(nameof(HttpConnectionContext)), pair.Transport, pair.Application, new()); + var connection = CreateHttpConnectionContext(pair); using (var feature = new TestWebSocketConnectionFeature()) { @@ -304,7 +309,7 @@ public async Task ServerGracefullyClosesWhenClientSendsCloseFrameThenApplication using (StartVerifiableLog()) { var pair = DuplexPipe.CreateConnectionPair(PipeOptions.Default, PipeOptions.Default); - var connection = new HttpConnectionContext("foo", connectionToken: null, LoggerFactory.CreateLogger(nameof(HttpConnectionContext)), pair.Transport, pair.Application, new()); + var connection = CreateHttpConnectionContext(pair); using (var feature = new TestWebSocketConnectionFeature()) { @@ -346,7 +351,7 @@ public async Task SubProtocolSelectorIsUsedToSelectSubProtocol() using (StartVerifiableLog()) { var pair = DuplexPipe.CreateConnectionPair(PipeOptions.Default, PipeOptions.Default); - var connection = new HttpConnectionContext("foo", connectionToken: null, LoggerFactory.CreateLogger(nameof(HttpConnectionContext)), pair.Transport, pair.Application, new()); + var connection = CreateHttpConnectionContext(pair); using (var feature = new TestWebSocketConnectionFeature()) { From fd622374f67e06b49aed892ffc28e929f5e5e529 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Thu, 18 May 2023 14:40:52 +0800 Subject: [PATCH 9/9] Apply suggestions from code review Co-authored-by: Chris Ross --- .../Kestrel/Core/src/Internal/Infrastructure/KestrelMetrics.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/KestrelMetrics.cs b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/KestrelMetrics.cs index 6f0c62a7d299..71102514ee60 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/KestrelMetrics.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/KestrelMetrics.cs @@ -123,6 +123,7 @@ private void ConnectionStopCore(in ConnectionMetricsContext metricsContext, Exce public void ConnectionRejected(in ConnectionMetricsContext metricsContext) { + // Check live rather than cached state because this is just a counter, it's not a start/stop event like the other metrics. if (_rejectedConnectionsCounter.Enabled) { ConnectionRejectedCore(metricsContext); @@ -297,6 +298,7 @@ private static void InitializeConnectionTags(ref TagList tags, in ConnectionMetr public ConnectionMetricsContext CreateContext(BaseConnectionContext connection) { + // Cache the state at the start of the connection so we produce consistent start/stop events. return new ConnectionMetricsContext(connection, _currentConnectionsCounter.Enabled, _connectionDuration.Enabled, _queuedConnectionsCounter.Enabled, _queuedRequestsCounter.Enabled, _currentUpgradedRequestsCounter.Enabled, _currentTlsHandshakesCounter.Enabled);