Skip to content
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

Get rate limit count and remaining time limit. #2081

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

p4paul
Copy link

@p4paul p4paul commented Jan 22, 2024

resolves #2080

// Simple example...
.get("/", ctx -> {
    NaiveRateLimit.requestPerTimeUnit(ctx, 5, TimeUnit.MINUTES);
    ConcurrentHashMap<TimeUnit, RateLimiter> limiters = RateLimitUtil.INSTANCE.getLimiters();
    int count = limiters.get(TimeUnit.MINUTES).getCounter(ctx);
    long remainder = limiters.get(TimeUnit.MINUTES).getRemainder(TimeUnit.MILLISECONDS);

    ctx.result("Count: " + count + " Remainder: " + remainder);
})

// Simple example...
.get("/", ctx -> {
    NaiveRateLimit.requestPerTimeUnit(ctx, 5, TimeUnit.MINUTES);
    ConcurrentHashMap<TimeUnit, RateLimiter> limiters = RateLimitUtil.INSTANCE.getLimiters();
    int count = limiters.get(TimeUnit.MINUTES).getCounter(ctx);
    long remainder = limiters.get(TimeUnit.MINUTES).getRemainder(TimeUnit.MILLISECONDS);

    ctx.result("Count: " + count + " Remainder: " + remainder);
})
@tipsy tipsy force-pushed the master branch 2 times, most recently from 0e2b2a8 to 4b4a470 Compare February 1, 2024 14:30
@@ -38,6 +40,14 @@ class RateLimiter(val timeUnit: TimeUnit) {
}
}
}

fun getCounter(ctx: Context) : Int? {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
fun getCounter(ctx: Context) : Int? {
fun counter(ctx: Context) : Int? {

return keyToRequestCount.get(RateLimitUtil.keyFunction(ctx))
}

fun getRemainder(timeUnit: TimeUnit) : Long? {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
fun getRemainder(timeUnit: TimeUnit) : Long? {
fun remainder(timeUnit: TimeUnit) : Long? {

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait, why does it take a TimeUnit ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You will want a different time unit for the remainder. e.g. take a rate limit of 5 requests per SECOND where the remaining time before the SECOND expires is in MILLISECONDS...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps I could use a float... 0.25 left of your TimeUnit; then you would not need another TimeUnit

@tipsy
Copy link
Member

tipsy commented Feb 2, 2024

Thanks @p4paul !

I could merge this, but the way it's written kind of forces you to do this for each handler where you use the rate limiter. We could provide the rate limiter with a config option to run a lambda, so that you could do something like:

NaiveRateLimiter.responseHeaders { ctx ->
    "headername" to headerValue
}

@tipsy
Copy link
Member

tipsy commented Feb 2, 2024

Could do

object RateLimitUtil {
    var responseHeaderFunction = { ctx: Context, timeUnit: TimeUnit, numRequests: Int -> } // signature could be different
}

Then we'd call it for each request:

fun incrementCounter(ctx: Context, requestLimit: Int) {
    ...
    RateLimitUtil.responseHeaderFunction(ctx, timeUnit, requestLimit) // signature could be different
}

It would be used like this:

RateLimitUtil.responseHeaderFunction = { ctx, timeUnit, numRequests ->
    ctx.header("X-Rate-Limit-Limit", ...)
    ctx.header("X-Rate-Limit-Remaining", ...)
    ctx.header("X-Rate-Limit-Reset", ...)
}

What do you think?

@p4paul
Copy link
Author

p4paul commented Feb 5, 2024

I like the lambda implementation for setting the headers. Feel free to discard or edit this PR as needed.

@tipsy
Copy link
Member

tipsy commented Feb 5, 2024

I like the lambda implementation for setting the headers. Feel free to discard or edit this PR as needed.

I'd be happy to assist if you'd like to give it a try ...? 😁

@tipsy tipsy force-pushed the master branch 2 times, most recently from 41640d2 to 2b4c30d Compare February 23, 2024 13:34
@tipsy tipsy force-pushed the master branch 4 times, most recently from 765b7d0 to 6faa186 Compare May 8, 2024 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Is there a way to read the current rate limit.
2 participants