[API Proposal]: Add TResource in RateLimitPartition.Factory

[Kahbazi]

Background and motivation

RateLimitPartition.Factory is used to create the RateLimiter for a specific partition. At the moment only partition key is available in the creation but what if the type of the limiter or the values of the limiter is depend on the TResource itself? I suggest to add TResource in RateLimitPartition.Factory.

cc/ @BrennanConroy @halter73 @wtgodbe

API Proposal

namespace System.Threading.RateLimiting;

- public struct RateLimitPartition<TKey>
+ public struct RateLimitPartition<TResource, TKey>
{
-    public Func<TKey, RateLimiter> Factory { get; }
+    public Func<TResource, TKey, RateLimiter> Factory { get; }
}

API Usage

PartitionedRateLimiter.Create<HttpContext, string>(context =>
{
    var name = context.GetEndpoint().DisplayName;

    return RateLimitPartition.CreateSlidingWindowLimiter<string>(name, (httpContext, key) =>
    {
        var metadata = httpContext.GetEndpoint().Metadata.GetMetadata<IRateLimit>();
        return new SlidingWindowRateLimiterOptions(
            permitLimit: metadata.permitLimit,
            queueProcessingOrder: metadata.QueueProcessingOrder,
            queueLimit: metadata.queueLimit,
            window: metadata.window,
            segmentsPerWindow: metadata.segmentsPerWindow);
    });
});

Alternative Designs

This could be achieved now, but it would introduce an allocation because of the closure.

PartitionedRateLimiter.Create<HttpContext, string>(context =>
{
    var name = context.GetEndpoint().DisplayName;

    return RateLimitPartition.CreateSlidingWindowLimiter<string>(name, key =>
    {
        var metadata = context.GetEndpoint().Metadata.GetMetadata<IRateLimit>();
        return new SlidingWindowRateLimiterOptions(
            permitLimit: metadata.permitLimit,
            queueProcessingOrder: metadata.QueueProcessingOrder,
            queueLimit: metadata.queueLimit,
            window: metadata.window,
            segmentsPerWindow: metadata.segmentsPerWindow);
    });
});

Risks

No response

[dotnet-issue-labeler[bot]]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[ghost]

Tagging subscribers to this area: @mangod9 See info in area-owners.md if you want to be subscribed.

Issue Details

---

### Background and motivation [`RateLimitPartition.Factory`](https://github.com/dotnet/runtime/blob/f3483388f770602373cbf031ffbce108784bcf4a/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/RateLimitPartition.T.cs#L31) is used to create the `RateLimiter` for a specific partition. At the moment only partition key is available in the creation but what if the type of the limiter or the values of the limiter is depend on the `TResource` itself? I suggest to add `TResource` in RateLimitPartition.Factory. cc/ @BrennanConroy @halter73 @wtgodbe ### API Proposal ```diff namespace System.Threading.RateLimiting; - public struct RateLimitPartition + public struct RateLimitPartition { - public Func Factory { get; } + public Func Factory { get; } } ``` ### API Usage ```csharp PartitionedRateLimiter.Create(context => { var name = context.GetEndpoint().DisplayName; return RateLimitPartition.CreateSlidingWindowLimiter(name, (httpContext, key) => { var metadata = httpContext.GetEndpoint().Metadata.GetMetadata(); return new SlidingWindowRateLimiterOptions( permitLimit: metadata.permitLimit, queueProcessingOrder: metadata.QueueProcessingOrder, queueLimit: metadata.queueLimit, window: metadata.window, segmentsPerWindow: metadata.segmentsPerWindow); }); }); ``` ### Alternative Designs This could be achieved now, but it would introduce an allocation because of the closure. ```csharp PartitionedRateLimiter.Create(context => { var name = context.GetEndpoint().DisplayName; return RateLimitPartition.CreateSlidingWindowLimiter(name, key => { var metadata = context.GetEndpoint().Metadata.GetMetadata(); return new SlidingWindowRateLimiterOptions( permitLimit: metadata.permitLimit, queueProcessingOrder: metadata.QueueProcessingOrder, queueLimit: metadata.queueLimit, window: metadata.window, segmentsPerWindow: metadata.segmentsPerWindow); }); }); ``` ### Risks _No response_

Author:	Kahbazi
Assignees:	-
Labels:	`api-suggestion`, `area-System.Threading`, `untriaged`
Milestone:	-

[BrennanConroy]

My issue with this is that once the key has been created, the Factory will only be called once (not entirely true, but for the purposes of this discussion we'll assume once). But the factory here changes behavior based on the resource passed in, which would only apply on the first resource for that specific key. And because the factory is called once per key (which hopefully applies to multiple resources/requests), the allocations aren't a big concern anymore (amortizes to 0 given enough requests).

This may be intended behavior if you fully understand how the system works, but I think it starts blurring the lines and most people will not realize the behavior of the factory. We've already tried renaming the RateLimitPartition.Get methods from RateLimitPartition.Create to try making it a little more obvious that it's not creating a limiter every time. So we already realize this API can be confusing.

I think a better approach would be to make your IRateLimit contain a RateLimitPartition<TKey>/TKey pair so that you can construct the partition per resource.

PartitionedRateLimiter.Create<HttpContext, string>(context =>
{
    var displayName = context.GetEndpoint().DisplayName;

    var metadata = context.GetEndpoint().Metadata.GetMetadata<IRateLimit>();
    return RateLimitPartition.CreateX(metadata.Key /* or displayName */, metadata.Factory);
    // or
    // return metadata.Partition;
});

This also has the advantage of not hardcoding the rate limit type to SlidingWindowRateLimiter in the PartitionedRateLimiter, which the previous example of IRateLimit usage did. If that's intended, then maybe there should be an interface per limiter type like ISlidingWindowRateLimit, otherwise the IRateLimit interface may need to have a bunch of extra properties on it to support all limiter types.

---

Having said all that, I am willing to be convinced otherwise if we think we can reduce the confusion somehow.

One last idea for how to avoid the closure if you really want the resource inside the factory is to create a custom key type which contains the resource.

struct CustomKey
{
    public string Key { get; set; }
    public HttpContext Context { get; set; }
}

var customKey = new CustomKey(name, resource);
return RateLimitPartition.CreateSlidingWindowLimiter<CustomKey>(customKey, static (key) =>
{
    var metadata = key.Context.GetEndpoint().Metadata.GetMetadata<IRateLimit>();
    var options = new SlidingWindowRateLimiterOptions(
        permitLimit: metadata.permitLimit,
        queueProcessingOrder: metadata.QueueProcessingOrder,
        queueLimit: metadata.queueLimit,
        window: metadata.window,
        segmentsPerWindow: metadata.segmentsPerWindow);
    key.Context = null; // don't want to hold onto the HttpContext
    return options;
});

[Kahbazi]

But the factory here changes behavior based on the resource passed in, which would only apply on the first resource for that specific key.

Yeah I'm aware of that. I would have to guarantee that for each specific resource key, factory have the same behavior.

I think a better approach would be to make your IRateLimit contain a RateLimitPartition/TKey pair so that you can construct the partition per resource.

This would work for hard-coded config, but I was thinking to have HttpContext.RequestedService and get the options from database or some configuration. This way I could have dynamic options. I know it won't affect immediately.

...Unless, there will be a RemoveAllRateLimiter method. 🤔 😅

One last idea for how to avoid the closure if you really want the resource inside the factory is to create a custom key type which contains the resource.

I have thought of a custom key and it would work for scenarios in application code. But it's not applicable for rate limiting middleware since the key in there is HttpContext and can't be changed.

Having said all that, I am willing to be convinced otherwise if we think we can reduce the confusion somehow.

Yeah these APIs are really hard to follow as it is. I did spend a lot to understand them. I think in conclusion the point of this API change is to reduce closure allocation. Nothing blocks me to do what I want with some extra allocation.

[BrennanConroy]

But it's not applicable for rate limiting middleware since the key in there is HttpContext and can't be changed.

The key is chosen by the user:

builder.Services.AddRateLimiter(options =>
{
    options.GlobalLimiter = PartitionedRateLimiter.Create<HttpContext, CustomKey>(context =>
    {
        var customKey = new CustomKey(name, resource);
        return RateLimitPartition.CreateSlidingWindowLimiter<CustomKey>(customKey, static (key) =>
        {
            // ...
        });
    });
});