zalando / skipper

An HTTP router and reverse proxy for service composition, including use cases like Kubernetes Ingress
https://opensource.zalando.com/skipper/
Other
3.07k stars 347 forks source link

Allow user to add custom metrics #490

Open last-ent opened 6 years ago

last-ent commented 6 years ago

While using skipper as a proxy, I would like to monitor some custom states that might change because of filters. I would like have them as part of the skipper's /metrics endpoint so that we can rely on a single endpoint to provide us with these new states as well.

For example consider that we have 3 filters calling on three external http endpoints as part of the filter logic and each of these filters has a circuit breaker around these http calls - CB1, CB2, CB3. Let imagine that CB1 is in Closed State, CB2 is in Open State & CB3 is in Partially Open State.

As a part of my monitoring system, I would like to keep track of these CB states as well. Right now I would need to expose them on another endpoint within my main proxy application. Instead having them on /metrics would be helpful.

Given that we create the skipper.Options and then run the application as skipper.Run(options). There doesn't seem to be a provision to add any extra metrics.

I propose that skipper.Options has a new field called CustomMetrics of type map[string]interface{}

As a result, when I call on metrics endpoint I can expect something as follows.

GET /metrics
{
// Standard set of metrics

// Custom Metrics
"CB1": "Closed",
"CB2": "Open",
"CB3": "Half Open",
}
lmineiro commented 6 years ago

This has been discussed before.

Do you think what was contributed with https://github.com/zalando/skipper/pull/384 is enough for your needs?

last-ent commented 6 years ago

@lmineiro the discussion in the ticket deals with adding counter & measuring duration for a given key. Unfortunately this does not fit our requirements as we need a way to update values for different keys/metrics.

aryszka commented 6 years ago

the metrics.Default object allows custom keys for duration measures and counters. The same interface is exposed on the filter context, where the keys will be prefixed with the filter name. How I understand, what's missing:

  1. allow other types of custom metrics, not only durations and counters

(2. not strictly belongs here, but it would be nice if the state of the breakers would be possible to monitor anyway)

@last-ent do you think 1. would cover what you want to achieve?

last-ent commented 6 years ago

@aryszka Yes, 1. is exactly what I am looking for and 2. is just a special case of 1. IMO.

Is this already available in Skipper? Otherwise I wouldn't mind working on this.

aryszka commented 6 years ago

i think what's missing for 1. is to add a DecCounter(key string) next to the IncCounter(key string) method. Or something like ToggleState(key string) that internally can rely on the counter with +1/-1. We use this metrics library: https://godoc.org/github.com/rcrowley/go-metrics , feel free to pick its best matching capacity. I am actually not so sure what is the standard and nice way to monitor such states, @lmineiro @szuecs do you have any advice?

The method would need to be added to this interface:

https://godoc.org/github.com/zalando/skipper/filters#Metrics

... and to this struct:

https://godoc.org/github.com/zalando/skipper/metrics#Metrics

last-ent commented 6 years ago

@aryszka I agree that DecCounter(key string) should be added but I don't think it covers the complete picture.

If we consider the Circuit Breaker & its states (I will stick to this example for now): I have 3 three states to consider - Open, Close & Half Open. I can refer to them in metrics as int values or string values.

Open => -1 (or) "O" Half Open => 0 (or) "H" Closed => 1 (or) "C"

If I need to change the state from Open to Closed, it will not be easy to achieve especially since I do not know what the state of the metric is on Metrics. Perhaps a Set(key string, value interface{}) also makes sense?

szuecs commented 6 years ago

@aryszka sounds good to me.

last-ent commented 6 years ago

Alright, I will start working on this.