Skipper: Metrics per ingress defined route

[szuecs]

Today we have already many, many metrics, but to make it more easy for Kubernetes users, it might makes sense to have metrics by ingress rule definition.

In Kubernetes Ingress you define routes to serve and it makes sense skipper would have a metrics collector that serve metrics for that. Some people may want to get metrics for a specific path while others only use host header dispatch and / as path.

• Flag to enable: --kubernetes-metrics

Open Questions:

• [x] Should we split metrics by http verbs? (I would say yes)
• [x] In case of having more than one path in an ingress, for example /foo and /: should we count /foo calls also into the bucket of /? only defined paths in ingress

[aryszka]

1. metrics per verbs:

by default, it is a good idea to collect the metrics separately because one can still combine them in a monitoring system. (One thing to consider here though, is that with a large amount of ingresses, the polling of the metrics can cause high load, so there should be probably an option to use more limited metrics. Fine to implement in a subsequent step.)

1. '/foo' and '/:' buckets:

i think this should based on the routes. This means that if the matched route has Path("/:somevar") then we collect the metrics as /:somevar. See also 3.

1. security aspect

we need to make sure that the max number of metrics is limited by the ingress configuration, however dynamic it is, and avoid a situation when an attacker can grow the number of metrics by just iterating over random paths. I think this can be solved by defining the metrics per routes.

1. existing functionality

see https://godoc.org/github.com/zalando/skipper/filters#Metrics . It allows to collect metrics with custom keys from filters, prefixed with the filter name. The ingress metrics could be a specialization of it. It's accessible on the filtercontext. I think we can build on top of this approach and migrate away from using the global metrics object (metrics.Default). Currently, this context metrics object has a bug: it messes up the keys in cases the filter has any concurrent functionality internally.

[szuecs]

@aryszka in a non ingress installation and for debugging purposes only, it makes also sense to create metrics that "explode" for example per called path, but it should be clear, that it is not "secure" to use.

[hrzbrg]

I would really like to see this in order to have cleaner metric labels with information from the ingress like path & service.

[hjacobs]

@hrzbrg would you be willing to contribute such feature via PR?

[szuecs]

If we have api monitoring feature and https://github.com/zalando/skipper/pull/842 , I think we can close this.
Any other comments on that?

[aryszka]

agree. We can reopen it if different metrics are required.