Skip to content

xds: enable orca to lrs propagation by default#12836

Merged
shivaspeaks merged 1 commit into
grpc:masterfrom
shivaspeaks:default-true-orca-to-lrs
Jun 2, 2026
Merged

xds: enable orca to lrs propagation by default#12836
shivaspeaks merged 1 commit into
grpc:masterfrom
shivaspeaks:default-true-orca-to-lrs

Conversation

@shivaspeaks
Copy link
Copy Markdown
Member

@shivaspeaks shivaspeaks commented Jun 2, 2026

When the GRPC_EXPERIMENTAL_XDS_ORCA_LRS_PROPAGATION flag defaults to true, the following minor behavioral changes in backend metrics reporting will take effect:

  • Named Metrics Filtering & Prefixing:
    Before: All custom/named metrics were reported as-is under their original name keys (e.g., "named1").
    Now: Custom/named metrics are filtered based on the CDS-configured metrics specification. If a metric matches the propagation filter, it is now reported with the "named_metrics." prefix (e.g., "named_metrics.named1"). Unmatched named metrics are excluded.

  • CdsUpdate Structure for Empty Metric Specs (Runtime No-op):
    Change: When the CDS cluster resource contains an empty lrs_report_endpoint_metrics list, the parser in XdsClusterResource now leaves backendMetricPropagation as null in the parsed CdsUpdate object rather than instantiating an empty configuration object.
    Why: This ensures backward compatibility with Mockito verification tests that assert exact equality on CdsUpdate objects and expect unconfigured fields to be null.

@shivaspeaks shivaspeaks merged commit 1e85674 into grpc:master Jun 2, 2026
17 of 18 checks passed
Copy link
Copy Markdown
Collaborator

@danielzhaotongliu danielzhaotongliu left a comment

Choose a reason for hiding this comment

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

This generally LGTM.

Note that there are (very) minor behaviour changes here that should be no-op in this PR, but should be called out in the PR description for posterity.

BackendMetricPropagation backendMetricPropagation = null;

if (isEnabledOrcaLrsPropagation) {
if (isEnabledOrcaLrsPropagation && !cluster.getLrsReportEndpointMetricsList().isEmpty()) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

For my education, is this code being added here as a minor optimization to only propagate the list is not empty?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes and when a CDS resource has no metric specifications configured, we want backendMetricPropagation in CdsUpdate to remain null (representing "not configured") rather than instantiating an empty configuration object.

This was also impacting the behavior of XdsDependencyManager. Existing unit tests in XdsDependencyManagerTest verify parsed CdsUpdate objects using Mockito mock verifications that rely on strict .equals() checks. If we instantiated a non-null empty BackendMetricPropagation object for empty lists, those existing tests would fail because their expected mocks assume null for unconfigured fields.

And yeah it avoids unnecessary object allocation when there is nothing to propagate.

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