Skip to content

stats: utilize expiration#6529

Merged
istio-testing merged 1 commit intoistio:masterfrom
kyessenov:cel_scope_evict
Sep 4, 2025
Merged

stats: utilize expiration#6529
istio-testing merged 1 commit intoistio:masterfrom
kyessenov:cel_scope_evict

Conversation

@kyessenov
Copy link
Copy Markdown
Contributor

Change-Id: I97c10eac08443566eca6f53dc73b86c2e51fa095

Replacing rotating scopes with the more accurate per-stat expiration implementation envoyproxy/envoy#40395

Change-Id: I97c10eac08443566eca6f53dc73b86c2e51fa095
Signed-off-by: Kuat Yessenov <kuat@google.com>
@kyessenov kyessenov requested a review from a team as a code owner September 3, 2025 23:37
@istio-policy-bot
Copy link
Copy Markdown

😊 Welcome @kyessenov! This is either your first contribution to the Istio proxy 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.

@istio-testing istio-testing added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Sep 3, 2025
Copy link
Copy Markdown
Member

@howardjohn howardjohn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

qq - the envoy PR seems to suggest it only works with otlp (or other delta protocols) while I think the old istio approach worked with Prometheus as well. Is the Prometheus behavior changed by this pr?

@kyessenov
Copy link
Copy Markdown
Contributor Author

The old behavior didn't work with prometheus either AFAICT, so it didn't change. The behavior is the same if you create all stats at once, and then wait some time.

@howardjohn
Copy link
Copy Markdown
Member

I believe it did which is why there was the timer - to make sure it was sufficiently large to ensure Prometheus had time to scrape?

@zirain would know for sure


// Metric scope rotation interval. Set to 0 to disable the metric scope rotation.
// Defaults to 0.
// DEPRECATED.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just for note, we need to remove all related stuffs(e.g. the env flags) from istio since it won't work anymore.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's a follow up. There's a difference in configuration since this is now a per-Envoy config, so maybe a pod annotation rather than telemetry API.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or just mentioned in custom bootstrap.

@zirain
Copy link
Copy Markdown
Member

zirain commented Sep 4, 2025

qq - the envoy PR seems to suggest it only works with otlp (or other delta protocols) while I think the old istio approach worked with Prometheus as well. Is the Prometheus behavior changed by this pr?

IIUC, this worked at the underlying mechanism of envoy. which means it works for both metric service and /stats/prometheus endpoint.

The old behavior didn't work with prometheus either AFAICT, so it didn't change. The behavior is the same if you create all stats at once, and then wait some time.

this new behavior will work if you enable stats_eviction in bootstrap, is that right?

@kyessenov
Copy link
Copy Markdown
Contributor Author

qq - the envoy PR seems to suggest it only works with otlp (or other delta protocols) while I think the old istio approach worked with Prometheus as well. Is the Prometheus behavior changed by this pr?

IIUC, this worked at the underlying mechanism of envoy. which means it works for both metric service and /stats/prometheus endpoint.

It worked in the sense that prometheus would handle disappearing stats, even though that's not normal. The difference is that before a stat would disappear all the time, but now only when it's not being emitted.

The old behavior didn't work with prometheus either AFAICT, so it didn't change. The behavior is the same if you create all stats at once, and then wait some time.

this new behavior will work if you enable stats_eviction in bootstrap, is that right?

Yes, it requires that field to turn on. It does nothing without it.

Copy link
Copy Markdown
Member

@zirain zirain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

This make things easier.

@istio-testing istio-testing merged commit ce65656 into istio:master Sep 4, 2025
7 checks passed
@ringerc
Copy link
Copy Markdown

ringerc commented Dec 3, 2025

this new behavior will work if you enable stats_eviction in bootstrap, is that right?

It sounds like that's what istio/api#3562 and istio/istio#57736 do, right?

@zirain
Copy link
Copy Markdown
Member

zirain commented Dec 4, 2025

this new behavior will work if you enable stats_eviction in bootstrap, is that right?

It sounds like that's what istio/api#3562 and istio/istio#57736 do, right?

bingo

@ringerc
Copy link
Copy Markdown

ringerc commented Dec 31, 2025

Docs update in istio/istio.io#17043

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants