Microsoft.AspNetCore.RateLimiting RateLimiterPolicy GetPartitionAsync

[cristipufu]

Background and Motivation

I want to save API rate limits in the database and have the possibility of changing them at runtime (eg: based on the customer's subscription/licensing/etc).

The problem is that IRateLimiterPolicy doesn't have an async method

public class ClientIdRateLimiterPolicy : IRateLimiterPolicy<string>
{
    private readonly IServiceProvider _serviceProvider;

    public ClientIdRateLimiterPolicy(IServiceProvider serviceProvider)
    {
        _serviceProvider = serviceProvider;
    }

    public RateLimitPartition<string> GetPartition(HttpContext httpContext)
    {
        var clientId = httpContext.Request.Headers["X-ClientId"].ToString();

        using var scope = _serviceProvider.CreateScope();
        var dbContext = scope.ServiceProvider.GetRequiredService<SampleDbContext>();

        var rateLimit = dbContext.Clients.Where(x => x.Identifier == clientId).Select(x => x.RateLimit).FirstOrDefault();

        return RateLimitPartition.GetConcurrencyRateLimiter(clientId, key => new ConcurrencyRateLimiterOptions
        {
            PermitLimit = rateLimit?.PermitLimit ?? 1
        });
    }
}

Proposed API

public interface IRateLimiterPolicy<TPartitionKey>
{
    Func<OnRejectedContext, CancellationToken, ValueTask>? OnRejected { get; }

-   RateLimitPartition<TPartitionKey> GetPartition(HttpContext httpContext);
    
+   ValueTask<RateLimitPartition<TPartitionKey>> GetPartitionAsync(HttpContext httpContext);
}

Usage Examples

public class ClientIdRateLimiterPolicy : IRateLimiterPolicy<string>
{
    private readonly IServiceProvider _serviceProvider;

    public ClientIdRateLimiterPolicy(IServiceProvider serviceProvider)
    {
        _serviceProvider = serviceProvider;
    }

    public async ValueTask<RateLimitPartition<string>> GetPartitionAsync(HttpContext httpContext)
    {
        var clientId = httpContext.Request.Headers["X-ClientId"].ToString();

        using var scope = _serviceProvider.CreateScope();
        var dbContext = scope.ServiceProvider.GetRequiredService<SampleDbContext>();

        var rateLimit = await redisCache.GetAsync(clientId);
        var rateLimit = await dbContext.Clients.Where(x => x.Identifier == clientId).Select(x => x.RateLimit).FirstOrDefaultAsync();

        return RateLimitPartition.GetConcurrencyRateLimiter(clientId, key => new ConcurrencyRateLimiterOptions
        {
            PermitLimit = rateLimit?.PermitLimit ?? 1
        });
    }
}

Risks

Find a way w/o breaking changes

[halter73]

Thanks for submitting the API proposal and taking the time to fully fill out the template!

As you note in the risks section, removing a member from an interface is a breaking change which we try really hard to avoid. It is sometimes possible to add members with default implementations, but we also try to avoid that if it's unnecessary. This is particularly true if the new member would be mostly redundant.

This does seem pretty useful though. Particularly when you using our built-in in-memory RateLimitPartition with configuration from a database as you demonstrate in your example usage. You might want to avoid this pattern if the client could connect to multiple instances, but there are certainly scenarios where that is not the case. You also have to be cognizant of the fact that while GetPartition(Async) is called for ever rate-limited request, usually only the key gets used to reference the existing cached rate limiter. That's why "Get" is in RateLimitPartition.GetConcurrencyLimiter. So if the options change, it might not be picked up until the app restarts.

If we feel it's important to be able to asynchronously read rate limiting config, another option might be adding an API to convert a Task<RateLimiter> to a plain RateLimiter. In this scenario, RateLimiter.AcquireAsync would wait until the real inner rate limiter was available before delegating and RateLimiter.AttemptAcquire would return a failed lease.

Maybe PartitionedRateLimiter<TResource> could use similar treatment. The problem is likely to be discoverability. I'm not exactly sure where we'd put these methods or what to name them. It feels somewhat like PartitionedRateLimiter.CreateChained in that these would be static methods that return a rate limiter that delegate rate limiter calls, so it doesn't feel unprecedented.

Another problem is that a Task<RateLimitPartition<TKey>> to a RateLimitPartition<TKey> conversion isn't really possible, because the key needs to be synchronously retrievable. That doesn't seem to be a problem for the use case demonstrated above where the clientId is pulled from a request header, but I could see it mattering if you needed to asynchronously query what rate limiting group a request belonged to.

[mitchdenny]

I think it is important to distinguish between rate limiting for the purpose of protecting the app/service and rate limiting for the purpose of what I would call "accounting".

Whilst you could implement a rate limiter which makes a call to an external service to enforce "accounting limits" which might be derived from something like a payment tier it might be a better option to separate that logic from the kind of rate limits that you use to make sure that your service remains available.

That said having an async implementation GetPartition(...) would be useful.

[halter73]

This started as an edit to my previous comment, but it became big enough to deserve it's own comment for the folks following along via email.

I think your best bet for now is to read the rate-limit asynchronously before calling into the rate limiting middleware. You can look for the EnableRateLimitingAttribute to make sure you're not doing this unnecessarily when the endpoint doesn't have your policy defined. This attribute should be present in the endpoint metadata whether or not it was configured as an attribute on the method or via the RequireRateLimiting() extension method.

// ...

app.Use(async (httpContext, next) =>
{
    var metadata = httpContext.GetEndpoint()?.Metadata;
    if (metadata?.GetMetadata<EnableRateLimitingAttribute>()?.PolicyName == "my-client-id-policy"
        && metadata?.GetMetadata<DisableRateLimitingAttribute>() is null)
    {
        using var scope = httpContext.RequestServices.CreateScope();
        var dbContext = scope.ServiceProvider.GetRequiredService<SampleDbContext>();

        var rateLimit = await dbContext.Clients.Where(x => x.Identifier == clientId).Select(x => x.RateLimit).FirstOrDefaultAsync();
        httpContext.Items["rate-limit"] = rateLimit;
    }

    await next(httpContext);
});

app.UseRateLimiter();

// ...

public class ClientIdRateLimiterPolicy : IRateLimiterPolicy<string>
{
    public Func<OnRejectedContext, CancellationToken, ValueTask>? OnRejected => null;

    public RateLimitPartition<string> GetPartition(HttpContext httpContext)
    {
        var clientId = httpContext.Request.Headers["X-ClientId"].ToString();

        return RateLimitPartition.GetConcurrencyLimiter(clientId, key => new ConcurrencyLimiterOptions
        {
            PermitLimit = (int?)httpContext.Items["rate-limit"] ?? 1
        });
    }
}

The downside of this logic is that it will get run even if the middleware only needed the key and not the rate limiter instance from the factory. The originally proposed API has the same downside though.

Yet another alternative might be to add something like Func<TKey, HttpContext, ValueTask<RateLimiter>>? RateLimitPartition<TKey>.AsyncFactory and updating our callers to use that if it's set. We'd also need corresponding RateLimitPartition.GetConcurrencyLimiter overloads or similar that took async factories to go with it.

[adityamandaleeka]

Checking back in on this issue after the holidays. Is the workaround above sufficient or should we pursue the alternative at the end of @halter73's comment?

[adityamandaleeka]

Since we haven't heard anything, I'll assume the workaround is sufficient and close this issue. Let us know if this isn't the case.

[cristipufu]

I believe that the option to make an async call in cases like this should be added to the framework's APIs, the workaround is just a workaround