From b8e253afdc129e383b541960b86bc2de0b755d31 Mon Sep 17 00:00:00 2001 From: MV Shiva Prasad Date: Tue, 2 Jun 2026 13:51:28 +0530 Subject: [PATCH] xds: set orca to lrs default flag true --- .../java/io/grpc/xds/XdsClusterResource.java | 2 +- .../io/grpc/xds/client/LoadStatsManager2.java | 8 +- .../grpc/xds/ClusterImplLoadBalancerTest.java | 33 ++++---- .../grpc/xds/GrpcXdsClientImplDataTest.java | 3 +- .../io/grpc/xds/LoadReportClientTest.java | 16 ++-- .../xds/client/LoadStatsManager2Test.java | 75 ++++++++++--------- 6 files changed, 75 insertions(+), 62 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/XdsClusterResource.java b/xds/src/main/java/io/grpc/xds/XdsClusterResource.java index 41c2f734830..34035ab1da8 100644 --- a/xds/src/main/java/io/grpc/xds/XdsClusterResource.java +++ b/xds/src/main/java/io/grpc/xds/XdsClusterResource.java @@ -225,7 +225,7 @@ private static StructOrError parseNonAggregateCluster( boolean isHttp11ProxyAvailable = false; BackendMetricPropagation backendMetricPropagation = null; - if (isEnabledOrcaLrsPropagation) { + if (isEnabledOrcaLrsPropagation && !cluster.getLrsReportEndpointMetricsList().isEmpty()) { backendMetricPropagation = BackendMetricPropagation.fromMetricSpecs( cluster.getLrsReportEndpointMetricsList()); } diff --git a/xds/src/main/java/io/grpc/xds/client/LoadStatsManager2.java b/xds/src/main/java/io/grpc/xds/client/LoadStatsManager2.java index cd858dccd99..c2e3e8f3f86 100644 --- a/xds/src/main/java/io/grpc/xds/client/LoadStatsManager2.java +++ b/xds/src/main/java/io/grpc/xds/client/LoadStatsManager2.java @@ -59,7 +59,7 @@ public final class LoadStatsManager2 { Map>>> allLoadStats = new HashMap<>(); private final Supplier 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 stopwatchSupplier) { @@ -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(); } diff --git a/xds/src/test/java/io/grpc/xds/ClusterImplLoadBalancerTest.java b/xds/src/test/java/io/grpc/xds/ClusterImplLoadBalancerTest.java index 9277675385a..6cf6fa22a8a 100644 --- a/xds/src/test/java/io/grpc/xds/ClusterImplLoadBalancerTest.java +++ b/xds/src/test/java/io/grpc/xds/ClusterImplLoadBalancerTest.java @@ -325,7 +325,8 @@ public void recordLoadStats() { null, Collections.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); @@ -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 diff --git a/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplDataTest.java b/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplDataTest.java index 3419a3c804e..bc45bb526fd 100644 --- a/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplDataTest.java +++ b/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplDataTest.java @@ -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 metricSpecs = ImmutableList.of( @@ -2683,7 +2684,7 @@ public void processCluster_parsesOrcaLrsPropagationMetrics() throws ResourceInva assertThat(propagationConfig.shouldPropagateNamedMetric("unknown_metric_spec")) .isFalse(); - LoadStatsManager2.isEnabledOrcaLrsPropagation = false; + LoadStatsManager2.isEnabledOrcaLrsPropagation = originalVal; } @Test diff --git a/xds/src/test/java/io/grpc/xds/LoadReportClientTest.java b/xds/src/test/java/io/grpc/xds/LoadReportClientTest.java index 9bdf86132b6..80eb5cc1f47 100644 --- a/xds/src/test/java/io/grpc/xds/LoadReportClientTest.java +++ b/xds/src/test/java/io/grpc/xds/LoadReportClientTest.java @@ -51,6 +51,7 @@ 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; @@ -58,6 +59,7 @@ 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; @@ -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 @@ -205,7 +209,8 @@ 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(); } @@ -213,7 +218,8 @@ private void addFakeStatsData() { 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(); } @@ -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); @@ -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); @@ -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); diff --git a/xds/src/test/java/io/grpc/xds/client/LoadStatsManager2Test.java b/xds/src/test/java/io/grpc/xds/client/LoadStatsManager2Test.java index a0642f7e4bb..e6af888f353 100644 --- a/xds/src/test/java/io/grpc/xds/client/LoadStatsManager2Test.java +++ b/xds/src/test/java/io/grpc/xds/client/LoadStatsManager2Test.java @@ -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()); @@ -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++) { @@ -106,18 +109,18 @@ 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); @@ -125,12 +128,12 @@ public void recordAndGetReport() { 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)); @@ -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)); @@ -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 @@ -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)); @@ -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();