-
Notifications
You must be signed in to change notification settings - Fork 595
stats: add bootstrap annotations for stats flush and eviction #3562
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -191,6 +191,35 @@ annotations: | |
| resources: | ||
| - Pod | ||
|
|
||
| - name: sidecar.istio.io/statsFlushInterval | ||
| featureStatus: Alpha | ||
| description: Specifies the flush interval for push-based stat sinks, e.g. OTLP. Default interval is `5s`. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. qq: if this is only for push-based stats sinks, what about prometheus (pull based)?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Flush interval doesn't matter for prometheus. However, eviction piggy backs on flush timer, and eviction works for prometheus.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe more concretely... I send 1 request to When, with prometheus, will this metric stop showing up in /stats/prometheus with/without this annotation set?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For flush annotation: it will show up immediately and stay forever.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks! Sorry I was being a bit dense, github UI was hiding part of the PR
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @howardjohn @keithmattix can we move this forward? |
||
| deprecated: false | ||
| hidden: false | ||
| resources: | ||
| - Pod | ||
|
|
||
| - name: sidecar.istio.io/statsEvictionInterval | ||
| featureStatus: Alpha | ||
| description: Specifies the expiration interval for the Istio standard | ||
| metrics. This gets rounded to a multiple of the flush interval. A time | ||
| series is expected to be evicted after 2 iterations of this interval from | ||
| the last measurement. | ||
| deprecated: false | ||
| hidden: false | ||
| resources: | ||
| - Pod | ||
|
|
||
| - name: sidecar.istio.io/statsHistogramBins | ||
| featureStatus: Alpha | ||
| description: Specifies the bin size per time series for the Istio standard | ||
| metrics histograms. Reducing this value from the default `100` decreases | ||
| overall memory usage for sparse and/or high cardinality histograms. | ||
| deprecated: false | ||
| hidden: false | ||
| resources: | ||
| - Pod | ||
|
|
||
| - name: sidecar.istio.io/userVolume | ||
| featureStatus: Alpha | ||
| description: Specifies one or more user volumes (as a JSON array) to be added to | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it's interpreted by pilot-agent which is the same for gateways and sidecars.