dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.57k stars 10.05k forks source link

Support Multiple PartitionedRateLimiter Per Endpoint #42691

Open Kahbazi opened 2 years ago

Kahbazi commented 2 years ago

Is there an existing issue for this?

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

Right now RateLimiting middleware has two PartitionedRateLimiter. A global and an endpoint limiter. This means for each endpoint there could be at most two level of rate limiters and also the global one is somehow limited because it must be same for all endpoints. So I can not limit my endpoints based on more than two partitions. As an example I need to limit first based on request IP, then current user Id and then based on the current endpoint. Let's say I need to limit 10 requests per second per IP, no matter which endpoint. And also limit 5 requests per second per User Id.

Also I can't have different window and limit based on one partition. Again let's say I need to limit 10 requests per second per IP and also limit 40 requests per minute per IP.

Describe the solution you'd like

I'm suggesting to create PartitionedRateLimiter<HttpContext> based on policy and have a Dictionary<string, PartitionedRateLimiter<HttpContext> which policy is the key. The endpoints could have multiple IRequireRateLimitMetadata and every one of them is a policy with its own PartitionedRateLimiter. And the middleware would always call Acquire on each of these limiters whether IsAcquired is true or false and only limit the request if one of them has IsAcquired = false.

This way part of the limiter like checking based on User Id could be shared between multiple endpoints too.

Additional context

cc @wtgodbe @BrennanConroy @halter

wtgodbe commented 2 years ago

This should be achievable through the CreateChained API in runtime - it allows you to pass in multiple PartitionedRateLimiters which it will combine into 1 PartitionedRateLimiter that runs all of your input limiters in sequence. Then you could apply that single chained limiter to the endpoint.

wtgodbe commented 2 years ago

Going to close this as I believe it's covered by the above comment - feel free to re-open if there's additional functionality in this request that I'm missing.

Kahbazi commented 2 years ago

@wtgodbe I believe the issue should be re-opened. It seems ChainedPartitionedRateLimiter is the answer on how to implement my requirements (with a small change that I need to call Acquire on every limiter whether IsAcquired is true or false), but I don't see any way to achieve that using the current implementation of the middleware. Is there any sample on how to do that? And also I can't use the global limiter because I would need different ChainedPartitionedRateLimiter per endpoint.

wtgodbe commented 2 years ago

I see, I think you're right that we don't currently support that (endpoint policies are RateLimitPartition based, not PartitionedRateLimiter based). We'll consider this for future passes

adityamandaleeka commented 2 years ago

Triage: we're open to this but we'd like to see more use cases for this sort of pattern.

ghost commented 2 years ago

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

adityamandaleeka commented 2 years ago

We have another request for this: https://github.com/dotnet/aspnetcore/issues/44907

BondarencoM commented 2 years ago

I also want to rate limit an endpoint with one partition but multiple time spans (e.g. allow 3 requests per second but only 100 per hour). As far as I can tell this is currently not possible but feels like a very basic use case.

stefannikolei commented 1 year ago

+100 for @BondarencoM.

ATM it seems not possible to create a policy, which calls PartitionedRateLimiter.CreateChained().

I also have the usecase to create a policy for an endpoint which eg. allows 1 Request per Minute and 10 Requests per Hour. Atm this does not seem to be easy achievable.

juasanpar commented 1 year ago

Greetings,

I have a very similar requirement of adding a RateLimiting policy of limiting the requests to 50 per minute and also not allowing more that 10 concurrent. At the moment this is only achievable using PartitionedRateLimiter.CreateChained() as you have already mentioned, but only for the global limiter. I have an API with several endpoints that need different limiting policies, so this is not the best option for me. Is there any progress on this request or is there any other way to achieve my purpose?

Thanks for your work and help!

wtgodbe commented 1 year ago

I agree we should do this for 8. Right now, you apply RateLimitPartitions to endpoints, not PartitionedRateLimiters - the latter would be necessary to solve the problem. I think we could solve it by doing an internal CreateChained for PartitionedRateLimiters added to endpoints

ghost commented 1 year ago

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.

MrChriZ commented 1 year ago

I've just come at this from exactly the same direction as @BondarencoM.

LinqToException commented 1 year ago

I've spent the last hour frustrated looking for a solution for this problem as well.

In my case, I have an endpoint that invokes another, 3rd party service, which has its own rate limiting. As such, I would have liked to create a policy on that single endpoint that

stukalin commented 1 year ago

I'd like to +1 on this one. We'd like to migrate away from the https://github.com/stefanprodan/AspNetCoreRateLimit project and some of our policies will be expressed by a chain of fixed window validators but they should be a custom IRateLimiterPolicy and apparenlty it's not possible to specify a chain as a non-global rate limiter.

sizzle168 commented 1 year ago

We face the same challenge that @stukalin describes.

GiampaoloGabba commented 1 year ago

+1 for this. i tried to apply multiple policy to an endpoint but only the last one applied work. Imho this is a very basic feature for a rate limiter, i see that didnt make it for .net8, i hope you consider it for future releases. I really wanted to use the rate limiting feature bundled with aspnetcore but is not flexible enough for my usecase.

CreepMania commented 11 months ago

+1

We tried to implement a token bucket policy on top of fixed window policy but hit a wall.

odin568 commented 11 months ago

Unfortunately did not make it in .NET 8 :(

MikeNolan678 commented 8 months ago

+1

Has anyone been able to find a solution to this, which doesn't involve using the AspNetCoreRateLimit NuGet package?

atomaras commented 8 months ago

Personally planning on building my own and following this guide https://developer.redis.com/develop/dotnet/aspnetcore/rate-limiting/middleware which has the added benefit (aside from the ability to do distributed rate limiting) of sending all rate limit rules that apply to a single endpoint over to Redis in a single request.

MikeNolan678 commented 8 months ago

Personally planning on building my own and following this guide https://developer.redis.com/develop/dotnet/aspnetcore/rate-limiting/middleware which has the added benefit (aside from the ability to do distributed rate limiting) of sending all rate limit rules that apply to a single endpoint over to Redis in a single request.

Nice! Thanks for sharing. I ended up using the ASPNetCoreRateLimit package in the end.

atomaras commented 8 months ago

Spent a day so you don't have to. Here's the Redis Lua script (co-written by ChatGPT ofc) that implements the Sliding Window Rate Limit algo with support for multiple rules per invocation AND support for returning the remaining_tokens so you can power the X-Rate-Limit-Remaining header in your web api https://gist.github.com/atomaras/925a13f07c24df7f15dcc4fb7bc89c81