Proposal: Add explicit unlimited rate limits

[thao-wish]

Currently we can achieve implicit unlimited rate limits by not specifying a rate limit in the configuration. However, we'll lose the metrics (total_hits, within_limits) for those rate limits. The proposal is to add an explicit unlimited rate limit. An example config file could look like this:

domain: mongo_cps
descriptors:
  - key: database
    value: users
    rate_limit:
      unit: second
      requests_per_unit: unlimited

We can skip the trip to redis/memcached if unlimited rate limits are requested. Please let me know if the team is open to this request or discuss any alternatives. The main concern is to be able to get the metrics for the unlimited rate limits.

[mattklein123]

IMO I would rather just implement #218 which I think covers this case?

[thao-wish]

IMO I would rather just implement #218 which I think covers this case?

I think it's a bit different. Please correct me if I'm wrong.

Test limits are processed in the same way as real limits, so they still have to go through redis/memcached. Currently the unlimited rate limits do not go to redis/memcached, so we want the new unlimited rate limits to be the same. It's also good for performance.

[mattklein123]

Yeah fair enough. I agree we can look at this independently.

[lmajercak-wish]

Hey @mattklein123, I would like to pick this up.

I've already looked into this briefly and it seems like the biggest issue here will be deciding what to return for rate limits which are set to 'unlimited'. Specifically, we have the uint32 requests_per_unit and uint32 limit_remaining fields.

Any suggestions on what we do with these? If we omitted them, then I think there isn't a difference between a response for an unlimited ratelimit and one that does not exist, which might cause confusion. Alternatively, I guess we could return something like uint32 max value, but that does not seem ideal either.

[lmajercak-wish]

I just realized that, if the limit is set to 0, these two fields are omitted. I guess we could do the same for unlimited ones then?

[lmajercak-wish]

Opened #261

[zakhenry]

This appears to be implemented now, can this issue be closed?