extension API Discussion

[rmanibus]

This issue is a thread for polishing the extension API

here is a first draft of the API that would be exposed by this extension:

quarkus:
  rate-limiter:
    limits:
      group1:
        burst-protection:
          max-usage: 10
          duration: 1S
        fair-use:
          max-usage: 100
          duration: 5m

@RateLimited(limitsKey = "group1")
void limitedMethod()

if multiple methods share the same limits key, user should be able to either share or isolate buckets for those methods.

Segmentation Key:

• UNSEGMENTED: the limit is shared for all the incoming trafic
• SEGMENT_BY_IP: one bucket is created per IP address, should throw if request context is not enabled
• CUSTOM: should allow a user defined segmentation method, working with CDI (use case: segment by user id)

Bucket registry:
Local for now, but network registry should be supported in the future (hazelcast, redis ...)

[rmanibus]

@melloware, I pushed the first commit, you can use this thread for API related discussions

[rmanibus]

I would prefer the configuration to look like

quarkus:
  rate-limiter:
    limits:
      group1:
        - name: burst-protection
          max-usage: 10
          duration: 1S
        - name: fair-use
          max-usage: 100
          duration: 5m

but when I try to use a List instead of a map, I have an exception saying it did not find any transformer for my nested config group.
Does someone have any idea how I could achieve this ?

[melloware]

OK

• rename fr.rmanibus to io.quarkiverse looks like you took care of!
• RateLimiterConfig is in Runtime package but I have seen a lot of other projects have that class in Deployment package. Wasn't sure if one was more correct or not?
• quarkus-extension.yaml you probably want to add keywords and doc location etc you can look at Quarkiverse POI for good example
• Dev UI card I can help get it started and send you a PR. Maybe we can then make it smarter by showing real time data of what is being limited etc?

[rmanibus]

@melloware

• RateLimiterConfig: interesting, maybe @gastaldi has an answer for this one (should it be in runtime or deployment)
• quarkus-extension.yaml: noted, I will take care of it once I have a working solution
• There is a lot of things that could be exposed in the dev UI for this extension, but it should also be exposed via metrics for monitoring purpose during production use.
• How do you feel about the proposed API ?

My main concern is to find a solution to plug a custom segmentation key resolver for each rate limited endpoint.
Maybe with something like:

@RateLimited(limitsKey = "group1", segmentationKeyResolver=MyResolver.class)

where MyResolver.class could be a CDI bean ?

[melloware]

As for the API I like your ideas so far. As for Config your List question I am definitely not a config expert maybe ask on the Zulip channel or Mailing List and get some feedback?

Also I noticed Bucket4J talks about using any JCache implementation but I don't see where you have any of that or does it use Caffeine by default?

[rmanibus]

yeah that's what I was trying to express with the last paragraph, I will focus on a in memory store for now and I will extend to shared registries in the future. Bucket4J support a tons of implementations (Hazelcast, Redis ...), this is definitely in the roadmap for this project.

[melloware]

Understood! We are on the same page.

[gastaldi]

About the RateLimiterConfig it's normal to place it in the runtime/ module (eg. https://github.com/quarkusio/quarkus/tree/main/extensions/flyway/runtime/src/main/java/io/quarkus/flyway/runtime).

[gastaldi]

@rmanibus if using Caffeine as the store implementation, use quarkus-cache, as it already builds on it

[melloware]

Also seeing this INFO message...

2023-04-12 14:59:07,068 INFO  [io.qua.arc.pro.Interceptors] (build-47) 
The interceptor io.quarkiverse.bucket4j.runtime.RateLimitedInterceptor does not declare any @Priority. It will be assigned a default priority value of 0.

[melloware]

In Runtime you should remove..

<dependency>
            <groupId>com.github.ben-manes.caffeine</groupId>
            <artifactId>caffeine</artifactId>
        </dependency>

And replace it with...

        <dependency>
            <groupId>io.quarkus</groupId>
            <artifactId>quarkus-cache</artifactId>
        </dependency>

That brings in Caffeine and will work in Native mode.

[rmanibus]

@melloware Done, I think this is ready for a first experimental release !

[rmanibus]

still need to work on the dev tools and metrics but the core functionality is here

[gastaldi]

Any thoughts on renaming the master branch to main?

[rmanibus]

@gastaldi is there something to change in the CI config ?

[gastaldi]

@rmanibus no need to, already changed it

[rmanibus]

@gastaldi I pushed a branch called main, could you set it as default and protect it (and also unprotect current master) ?

[gastaldi]

@gastaldi I pushed a branch called main, could you set it as default

Done

and protect it (and also unprotect current master) ?

Protecting the branch may break the release process, as it pushes content to the main branch without a PR, so that needs to be better investigated