Skip to content

Rate limiting middleware - Statistics about rate limiters #44140

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
1 task done
maartenba opened this issue Sep 23, 2022 · 10 comments
Open
1 task done

Rate limiting middleware - Statistics about rate limiters #44140

maartenba opened this issue Sep 23, 2022 · 10 comments
Assignees
Labels
area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlewares area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions feature-rate-limit Work related to use of rate limit primitives

Comments

@maartenba
Copy link

maartenba commented Sep 23, 2022

Is there an existing issue for this?

  • I have searched the existing issues

Is your feature request related to a problem? Please describe the problem.

The ASP.NET Core rate limiting middleware is great, but "limited" in terms of what you an communicate with your users. Let's start with some code that you can write today in .NET 7:

builder.Services.AddRateLimiter(options =>
{
    options.OnRejected = async (context, token) =>
    {
        context.HttpContext.Response.StatusCode = 429;
        if (context.Lease.TryGetMetadata(MetadataName.RetryAfter, out var retryAfter))
        {
            await context.HttpContext.Response.WriteAsync(
                $"Too many requests. Please try again after {retryAfter.TotalMinutes} minute(s). " +
                $"Read more about our rate limits at https://example.org/docs/ratelimiting.", cancellationToken: token);
        }
        else
        {
            await context.HttpContext.Response.WriteAsync(
                "Too many requests. Please try again later. " +
                "Read more about our rate limits at https://example.org/docs/ratelimiting.", cancellationToken: token);
        }
    };

    // ...
});

When rate limits are triggered, a response is returned that tells the user they are rate limited, where to find more information, and (if the MetadataName.RetryAfter data is available), when to retry.

This is quite limited. There's no access to which rate limiter fired, and what its statistics are.

Additionally, the current RateLimitLease is only accessible when rate limiting is fired - not for every request. If you would want to return statistics about your limits (e.g. like GitHub does), you'll find it impossible to get statistics about the current lease in a custom middleware that can write out these additional headers.

Describe the solution you'd like

Here's a middleware that has access to some of data that I'd want to access:

public class RateLimitStatisticsMiddleware
{
    private readonly RequestDelegate _next;
    private readonly IOptions<RateLimiterOptions> _options;

    public RateLimitStatisticsMiddleware(RequestDelegate next, IOptions<RateLimiterOptions> options)
    {
        _next = next;
        _options = options;
    }

    public Task Invoke(HttpContext context)
    {
        // Note: This should also work on endpoint limiters, but those are not available.
        // There is no current "rate limit context" of sorts. Including the policy name etc.
        var globalLimiter = _options.Value.GlobalLimiter;
        if (globalLimiter != null)
        {
            var statistics = globalLimiter.GetStatistics(context);
            if (statistics != null)
            {
                // Note: It would be great to be able to get the TokenLimit from the "current rate limiter context"
                context.Response.Headers.Add("X-Rate-Limit-Limit", "20");

                // Note: It would be great to be able to get the Window from the "current rate limiter context"
                context.Response.Headers.Add("X-Rate-Limit-Reset", DateTimeOffset.UtcNow.ToString("O"));

                // This one is there today
                context.Response.Headers.Add("X-Rate-Limit-Remaining", statistics.CurrentAvailablePermits.ToString());
            }
        }

        return _next(context);
    }
}

The dream scenario for a better ASP.NET Core rate limiter middleware would be to:

  • Have access to more statistics about the rate limiter (which policy fired, what partition, what are the limiter's options so I can emit a message that says "you can only make 10 requests per minute", ...) in the rejected callbacks.
  • Have a feature on the current HttpContext that gives access to the current rate limit context, so these details can also be returned on successful requests, or added to telemetry.

Additional context

Most rate limiters in System.Threading.RateLimiting don't provide additional statistics. This feature will need changes in both ASP.NET Core and the .NET framework.

@adityamandaleeka
Copy link
Member

@wtgodbe wtgodbe self-assigned this Sep 26, 2022
@wtgodbe wtgodbe added this to the .NET 8 Planning milestone Sep 26, 2022
@ghost
Copy link

ghost commented Sep 26, 2022

Thanks for contacting us.

We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@wtgodbe
Copy link
Member

wtgodbe commented Sep 26, 2022

I definitely think we should do this for dotnet 8

@cristipufu
Copy link

Have a feature on the current HttpContext that gives access to the current rate limit context, so these details can also be returned on successful requests, or added to telemetry.

How would we handle this for multiple policies (eg global + endpoint)?

@maartenba
Copy link
Author

How does the runtime handle such case? I assume the most strict is applied?

@cristipufu
Copy link

cristipufu commented Dec 4, 2022

I think that

  • we should have an OnAcquired func where we would have access to the successful lease which stores metadata information about the current rate limiter, similar to the OnRejected func (this should be sufficient to return rate limiting http headers on successful requests as well) - the issue here would be with multiple rate limiters on the same endpoint, I'm not sure how easy it is to decide which one is the most strict, but I don't think it actually matters that much.

  • we should somehow have access to the cached rate limiters, I want to be able to call GetStatistics() on the application's rate limiters once in a while (in a background job) and map that data with OpenTelemetry.Metrics and show a nice graph of successful/failed requests per rate limiter with Promotheus. Thinking about it, this could also be achieved by writing a metric when OnAcquired/OnRejected and adding metadata about the current PartitionKey

@MadL1me
Copy link
Contributor

MadL1me commented Dec 12, 2022

@maartenba you said:

Most rate limiters in System.Threading.RateLimiting don't provide additional statistics. This feature will need changes in both ASP.NET Core and the .NET framework.

Can you please explain on the process how to do this? Do I need to send link to this issue in PR to other repo? Where I can find BCL (with rate .net base rate limiting) repo on a github? Do I need somehow to contact devs via issue submitting, or should I right away submit a PR? It would be great to have answers to those

@maartenba
Copy link
Author

@MadL1me The BCL classes are in https://github.com/dotnet/runtime/tree/main/src/libraries/System.Threading.RateLimiting

I am not Microsoft, so no idea how to do a PR across multiple repos/how to suggest such changes. @wtgodbe may be able to comment on that?

@wtgodbe
Copy link
Member

wtgodbe commented Dec 13, 2022

We ingest changes from dotnet/runtime via automated dependency updates. If your change to the BCL base limiter classes requires API change, you'll need to open a formal API proposal. Otherwise you can just open PR in dotnet/runtime and ask for review from myself, @BrennanConroy, and @Tratcher. Once your PR in is merged into dotnet/runtime, after a few hours an automated dependency update PR should get opened in this repo (will look like #45448). Once that's merged, your changes from dotnet/runtime will be available in this repo, and you can open your follow-up PR here.

@anandsowm
Copy link

Yes, would be very useful feature. Looking at the different classes involved, there is no way to know how many permits have been issued so far for a given partition key. That would be good to have. Also is there a way to retain the count of leases issued between system restarts? It is right now stored in-memory, try to provide a way to plug in a persistence component.

@amcasey amcasey added area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlewares area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Aug 24, 2023
@samsp-msft samsp-msft added the feature-rate-limit Work related to use of rate limit primitives label Feb 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlewares area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions feature-rate-limit Work related to use of rate limit primitives
Projects
None yet
Development

No branches or pull requests

12 participants