Skip to content

xds: enable orca to lrs propagation by default #12836

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.

Sign up for GitHub

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

Merged
shivaspeaks merged 1 commit into from
Jun 2, 2026
Merged

xds: enable orca to lrs propagation by default #12836

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion xds/src/main/java/io/grpc/xds/XdsClusterResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ private static StructOrError<CdsUpdate.Builder> parseNonAggregateCluster(
boolean isHttp11ProxyAvailable = false;
BackendMetricPropagation backendMetricPropagation = null;

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

@danielzhaotongliu danielzhaotongliu Jun 3, 2026

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

@shivaspeaks shivaspeaks Jun 3, 2026

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.

backendMetricPropagation = BackendMetricPropagation.fromMetricSpecs(
cluster.getLrsReportEndpointMetricsList());
}
Expand Down
8 changes: 5 additions & 3 deletions xds/src/main/java/io/grpc/xds/client/LoadStatsManager2.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public final class LoadStatsManager2 {
Map<Locality, ReferenceCounted<ClusterLocalityStats>>>> allLoadStats = new HashMap<>();
private final Supplier<Stopwatch> stopwatchSupplier;
public static boolean isEnabledOrcaLrsPropagation =
GrpcUtil.getFlag("GRPC_EXPERIMENTAL_XDS_ORCA_LRS_PROPAGATION", false);
GrpcUtil.getFlag("GRPC_EXPERIMENTAL_XDS_ORCA_LRS_PROPAGATION", true);

@VisibleForTesting
public LoadStatsManager2(Supplier<Stopwatch> stopwatchSupplier) {
Expand Down Expand Up @@ -345,12 +345,14 @@ public final class ClusterLocalityStats {

private ClusterLocalityStats(
String clusterName, @Nullable String edsServiceName, Locality locality,
Stopwatch stopwatch, BackendMetricPropagation backendMetricPropagation) {
Stopwatch stopwatch, @Nullable BackendMetricPropagation backendMetricPropagation) {
this.clusterName = checkNotNull(clusterName, "clusterName");
this.edsServiceName = edsServiceName;
this.locality = checkNotNull(locality, "locality");
this.stopwatch = checkNotNull(stopwatch, "stopwatch");
this.backendMetricPropagation = backendMetricPropagation;
this.backendMetricPropagation = backendMetricPropagation != null
? backendMetricPropagation
: BackendMetricPropagation.fromMetricSpecs(null);
stopwatch.reset().start();
}

Expand Down
33 changes: 17 additions & 16 deletions xds/src/test/java/io/grpc/xds/ClusterImplLoadBalancerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,8 @@ public void recordLoadStats() {
null, Collections.<DropOverload>emptyList(),
GracefulSwitchLoadBalancer.createLoadBalancingPolicyConfig(
weightedTargetProvider, weightedTargetConfig),
null, Collections.emptyMap(), null);
null, Collections.emptyMap(),
BackendMetricPropagation.fromMetricSpecs(Arrays.asList("named_metrics.*")));
EquivalentAddressGroup endpoint = makeAddress("endpoint-addr", locality);
deliverAddressesAndConfig(Collections.singletonList(endpoint), config);
FakeLoadBalancer leafBalancer = Iterables.getOnlyElement(downstreamBalancers);
Expand Down Expand Up @@ -368,24 +369,24 @@ public void recordLoadStats() {
assertThat(localityStats.totalSuccessfulRequests()).isEqualTo(1L);
assertThat(localityStats.totalErrorRequests()).isEqualTo(1L);
assertThat(localityStats.totalRequestsInProgress()).isEqualTo(1L);
assertThat(localityStats.loadMetricStatsMap().containsKey("named1")).isTrue();
assertThat(localityStats.loadMetricStatsMap().containsKey("named_metrics.named1")).isTrue();
assertThat(
localityStats.loadMetricStatsMap().get("named1").numRequestsFinishedWithMetric()).isEqualTo(
2L);
assertThat(localityStats.loadMetricStatsMap().get("named1").totalMetricValue()).isWithin(
TOLERANCE).of(3.14159 + 2.718);
assertThat(localityStats.loadMetricStatsMap().containsKey("named2")).isTrue();
localityStats.loadMetricStatsMap().get("named_metrics.named1")
.numRequestsFinishedWithMetric()).isEqualTo(2L);
assertThat(localityStats.loadMetricStatsMap().get("named_metrics.named1")
.totalMetricValue()).isWithin(TOLERANCE).of(3.14159 + 2.718);
assertThat(localityStats.loadMetricStatsMap().containsKey("named_metrics.named2")).isTrue();
assertThat(
localityStats.loadMetricStatsMap().get("named2").numRequestsFinishedWithMetric()).isEqualTo(
2L);
assertThat(localityStats.loadMetricStatsMap().get("named2").totalMetricValue()).isWithin(
TOLERANCE).of(-1.618 + 1.414);
assertThat(localityStats.loadMetricStatsMap().containsKey("named3")).isTrue();
localityStats.loadMetricStatsMap().get("named_metrics.named2")
.numRequestsFinishedWithMetric()).isEqualTo(2L);
assertThat(localityStats.loadMetricStatsMap().get("named_metrics.named2")
.totalMetricValue()).isWithin(TOLERANCE).of(-1.618 + 1.414);
assertThat(localityStats.loadMetricStatsMap().containsKey("named_metrics.named3")).isTrue();
assertThat(
localityStats.loadMetricStatsMap().get("named3").numRequestsFinishedWithMetric()).isEqualTo(
1L);
assertThat(localityStats.loadMetricStatsMap().get("named3").totalMetricValue()).isWithin(
TOLERANCE).of(0.009);
localityStats.loadMetricStatsMap().get("named_metrics.named3")
.numRequestsFinishedWithMetric()).isEqualTo(1L);
assertThat(localityStats.loadMetricStatsMap().get("named_metrics.named3")
.totalMetricValue()).isWithin(TOLERANCE).of(0.009);

streamTracer3.streamClosed(Status.OK);
subchannel.shutdown(); // stats recorder released
Expand Down
3 changes: 2 additions & 1 deletion xds/src/test/java/io/grpc/xds/GrpcXdsClientImplDataTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2652,6 +2652,7 @@ public void parseNonAggregateCluster_withHttp11ProxyTransportSocket() throws Exc

@Test
public void processCluster_parsesOrcaLrsPropagationMetrics() throws ResourceInvalidException {
boolean originalVal = LoadStatsManager2.isEnabledOrcaLrsPropagation;
LoadStatsManager2.isEnabledOrcaLrsPropagation = true;

ImmutableList<String> metricSpecs = ImmutableList.of(
Expand Down Expand Up @@ -2683,7 +2684,7 @@ public void processCluster_parsesOrcaLrsPropagationMetrics() throws ResourceInva
assertThat(propagationConfig.shouldPropagateNamedMetric("unknown_metric_spec"))
.isFalse();

LoadStatsManager2.isEnabledOrcaLrsPropagation = false;
LoadStatsManager2.isEnabledOrcaLrsPropagation = originalVal;
}

@Test
Expand Down
16 changes: 11 additions & 5 deletions xds/src/test/java/io/grpc/xds/LoadReportClientTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,15 @@
import io.grpc.internal.FakeClock;
import io.grpc.stub.StreamObserver;
import io.grpc.testing.GrpcCleanupRule;
import io.grpc.xds.client.BackendMetricPropagation;
import io.grpc.xds.client.EnvoyProtoData;
import io.grpc.xds.client.LoadReportClient;
import io.grpc.xds.client.LoadStatsManager2;
import io.grpc.xds.client.LoadStatsManager2.ClusterDropStats;
import io.grpc.xds.client.LoadStatsManager2.ClusterLocalityStats;
import io.grpc.xds.client.Locality;
import java.util.ArrayDeque;
import java.util.Arrays;
import java.util.List;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
Expand Down Expand Up @@ -91,6 +93,8 @@ public class LoadReportClientTest {
private static final String EDS_SERVICE_NAME2 = "backend-service-bar.googleapis.com";
private static final Locality LOCALITY1 = Locality.create("region1", "zone1", "subZone1");
private static final Locality LOCALITY2 = Locality.create("region2", "zone2", "subZone2");
private static final BackendMetricPropagation PROPAGATE_ALL =
BackendMetricPropagation.fromMetricSpecs(Arrays.asList("named_metrics.*"));
private static final FakeClock.TaskFilter LOAD_REPORTING_TASK_FILTER =
new FakeClock.TaskFilter() {
@Override
Expand Down Expand Up @@ -205,15 +209,17 @@ private void addFakeStatsData() {
dropStats2.recordDroppedRequest("throttle");
}
ClusterLocalityStats localityStats1 =
loadStatsManager.getClusterLocalityStats(CLUSTER1, EDS_SERVICE_NAME1, LOCALITY1);
loadStatsManager.getClusterLocalityStats(
CLUSTER1, EDS_SERVICE_NAME1, LOCALITY1, PROPAGATE_ALL);
for (int i = 0; i < 31; i++) {
localityStats1.recordCallStarted();
}
localityStats1.recordBackendLoadMetricStats(ImmutableMap.of("named1", 3.14159));
localityStats1.recordBackendLoadMetricStats(ImmutableMap.of("named1", 1.618));
localityStats1.recordBackendLoadMetricStats(ImmutableMap.of("named1", -2.718));
ClusterLocalityStats localityStats2 =
loadStatsManager.getClusterLocalityStats(CLUSTER2, EDS_SERVICE_NAME2, LOCALITY2);
loadStatsManager.getClusterLocalityStats(
CLUSTER2, EDS_SERVICE_NAME2, LOCALITY2, PROPAGATE_ALL);
for (int i = 0; i < 45; i++) {
localityStats2.recordCallStarted();
}
Expand Down Expand Up @@ -263,7 +269,7 @@ public void periodicLoadReporting() {
assertThat(localityStats.getLoadMetricStatsCount()).isEqualTo(1);
EndpointLoadMetricStats loadMetricStats = Iterables.getOnlyElement(
localityStats.getLoadMetricStatsList());
assertThat(loadMetricStats.getMetricName()).isEqualTo("named1");
assertThat(loadMetricStats.getMetricName()).isEqualTo("named_metrics.named1");
assertThat(loadMetricStats.getNumRequestsFinishedWithMetric()).isEqualTo(3L);
assertThat(loadMetricStats.getTotalMetricValue()).isEqualTo(3.14159 + 1.618 - 2.718);

Expand Down Expand Up @@ -353,7 +359,7 @@ public void periodicLoadReporting() {
assertThat(localityStats2.getLoadMetricStatsCount()).isEqualTo(1);
EndpointLoadMetricStats loadMetricStats2 = Iterables.getOnlyElement(
localityStats2.getLoadMetricStatsList());
assertThat(loadMetricStats2.getMetricName()).isEqualTo("named2");
assertThat(loadMetricStats2.getMetricName()).isEqualTo("named_metrics.named2");
assertThat(loadMetricStats2.getNumRequestsFinishedWithMetric()).isEqualTo(1L);
assertThat(loadMetricStats2.getTotalMetricValue()).isEqualTo(1.414);

Expand Down Expand Up @@ -530,7 +536,7 @@ public void lrsStreamClosedAndRetried() {
assertThat(localityStats.getLoadMetricStatsCount()).isEqualTo(1);
EndpointLoadMetricStats loadMetricStats = Iterables.getOnlyElement(
localityStats.getLoadMetricStatsList());
assertThat(loadMetricStats.getMetricName()).isEqualTo("named1");
assertThat(loadMetricStats.getMetricName()).isEqualTo("named_metrics.named1");
assertThat(loadMetricStats.getNumRequestsFinishedWithMetric()).isEqualTo(3L);
assertThat(loadMetricStats.getTotalMetricValue()).isEqualTo(3.14159 + 1.618 - 2.718);

Expand Down
75 changes: 39 additions & 36 deletions xds/src/test/java/io/grpc/xds/client/LoadStatsManager2Test.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@ public class LoadStatsManager2Test {
private static final Locality LOCALITY3 =
Locality.create("test_region3", "test_zone3", "test_subzone3");

private static final BackendMetricPropagation PROPAGATE_ALL =
BackendMetricPropagation.fromMetricSpecs(Arrays.asList("named_metrics.*"));

private final FakeClock fakeClock = new FakeClock();
private final LoadStatsManager2 loadStatsManager =
new LoadStatsManager2(fakeClock.getStopwatchSupplier());
Expand All @@ -64,11 +67,11 @@ public void recordAndGetReport() {
ClusterDropStats dropCounter2 = loadStatsManager.getClusterDropStats(
CLUSTER_NAME1, EDS_SERVICE_NAME2);
ClusterLocalityStats loadCounter1 = loadStatsManager.getClusterLocalityStats(
CLUSTER_NAME1, EDS_SERVICE_NAME1, LOCALITY1);
CLUSTER_NAME1, EDS_SERVICE_NAME1, LOCALITY1, PROPAGATE_ALL);
ClusterLocalityStats loadCounter2 = loadStatsManager.getClusterLocalityStats(
CLUSTER_NAME1, EDS_SERVICE_NAME1, LOCALITY2);
CLUSTER_NAME1, EDS_SERVICE_NAME1, LOCALITY2, PROPAGATE_ALL);
ClusterLocalityStats loadCounter3 = loadStatsManager.getClusterLocalityStats(
CLUSTER_NAME2, null, LOCALITY3);
CLUSTER_NAME2, null, LOCALITY3, PROPAGATE_ALL);
dropCounter1.recordDroppedRequest("lb");
dropCounter1.recordDroppedRequest("throttle");
for (int i = 0; i < 19; i++) {
Expand Down Expand Up @@ -106,31 +109,31 @@ public void recordAndGetReport() {
assertThat(loadStats1.totalSuccessfulRequests()).isEqualTo(1L);
assertThat(loadStats1.totalErrorRequests()).isEqualTo(0L);
assertThat(loadStats1.totalRequestsInProgress()).isEqualTo(19L - 1L);
assertThat(loadStats1.loadMetricStatsMap().containsKey("named1")).isTrue();
assertThat(loadStats1.loadMetricStatsMap().containsKey("named2")).isTrue();
assertThat(loadStats1.loadMetricStatsMap().containsKey("named_metrics.named1")).isTrue();
assertThat(loadStats1.loadMetricStatsMap().containsKey("named_metrics.named2")).isTrue();
assertThat(
loadStats1.loadMetricStatsMap().get("named1").numRequestsFinishedWithMetric()).isEqualTo(
4L);
assertThat(loadStats1.loadMetricStatsMap().get("named1").totalMetricValue()).isWithin(TOLERANCE)
.of(3.14159 + 1.618 + 99 - 97.23);
loadStats1.loadMetricStatsMap().get("named_metrics.named1")
.numRequestsFinishedWithMetric()).isEqualTo(4L);
assertThat(loadStats1.loadMetricStatsMap().get("named_metrics.named1").totalMetricValue())
.isWithin(TOLERANCE).of(3.14159 + 1.618 + 99 - 97.23);
assertThat(
loadStats1.loadMetricStatsMap().get("named2").numRequestsFinishedWithMetric()).isEqualTo(
1L);
assertThat(loadStats1.loadMetricStatsMap().get("named2").totalMetricValue()).isWithin(TOLERANCE)
.of(-2.718);
loadStats1.loadMetricStatsMap().get("named_metrics.named2")
.numRequestsFinishedWithMetric()).isEqualTo(1L);
assertThat(loadStats1.loadMetricStatsMap().get("named_metrics.named2").totalMetricValue())
.isWithin(TOLERANCE).of(-2.718);

UpstreamLocalityStats loadStats2 =
findLocalityStats(stats1.upstreamLocalityStatsList(), LOCALITY2);
assertThat(loadStats2.totalIssuedRequests()).isEqualTo(9L);
assertThat(loadStats2.totalSuccessfulRequests()).isEqualTo(0L);
assertThat(loadStats2.totalErrorRequests()).isEqualTo(1L);
assertThat(loadStats2.totalRequestsInProgress()).isEqualTo(9L - 1L);
assertThat(loadStats2.loadMetricStatsMap().containsKey("named3")).isTrue();
assertThat(loadStats2.loadMetricStatsMap().containsKey("named_metrics.named3")).isTrue();
assertThat(
loadStats2.loadMetricStatsMap().get("named3").numRequestsFinishedWithMetric()).isEqualTo(
1L);
assertThat(loadStats2.loadMetricStatsMap().get("named3").totalMetricValue()).isWithin(TOLERANCE)
.of(0.0009);
loadStats2.loadMetricStatsMap().get("named_metrics.named3")
.numRequestsFinishedWithMetric()).isEqualTo(1L);
assertThat(loadStats2.loadMetricStatsMap().get("named_metrics.named3").totalMetricValue())
.isWithin(TOLERANCE).of(0.0009);

ClusterStats stats2 = findClusterStats(allStats, CLUSTER_NAME1, EDS_SERVICE_NAME2);
assertThat(stats2.loadReportIntervalNano()).isEqualTo(TimeUnit.SECONDS.toNanos(5L + 10L));
Expand Down Expand Up @@ -221,9 +224,9 @@ public void dropCounterDelayedDeletionAfterReported() {
@Test
public void sharedLoadCounterStatsAggregation() {
ClusterLocalityStats ref1 = loadStatsManager.getClusterLocalityStats(
CLUSTER_NAME1, EDS_SERVICE_NAME1, LOCALITY1);
CLUSTER_NAME1, EDS_SERVICE_NAME1, LOCALITY1, PROPAGATE_ALL);
ClusterLocalityStats ref2 = loadStatsManager.getClusterLocalityStats(
CLUSTER_NAME1, EDS_SERVICE_NAME1, LOCALITY1);
CLUSTER_NAME1, EDS_SERVICE_NAME1, LOCALITY1, PROPAGATE_ALL);
ref1.recordCallStarted();
ref1.recordBackendLoadMetricStats(ImmutableMap.of("named1", 1.618));
ref1.recordBackendLoadMetricStats(ImmutableMap.of("named1", 3.14159));
Expand All @@ -241,18 +244,18 @@ public void sharedLoadCounterStatsAggregation() {
assertThat(localityStats.totalSuccessfulRequests()).isEqualTo(1L);
assertThat(localityStats.totalErrorRequests()).isEqualTo(1L);
assertThat(localityStats.totalRequestsInProgress()).isEqualTo(1L + 2L - 1L - 1L);
assertThat(localityStats.loadMetricStatsMap().containsKey("named1")).isTrue();
assertThat(localityStats.loadMetricStatsMap().containsKey("named2")).isTrue();
assertThat(localityStats.loadMetricStatsMap().containsKey("named_metrics.named1")).isTrue();
assertThat(localityStats.loadMetricStatsMap().containsKey("named_metrics.named2")).isTrue();
assertThat(
localityStats.loadMetricStatsMap().get("named1").numRequestsFinishedWithMetric()).isEqualTo(
3L);
assertThat(localityStats.loadMetricStatsMap().get("named1").totalMetricValue()).isWithin(
TOLERANCE).of(1.618 + 3.14159 - 1);
localityStats.loadMetricStatsMap().get("named_metrics.named1")
.numRequestsFinishedWithMetric()).isEqualTo(3L);
assertThat(localityStats.loadMetricStatsMap().get("named_metrics.named1")
.totalMetricValue()).isWithin(TOLERANCE).of(1.618 + 3.14159 - 1);
assertThat(
localityStats.loadMetricStatsMap().get("named2").numRequestsFinishedWithMetric()).isEqualTo(
1L);
assertThat(localityStats.loadMetricStatsMap().get("named2").totalMetricValue()).isEqualTo(
2.718);
localityStats.loadMetricStatsMap().get("named_metrics.named2")
.numRequestsFinishedWithMetric()).isEqualTo(1L);
assertThat(localityStats.loadMetricStatsMap().get("named_metrics.named2")
.totalMetricValue()).isEqualTo(2.718);
}

@Test
Expand Down Expand Up @@ -311,7 +314,7 @@ public void recordMetrics_orcaLrsPropagationEnabled_wildcardNamedMetrics() {
@Test
public void loadCounterDelayedDeletionAfterAllInProgressRequestsReported() {
ClusterLocalityStats counter = loadStatsManager.getClusterLocalityStats(
CLUSTER_NAME1, EDS_SERVICE_NAME1, LOCALITY1);
CLUSTER_NAME1, EDS_SERVICE_NAME1, LOCALITY1, PROPAGATE_ALL);
counter.recordCallStarted();
counter.recordCallStarted();
counter.recordBackendLoadMetricStats(ImmutableMap.of("named1", 2.718));
Expand All @@ -325,12 +328,12 @@ public void loadCounterDelayedDeletionAfterAllInProgressRequestsReported() {
assertThat(localityStats.totalSuccessfulRequests()).isEqualTo(0L);
assertThat(localityStats.totalErrorRequests()).isEqualTo(0L);
assertThat(localityStats.totalRequestsInProgress()).isEqualTo(2L);
assertThat(localityStats.loadMetricStatsMap().containsKey("named1")).isTrue();
assertThat(localityStats.loadMetricStatsMap().containsKey("named_metrics.named1")).isTrue();
assertThat(
localityStats.loadMetricStatsMap().get("named1").numRequestsFinishedWithMetric()).isEqualTo(
2L);
assertThat(localityStats.loadMetricStatsMap().get("named1").totalMetricValue()).isEqualTo(
2.718 + 1.414);
localityStats.loadMetricStatsMap().get("named_metrics.named1")
.numRequestsFinishedWithMetric()).isEqualTo(2L);
assertThat(localityStats.loadMetricStatsMap().get("named_metrics.named1")
.totalMetricValue()).isEqualTo(2.718 + 1.414);

// release the counter, but requests still in-flight
counter.release();
Expand Down
Loading