[API Proposal]: make RateLimiterOptions follow Options pattern

[wtgodbe]

Background and motivation

Enables us to instantiate RateLimiters in the RateLimitingMiddleware following the traditional pattern (with an Action): dotnet/aspnetcore#42667 (comment)

API Proposal

namespace System.Threading.RateLimiting;

public class sealed TokenBucketRateLimiterOptions
{
-     public TokenBucketRateLimiterOptions(int tokenLimit, QueueProcessingOrder queueProcessingOrder, int queueLimit, TimeSpan replenishmentPeriod, int tokensPerPeriod, bool autoReplenishment = true);
+     public TokenBucketRateLimiterOptions();

-     public TimeSpan ReplenishmentPeriod { get; }
+     public TimeSpan ReplenishmentPeriod { get;  set; }
-     public int TokensPerPeriod { get; }
+     public int TokensPerPeriod { get; set; }
-     public bool AutoReplenishment { get; }
+     public bool AutoReplenishment { get; set; }
-     public int TokenLimit { get; }
+     public int TokenLimit { get; set; }
-     public QueueProcessingOrder QueueProcessingOrder { get; }
+     public QueueProcessingOrder QueueProcessingOrder { get; set; }
-     public int QueueLimit { get; }
+     public int QueueLimit { get; set; }
}

public class sealed SlidingWindowRateLimiterOptions
{
-     public SlidingWindowRateLimiterOptions(int permitLimit, QueueProcessingOrder queueProcessingOrder, int queueLimit, TimeSpan window, int segmentsPerWindow, bool autoReplenishment = true);
+     public SlidingWindowRateLimiterOptions();

-     public TimeSpan Window { get; }
+     public TimeSpan Window { get;  set; }
-     public int SegmentsPerWindow { get; }
+     public int SegmentsPerWindow { get; set; }
-     public bool AutoReplenishment { get; }
+     public bool AutoReplenishment { get; set; }
-     public int PermitLimit { get; }
+     public int PermitLimit { get; set; }
-     public QueueProcessingOrder QueueProcessingOrder { get; }
+     public QueueProcessingOrder QueueProcessingOrder { get; set; }
-     public int QueueLimit { get; }
+     public int QueueLimit { get; set; }
}

public class sealed FixedWindowRateLimiterOptions
{
-     public FixedWindowRateLimiterOptions(int permitLimit, QueueProcessingOrder queueProcessingOrder, int queueLimit, TimeSpan window, bool autoReplenishment = true);
+     public FixedWindowRateLimiterOptions();

-     public TimeSpan Window { get; }
+     public TimeSpan Window { get;  set; }
-     public bool AutoReplenishment { get; }
+     public bool AutoReplenishment { get; set; }
-     public int PermitLimit { get; }
+     public int PermitLimit { get; set; }
-     public QueueProcessingOrder QueueProcessingOrder { get; }
+     public QueueProcessingOrder QueueProcessingOrder { get; set; }
-     public int QueueLimit { get; }
+     public int QueueLimit { get; set; }
}

public class sealed ConcurrencyLimiterOptions
{
-     public ConcurrencyLimiterOptions(int permitLimit, QueueProcessingOrder queueProcessingOrder, int queueLimit);
+     public ConcurrencyLimiterOptions();

-     public int PermitLimit { get; }
+     public int PermitLimit { get; set; }
-     public QueueProcessingOrder QueueProcessingOrder { get; }
+     public QueueProcessingOrder QueueProcessingOrder { get; set; }
-     public int QueueLimit { get; }
+     public int QueueLimit { get; set; }
}

With 0 defaults, and verbose exception messages in the Limiter constructors.

API Usage

var options = new RateLimiterOptions()
        .AddTokenBucketLimiter("myLimiter", options =>
        {
            options.TokenLimit = 10;
            options.QueueProcessingOrder = QueueProcessingOrder.NewestFirst;
            options.QueueLimit = 10;
            options.ReplenishmentPeriod = TimeSpan.FromSeconds(30);
            options.TokensPerPeriod = 5;
        });

Alternative Designs

We could keep using it the way it is.

Risks

Hard to think of any

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.

[BrennanConroy]

What is public TokenBucketRateLimiterOptions(T item)?

with reasonable defaults

Are you proposing defaults or keeping the current ctors as-is (that require specifying all values)?

[wtgodbe]

What is public TokenBucketRateLimiterOptions(T item)

A typo 😆 should be an empty constructor.

Are you proposing defaults or keeping the current ctors as-is (that require specifying all values)?

There need to be defaults for anything non-nullable, right? I'd like to keep the existing ctors as-is if possible as well

[BrennanConroy]

There need to be defaults for anything non-nullable, right?

Right (although these types mostly all have implicit defaults), API review will probably want to know what the defaults are though which is why I'm asking.

[wtgodbe]

It's hard to know what a reasonable default is for, e.g., ReplenishmentPeriod or things like TokensPerPeriod & QueueLimit. We could either try to come up with a useful default, or do something arbitrary like make everything 1. The latter is simple but definitely less useful, though I'm leaning towards it since it seems like a futile exercise to try to come up with defaults that actually are useful.

[BrennanConroy]

seems like a futile exercise to try to come up with defaults that actually are useful

Agreed, it's very scenario specific.

My thoughts for defaults:

public TimeSpan ReplenishmentPeriod { get;  set; } = TimeSpan.FromSeconds(1);
public int TokensPerPeriod { get; set; } = 1;
public bool AutoReplenishment { get; set; } = true;
public int TokenLimit { get; set; } = 1;
public QueueProcessingOrder QueueProcessingOrder { get; set; } = QueueProcessingOrder.OldestFirst;
public int QueueLimit { get; set; } = 1;

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

Issue Details

---

Background and motivation

Enables us to instantiate RateLimiters in the RateLimitingMiddleware following the traditional pattern (with an Action): dotnet/aspnetcore#42667 (comment)

API Proposal

namespace System.Threading.RateLimiting;

public class sealed TokenBucketRateLimiterOptions
{
+     public TokenBucketRateLimiterOptions()

-     public TimeSpan ReplenishmentPeriod { get; }
+     public TimeSpan ReplenishmentPeriod { get;  set; } 
-     public int TokensPerPeriod { get; }
+     public int TokensPerPeriod { get; set; }
-     public bool AutoReplenishment { get; }
+     public bool AutoReplenishment { get; set; }
-     public int TokenLimit { get; }
+     public int TokenLimit { get; set; }
-     public QueueProcessingOrder QueueProcessingOrder { get; }
+     public QueueProcessingOrder QueueProcessingOrder { get; set; }
-     public int QueueLimit { get; }
+     public int QueueLimit { get; set; } 
}

Plus the equivalent changes for SlidingWindowRateLimiterOptions, ConcurrencyLimiterOptions, and FixedWindowRateLimiterOptions, with reasonable defaults.

API Usage

var options = new RateLimiterOptions()
        .AddTokenBucketLimiter("myLimiter", options =>
        {
            options.TokenLimit = 10;
            options.QueueProcessingOrder = QueueProcessingOrder.NewestFirst;
            options.QueueLimit = 10;
            options.ReplenishmentPeriod = TimeSpan.FromSeconds(30);
            options.TokensPerPeriod = 5;
        });

Alternative Designs

We could keep using it the way it is.

Risks

Hard to think of any

Author:	wtgodbe
Assignees:	-
Labels:	area-System.Threading, blocking, untriaged, api-ready-for-review
Milestone:	-

[terrajobst]

Video

• Looks good as proposed
  • @wtgodbe will update the issue (and this comment) with all the option types (done)
• One problem with the option callback pattern is that there is no way to annotate what is required

namespace System.Threading.RateLimiting;

public class sealed TokenBucketRateLimiterOptions
{
-     public TokenBucketRateLimiterOptions(int tokenLimit, QueueProcessingOrder queueProcessingOrder, int queueLimit, TimeSpan replenishmentPeriod, int tokensPerPeriod, bool autoReplenishment = true);
+     public TokenBucketRateLimiterOptions();

-     public TimeSpan ReplenishmentPeriod { get; }
+     public TimeSpan ReplenishmentPeriod { get;  set; }
-     public int TokensPerPeriod { get; }
+     public int TokensPerPeriod { get; set; }
-     public bool AutoReplenishment { get; }
+     public bool AutoReplenishment { get; set; }
-     public int TokenLimit { get; }
+     public int TokenLimit { get; set; }
-     public QueueProcessingOrder QueueProcessingOrder { get; }
+     public QueueProcessingOrder QueueProcessingOrder { get; set; }
-     public int QueueLimit { get; }
+     public int QueueLimit { get; set; }
}

public class sealed SlidingWindowRateLimiterOptions
{
-     public SlidingWindowRateLimiterOptions(int permitLimit, QueueProcessingOrder queueProcessingOrder, int queueLimit, TimeSpan window, int segmentsPerWindow, bool autoReplenishment = true);
+     public SlidingWindowRateLimiterOptions();

-     public TimeSpan Window { get; }
+     public TimeSpan Window { get;  set; }
-     public int SegmentsPerWindow { get; }
+     public int SegmentsPerWindow { get; set; }
-     public bool AutoReplenishment { get; }
+     public bool AutoReplenishment { get; set; }
-     public int PermitLimit { get; }
+     public int PermitLimit { get; set; }
-     public QueueProcessingOrder QueueProcessingOrder { get; }
+     public QueueProcessingOrder QueueProcessingOrder { get; set; }
-     public int QueueLimit { get; }
+     public int QueueLimit { get; set; }
}

public class sealed FixedWindowRateLimiterOptions
{
-     public FixedWindowRateLimiterOptions(int permitLimit, QueueProcessingOrder queueProcessingOrder, int queueLimit, TimeSpan window, bool autoReplenishment = true);
+     public FixedWindowRateLimiterOptions();

-     public TimeSpan Window { get; }
+     public TimeSpan Window { get;  set; }
-     public bool AutoReplenishment { get; }
+     public bool AutoReplenishment { get; set; }
-     public int PermitLimit { get; }
+     public int PermitLimit { get; set; }
-     public QueueProcessingOrder QueueProcessingOrder { get; }
+     public QueueProcessingOrder QueueProcessingOrder { get; set; }
-     public int QueueLimit { get; }
+     public int QueueLimit { get; set; }
}

public class sealed ConcurrencyLimiterOptions
{
-     public ConcurrencyLimiterOptions(int permitLimit, QueueProcessingOrder queueProcessingOrder, int queueLimit);
+     public ConcurrencyLimiterOptions();

-     public int PermitLimit { get; }
+     public int PermitLimit { get; set; }
-     public QueueProcessingOrder QueueProcessingOrder { get; }
+     public QueueProcessingOrder QueueProcessingOrder { get; set; }
-     public int QueueLimit { get; }
+     public int QueueLimit { get; set; }
}