Adding rate_limit_name as tag on metrics

[bjoernw]

Right now I'm collecting data about individual rate limits on the client side but it would be nice if I didn't have to add this to every client across languages. Being able to stratify the metrics by rate limit name would be useful.

Mostly thinking about these:

var metricCheckCounter = prometheus.NewCounter(prometheus.CounterOpts{
    Name: "gubernator_check_counter",
    Help: "The number of rate limits checked.",
})
var metricOverLimitCounter = prometheus.NewCounter(prometheus.CounterOpts{
    Name: "gubernator_over_limit_counter",
    Help: "The number of rate limit checks that are over the limit.",
})

Happy to work on this

[bjoernw]

@thrawn01 any objections?

[Baliedge]

I recommend against this because the cardinality of rate limit names is unlimited. Instead, your suggested alternative is correct to create a metric around your client-side call to Gubernator.

[thrawn01]

The code listed in the proposal doesn't include the name, just a counter of checks and how many were over the limit. I don't see an issue with that at all. If the metric did include the name of the rate limit that would quickly break Prometheus.

If it's just a counter of checks and over limits it should be fine!

[Baliedge]

My apologies, I misread the intentions.

[bjoernw]

@Baliedge understood correctly actually. I just pointed out the 2 counters that I would like to have the ability to add additional tags like the rate limit name. The cardinality issue is understandable and adding the name by default wouldn't make sense. It's ok I augmented the client code and it does that now for the few limits that I need to support.