Remove route and method tags from rate limiting (#48707)

Author: JamesNK

src/Middleware/RateLimiting/src/MetricsContext.cs

@@ -6,16 +6,12 @@ namespace Microsoft.AspNetCore.RateLimiting;
 internal readonly struct MetricsContext
 {
     public readonly string? PolicyName;
-    public readonly string? Method;
-    public readonly string? Route;
     public readonly bool CurrentLeasedRequestsCounterEnabled;
     public readonly bool CurrentQueuedRequestsCounterEnabled;
 
-    public MetricsContext(string? policyName, string? method, string? route, bool currentLeasedRequestsCounterEnabled, bool currentQueuedRequestsCounterEnabled)
+    public MetricsContext(string? policyName, bool currentLeasedRequestsCounterEnabled, bool currentQueuedRequestsCounterEnabled)
     {
         PolicyName = policyName;
-        Method = method;
-        Route = route;
         CurrentLeasedRequestsCounterEnabled = currentLeasedRequestsCounterEnabled;
         CurrentQueuedRequestsCounterEnabled = currentQueuedRequestsCounterEnabled;
     }

src/Middleware/RateLimiting/src/RateLimitingMetrics.cs

@@ -162,18 +162,10 @@ private static void InitializeRateLimitingTags(ref TagList tags, in MetricsConte
         {
             tags.Add("policy", metricsContext.PolicyName);
         }
-        if (metricsContext.Method is not null)
-        {
-            tags.Add("method", metricsContext.Method);
-        }
-        if (metricsContext.Route is not null)
-        {
-            tags.Add("route", metricsContext.Route);
-        }
     }
 
-    public MetricsContext CreateContext(string? policyName, string? method, string? route)
+    public MetricsContext CreateContext(string? policyName)
     {
-        return new MetricsContext(policyName, method, route, _currentLeasedRequestsCounter.Enabled, _currentQueuedRequestsCounter.Enabled);
+        return new MetricsContext(policyName, _currentLeasedRequestsCounter.Enabled, _currentQueuedRequestsCounter.Enabled);
     }
 }

src/Middleware/RateLimiting/src/RateLimitingMiddleware.cs

@@ -4,7 +4,6 @@
 using System.Diagnostics;
 using System.Threading.RateLimiting;
 using Microsoft.AspNetCore.Http;
-using Microsoft.AspNetCore.Http.Metadata;
 using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Options;
 
@@ -78,23 +77,18 @@ public Task Invoke(HttpContext context)
             return _next(context);
         }
 
-        // 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<IRouteDiagnosticsMetadata>()?.Route;
-        var method = route is not null ? context.Request.Method : null;
-
-        return InvokeInternal(context, enableRateLimitingAttribute, method, route);
+        return InvokeInternal(context, enableRateLimitingAttribute);
     }
 
-    private async Task InvokeInternal(HttpContext context, EnableRateLimitingAttribute? enableRateLimitingAttribute, string? method, string? route)
+    private async Task InvokeInternal(HttpContext context, EnableRateLimitingAttribute? enableRateLimitingAttribute)
     {
         var policyName = enableRateLimitingAttribute?.PolicyName;
 
         // 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 = _metrics.CreateContext(policyName, method, route);
+        var metricsContext = _metrics.CreateContext(policyName);
 
         using var leaseContext = await TryAcquireAsync(context, metricsContext);
 

src/Middleware/RateLimiting/test/RateLimitingMetricsTests.cs

@@ -93,7 +93,7 @@ public async Task Metrics_Success()
         await syncPoint.WaitForSyncPoint().DefaultTimeout();
 
         Assert.Collection(currentLeaseRequestsRecorder.GetMeasurements(),
-            m => AssertCounter(m, 1, null, null, null));
+            m => AssertCounter(m, 1, null));
         Assert.Empty(leaseRequestDurationRecorder.GetMeasurements());
 
         syncPoint.Continue();
@@ -104,10 +104,10 @@ public async Task Metrics_Success()
         Assert.Equal(StatusCodes.Status200OK, context.Response.StatusCode);
 
         Assert.Collection(currentLeaseRequestsRecorder.GetMeasurements(),
-            m => AssertCounter(m, 1, null, null, null),
-            m => AssertCounter(m, -1, null, null, null));
+            m => AssertCounter(m, 1, null),
+            m => AssertCounter(m, -1, null));
         Assert.Collection(leaseRequestDurationRecorder.GetMeasurements(),
-            m => AssertDuration(m, null, null, null));
+            m => AssertDuration(m, null));
         Assert.Empty(currentRequestsQueuedRecorder.GetMeasurements());
         Assert.Empty(queuedRequestDurationRecorder.GetMeasurements());
         Assert.Empty(leaseFailedRequestsRecorder.GetMeasurements());
@@ -156,7 +156,7 @@ public async Task Metrics_ListenInMiddleOfRequest_CurrentLeasesNotDecreased()
 
         Assert.Empty(currentLeaseRequestsRecorder.GetMeasurements());
         Assert.Collection(leaseRequestDurationRecorder.GetMeasurements(),
-            m => AssertDuration(m, null, null, null));
+            m => AssertDuration(m, null));
     }
 
     [Fact]
@@ -214,7 +214,7 @@ public async Task Metrics_Queued()
 
         // Assert second request is queued.
         Assert.Collection(currentRequestsQueuedRecorder.GetMeasurements(),
-            m => AssertCounter(m, 1, "GET", "/", "concurrencyPolicy"));
+            m => AssertCounter(m, 1, "concurrencyPolicy"));
         Assert.Empty(queuedRequestDurationRecorder.GetMeasurements());
 
         // Allow both requests to finish.
@@ -224,10 +224,10 @@ public async Task Metrics_Queued()
         await middlewareTask2.DefaultTimeout();
 
         Assert.Collection(currentRequestsQueuedRecorder.GetMeasurements(),
-            m => AssertCounter(m, 1, "GET", "/", "concurrencyPolicy"),
-            m => AssertCounter(m, -1, "GET", "/", "concurrencyPolicy"));
+            m => AssertCounter(m, 1, "concurrencyPolicy"),
+            m => AssertCounter(m, -1, "concurrencyPolicy"));
         Assert.Collection(queuedRequestDurationRecorder.GetMeasurements(),
-            m => AssertDuration(m, "GET", "/", "concurrencyPolicy"));
+            m => AssertDuration(m, "concurrencyPolicy"));
     }
 
     [Fact]
@@ -296,22 +296,18 @@ public async Task Metrics_ListenInMiddleOfQueued_CurrentQueueNotDecreased()
 
         Assert.Empty(currentRequestsQueuedRecorder.GetMeasurements());
         Assert.Collection(queuedRequestDurationRecorder.GetMeasurements(),
-            m => AssertDuration(m, "GET", "/", "concurrencyPolicy"));
+            m => AssertDuration(m, "concurrencyPolicy"));
     }
 
-    private static void AssertCounter(Measurement<long> measurement, long value, string method, string route, string policy)
+    private static void AssertCounter(Measurement<long> measurement, long value, 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<double> measurement, string method, string route, string policy)
+    private static void AssertDuration(Measurement<double> measurement, string policy)
     {
         Assert.True(measurement.Value > 0);
-        AssertTag(measurement.Tags, "method", method);
-        AssertTag(measurement.Tags, "route", route);
         AssertTag(measurement.Tags, "policy", policy);
     }