stats: add bootstrap annotations for stats flush and eviction

[kyessenov]

Change-Id: I6c64ddd71aee82b570c7df25f13e2d710a046558

[istio-policy-bot]

😊 Welcome @kyessenov! This is either your first contribution to the Istio api repo, or it's been
a while since you've been here.

You can learn more about the Istio working groups, Code of Conduct, and contribution guidelines
by referring to Contributing to Istio.

Thanks for contributing!

Courtesy of your friendly welcome wagon.

[keithmattix]

qq: if this is only for push-based stats sinks, what about prometheus (pull based)?

[kyessenov]

Flush interval doesn't matter for prometheus. However, eviction piggy backs on flush timer, and eviction works for prometheus.

[howardjohn]

I don't get it. if it doesn't matter, but we evict when we flush, then it does matter...? And if the default is 5s, isn't that going to break prometheus?

[kyessenov]

Envoy always runs a stats flush timer, regardless of prometheus. During that timer, server stats are published (e.g. memory allocations, etc), and some other work is done, e.g. eviction.

Prometheus endpoint can snip stats are any point through, in between flushes.

So flush impacts prometheus only indirectly, e.g. by updating/removing some metrics.

[howardjohn]

Maybe more concretely... I send 1 request to foo.com. I get an istio_request_total metric for it.

When, with prometheus, will this metric stop showing up in /stats/prometheus with/without this annotation set?

[kyessenov]

For flush annotation: it will show up immediately and stay forever.
For eviction annotation: same, but it will disappear after 2*eviction interval unless the metric gets another hit.

[howardjohn]

Thanks! Sorry I was being a bit dense, github UI was hiding part of the PR

[zirain]

@howardjohn @keithmattix can we move this forward?

[kyessenov]

/retest

[zirain]

LGTM, Istio don't have first class API for bootstrap, this's the best option for now.

[kyessenov]

@howardjohn @keithmattix How does this get moved forward?

[keithmattix]

Does the sidecar prefix mean this doesn't work for gateways/waypoints? I would guess no (that this is just convention) but wanted to confirm

[kyessenov]

No, it's interpreted by pilot-agent which is the same for gateways and sidecars.

[ringerc]

Is there any way to control defaults for sidecar.istio.io/statsEvictionInterval settings via the Istio Telemetry resource, rather than having to annotate every single workload?

[zirain]

Is there any way to control defaults for sidecar.istio.io/statsEvictionInterval settings via the Istio Telemetry resource, rather than having to annotate every single workload?

this's part of bootstrap, which is not dynamiced.

[ringerc]

For others who want to adopt this Istio-cluster-wide, add it to your injectedAnnotations in your ConfigMap/istio-sidecar-injector in the (by default) istio-system namespace.

E.g.

apiVersion: v1
kind: ConfigMap
metadata:
  name: istio-sidecar-injector
  namespace: istio-system
  # ...
data:
  config: |-
    # ...
    injectedAnnotations:
      # The stats eviction interval should be long enough to preserve
      # continuity of samples over infrequent communications between
      # components, while avoiding infinite accumulation of series for
      # workloads that no longer exist.
      sidecar.istio.io/statsEvictionInterval: "15m"
      # We don't need the default 5s frequency in our stack, so flush every
      # 15s. Only affects push stats, and rounding for the eviction interval.      
      sidecar.istio.io/statsFlushInterval: "15s"

It'd be handy if https://istio.io/latest/docs/reference/config/annotations/ mentioned the default sidecar template; I'll see if I can cook up a docs patch for that.