istio / old_mixer_repo

Deprecated home of Istio's Mixer and its adapters, now in istio/istio's mixer dir
https://github.com/istio/istio/tree/master/mixer
Apache License 2.0
67 stars 93 forks source link

Additional default gauge metric for request duration #965

Open fabolive opened 7 years ago

fabolive commented 7 years ago

Currently, Istio is reporting to Prometheus two metrics, namely, request_count and request_duration, as per the definition here: https://github.com/istio/mixer/blob/master/testdata/configroot/scopes/global/descriptors.yml#L95

The former is a Prometheus count, whereas the latter is a Prometheus histogram. Even though these are useful, I think we also need a default gauge metric for request duration. The current histogram is great for aggregations and for computing quantiles, but prevents users from performing their own custom analytics on the actual observations, which are masked by the pre-defined cumulative histogram buckets. Thus, a Prometheus gauge metric for request duration, in addition to the current ones, might be interesting.

cc: @esnible, @rshriram

douglas-reid commented 7 years ago

I believe the path forward here is to remove the counter/histogram distinction at the Mixer level. In other words, whether or not request_count is counter and request_duration is a histogram should be adapter-level configuration. Mixer should solely be concerned with providing the Metric instances based on constructors (attribute expressions).

This separation is part of the focus of the 0.2 Adapter subsystem design proposal (review happening on 7/31). An early example of what this might look like can be seen in the PR for Stackdriver Metrics Adapter (https://github.com/istio/mixer/pull/846).

douglas-reid commented 7 years ago

I should mention that operators are free to use GAUGE today. The metrics descriptor provides a GAUGE MetricKind (https://github.com/istio/api/blob/master/mixer/v1/config/descriptor/metric_descriptor.proto#L76) and the prometheus adapter has code meant to deal with GAUGEs (https://github.com/istio/mixer/blob/master/adapter/prometheus/prometheus.go#L85)

geeknoid commented 7 years ago

@douglas-reid Should we close this issue?