Skip to content

Change OrcaWeightManager to use metric_names_for_computing_utilization first.#44196

Merged
adisuissa merged 4 commits intoenvoyproxy:mainfrom
efimki:metrics_order
Apr 10, 2026
Merged

Change OrcaWeightManager to use metric_names_for_computing_utilization first.#44196
adisuissa merged 4 commits intoenvoyproxy:mainfrom
efimki:metrics_order

Conversation

@efimki
Copy link
Copy Markdown
Member

@efimki efimki commented Mar 31, 2026

Commit Message:

This PR changes preferred order of ORCA endpoint metrics used to compute utilization in OrcaWeightManager used by ClientSideWeightedRoundRobin lb policy to:

  1. Metrics configured in metric_names_for_computing_utilization.
  2. application_utilization
  3. cpu_utilization

Additional Description:

The discussion on gRPC A114 has brought up the point that current order of ORCA endpoint metrics: application_utilization -> metric_names_for_computing_utilization configuration -> cpu_utilization means that if endpoint load report includes application_utilization, then metric_names_for_computing_utilization configuration is completely ignored, which is not ideal.

Changing preferred order to start with metric_names_for_computing_utilization still allows fallback to application_utilization and cpu_utilization if no metrics are configured.

Risk Level: Low
Testing:

  • bazel test //test/extensions/load_balancing_policies/common:orca_weight_manager_test
  • bazel test //test/extensions/load_balancing_policies/client_side_weighted_round_robin:client_side_weighted_round_robin_lb_test

Protected by: envoy.reloadable_features.orca_weight_manager_use_named_metrics_first
Release Notes: Yes

…tilization for utilization.

Signed-off-by: Misha Efimov <mef@google.com>
@repokitteh-read-only
Copy link
Copy Markdown

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #44196 was opened by efimki.

see: more, trace.

@efimki
Copy link
Copy Markdown
Member Author

efimki commented Mar 31, 2026

@markdroth FYI

@markdroth
Copy link
Copy Markdown
Contributor

Thanks for taking care of this, @efimki!

@adisuissa adisuissa self-assigned this Apr 1, 2026
Copy link
Copy Markdown
Contributor

@adisuissa adisuissa left a comment

Choose a reason for hiding this comment

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

High-level LGTM.

  1. AFAICT this is a breaking-change, and should have a runtime guard against it (as well as release notes).
  2. It seems that there are no tests that validate this ordering behavior. There should be probably be tests (with accompanying comments) if this ordering is required.

Signed-off-by: Misha Efimov <mef@google.com>
@repokitteh-read-only
Copy link
Copy Markdown

CC @envoyproxy/runtime-guard-changes: FYI only for changes made to (source/common/runtime/runtime_features.cc).

🐱

Caused by: #44196 was synchronize by efimki.

see: more, trace.

Copy link
Copy Markdown
Contributor

@adisuissa adisuissa left a comment

Choose a reason for hiding this comment

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

Minor nit, but otherwise lgtm.

efimki added 2 commits April 6, 2026 17:51
Signed-off-by: Misha Efimov <mef@google.com>
Signed-off-by: Misha Efimov <mef@google.com>
Copy link
Copy Markdown
Contributor

@adisuissa adisuissa 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!

@adisuissa adisuissa merged commit 341d7a7 into envoyproxy:main Apr 10, 2026
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants