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

Proposal: flag to use tags instead of metric names to distinguish rate limit configs #162

Open
dweitzman opened this issue Jul 27, 2020 · 3 comments

Comments

@dweitzman
Copy link
Contributor

dweitzman commented Jul 27, 2020

Checking in here before sending out code for this.

The proposal is to add a setting like 'STATS_WITH_TAGS' that changes from reporting config stats like:

ratelimit.service.rate_limit.<domain1>.<k1>_<v1>.<k2>.total_hits
ratelimit.service.rate_limit.<domain1>.<k3>.total_hits

To something more like:

ratelimit.service.rate_limit.total_hits config=<domain1>.<k1>_<v1>.<k2>
ratelimit.service.rate_limit.total_hits config=.<domain1>.<k1>_<v1>.<k2>

The non-tagged stats would be deprecated, and on a long-term timeframe maybe the stats reporting would migrate away from the gostats library to the opentelemetry library when it's considered sufficiently mature.

Motivations:

  • Some (all?...) dashboard systems make it easier to do aggregation over tags vs over metrics. For example, today both ratelimit.service.rate_limit.<config_name>.over_limit and ratelimit.service.rate_limit.<config_name>.over_limit_with_cache are metric names, and the dashboarding system I'm working with seems to have inflexible aggregration + globbing so that there doesn't seem to be a way to make a graph including the "over_limit" that doesn't also include the "over_limit_with_cache"
  • statsd doesn't have an official protocol and implementations are not all compatible, so moving away from it as a default over time toward supporting protocols real standards feels sensible. Some people will also probably want to ingest into places like Prometheus where stats are more expressive than statsd and can report descriptions of what the metrics are.
@mattklein123
Copy link
Member

statsd is still widely used, so I don't think we can deprecate it.

My recommendation would be to support both formats, much like Envoy does.

@dweitzman
Copy link
Contributor Author

My thought for the short term is to factor out an interface for reporting stats so that all the direct dependencies on the gostats library can be isolated within one file.

Being able to replace the entire stats backend with an environment variable or by changing a single file seems useful (it'd make my life easier, at least)

@mattklein123
Copy link
Member

Being able to replace the entire stats backend with an environment variable or by changing a single file seems useful (it'd make my life easier, at least)

Sure sounds good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants