From 6f6bb59638a25a436b0ebd8a7c5dea572d087b39 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Tue, 18 Apr 2023 15:19:01 +0800 Subject: [PATCH 1/6] Add metrics to rate limiting --- ...oft.AspNetCore.Hosting.Abstractions.csproj | 22 -- ...rosoft.AspNetCore.Http.Abstractions.csproj | 23 ++ .../RateLimiting/src/LeaseContext.cs | 2 +- ...RateLimiterApplicationBuilderExtensions.cs | 17 ++ .../RateLimiterServiceCollectionExtensions.cs | 2 + .../RateLimiting/src/RateLimitingMetrics.cs | 153 ++++++++++++ .../src/RateLimitingMiddleware.cs | 56 ++++- .../RateLimiting/src/Resources.resx | 123 ++++++++++ ...osoft.AspNetCore.RateLimiting.Tests.csproj | 3 + ...mitingApplicationBuilderExtensionsTests.cs | 15 +- .../test/RateLimitingMetricsTests.cs | 225 ++++++++++++++++++ .../test/RateLimitingMiddlewareTests.cs | 23 +- .../test/TestPartitionedRateLimiter.cs | 15 +- ...soft.AspNetCore.Server.Kestrel.Core.csproj | 1 - 14 files changed, 631 insertions(+), 49 deletions(-) create mode 100644 src/Middleware/RateLimiting/src/RateLimitingMetrics.cs create mode 100644 src/Middleware/RateLimiting/src/Resources.resx create mode 100644 src/Middleware/RateLimiting/test/RateLimitingMetricsTests.cs diff --git a/src/Hosting/Abstractions/src/Microsoft.AspNetCore.Hosting.Abstractions.csproj b/src/Hosting/Abstractions/src/Microsoft.AspNetCore.Hosting.Abstractions.csproj index 4a53a07a61b4..9b5d4268cb21 100644 --- a/src/Hosting/Abstractions/src/Microsoft.AspNetCore.Hosting.Abstractions.csproj +++ b/src/Hosting/Abstractions/src/Microsoft.AspNetCore.Hosting.Abstractions.csproj @@ -11,28 +11,6 @@ true - - - - - - - - - - - - - - - - - - - - - - diff --git a/src/Http/Http.Abstractions/src/Microsoft.AspNetCore.Http.Abstractions.csproj b/src/Http/Http.Abstractions/src/Microsoft.AspNetCore.Http.Abstractions.csproj index ff0c54dadae1..ccdbcde598a7 100644 --- a/src/Http/Http.Abstractions/src/Microsoft.AspNetCore.Http.Abstractions.csproj +++ b/src/Http/Http.Abstractions/src/Microsoft.AspNetCore.Http.Abstractions.csproj @@ -53,6 +53,29 @@ Microsoft.AspNetCore.Http.HttpResponse + + + + + + + + + + + + + + + + + + + + + + + diff --git a/src/Middleware/RateLimiting/src/LeaseContext.cs b/src/Middleware/RateLimiting/src/LeaseContext.cs index 1b78c3f8bf85..98068291bf34 100644 --- a/src/Middleware/RateLimiting/src/LeaseContext.cs +++ b/src/Middleware/RateLimiting/src/LeaseContext.cs @@ -22,4 +22,4 @@ internal enum RequestRejectionReason EndpointLimiter, GlobalLimiter, RequestCanceled -} \ No newline at end of file +} diff --git a/src/Middleware/RateLimiting/src/RateLimiterApplicationBuilderExtensions.cs b/src/Middleware/RateLimiting/src/RateLimiterApplicationBuilderExtensions.cs index cd1ec7b82604..360c3baafb3a 100644 --- a/src/Middleware/RateLimiting/src/RateLimiterApplicationBuilderExtensions.cs +++ b/src/Middleware/RateLimiting/src/RateLimiterApplicationBuilderExtensions.cs @@ -2,7 +2,9 @@ // The .NET Foundation licenses this file to you under the MIT license. using Microsoft.AspNetCore.RateLimiting; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; +using Resources = Microsoft.AspNetCore.RateLimiting.Resources; namespace Microsoft.AspNetCore.Builder; @@ -20,6 +22,8 @@ public static IApplicationBuilder UseRateLimiter(this IApplicationBuilder app) { ArgumentNullException.ThrowIfNull(app); + VerifyServicesAreRegistered(app); + return app.UseMiddleware(); } @@ -34,6 +38,19 @@ public static IApplicationBuilder UseRateLimiter(this IApplicationBuilder app, R ArgumentNullException.ThrowIfNull(app); ArgumentNullException.ThrowIfNull(options); + VerifyServicesAreRegistered(app); + return app.UseMiddleware(Options.Create(options)); } + + private static void VerifyServicesAreRegistered(IApplicationBuilder app) + { + var serviceProviderIsService = app.ApplicationServices.GetService(); + if (serviceProviderIsService != null && !serviceProviderIsService.IsService(typeof(RateLimitingMetrics))) + { + throw new InvalidOperationException(Resources.FormatUnableToFindServices( + nameof(IServiceCollection), + nameof(RateLimiterServiceCollectionExtensions.AddRateLimiter))); + } + } } diff --git a/src/Middleware/RateLimiting/src/RateLimiterServiceCollectionExtensions.cs b/src/Middleware/RateLimiting/src/RateLimiterServiceCollectionExtensions.cs index ac3c6718000f..09f6f7ba7c5c 100644 --- a/src/Middleware/RateLimiting/src/RateLimiterServiceCollectionExtensions.cs +++ b/src/Middleware/RateLimiting/src/RateLimiterServiceCollectionExtensions.cs @@ -22,6 +22,8 @@ public static IServiceCollection AddRateLimiter(this IServiceCollection services ArgumentNullException.ThrowIfNull(services); ArgumentNullException.ThrowIfNull(configureOptions); + services.AddMetrics(); + services.AddSingleton(); services.Configure(configureOptions); return services; } diff --git a/src/Middleware/RateLimiting/src/RateLimitingMetrics.cs b/src/Middleware/RateLimiting/src/RateLimitingMetrics.cs new file mode 100644 index 000000000000..00d62159d3e1 --- /dev/null +++ b/src/Middleware/RateLimiting/src/RateLimitingMetrics.cs @@ -0,0 +1,153 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Diagnostics; +using System.Diagnostics.Metrics; +using System.Runtime.CompilerServices; +using Microsoft.Extensions.Metrics; + +namespace Microsoft.AspNetCore.RateLimiting; + +internal sealed class RateLimitingMetrics : IDisposable +{ + public const string MeterName = "Microsoft.AspNetCore.RateLimiting"; + + private readonly Meter _meter; + private readonly UpDownCounter _currentLeaseRequestsCounter; + private readonly Histogram _leaseRequestDurationCounter; + private readonly UpDownCounter _currentRequestsQueuedCounter; + private readonly Histogram _queuedRequestDurationCounter; + private readonly Counter _leaseFailedRequestsCounter; + + public RateLimitingMetrics(IMeterFactory meterFactory) + { + _meter = meterFactory.CreateMeter(MeterName); + + _currentLeaseRequestsCounter = _meter.CreateUpDownCounter( + "current-lease-requests", + description: "Number of HTTP requests that are currently active on the server that hold a rate limiting lease."); + + _leaseRequestDurationCounter = _meter.CreateHistogram( + "lease-request-duration", + unit: "s", + description: "The duration of rate limiting leases held by HTTP requests on the server."); + + _currentRequestsQueuedCounter = _meter.CreateUpDownCounter( + "current-requests-queued", + description: "Number of HTTP requests that are currently queued, waiting to acquire a rate limiting lease."); + + _queuedRequestDurationCounter = _meter.CreateHistogram( + "queued-request-duration", + unit: "s", + description: "The duration of HTTP requests in a queue, waiting to acquire a rate limiting lease."); + + _leaseFailedRequestsCounter = _meter.CreateCounter( + "lease-failed-requests", + 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 void LeaseFailed(string? policyName, string? route, RequestRejectionReason reason) + { + if (_leaseFailedRequestsCounter.Enabled) + { + LeaseFailedCore(policyName, route, reason); + } + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private void LeaseFailedCore(string? policyName, string? route, RequestRejectionReason reason) + { + var tags = new TagList(); + InitializeRateLimitingTags(ref tags, policyName, route); + tags.Add("reason", reason.ToString()); + _leaseFailedRequestsCounter.Add(1, tags); + } + + public void LeaseStart(string? policyName, string? route) + { + if (_currentLeaseRequestsCounter.Enabled) + { + LeaseStartCore(policyName, route); + } + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private void LeaseStartCore(string? policyName, string? route) + { + var tags = new TagList(); + InitializeRateLimitingTags(ref tags, policyName, route); + _currentLeaseRequestsCounter.Add(1, tags); + } + + public void LeaseEnd(string? policyName, string? route, long startTimestamp, long currentTimestamp) + { + if (_currentLeaseRequestsCounter.Enabled || _leaseRequestDurationCounter.Enabled) + { + LeaseEndCore(policyName, route, startTimestamp, currentTimestamp); + } + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private void LeaseEndCore(string? policyName, string? route, long startTimestamp, long currentTimestamp) + { + var tags = new TagList(); + InitializeRateLimitingTags(ref tags, policyName, route); + + _currentLeaseRequestsCounter.Add(-1, tags); + + var duration = Stopwatch.GetElapsedTime(startTimestamp, currentTimestamp); + _leaseRequestDurationCounter.Record(duration.TotalSeconds, tags); + } + + public void QueueStart(string? policyName, string? route) + { + if (_currentRequestsQueuedCounter.Enabled) + { + QueueStartCore(policyName, route); + } + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private void QueueStartCore(string? policyName, string? route) + { + var tags = new TagList(); + InitializeRateLimitingTags(ref tags, policyName, route); + _currentRequestsQueuedCounter.Add(1, tags); + } + + public void QueueEnd(string? policyName, string? route, long startTimestamp, long currentTimestamp) + { + if (_currentRequestsQueuedCounter.Enabled || _queuedRequestDurationCounter.Enabled) + { + QueueEndCore(policyName, route, startTimestamp, currentTimestamp); + } + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private void QueueEndCore(string? policyName, string? route, long startTimestamp, long currentTimestamp) + { + var tags = new TagList(); + InitializeRateLimitingTags(ref tags, policyName, route); + _currentRequestsQueuedCounter.Add(-1, tags); + + var duration = Stopwatch.GetElapsedTime(startTimestamp, currentTimestamp); + _queuedRequestDurationCounter.Record(duration.TotalSeconds, tags); + } + + public void Dispose() + { + _meter.Dispose(); + } + + private static void InitializeRateLimitingTags(ref TagList tags, string? policyName, string? route) + { + if (policyName is not null) + { + tags.Add("policy", policyName); + } + if (route is not null) + { + tags.Add("route", route); + } + } +} diff --git a/src/Middleware/RateLimiting/src/RateLimitingMiddleware.cs b/src/Middleware/RateLimiting/src/RateLimitingMiddleware.cs index d743f77feea6..cdd0de49b9b1 100644 --- a/src/Middleware/RateLimiting/src/RateLimitingMiddleware.cs +++ b/src/Middleware/RateLimiting/src/RateLimitingMiddleware.cs @@ -1,8 +1,10 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Diagnostics; using System.Threading.RateLimiting; using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http.Metadata; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; @@ -16,6 +18,7 @@ internal sealed partial class RateLimitingMiddleware private readonly RequestDelegate _next; private readonly Func? _defaultOnRejected; private readonly ILogger _logger; + private readonly RateLimitingMetrics _metrics; private readonly PartitionedRateLimiter? _globalLimiter; private readonly PartitionedRateLimiter _endpointLimiter; private readonly int _rejectionStatusCode; @@ -29,14 +32,17 @@ internal sealed partial class RateLimitingMiddleware /// The used for logging. /// The options for the middleware. /// The service provider. - public RateLimitingMiddleware(RequestDelegate next, ILogger logger, IOptions options, IServiceProvider serviceProvider) + /// The rate limiting metrics. + public RateLimitingMiddleware(RequestDelegate next, ILogger logger, IOptions options, IServiceProvider serviceProvider, RateLimitingMetrics metrics) { ArgumentNullException.ThrowIfNull(next); ArgumentNullException.ThrowIfNull(logger); ArgumentNullException.ThrowIfNull(serviceProvider); + ArgumentNullException.ThrowIfNull(metrics); _next = next; _logger = logger; + _metrics = metrics; _defaultOnRejected = options.Value.OnRejected; _rejectionStatusCode = options.Value.RejectionStatusCode; _policyMap = new Dictionary(options.Value.PolicyMap); @@ -49,7 +55,6 @@ public RateLimitingMiddleware(RequestDelegate next, ILogger()?.Route); } - private async Task InvokeInternal(HttpContext context, EnableRateLimitingAttribute? enableRateLimitingAttribute) + private async Task InvokeInternal(HttpContext context, EnableRateLimitingAttribute? enableRateLimitingAttribute, string? route) { - using var leaseContext = await TryAcquireAsync(context); + var policyName = enableRateLimitingAttribute?.PolicyName; + using var leaseContext = await TryAcquireAsync(context, policyName, route); + if (leaseContext.Lease?.IsAcquired == true) { - await _next(context); + var startTimestamp = Stopwatch.GetTimestamp(); + try + { + + _metrics.LeaseStart(policyName, route); + await _next(context); + } + finally + { + _metrics.LeaseEnd(policyName, route, startTimestamp, Stopwatch.GetTimestamp()); + } } else { + _metrics.LeaseFailed(policyName, route, leaseContext.RequestRejectionReason!.Value); + // If the request was canceled, do not call OnRejected, just return. if (leaseContext.RequestRejectionReason == RequestRejectionReason.RequestCanceled) { @@ -107,7 +126,6 @@ private async Task InvokeInternal(HttpContext context, EnableRateLimitingAttribu } else { - var policyName = enableRateLimitingAttribute?.PolicyName; if (policyName is not null && _policyMap.TryGetValue(policyName, out policy) && policy.OnRejected is not null) { thisRequestOnRejected = policy.OnRejected; @@ -122,15 +140,31 @@ private async Task InvokeInternal(HttpContext context, EnableRateLimitingAttribu } } - private ValueTask TryAcquireAsync(HttpContext context) + private async ValueTask TryAcquireAsync(HttpContext context, string? policyName, string? route) { var leaseContext = CombinedAcquire(context); if (leaseContext.Lease?.IsAcquired == true) { - return ValueTask.FromResult(leaseContext); + return leaseContext; } - return CombinedWaitAsync(context, context.RequestAborted); + var waitTask = CombinedWaitAsync(context, context.RequestAborted); + // If the task returns immediately then the request wasn't queued. + if (waitTask.IsCompleted) + { + return await waitTask; + } + + var startTimestamp = Stopwatch.GetTimestamp(); + try + { + _metrics.QueueStart(policyName, route); + return await waitTask; + } + finally + { + _metrics.QueueEnd(policyName, route, startTimestamp, Stopwatch.GetTimestamp()); + } } private LeaseContext CombinedAcquire(HttpContext context) @@ -253,4 +287,4 @@ private static partial class RateLimiterLog [LoggerMessage(3, LogLevel.Debug, "The request was canceled.", EventName = "RequestCanceled")] internal static partial void RequestCanceled(ILogger logger); } -} \ No newline at end of file +} diff --git a/src/Middleware/RateLimiting/src/Resources.resx b/src/Middleware/RateLimiting/src/Resources.resx new file mode 100644 index 000000000000..f50493b9d4a9 --- /dev/null +++ b/src/Middleware/RateLimiting/src/Resources.resx @@ -0,0 +1,123 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + text/microsoft-resx + + + 2.0 + + + System.Resources.ResXResourceReader, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 + + + System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 + + + Unable to find the required services. Please add all the required services by calling '{0}.{1}' in the application startup code. + + \ No newline at end of file diff --git a/src/Middleware/RateLimiting/test/Microsoft.AspNetCore.RateLimiting.Tests.csproj b/src/Middleware/RateLimiting/test/Microsoft.AspNetCore.RateLimiting.Tests.csproj index 9712d186644a..22890a28eae9 100644 --- a/src/Middleware/RateLimiting/test/Microsoft.AspNetCore.RateLimiting.Tests.csproj +++ b/src/Middleware/RateLimiting/test/Microsoft.AspNetCore.RateLimiting.Tests.csproj @@ -7,5 +7,8 @@ + + + diff --git a/src/Middleware/RateLimiting/test/RateLimitingApplicationBuilderExtensionsTests.cs b/src/Middleware/RateLimiting/test/RateLimitingApplicationBuilderExtensionsTests.cs index 45afd2182a4d..55d4b0b3ab7b 100644 --- a/src/Middleware/RateLimiting/test/RateLimitingApplicationBuilderExtensionsTests.cs +++ b/src/Middleware/RateLimiting/test/RateLimitingApplicationBuilderExtensionsTests.cs @@ -10,7 +10,6 @@ namespace Microsoft.AspNetCore.RateLimiting; public class RateLimitingApplicationBuilderExtensionsTests : LoggedTest { - [Fact] public void UseRateLimiter_ThrowsOnNullAppBuilder() { @@ -24,6 +23,18 @@ public void UseRateLimiter_ThrowsOnNullOptions() Assert.Throws(() => appBuilder.UseRateLimiter(null)); } + [Fact] + public void UseRateLimiter_RequireServices() + { + var services = new ServiceCollection(); + var serviceProvider = services.BuildServiceProvider(); + var appBuilder = new ApplicationBuilder(serviceProvider); + + // Act + var ex = Assert.Throws(() => appBuilder.UseRateLimiter()); + Assert.Equal("Unable to find the required services. Please add all the required services by calling 'IServiceCollection.AddRateLimiter' in the application startup code.", ex.Message); + } + [Fact] public void UseRateLimiter_RespectsOptions() { @@ -34,7 +45,7 @@ public void UseRateLimiter_RespectsOptions() // These should not get used var services = new ServiceCollection(); - services.Configure(options => + services.AddRateLimiter(options => { options.GlobalLimiter = new TestPartitionedRateLimiter(new TestRateLimiter(false)); options.RejectionStatusCode = 404; diff --git a/src/Middleware/RateLimiting/test/RateLimitingMetricsTests.cs b/src/Middleware/RateLimiting/test/RateLimitingMetricsTests.cs new file mode 100644 index 000000000000..decb07444c3e --- /dev/null +++ b/src/Middleware/RateLimiting/test/RateLimitingMetricsTests.cs @@ -0,0 +1,225 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Diagnostics.Metrics; +using System.Threading.RateLimiting; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Internal; +using Microsoft.AspNetCore.Routing; +using Microsoft.AspNetCore.Routing.Patterns; +using Microsoft.AspNetCore.Testing; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Metrics; +using Microsoft.Extensions.Options; +using Moq; + +namespace Microsoft.AspNetCore.RateLimiting; + +public class RateLimitingMetricsTests +{ + [Fact] + public async Task Metrics_Rejected() + { + // Arrange + var meterFactory = new TestMeterFactory(); + var meterRegistry = new TestMeterRegistry(meterFactory.Meters); + + var options = CreateOptionsAccessor(); + options.Value.GlobalLimiter = new TestPartitionedRateLimiter(new TestRateLimiter(false)); + + var middleware = CreateTestRateLimitingMiddleware(options, meterFactory: meterFactory); + var meter = meterFactory.Meters.Single(); + + var context = new DefaultHttpContext(); + + using var leaseRequestDurationRecorder = new InstrumentRecorder(meterRegistry, RateLimitingMetrics.MeterName, "lease-request-duration"); + using var currentLeaseRequestsRecorder = new InstrumentRecorder(meterRegistry, RateLimitingMetrics.MeterName, "current-lease-requests"); + using var currentRequestsQueuedRecorder = new InstrumentRecorder(meterRegistry, RateLimitingMetrics.MeterName, "current-requests-queued"); + using var queuedRequestDurationRecorder = new InstrumentRecorder(meterRegistry, RateLimitingMetrics.MeterName, "queued-request-duration"); + using var leaseFailedRequestsRecorder = new InstrumentRecorder(meterRegistry, RateLimitingMetrics.MeterName, "lease-failed-requests"); + + // Act + await middleware.Invoke(context).DefaultTimeout(); + + // Assert + Assert.Equal(StatusCodes.Status503ServiceUnavailable, context.Response.StatusCode); + + Assert.Empty(currentLeaseRequestsRecorder.GetMeasurements()); + Assert.Empty(leaseRequestDurationRecorder.GetMeasurements()); + Assert.Empty(currentRequestsQueuedRecorder.GetMeasurements()); + Assert.Empty(queuedRequestDurationRecorder.GetMeasurements()); + Assert.Collection(leaseFailedRequestsRecorder.GetMeasurements(), + m => + { + Assert.Equal(1, m.Value); + Assert.Equal("GlobalLimiter", (string)m.Tags.ToArray().Single(t => t.Key == "reason").Value); + }); + } + + [Fact] + public async Task Metrics_Success() + { + // Arrange + var syncPoint = new SyncPoint(); + + var meterFactory = new TestMeterFactory(); + var meterRegistry = new TestMeterRegistry(meterFactory.Meters); + + var options = CreateOptionsAccessor(); + options.Value.GlobalLimiter = new TestPartitionedRateLimiter(new TestRateLimiter(true)); + + var middleware = CreateTestRateLimitingMiddleware( + options, + meterFactory: meterFactory, + next: async c => + { + await syncPoint.WaitToContinue(); + }); + var meter = meterFactory.Meters.Single(); + + var context = new DefaultHttpContext(); + + using var leaseRequestDurationRecorder = new InstrumentRecorder(meterRegistry, RateLimitingMetrics.MeterName, "lease-request-duration"); + using var currentLeaseRequestsRecorder = new InstrumentRecorder(meterRegistry, RateLimitingMetrics.MeterName, "current-lease-requests"); + using var currentRequestsQueuedRecorder = new InstrumentRecorder(meterRegistry, RateLimitingMetrics.MeterName, "current-requests-queued"); + using var queuedRequestDurationRecorder = new InstrumentRecorder(meterRegistry, RateLimitingMetrics.MeterName, "queued-request-duration"); + using var leaseFailedRequestsRecorder = new InstrumentRecorder(meterRegistry, RateLimitingMetrics.MeterName, "lease-failed-requests"); + + // Act + var middlewareTask = middleware.Invoke(context); + + await syncPoint.WaitForSyncPoint().DefaultTimeout(); + + Assert.Collection(currentLeaseRequestsRecorder.GetMeasurements(), + m => AssertCounter(m, 1, null, null)); + Assert.Empty(leaseRequestDurationRecorder.GetMeasurements()); + + syncPoint.Continue(); + + await middlewareTask.DefaultTimeout(); + + // Assert + Assert.Equal(StatusCodes.Status200OK, context.Response.StatusCode); + + Assert.Collection(currentLeaseRequestsRecorder.GetMeasurements(), + m => AssertCounter(m, 1, null, null), + m => AssertCounter(m, -1, null, null)); + Assert.Collection(leaseRequestDurationRecorder.GetMeasurements(), + m => AssertDuration(m, null, null)); + Assert.Empty(currentRequestsQueuedRecorder.GetMeasurements()); + Assert.Empty(queuedRequestDurationRecorder.GetMeasurements()); + Assert.Empty(leaseFailedRequestsRecorder.GetMeasurements()); + } + + [Fact] + public async Task Metrics_Queued() + { + // Arrange + var syncPoint = new SyncPoint(); + + var meterFactory = new TestMeterFactory(); + var meterRegistry = new TestMeterRegistry(meterFactory.Meters); + + var services = new ServiceCollection(); + + services.AddRateLimiter(_ => _ + .AddConcurrencyLimiter(policyName: "concurrencyPolicy", options => + { + options.PermitLimit = 1; + options.QueueProcessingOrder = QueueProcessingOrder.OldestFirst; + options.QueueLimit = 1; + })); + var serviceProvider = services.BuildServiceProvider(); + + var middleware = CreateTestRateLimitingMiddleware( + serviceProvider.GetRequiredService>(), + meterFactory: meterFactory, + next: async c => + { + await syncPoint.WaitToContinue(); + }, + serviceProvider: serviceProvider); + var meter = meterFactory.Meters.Single(); + + var routeEndpointBuilder = new RouteEndpointBuilder(c => Task.CompletedTask, RoutePatternFactory.Parse("/"), 0); + routeEndpointBuilder.Metadata.Add(new EnableRateLimitingAttribute("concurrencyPolicy")); + var endpoint = routeEndpointBuilder.Build(); + + using var leaseRequestDurationRecorder = new InstrumentRecorder(meterRegistry, RateLimitingMetrics.MeterName, "lease-request-duration"); + using var currentLeaseRequestsRecorder = new InstrumentRecorder(meterRegistry, RateLimitingMetrics.MeterName, "current-lease-requests"); + using var currentRequestsQueuedRecorder = new InstrumentRecorder(meterRegistry, RateLimitingMetrics.MeterName, "current-requests-queued"); + using var queuedRequestDurationRecorder = new InstrumentRecorder(meterRegistry, RateLimitingMetrics.MeterName, "queued-request-duration"); + using var leaseFailedRequestsRecorder = new InstrumentRecorder(meterRegistry, RateLimitingMetrics.MeterName, "lease-failed-requests"); + + // Act + var context1 = new DefaultHttpContext(); + context1.SetEndpoint(endpoint); + var middlewareTask1 = middleware.Invoke(context1); + + // Wait for first request to reach server and block it. + await syncPoint.WaitForSyncPoint().DefaultTimeout(); + + var context2 = new DefaultHttpContext(); + context2.SetEndpoint(endpoint); + var middlewareTask2 = middleware.Invoke(context1); + + // Assert second request is queued. + Assert.Collection(currentRequestsQueuedRecorder.GetMeasurements(), + m => AssertCounter(m, 1, "/", "concurrencyPolicy")); + Assert.Empty(queuedRequestDurationRecorder.GetMeasurements()); + + // Allow both requests to finish. + syncPoint.Continue(); + + await middlewareTask1.DefaultTimeout(); + await middlewareTask2.DefaultTimeout(); + + Assert.Collection(currentRequestsQueuedRecorder.GetMeasurements(), + m => AssertCounter(m, 1, "/", "concurrencyPolicy"), + m => AssertCounter(m, -1, "/", "concurrencyPolicy")); + Assert.Collection(queuedRequestDurationRecorder.GetMeasurements(), + m => AssertDuration(m, "/", "concurrencyPolicy")); + } + + private static void AssertCounter(Measurement measurement, long value, string route, string policy) + { + Assert.Equal(value, measurement.Value); + AssertTag(measurement.Tags, "route", route); + AssertTag(measurement.Tags, "policy", policy); + } + + private static void AssertDuration(Measurement measurement, string route, string policy) + { + Assert.True(measurement.Value > 0); + AssertTag(measurement.Tags, "route", route); + AssertTag(measurement.Tags, "policy", policy); + } + + private static void AssertTag(ReadOnlySpan> tags, string tagName, T expected) + { + if (expected == null) + { + Assert.DoesNotContain(tags.ToArray(), t => t.Key == tagName); + } + else + { + Assert.Equal(expected, (T)tags.ToArray().Single(t => t.Key == tagName).Value); + } + } + + private RateLimitingMiddleware CreateTestRateLimitingMiddleware(IOptions options, ILogger logger = null, IServiceProvider serviceProvider = null, IMeterFactory meterFactory = null, RequestDelegate next = null) + { + next ??= c => Task.CompletedTask; + return new RateLimitingMiddleware( + next, + logger ?? new NullLoggerFactory().CreateLogger(), + options, + serviceProvider ?? Mock.Of(), + new RateLimitingMetrics(meterFactory ?? new TestMeterFactory())); + } + + private IOptions CreateOptionsAccessor() => Options.Create(new RateLimiterOptions()); +} diff --git a/src/Middleware/RateLimiting/test/RateLimitingMiddlewareTests.cs b/src/Middleware/RateLimiting/test/RateLimitingMiddlewareTests.cs index 24c6ccd93990..b64c7a258195 100644 --- a/src/Middleware/RateLimiting/test/RateLimitingMiddlewareTests.cs +++ b/src/Middleware/RateLimiting/test/RateLimitingMiddlewareTests.cs @@ -8,6 +8,7 @@ using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Logging.Testing; +using Microsoft.Extensions.Metrics; using Microsoft.Extensions.Options; using Moq; @@ -25,7 +26,8 @@ public void Ctor_ThrowsExceptionsWhenNullArgs() null, new NullLoggerFactory().CreateLogger(), options, - Mock.Of())); + Mock.Of(), + new RateLimitingMetrics(new TestMeterFactory()))); Assert.Throws(() => new RateLimitingMiddleware(c => { @@ -33,7 +35,8 @@ public void Ctor_ThrowsExceptionsWhenNullArgs() }, null, options, - Mock.Of())); + Mock.Of(), + new RateLimitingMetrics(new TestMeterFactory()))); Assert.Throws(() => new RateLimitingMiddleware(c => { @@ -41,6 +44,16 @@ public void Ctor_ThrowsExceptionsWhenNullArgs() }, new NullLoggerFactory().CreateLogger(), options, + null, + new RateLimitingMetrics(new TestMeterFactory()))); + + Assert.Throws(() => new RateLimitingMiddleware(c => + { + return Task.CompletedTask; + }, + new NullLoggerFactory().CreateLogger(), + options, + Mock.Of(), null)); } @@ -59,7 +72,8 @@ public async Task RequestsCallNextIfAccepted() }, new NullLoggerFactory().CreateLogger(), options, - Mock.Of()); + Mock.Of(), + new RateLimitingMetrics(new TestMeterFactory())); // Act await middleware.Invoke(new DefaultHttpContext()); @@ -637,7 +651,8 @@ private RateLimitingMiddleware CreateTestRateLimitingMiddleware(IOptions(), options, - serviceProvider ?? Mock.Of()); + serviceProvider ?? Mock.Of(), + new RateLimitingMetrics(new TestMeterFactory())); private IOptions CreateOptionsAccessor() => Options.Create(new RateLimiterOptions()); } diff --git a/src/Middleware/RateLimiting/test/TestPartitionedRateLimiter.cs b/src/Middleware/RateLimiting/test/TestPartitionedRateLimiter.cs index fa19f79779b7..ea9082858fd3 100644 --- a/src/Middleware/RateLimiting/test/TestPartitionedRateLimiter.cs +++ b/src/Middleware/RateLimiting/test/TestPartitionedRateLimiter.cs @@ -12,18 +12,18 @@ namespace Microsoft.AspNetCore.RateLimiting; internal class TestPartitionedRateLimiter : PartitionedRateLimiter { - private List limiters = new List(); + private List _limiters = new List(); public TestPartitionedRateLimiter() { } public TestPartitionedRateLimiter(RateLimiter limiter) { - limiters.Add(limiter); + _limiters.Add(limiter); } public void AddLimiter(RateLimiter limiter) { - limiters.Add(limiter); + _limiters.Add(limiter); } public override RateLimiterStatistics GetStatistics(TResource resourceID) @@ -36,9 +36,9 @@ protected override RateLimitLease AttemptAcquireCore(TResource resourceID, int p if (permitCount != 1) { throw new ArgumentException("Tests only support 1 permit at a time"); - } + } var leases = new List(); - foreach (var limiter in limiters) + foreach (var limiter in _limiters) { var lease = limiter.AttemptAcquire(); if (lease.IsAcquired) @@ -64,7 +64,7 @@ protected override async ValueTask AcquireAsyncCore(TResource re throw new ArgumentException("Tests only support 1 permit at a time"); } var leases = new List(); - foreach (var limiter in limiters) + foreach (var limiter in _limiters) { leases.Add(await limiter.AcquireAsync()); } @@ -77,9 +77,8 @@ protected override async ValueTask AcquireAsyncCore(TResource re unusedLease.Dispose(); } return new TestRateLimitLease(false, null); - } + } } return new TestRateLimitLease(true, leases); - } } diff --git a/src/Servers/Kestrel/Core/src/Microsoft.AspNetCore.Server.Kestrel.Core.csproj b/src/Servers/Kestrel/Core/src/Microsoft.AspNetCore.Server.Kestrel.Core.csproj index f41aa21dc843..b2f421d9f5a6 100644 --- a/src/Servers/Kestrel/Core/src/Microsoft.AspNetCore.Server.Kestrel.Core.csproj +++ b/src/Servers/Kestrel/Core/src/Microsoft.AspNetCore.Server.Kestrel.Core.csproj @@ -20,7 +20,6 @@ - From 14905fa801d3d3fe1db5ddc278627812b35e6a16 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Tue, 18 Apr 2023 15:20:58 +0800 Subject: [PATCH 2/6] Update --- src/Middleware/RateLimiting/test/TestPartitionedRateLimiter.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Middleware/RateLimiting/test/TestPartitionedRateLimiter.cs b/src/Middleware/RateLimiting/test/TestPartitionedRateLimiter.cs index ea9082858fd3..f84dc427e692 100644 --- a/src/Middleware/RateLimiting/test/TestPartitionedRateLimiter.cs +++ b/src/Middleware/RateLimiting/test/TestPartitionedRateLimiter.cs @@ -12,7 +12,7 @@ namespace Microsoft.AspNetCore.RateLimiting; internal class TestPartitionedRateLimiter : PartitionedRateLimiter { - private List _limiters = new List(); + private readonly List _limiters = new List(); public TestPartitionedRateLimiter() { } From e834f81bdecfd3618e7b46ba77b57ce3b4c606d7 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Tue, 18 Apr 2023 15:35:57 +0800 Subject: [PATCH 3/6] Fix build --- .../Core/src/Microsoft.AspNetCore.Server.Kestrel.Core.csproj | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Servers/Kestrel/Core/src/Microsoft.AspNetCore.Server.Kestrel.Core.csproj b/src/Servers/Kestrel/Core/src/Microsoft.AspNetCore.Server.Kestrel.Core.csproj index b2f421d9f5a6..e4b328c02a61 100644 --- a/src/Servers/Kestrel/Core/src/Microsoft.AspNetCore.Server.Kestrel.Core.csproj +++ b/src/Servers/Kestrel/Core/src/Microsoft.AspNetCore.Server.Kestrel.Core.csproj @@ -20,7 +20,6 @@ - From d6d935f7eae41fcc6acf4294b937192b283af054 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Sat, 29 Apr 2023 08:58:34 +0800 Subject: [PATCH 4/6] API review feedback --- .../RateLimiting/src/RateLimitingMetrics.cs | 56 +++++++++++-------- .../src/RateLimitingMiddleware.cs | 27 +++++---- .../test/RateLimitingMetricsTests.cs | 43 +++++++------- 3 files changed, 73 insertions(+), 53 deletions(-) diff --git a/src/Middleware/RateLimiting/src/RateLimitingMetrics.cs b/src/Middleware/RateLimiting/src/RateLimitingMetrics.cs index 00d62159d3e1..b92eee18a93a 100644 --- a/src/Middleware/RateLimiting/src/RateLimitingMetrics.cs +++ b/src/Middleware/RateLimiting/src/RateLimitingMetrics.cs @@ -24,16 +24,16 @@ public RateLimitingMetrics(IMeterFactory meterFactory) _meter = meterFactory.CreateMeter(MeterName); _currentLeaseRequestsCounter = _meter.CreateUpDownCounter( - "current-lease-requests", + "current-leased-requests", description: "Number of HTTP requests that are currently active on the server that hold a rate limiting lease."); _leaseRequestDurationCounter = _meter.CreateHistogram( - "lease-request-duration", + "leased-request-duration", unit: "s", description: "The duration of rate limiting leases held by HTTP requests on the server."); _currentRequestsQueuedCounter = _meter.CreateUpDownCounter( - "current-requests-queued", + "current-queued-requests", description: "Number of HTTP requests that are currently queued, waiting to acquire a rate limiting lease."); _queuedRequestDurationCounter = _meter.CreateHistogram( @@ -46,52 +46,52 @@ 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 void LeaseFailed(string? policyName, string? route, RequestRejectionReason reason) + public void LeaseFailed(string? policyName, string? method, string? route, RequestRejectionReason reason) { if (_leaseFailedRequestsCounter.Enabled) { - LeaseFailedCore(policyName, route, reason); + LeaseFailedCore(policyName, method, route, reason); } } [MethodImpl(MethodImplOptions.NoInlining)] - private void LeaseFailedCore(string? policyName, string? route, RequestRejectionReason reason) + private void LeaseFailedCore(string? policyName, string? method, string? route, RequestRejectionReason reason) { var tags = new TagList(); - InitializeRateLimitingTags(ref tags, policyName, route); + InitializeRateLimitingTags(ref tags, policyName, method, route); tags.Add("reason", reason.ToString()); _leaseFailedRequestsCounter.Add(1, tags); } - public void LeaseStart(string? policyName, string? route) + public void LeaseStart(string? policyName, string? method, string? route) { if (_currentLeaseRequestsCounter.Enabled) { - LeaseStartCore(policyName, route); + LeaseStartCore(policyName, method, route); } } [MethodImpl(MethodImplOptions.NoInlining)] - private void LeaseStartCore(string? policyName, string? route) + private void LeaseStartCore(string? policyName, string? method, string? route) { var tags = new TagList(); - InitializeRateLimitingTags(ref tags, policyName, route); + InitializeRateLimitingTags(ref tags, policyName, method, route); _currentLeaseRequestsCounter.Add(1, tags); } - public void LeaseEnd(string? policyName, string? route, long startTimestamp, long currentTimestamp) + public void LeaseEnd(string? policyName, string? method, string? route, long startTimestamp, long currentTimestamp) { if (_currentLeaseRequestsCounter.Enabled || _leaseRequestDurationCounter.Enabled) { - LeaseEndCore(policyName, route, startTimestamp, currentTimestamp); + LeaseEndCore(policyName, method, route, startTimestamp, currentTimestamp); } } [MethodImpl(MethodImplOptions.NoInlining)] - private void LeaseEndCore(string? policyName, string? route, long startTimestamp, long currentTimestamp) + private void LeaseEndCore(string? policyName, string? method, string? route, long startTimestamp, long currentTimestamp) { var tags = new TagList(); - InitializeRateLimitingTags(ref tags, policyName, route); + InitializeRateLimitingTags(ref tags, policyName, method, route); _currentLeaseRequestsCounter.Add(-1, tags); @@ -99,37 +99,41 @@ private void LeaseEndCore(string? policyName, string? route, long startTimestamp _leaseRequestDurationCounter.Record(duration.TotalSeconds, tags); } - public void QueueStart(string? policyName, string? route) + public void QueueStart(string? policyName, string? method, string? route) { if (_currentRequestsQueuedCounter.Enabled) { - QueueStartCore(policyName, route); + QueueStartCore(policyName, method, route); } } [MethodImpl(MethodImplOptions.NoInlining)] - private void QueueStartCore(string? policyName, string? route) + private void QueueStartCore(string? policyName, string? method, string? route) { var tags = new TagList(); - InitializeRateLimitingTags(ref tags, policyName, route); + InitializeRateLimitingTags(ref tags, policyName, method, route); _currentRequestsQueuedCounter.Add(1, tags); } - public void QueueEnd(string? policyName, string? route, long startTimestamp, long currentTimestamp) + public void QueueEnd(string? policyName, string? method, string? route, RequestRejectionReason? reason, long startTimestamp, long currentTimestamp) { if (_currentRequestsQueuedCounter.Enabled || _queuedRequestDurationCounter.Enabled) { - QueueEndCore(policyName, route, startTimestamp, currentTimestamp); + QueueEndCore(policyName, method, route, reason, startTimestamp, currentTimestamp); } } [MethodImpl(MethodImplOptions.NoInlining)] - private void QueueEndCore(string? policyName, string? route, long startTimestamp, long currentTimestamp) + private void QueueEndCore(string? policyName, string? method, string? route, RequestRejectionReason? reason, long startTimestamp, long currentTimestamp) { var tags = new TagList(); - InitializeRateLimitingTags(ref tags, policyName, route); + InitializeRateLimitingTags(ref tags, policyName, method, route); _currentRequestsQueuedCounter.Add(-1, tags); + if (reason != null) + { + tags.Add("reason", reason.Value.ToString()); + } var duration = Stopwatch.GetElapsedTime(startTimestamp, currentTimestamp); _queuedRequestDurationCounter.Record(duration.TotalSeconds, tags); } @@ -139,12 +143,16 @@ public void Dispose() _meter.Dispose(); } - private static void InitializeRateLimitingTags(ref TagList tags, string? policyName, string? route) + private static void InitializeRateLimitingTags(ref TagList tags, string? policyName, string? method, string? route) { if (policyName is not null) { tags.Add("policy", policyName); } + if (method is not null) + { + tags.Add("method", method); + } if (route is not null) { tags.Add("route", route); diff --git a/src/Middleware/RateLimiting/src/RateLimitingMiddleware.cs b/src/Middleware/RateLimiting/src/RateLimitingMiddleware.cs index cdd0de49b9b1..297f659bcc10 100644 --- a/src/Middleware/RateLimiting/src/RateLimitingMiddleware.cs +++ b/src/Middleware/RateLimiting/src/RateLimitingMiddleware.cs @@ -77,13 +77,19 @@ public Task Invoke(HttpContext context) { return _next(context); } - return InvokeInternal(context, enableRateLimitingAttribute, endpoint?.Metadata.GetMetadata()?.Route); + + // Only include route and method if we have an endpoint with a route. + // A request always has a HTTP request, but it isn't useful unless combined with a route. + var route = endpoint?.Metadata.GetMetadata()?.Route; + var method = route is not null ? context.Request.Method : null; + + return InvokeInternal(context, enableRateLimitingAttribute, method, route); } - private async Task InvokeInternal(HttpContext context, EnableRateLimitingAttribute? enableRateLimitingAttribute, string? route) + private async Task InvokeInternal(HttpContext context, EnableRateLimitingAttribute? enableRateLimitingAttribute, string? method, string? route) { var policyName = enableRateLimitingAttribute?.PolicyName; - using var leaseContext = await TryAcquireAsync(context, policyName, route); + using var leaseContext = await TryAcquireAsync(context, policyName, method, route); if (leaseContext.Lease?.IsAcquired == true) { @@ -91,17 +97,17 @@ private async Task InvokeInternal(HttpContext context, EnableRateLimitingAttribu try { - _metrics.LeaseStart(policyName, route); + _metrics.LeaseStart(policyName, method, route); await _next(context); } finally { - _metrics.LeaseEnd(policyName, route, startTimestamp, Stopwatch.GetTimestamp()); + _metrics.LeaseEnd(policyName, method, route, startTimestamp, Stopwatch.GetTimestamp()); } } else { - _metrics.LeaseFailed(policyName, route, leaseContext.RequestRejectionReason!.Value); + _metrics.LeaseFailed(policyName, method, route, leaseContext.RequestRejectionReason!.Value); // If the request was canceled, do not call OnRejected, just return. if (leaseContext.RequestRejectionReason == RequestRejectionReason.RequestCanceled) @@ -140,7 +146,7 @@ private async Task InvokeInternal(HttpContext context, EnableRateLimitingAttribu } } - private async ValueTask TryAcquireAsync(HttpContext context, string? policyName, string? route) + private async ValueTask TryAcquireAsync(HttpContext context, string? policyName, string? method, string? route) { var leaseContext = CombinedAcquire(context); if (leaseContext.Lease?.IsAcquired == true) @@ -158,12 +164,13 @@ private async ValueTask TryAcquireAsync(HttpContext context, strin var startTimestamp = Stopwatch.GetTimestamp(); try { - _metrics.QueueStart(policyName, route); - return await waitTask; + _metrics.QueueStart(policyName, method, route); + leaseContext = await waitTask; + return leaseContext; } finally { - _metrics.QueueEnd(policyName, route, startTimestamp, Stopwatch.GetTimestamp()); + _metrics.QueueEnd(policyName, method, route, leaseContext.RequestRejectionReason, startTimestamp, Stopwatch.GetTimestamp()); } } diff --git a/src/Middleware/RateLimiting/test/RateLimitingMetricsTests.cs b/src/Middleware/RateLimiting/test/RateLimitingMetricsTests.cs index decb07444c3e..27a3114823cd 100644 --- a/src/Middleware/RateLimiting/test/RateLimitingMetricsTests.cs +++ b/src/Middleware/RateLimiting/test/RateLimitingMetricsTests.cs @@ -35,9 +35,9 @@ public async Task Metrics_Rejected() var context = new DefaultHttpContext(); - using var leaseRequestDurationRecorder = new InstrumentRecorder(meterRegistry, RateLimitingMetrics.MeterName, "lease-request-duration"); - using var currentLeaseRequestsRecorder = new InstrumentRecorder(meterRegistry, RateLimitingMetrics.MeterName, "current-lease-requests"); - using var currentRequestsQueuedRecorder = new InstrumentRecorder(meterRegistry, RateLimitingMetrics.MeterName, "current-requests-queued"); + using var leaseRequestDurationRecorder = new InstrumentRecorder(meterRegistry, RateLimitingMetrics.MeterName, "leased-request-duration"); + using var currentLeaseRequestsRecorder = new InstrumentRecorder(meterRegistry, RateLimitingMetrics.MeterName, "current-leased-requests"); + using var currentRequestsQueuedRecorder = new InstrumentRecorder(meterRegistry, RateLimitingMetrics.MeterName, "current-queued-requests"); using var queuedRequestDurationRecorder = new InstrumentRecorder(meterRegistry, RateLimitingMetrics.MeterName, "queued-request-duration"); using var leaseFailedRequestsRecorder = new InstrumentRecorder(meterRegistry, RateLimitingMetrics.MeterName, "lease-failed-requests"); @@ -81,10 +81,11 @@ public async Task Metrics_Success() var meter = meterFactory.Meters.Single(); var context = new DefaultHttpContext(); + context.Request.Method = "GET"; - using var leaseRequestDurationRecorder = new InstrumentRecorder(meterRegistry, RateLimitingMetrics.MeterName, "lease-request-duration"); - using var currentLeaseRequestsRecorder = new InstrumentRecorder(meterRegistry, RateLimitingMetrics.MeterName, "current-lease-requests"); - using var currentRequestsQueuedRecorder = new InstrumentRecorder(meterRegistry, RateLimitingMetrics.MeterName, "current-requests-queued"); + using var leaseRequestDurationRecorder = new InstrumentRecorder(meterRegistry, RateLimitingMetrics.MeterName, "leased-request-duration"); + using var currentLeaseRequestsRecorder = new InstrumentRecorder(meterRegistry, RateLimitingMetrics.MeterName, "current-leased-requests"); + using var currentRequestsQueuedRecorder = new InstrumentRecorder(meterRegistry, RateLimitingMetrics.MeterName, "current-queued-requests"); using var queuedRequestDurationRecorder = new InstrumentRecorder(meterRegistry, RateLimitingMetrics.MeterName, "queued-request-duration"); using var leaseFailedRequestsRecorder = new InstrumentRecorder(meterRegistry, RateLimitingMetrics.MeterName, "lease-failed-requests"); @@ -94,7 +95,7 @@ public async Task Metrics_Success() await syncPoint.WaitForSyncPoint().DefaultTimeout(); Assert.Collection(currentLeaseRequestsRecorder.GetMeasurements(), - m => AssertCounter(m, 1, null, null)); + m => AssertCounter(m, 1, null, null, null)); Assert.Empty(leaseRequestDurationRecorder.GetMeasurements()); syncPoint.Continue(); @@ -105,10 +106,10 @@ public async Task Metrics_Success() Assert.Equal(StatusCodes.Status200OK, context.Response.StatusCode); Assert.Collection(currentLeaseRequestsRecorder.GetMeasurements(), - m => AssertCounter(m, 1, null, null), - m => AssertCounter(m, -1, null, null)); + m => AssertCounter(m, 1, null, null, null), + m => AssertCounter(m, -1, null, null, null)); Assert.Collection(leaseRequestDurationRecorder.GetMeasurements(), - m => AssertDuration(m, null, null)); + m => AssertDuration(m, null, null, null)); Assert.Empty(currentRequestsQueuedRecorder.GetMeasurements()); Assert.Empty(queuedRequestDurationRecorder.GetMeasurements()); Assert.Empty(leaseFailedRequestsRecorder.GetMeasurements()); @@ -148,14 +149,15 @@ public async Task Metrics_Queued() routeEndpointBuilder.Metadata.Add(new EnableRateLimitingAttribute("concurrencyPolicy")); var endpoint = routeEndpointBuilder.Build(); - using var leaseRequestDurationRecorder = new InstrumentRecorder(meterRegistry, RateLimitingMetrics.MeterName, "lease-request-duration"); - using var currentLeaseRequestsRecorder = new InstrumentRecorder(meterRegistry, RateLimitingMetrics.MeterName, "current-lease-requests"); - using var currentRequestsQueuedRecorder = new InstrumentRecorder(meterRegistry, RateLimitingMetrics.MeterName, "current-requests-queued"); + using var leaseRequestDurationRecorder = new InstrumentRecorder(meterRegistry, RateLimitingMetrics.MeterName, "leased-request-duration"); + using var currentLeaseRequestsRecorder = new InstrumentRecorder(meterRegistry, RateLimitingMetrics.MeterName, "current-leased-requests"); + using var currentRequestsQueuedRecorder = new InstrumentRecorder(meterRegistry, RateLimitingMetrics.MeterName, "current-queued-requests"); using var queuedRequestDurationRecorder = new InstrumentRecorder(meterRegistry, RateLimitingMetrics.MeterName, "queued-request-duration"); using var leaseFailedRequestsRecorder = new InstrumentRecorder(meterRegistry, RateLimitingMetrics.MeterName, "lease-failed-requests"); // Act var context1 = new DefaultHttpContext(); + context1.Request.Method = "GET"; context1.SetEndpoint(endpoint); var middlewareTask1 = middleware.Invoke(context1); @@ -163,12 +165,13 @@ public async Task Metrics_Queued() await syncPoint.WaitForSyncPoint().DefaultTimeout(); var context2 = new DefaultHttpContext(); + context2.Request.Method = "GET"; context2.SetEndpoint(endpoint); var middlewareTask2 = middleware.Invoke(context1); // Assert second request is queued. Assert.Collection(currentRequestsQueuedRecorder.GetMeasurements(), - m => AssertCounter(m, 1, "/", "concurrencyPolicy")); + m => AssertCounter(m, 1, "GET", "/", "concurrencyPolicy")); Assert.Empty(queuedRequestDurationRecorder.GetMeasurements()); // Allow both requests to finish. @@ -178,22 +181,24 @@ public async Task Metrics_Queued() await middlewareTask2.DefaultTimeout(); Assert.Collection(currentRequestsQueuedRecorder.GetMeasurements(), - m => AssertCounter(m, 1, "/", "concurrencyPolicy"), - m => AssertCounter(m, -1, "/", "concurrencyPolicy")); + m => AssertCounter(m, 1, "GET", "/", "concurrencyPolicy"), + m => AssertCounter(m, -1, "GET", "/", "concurrencyPolicy")); Assert.Collection(queuedRequestDurationRecorder.GetMeasurements(), - m => AssertDuration(m, "/", "concurrencyPolicy")); + m => AssertDuration(m, "GET", "/", "concurrencyPolicy")); } - private static void AssertCounter(Measurement measurement, long value, string route, string policy) + private static void AssertCounter(Measurement measurement, long value, string method, string route, string policy) { Assert.Equal(value, measurement.Value); + AssertTag(measurement.Tags, "method", method); AssertTag(measurement.Tags, "route", route); AssertTag(measurement.Tags, "policy", policy); } - private static void AssertDuration(Measurement measurement, string route, string policy) + private static void AssertDuration(Measurement measurement, string method, string route, string policy) { Assert.True(measurement.Value > 0); + AssertTag(measurement.Tags, "method", method); AssertTag(measurement.Tags, "route", route); AssertTag(measurement.Tags, "policy", policy); } From f514919b2610fa75b19f8850e891ea3c5462a6cf Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Tue, 2 May 2023 16:58:53 +0800 Subject: [PATCH 5/6] PR feedback --- .../RateLimiting/src/MetricsContext.cs | 22 ++++ .../RateLimiting/src/RateLimitingMetrics.cs | 110 +++++++++------- .../src/RateLimitingMiddleware.cs | 23 ++-- .../test/RateLimitingMetricsTests.cs | 117 ++++++++++++++++++ .../Http2/Http2RequestTests.cs | 1 + 5 files changed, 219 insertions(+), 54 deletions(-) create mode 100644 src/Middleware/RateLimiting/src/MetricsContext.cs diff --git a/src/Middleware/RateLimiting/src/MetricsContext.cs b/src/Middleware/RateLimiting/src/MetricsContext.cs new file mode 100644 index 000000000000..36995e5a53a2 --- /dev/null +++ b/src/Middleware/RateLimiting/src/MetricsContext.cs @@ -0,0 +1,22 @@ +// 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; + +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 MetricsContext(string? policyName, string? method, string? route, bool currentLeaseRequestsCounterEnabled, bool currentRequestsQueuedCounterEnabled) + { + PolicyName = policyName; + Method = method; + Route = route; + CurrentLeaseRequestsCounterEnabled = currentLeaseRequestsCounterEnabled; + CurrentRequestsQueuedCounterEnabled = currentRequestsQueuedCounterEnabled; + } +} diff --git a/src/Middleware/RateLimiting/src/RateLimitingMetrics.cs b/src/Middleware/RateLimiting/src/RateLimitingMetrics.cs index b92eee18a93a..0816999cdf6e 100644 --- a/src/Middleware/RateLimiting/src/RateLimitingMetrics.cs +++ b/src/Middleware/RateLimiting/src/RateLimitingMetrics.cs @@ -13,9 +13,9 @@ internal sealed class RateLimitingMetrics : IDisposable public const string MeterName = "Microsoft.AspNetCore.RateLimiting"; private readonly Meter _meter; - private readonly UpDownCounter _currentLeaseRequestsCounter; - private readonly Histogram _leaseRequestDurationCounter; - private readonly UpDownCounter _currentRequestsQueuedCounter; + private readonly UpDownCounter _currentLeasedRequestsCounter; + private readonly Histogram _leasedRequestDurationCounter; + private readonly UpDownCounter _currentQueuedRequestsCounter; private readonly Histogram _queuedRequestDurationCounter; private readonly Counter _leaseFailedRequestsCounter; @@ -23,16 +23,16 @@ public RateLimitingMetrics(IMeterFactory meterFactory) { _meter = meterFactory.CreateMeter(MeterName); - _currentLeaseRequestsCounter = _meter.CreateUpDownCounter( + _currentLeasedRequestsCounter = _meter.CreateUpDownCounter( "current-leased-requests", description: "Number of HTTP requests that are currently active on the server that hold a rate limiting lease."); - _leaseRequestDurationCounter = _meter.CreateHistogram( + _leasedRequestDurationCounter = _meter.CreateHistogram( "leased-request-duration", unit: "s", description: "The duration of rate limiting leases held by HTTP requests on the server."); - _currentRequestsQueuedCounter = _meter.CreateUpDownCounter( + _currentQueuedRequestsCounter = _meter.CreateUpDownCounter( "current-queued-requests", description: "Number of HTTP requests that are currently queued, waiting to acquire a rate limiting lease."); @@ -46,96 +46,112 @@ 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 void LeaseFailed(string? policyName, string? method, string? route, RequestRejectionReason reason) + public bool CurrentLeasedRequestsCounterEnabled => _currentLeasedRequestsCounter.Enabled; + public bool CurrentQueuedRequestsCounterEnabled => _currentQueuedRequestsCounter.Enabled; + + public void LeaseFailed(in MetricsContext metricsContext, RequestRejectionReason reason) { if (_leaseFailedRequestsCounter.Enabled) { - LeaseFailedCore(policyName, method, route, reason); + LeaseFailedCore(metricsContext, reason); } } [MethodImpl(MethodImplOptions.NoInlining)] - private void LeaseFailedCore(string? policyName, string? method, string? route, RequestRejectionReason reason) + private void LeaseFailedCore(in MetricsContext metricsContext, RequestRejectionReason reason) { var tags = new TagList(); - InitializeRateLimitingTags(ref tags, policyName, method, route); + InitializeRateLimitingTags(ref tags, metricsContext); tags.Add("reason", reason.ToString()); _leaseFailedRequestsCounter.Add(1, tags); } - public void LeaseStart(string? policyName, string? method, string? route) + public void LeaseStart(in MetricsContext metricsContext) { - if (_currentLeaseRequestsCounter.Enabled) + if (metricsContext.CurrentLeaseRequestsCounterEnabled) { - LeaseStartCore(policyName, method, route); + LeaseStartCore(metricsContext); } } [MethodImpl(MethodImplOptions.NoInlining)] - private void LeaseStartCore(string? policyName, string? method, string? route) + public void LeaseStartCore(in MetricsContext metricsContext) { var tags = new TagList(); - InitializeRateLimitingTags(ref tags, policyName, method, route); - _currentLeaseRequestsCounter.Add(1, tags); + InitializeRateLimitingTags(ref tags, metricsContext); + _currentLeasedRequestsCounter.Add(1, tags); } - public void LeaseEnd(string? policyName, string? method, string? route, long startTimestamp, long currentTimestamp) + public void LeaseEnd(in MetricsContext metricsContext, long startTimestamp, long currentTimestamp) { - if (_currentLeaseRequestsCounter.Enabled || _leaseRequestDurationCounter.Enabled) + if (metricsContext.CurrentLeaseRequestsCounterEnabled || _leasedRequestDurationCounter.Enabled) { - LeaseEndCore(policyName, method, route, startTimestamp, currentTimestamp); + LeaseEndCore(metricsContext, startTimestamp, currentTimestamp); } } [MethodImpl(MethodImplOptions.NoInlining)] - private void LeaseEndCore(string? policyName, string? method, string? route, long startTimestamp, long currentTimestamp) + private void LeaseEndCore(in MetricsContext metricsContext, long startTimestamp, long currentTimestamp) { var tags = new TagList(); - InitializeRateLimitingTags(ref tags, policyName, method, route); + InitializeRateLimitingTags(ref tags, metricsContext); - _currentLeaseRequestsCounter.Add(-1, tags); + if (metricsContext.CurrentLeaseRequestsCounterEnabled) + { + _currentLeasedRequestsCounter.Add(-1, tags); + } - var duration = Stopwatch.GetElapsedTime(startTimestamp, currentTimestamp); - _leaseRequestDurationCounter.Record(duration.TotalSeconds, tags); + if (_leasedRequestDurationCounter.Enabled) + { + var duration = Stopwatch.GetElapsedTime(startTimestamp, currentTimestamp); + _leasedRequestDurationCounter.Record(duration.TotalSeconds, tags); + } } - public void QueueStart(string? policyName, string? method, string? route) + public void QueueStart(in MetricsContext metricsContext) { - if (_currentRequestsQueuedCounter.Enabled) + if (_currentQueuedRequestsCounter.Enabled) { - QueueStartCore(policyName, method, route); + QueueStartCore(metricsContext); } } [MethodImpl(MethodImplOptions.NoInlining)] - private void QueueStartCore(string? policyName, string? method, string? route) + private void QueueStartCore(in MetricsContext metricsContext) { var tags = new TagList(); - InitializeRateLimitingTags(ref tags, policyName, method, route); - _currentRequestsQueuedCounter.Add(1, tags); + InitializeRateLimitingTags(ref tags, metricsContext); + _currentQueuedRequestsCounter.Add(1, tags); } - public void QueueEnd(string? policyName, string? method, string? route, RequestRejectionReason? reason, long startTimestamp, long currentTimestamp) + public void QueueEnd(in MetricsContext metricsContext, RequestRejectionReason? reason, long startTimestamp, long currentTimestamp) { - if (_currentRequestsQueuedCounter.Enabled || _queuedRequestDurationCounter.Enabled) + if (metricsContext.CurrentRequestsQueuedCounterEnabled || _queuedRequestDurationCounter.Enabled) { - QueueEndCore(policyName, method, route, reason, startTimestamp, currentTimestamp); + QueueEndCore(metricsContext, reason, startTimestamp, currentTimestamp); } } [MethodImpl(MethodImplOptions.NoInlining)] - private void QueueEndCore(string? policyName, string? method, string? route, RequestRejectionReason? reason, long startTimestamp, long currentTimestamp) + private void QueueEndCore(in MetricsContext metricsContext, RequestRejectionReason? reason, long startTimestamp, long currentTimestamp) { var tags = new TagList(); - InitializeRateLimitingTags(ref tags, policyName, method, route); - _currentRequestsQueuedCounter.Add(-1, tags); + InitializeRateLimitingTags(ref tags, metricsContext); + + if (metricsContext.CurrentRequestsQueuedCounterEnabled) + { + _currentQueuedRequestsCounter.Add(-1, tags); + } - if (reason != null) + if (_queuedRequestDurationCounter.Enabled) { - tags.Add("reason", reason.Value.ToString()); + if (reason != null) + { + tags.Add("reason", reason.Value.ToString()); + } + var duration = Stopwatch.GetElapsedTime(startTimestamp, currentTimestamp); + _queuedRequestDurationCounter.Record(duration.TotalSeconds, tags); } - var duration = Stopwatch.GetElapsedTime(startTimestamp, currentTimestamp); - _queuedRequestDurationCounter.Record(duration.TotalSeconds, tags); } public void Dispose() @@ -143,19 +159,19 @@ public void Dispose() _meter.Dispose(); } - private static void InitializeRateLimitingTags(ref TagList tags, string? policyName, string? method, string? route) + private static void InitializeRateLimitingTags(ref TagList tags, in MetricsContext metricsContext) { - if (policyName is not null) + if (metricsContext.PolicyName is not null) { - tags.Add("policy", policyName); + tags.Add("policy", metricsContext.PolicyName); } - if (method is not null) + if (metricsContext.Method is not null) { - tags.Add("method", method); + tags.Add("method", metricsContext.Method); } - if (route is not null) + if (metricsContext.Route is not null) { - tags.Add("route", route); + tags.Add("route", metricsContext.Route); } } } diff --git a/src/Middleware/RateLimiting/src/RateLimitingMiddleware.cs b/src/Middleware/RateLimiting/src/RateLimitingMiddleware.cs index 297f659bcc10..2e494b8c5449 100644 --- a/src/Middleware/RateLimiting/src/RateLimitingMiddleware.cs +++ b/src/Middleware/RateLimiting/src/RateLimitingMiddleware.cs @@ -89,25 +89,34 @@ public Task Invoke(HttpContext context) private async Task InvokeInternal(HttpContext context, EnableRateLimitingAttribute? enableRateLimitingAttribute, string? method, string? route) { var policyName = enableRateLimitingAttribute?.PolicyName; - using var leaseContext = await TryAcquireAsync(context, policyName, method, route); + + // Cache the up/down counter enabled state at the start of the middleware. + // 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); + + using var leaseContext = await TryAcquireAsync(context, metricsContext); if (leaseContext.Lease?.IsAcquired == true) { var startTimestamp = Stopwatch.GetTimestamp(); + var currentLeaseStart = _metrics.CurrentLeasedRequestsCounterEnabled; try { - _metrics.LeaseStart(policyName, method, route); + _metrics.LeaseStart(metricsContext); await _next(context); } finally { - _metrics.LeaseEnd(policyName, method, route, startTimestamp, Stopwatch.GetTimestamp()); + _metrics.LeaseEnd(metricsContext, startTimestamp, Stopwatch.GetTimestamp()); } } else { - _metrics.LeaseFailed(policyName, method, route, leaseContext.RequestRejectionReason!.Value); + _metrics.LeaseFailed(metricsContext, leaseContext.RequestRejectionReason!.Value); // If the request was canceled, do not call OnRejected, just return. if (leaseContext.RequestRejectionReason == RequestRejectionReason.RequestCanceled) @@ -146,7 +155,7 @@ private async Task InvokeInternal(HttpContext context, EnableRateLimitingAttribu } } - private async ValueTask TryAcquireAsync(HttpContext context, string? policyName, string? method, string? route) + private async ValueTask TryAcquireAsync(HttpContext context, MetricsContext metricsContext) { var leaseContext = CombinedAcquire(context); if (leaseContext.Lease?.IsAcquired == true) @@ -164,13 +173,13 @@ private async ValueTask TryAcquireAsync(HttpContext context, strin var startTimestamp = Stopwatch.GetTimestamp(); try { - _metrics.QueueStart(policyName, method, route); + _metrics.QueueStart(metricsContext); leaseContext = await waitTask; return leaseContext; } finally { - _metrics.QueueEnd(policyName, method, route, leaseContext.RequestRejectionReason, startTimestamp, Stopwatch.GetTimestamp()); + _metrics.QueueEnd(metricsContext, leaseContext.RequestRejectionReason, startTimestamp, Stopwatch.GetTimestamp()); } } diff --git a/src/Middleware/RateLimiting/test/RateLimitingMetricsTests.cs b/src/Middleware/RateLimiting/test/RateLimitingMetricsTests.cs index 27a3114823cd..9f5f7388daf9 100644 --- a/src/Middleware/RateLimiting/test/RateLimitingMetricsTests.cs +++ b/src/Middleware/RateLimiting/test/RateLimitingMetricsTests.cs @@ -115,6 +115,53 @@ public async Task Metrics_Success() Assert.Empty(leaseFailedRequestsRecorder.GetMeasurements()); } + [Fact] + public async Task Metrics_ListenInMiddleOfRequest_CurrentLeasesNotDecreased() + { + // Arrange + var syncPoint = new SyncPoint(); + + var meterFactory = new TestMeterFactory(); + var meterRegistry = new TestMeterRegistry(meterFactory.Meters); + + var options = CreateOptionsAccessor(); + options.Value.GlobalLimiter = new TestPartitionedRateLimiter(new TestRateLimiter(true)); + + var middleware = CreateTestRateLimitingMiddleware( + options, + meterFactory: meterFactory, + next: async c => + { + await syncPoint.WaitToContinue(); + }); + var meter = meterFactory.Meters.Single(); + + var context = new DefaultHttpContext(); + context.Request.Method = "GET"; + + // Act + var middlewareTask = middleware.Invoke(context); + + await syncPoint.WaitForSyncPoint().DefaultTimeout(); + + using var leaseRequestDurationRecorder = new InstrumentRecorder(meterRegistry, RateLimitingMetrics.MeterName, "leased-request-duration"); + using var currentLeaseRequestsRecorder = new InstrumentRecorder(meterRegistry, RateLimitingMetrics.MeterName, "current-leased-requests"); + using var currentRequestsQueuedRecorder = new InstrumentRecorder(meterRegistry, RateLimitingMetrics.MeterName, "current-queued-requests"); + using var queuedRequestDurationRecorder = new InstrumentRecorder(meterRegistry, RateLimitingMetrics.MeterName, "queued-request-duration"); + using var leaseFailedRequestsRecorder = new InstrumentRecorder(meterRegistry, RateLimitingMetrics.MeterName, "lease-failed-requests"); + + syncPoint.Continue(); + + await middlewareTask.DefaultTimeout(); + + // Assert + Assert.Equal(StatusCodes.Status200OK, context.Response.StatusCode); + + Assert.Empty(currentLeaseRequestsRecorder.GetMeasurements()); + Assert.Collection(leaseRequestDurationRecorder.GetMeasurements(), + m => AssertDuration(m, null, null, null)); + } + [Fact] public async Task Metrics_Queued() { @@ -187,6 +234,76 @@ public async Task Metrics_Queued() m => AssertDuration(m, "GET", "/", "concurrencyPolicy")); } + [Fact] + public async Task Metrics_ListenInMiddleOfQueued_CurrentQueueNotDecreased() + { + // Arrange + var syncPoint = new SyncPoint(); + + var meterFactory = new TestMeterFactory(); + var meterRegistry = new TestMeterRegistry(meterFactory.Meters); + + var services = new ServiceCollection(); + + services.AddRateLimiter(_ => _ + .AddConcurrencyLimiter(policyName: "concurrencyPolicy", options => + { + options.PermitLimit = 1; + options.QueueProcessingOrder = QueueProcessingOrder.OldestFirst; + options.QueueLimit = 1; + })); + var serviceProvider = services.BuildServiceProvider(); + + var middleware = CreateTestRateLimitingMiddleware( + serviceProvider.GetRequiredService>(), + meterFactory: meterFactory, + next: async c => + { + await syncPoint.WaitToContinue(); + }, + serviceProvider: serviceProvider); + var meter = meterFactory.Meters.Single(); + + var routeEndpointBuilder = new RouteEndpointBuilder(c => Task.CompletedTask, RoutePatternFactory.Parse("/"), 0); + routeEndpointBuilder.Metadata.Add(new EnableRateLimitingAttribute("concurrencyPolicy")); + var endpoint = routeEndpointBuilder.Build(); + + // Act + var context1 = new DefaultHttpContext(); + context1.Request.Method = "GET"; + context1.SetEndpoint(endpoint); + var middlewareTask1 = middleware.Invoke(context1); + + // Wait for first request to reach server and block it. + await syncPoint.WaitForSyncPoint().DefaultTimeout(); + + var context2 = new DefaultHttpContext(); + context2.Request.Method = "GET"; + context2.SetEndpoint(endpoint); + var middlewareTask2 = middleware.Invoke(context1); + + // Start listening while the second request is queued. + + using var leaseRequestDurationRecorder = new InstrumentRecorder(meterRegistry, RateLimitingMetrics.MeterName, "leased-request-duration"); + using var currentLeaseRequestsRecorder = new InstrumentRecorder(meterRegistry, RateLimitingMetrics.MeterName, "current-leased-requests"); + using var currentRequestsQueuedRecorder = new InstrumentRecorder(meterRegistry, RateLimitingMetrics.MeterName, "current-queued-requests"); + using var queuedRequestDurationRecorder = new InstrumentRecorder(meterRegistry, RateLimitingMetrics.MeterName, "queued-request-duration"); + using var leaseFailedRequestsRecorder = new InstrumentRecorder(meterRegistry, RateLimitingMetrics.MeterName, "lease-failed-requests"); + + Assert.Empty(currentRequestsQueuedRecorder.GetMeasurements()); + Assert.Empty(queuedRequestDurationRecorder.GetMeasurements()); + + // Allow both requests to finish. + syncPoint.Continue(); + + await middlewareTask1.DefaultTimeout(); + await middlewareTask2.DefaultTimeout(); + + Assert.Empty(currentRequestsQueuedRecorder.GetMeasurements()); + Assert.Collection(queuedRequestDurationRecorder.GetMeasurements(), + m => AssertDuration(m, "GET", "/", "concurrencyPolicy")); + } + private static void AssertCounter(Measurement measurement, long value, string method, string route, string policy) { Assert.Equal(value, measurement.Value); diff --git a/src/Servers/Kestrel/test/Interop.FunctionalTests/Http2/Http2RequestTests.cs b/src/Servers/Kestrel/test/Interop.FunctionalTests/Http2/Http2RequestTests.cs index 0109d41a2d42..0edf0ee3ea5d 100644 --- a/src/Servers/Kestrel/test/Interop.FunctionalTests/Http2/Http2RequestTests.cs +++ b/src/Servers/Kestrel/test/Interop.FunctionalTests/Http2/Http2RequestTests.cs @@ -115,6 +115,7 @@ public async Task GET_RequestReturnsLargeData_GracefulShutdownDuringRequest_Requ private static async Task<(byte[], HttpResponseHeaders)> StartLongRunningRequestAsync(ILogger logger, IHost host, HttpMessageInvoker client) { var request = new HttpRequestMessage(HttpMethod.Get, $"http://127.0.0.1:{host.GetPort()}/"); + request.Headers.Host = "localhost2"; request.Version = HttpVersion.Version20; request.VersionPolicy = HttpVersionPolicy.RequestVersionExact; From d4c1ba3269c6e5d42ae370a9885431434ed0597e Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Wed, 3 May 2023 09:23:20 +0800 Subject: [PATCH 6/6] Update src/Middleware/RateLimiting/src/RateLimitingMetrics.cs Co-authored-by: Stephen Halter --- src/Middleware/RateLimiting/src/RateLimitingMetrics.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Middleware/RateLimiting/src/RateLimitingMetrics.cs b/src/Middleware/RateLimiting/src/RateLimitingMetrics.cs index 0816999cdf6e..cf7bbb533e7a 100644 --- a/src/Middleware/RateLimiting/src/RateLimitingMetrics.cs +++ b/src/Middleware/RateLimiting/src/RateLimitingMetrics.cs @@ -110,7 +110,7 @@ private void LeaseEndCore(in MetricsContext metricsContext, long startTimestamp, public void QueueStart(in MetricsContext metricsContext) { - if (_currentQueuedRequestsCounter.Enabled) + if (metricsContext.CurrentRequestsQueuedCounterEnabled) { QueueStartCore(metricsContext); }