rate_limit_quota: [API] Add time unit month and year

[tyxia]

Even though for us (who introduced the rate_limit_quota feature), month and year unit are too coarse-grained unit as our traffic will be huge and we will update the Envoy config on more frequent basis, there is business need/case for monthly and yearly subscription from @renuka-fernando side.
More detailed discussion can be found #23844.

Signed-off-by: tyxia tyxia@google.com

[repokitteh-read-only]

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #24031 was opened by tyxia.

see: more, trace.

[repokitteh-read-only]

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/).
envoyproxy/api-shepherds assignee is @adisuissa
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).

🐱

Caused by: #24031 was opened by tyxia.

see: more, trace.

[renuka-fernando]

I think we have to update this function too. But Month and Year do not have exact seconds, it varies from month to month, year to year, but I wish we do not want exact seconds here. @tyxia wdyt?

envoy/source/extensions/filters/http/ratelimit/ratelimit_headers.cc

Lines 67 to 82 in 5c56e45

	uint32_t XRateLimitHeaderUtils::convertRateLimitUnit(
	const envoy::service::ratelimit::v3::RateLimitResponse::RateLimit::Unit unit) {
	switch (unit) {
	case envoy::service::ratelimit::v3::RateLimitResponse::RateLimit::SECOND:
	return 1;
	case envoy::service::ratelimit::v3::RateLimitResponse::RateLimit::MINUTE:
	return 60;
	case envoy::service::ratelimit::v3::RateLimitResponse::RateLimit::HOUR:
	return 60 * 60;
	case envoy::service::ratelimit::v3::RateLimitResponse::RateLimit::DAY:
	return 24 * 60 * 60;
	case envoy::service::ratelimit::v3::RateLimitResponse::RateLimit::UNKNOWN:
	default:
	return 0;
	}
	}

[tyxia]

I think we have to update this function too. But Month and Year do not have exact seconds, it varies from month to month, year to year, but I wish we do not want exact seconds here. @tyxia wdyt?

envoy/source/extensions/filters/http/ratelimit/ratelimit_headers.cc

Lines 67 to 82 in 5c56e45

	uint32_t XRateLimitHeaderUtils::convertRateLimitUnit(
	const envoy::service::ratelimit::v3::RateLimitResponse::RateLimit::Unit unit) {
	switch (unit) {
	case envoy::service::ratelimit::v3::RateLimitResponse::RateLimit::SECOND:
	return 1;
	case envoy::service::ratelimit::v3::RateLimitResponse::RateLimit::MINUTE:
	return 60;
	case envoy::service::ratelimit::v3::RateLimitResponse::RateLimit::HOUR:
	return 60 * 60;
	case envoy::service::ratelimit::v3::RateLimitResponse::RateLimit::DAY:
	return 24 * 60 * 60;
	case envoy::service::ratelimit::v3::RateLimitResponse::RateLimit::UNKNOWN:
	default:
	return 0;
	}
	}

I think there is misunderstanding here. So do you want to include the month and year to existing rate limiting feature (api/envoy/service/ratelimit/v3/rls.proto) or the new global rate limiting feature I am working on? Or both?

The PR here is for new rate limiting feature and will not use the code you mentioned above.

[renuka-fernando]

I think we have to update this function too. But Month and Year do not have exact seconds, it varies from month to month, year to year, but I wish we do not want exact seconds here. @tyxia wdyt?

envoy/source/extensions/filters/http/ratelimit/ratelimit_headers.cc

Lines 67 to 82 in 5c56e45

	uint32_t XRateLimitHeaderUtils::convertRateLimitUnit(
	const envoy::service::ratelimit::v3::RateLimitResponse::RateLimit::Unit unit) {
	switch (unit) {
	case envoy::service::ratelimit::v3::RateLimitResponse::RateLimit::SECOND:
	return 1;
	case envoy::service::ratelimit::v3::RateLimitResponse::RateLimit::MINUTE:
	return 60;
	case envoy::service::ratelimit::v3::RateLimitResponse::RateLimit::HOUR:
	return 60 * 60;
	case envoy::service::ratelimit::v3::RateLimitResponse::RateLimit::DAY:
	return 24 * 60 * 60;
	case envoy::service::ratelimit::v3::RateLimitResponse::RateLimit::UNKNOWN:
	default:
	return 0;
	}
	}

I think there is misunderstanding here. So do you want to include the month and year to existing rate limiting feature (api/envoy/service/ratelimit/v3/rls.proto) or the new global rate limiting feature I am working on? Or both?

The PR here is for new rate limiting feature and will not use the code you mentioned above.

I am planning to use the existing rate limit feature (with the Envoy Rate-Limit service: https://github.com/envoyproxy/ratelimit). I think what you refer to as the new global rate limit feature is RLQS, that @markdroth mentioned in this comment envoyproxy/ratelimit#368 (review).

It is grateful if it is possible to have these units in the existing RLS feature too.
So both if possible 🙂.

[tyxia]

I think we have to update this function too. But Month and Year do not have exact seconds, it varies from month to month, year to year, but I wish we do not want exact seconds here. @tyxia wdyt?

envoy/source/extensions/filters/http/ratelimit/ratelimit_headers.cc

Lines 67 to 82 in 5c56e45

	uint32_t XRateLimitHeaderUtils::convertRateLimitUnit(
	const envoy::service::ratelimit::v3::RateLimitResponse::RateLimit::Unit unit) {
	switch (unit) {
	case envoy::service::ratelimit::v3::RateLimitResponse::RateLimit::SECOND:
	return 1;
	case envoy::service::ratelimit::v3::RateLimitResponse::RateLimit::MINUTE:
	return 60;
	case envoy::service::ratelimit::v3::RateLimitResponse::RateLimit::HOUR:
	return 60 * 60;
	case envoy::service::ratelimit::v3::RateLimitResponse::RateLimit::DAY:
	return 24 * 60 * 60;
	case envoy::service::ratelimit::v3::RateLimitResponse::RateLimit::UNKNOWN:
	default:
	return 0;
	}
	}

I think there is misunderstanding here. So do you want to include the month and year to existing rate limiting feature (api/envoy/service/ratelimit/v3/rls.proto) or the new global rate limiting feature I am working on? Or both?
The PR here is for new rate limiting feature and will not use the code you mentioned above.

I am planning to use the existing rate limit feature (with the Envoy Rate-Limit service: https://github.com/envoyproxy/ratelimit). I think what you refer to as the new global rate limit feature is RLQS, that @markdroth mentioned in this comment envoyproxy/ratelimit#368 (review).

It is grateful if it is possible to have these units in the existing RLS feature too. So both if possible 🙂.

Thanks for confirmation! The changes to existing RLS will be done in a separate PR.

[adisuissa]

LGTM.
/lgtm api

[adisuissa]

As discussed offline, this should be followed by a PR to update the previously defined Unit enum.