Skip to content

Commit 1e85674

Browse files
shivaspeaksshivaspeaks
authored
xds: enable orca to lrs propagation by default (grpc#12836)
1 parent 4561594 commit 1e85674

6 files changed

Lines changed: 75 additions & 62 deletions

File tree

‎xds/src/main/java/io/grpc/xds/XdsClusterResource.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,7 @@ private static StructOrError<CdsUpdate.Builder> parseNonAggregateCluster(
225225
boolean isHttp11ProxyAvailable = false;
226226
BackendMetricPropagation backendMetricPropagation = null;
227227

228-
if (isEnabledOrcaLrsPropagation) {
228+
if (isEnabledOrcaLrsPropagation && !cluster.getLrsReportEndpointMetricsList().isEmpty()) {
229229
backendMetricPropagation = BackendMetricPropagation.fromMetricSpecs(
230230
cluster.getLrsReportEndpointMetricsList());
231231
}

‎xds/src/main/java/io/grpc/xds/client/LoadStatsManager2.java‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ public final class LoadStatsManager2 {
5959
Map<Locality, ReferenceCounted<ClusterLocalityStats>>>> allLoadStats = new HashMap<>();
6060
private final Supplier<Stopwatch> stopwatchSupplier;
6161
public static boolean isEnabledOrcaLrsPropagation =
62-
GrpcUtil.getFlag("GRPC_EXPERIMENTAL_XDS_ORCA_LRS_PROPAGATION", false);
62+
GrpcUtil.getFlag("GRPC_EXPERIMENTAL_XDS_ORCA_LRS_PROPAGATION", true);
6363

6464
@VisibleForTesting
6565
public LoadStatsManager2(Supplier<Stopwatch> stopwatchSupplier) {
@@ -345,12 +345,14 @@ public final class ClusterLocalityStats {
345345

346346
private ClusterLocalityStats(
347347
String clusterName, @Nullable String edsServiceName, Locality locality,
348-
Stopwatch stopwatch, BackendMetricPropagation backendMetricPropagation) {
348+
Stopwatch stopwatch, @Nullable BackendMetricPropagation backendMetricPropagation) {
349349
this.clusterName = checkNotNull(clusterName, "clusterName");
350350
this.edsServiceName = edsServiceName;
351351
this.locality = checkNotNull(locality, "locality");
352352
this.stopwatch = checkNotNull(stopwatch, "stopwatch");
353-
this.backendMetricPropagation = backendMetricPropagation;
353+
this.backendMetricPropagation = backendMetricPropagation != null
354+
? backendMetricPropagation
355+
: BackendMetricPropagation.fromMetricSpecs(null);
354356
stopwatch.reset().start();
355357
}
356358

‎xds/src/test/java/io/grpc/xds/ClusterImplLoadBalancerTest.java‎

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,8 @@ public void recordLoadStats() {
325325
null, Collections.<DropOverload>emptyList(),
326326
GracefulSwitchLoadBalancer.createLoadBalancingPolicyConfig(
327327
weightedTargetProvider, weightedTargetConfig),
328-
null, Collections.emptyMap(), null);
328+
null, Collections.emptyMap(),
329+
BackendMetricPropagation.fromMetricSpecs(Arrays.asList("named_metrics.*")));
329330
EquivalentAddressGroup endpoint = makeAddress("endpoint-addr", locality);
330331
deliverAddressesAndConfig(Collections.singletonList(endpoint), config);
331332
FakeLoadBalancer leafBalancer = Iterables.getOnlyElement(downstreamBalancers);
@@ -368,24 +369,24 @@ public void recordLoadStats() {
368369
assertThat(localityStats.totalSuccessfulRequests()).isEqualTo(1L);
369370
assertThat(localityStats.totalErrorRequests()).isEqualTo(1L);
370371
assertThat(localityStats.totalRequestsInProgress()).isEqualTo(1L);
371-
assertThat(localityStats.loadMetricStatsMap().containsKey("named1")).isTrue();
372+
assertThat(localityStats.loadMetricStatsMap().containsKey("named_metrics.named1")).isTrue();
372373
assertThat(
373-
localityStats.loadMetricStatsMap().get("named1").numRequestsFinishedWithMetric()).isEqualTo(
374-
2L);
375-
assertThat(localityStats.loadMetricStatsMap().get("named1").totalMetricValue()).isWithin(
376-
TOLERANCE).of(3.14159 + 2.718);
377-
assertThat(localityStats.loadMetricStatsMap().containsKey("named2")).isTrue();
374+
localityStats.loadMetricStatsMap().get("named_metrics.named1")
375+
.numRequestsFinishedWithMetric()).isEqualTo(2L);
376+
assertThat(localityStats.loadMetricStatsMap().get("named_metrics.named1")
377+
.totalMetricValue()).isWithin(TOLERANCE).of(3.14159 + 2.718);
378+
assertThat(localityStats.loadMetricStatsMap().containsKey("named_metrics.named2")).isTrue();
378379
assertThat(
379-
localityStats.loadMetricStatsMap().get("named2").numRequestsFinishedWithMetric()).isEqualTo(
380-
2L);
381-
assertThat(localityStats.loadMetricStatsMap().get("named2").totalMetricValue()).isWithin(
382-
TOLERANCE).of(-1.618 + 1.414);
383-
assertThat(localityStats.loadMetricStatsMap().containsKey("named3")).isTrue();
380+
localityStats.loadMetricStatsMap().get("named_metrics.named2")
381+
.numRequestsFinishedWithMetric()).isEqualTo(2L);
382+
assertThat(localityStats.loadMetricStatsMap().get("named_metrics.named2")
383+
.totalMetricValue()).isWithin(TOLERANCE).of(-1.618 + 1.414);
384+
assertThat(localityStats.loadMetricStatsMap().containsKey("named_metrics.named3")).isTrue();
384385
assertThat(
385-
localityStats.loadMetricStatsMap().get("named3").numRequestsFinishedWithMetric()).isEqualTo(
386-
1L);
387-
assertThat(localityStats.loadMetricStatsMap().get("named3").totalMetricValue()).isWithin(
388-
TOLERANCE).of(0.009);
386+
localityStats.loadMetricStatsMap().get("named_metrics.named3")
387+
.numRequestsFinishedWithMetric()).isEqualTo(1L);
388+
assertThat(localityStats.loadMetricStatsMap().get("named_metrics.named3")
389+
.totalMetricValue()).isWithin(TOLERANCE).of(0.009);
389390

390391
streamTracer3.streamClosed(Status.OK);
391392
subchannel.shutdown(); // stats recorder released

‎xds/src/test/java/io/grpc/xds/GrpcXdsClientImplDataTest.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2652,6 +2652,7 @@ public void parseNonAggregateCluster_withHttp11ProxyTransportSocket() throws Exc
26522652

26532653
@Test
26542654
public void processCluster_parsesOrcaLrsPropagationMetrics() throws ResourceInvalidException {
2655+
boolean originalVal = LoadStatsManager2.isEnabledOrcaLrsPropagation;
26552656
LoadStatsManager2.isEnabledOrcaLrsPropagation = true;
26562657

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

2686-
LoadStatsManager2.isEnabledOrcaLrsPropagation = false;
2687+
LoadStatsManager2.isEnabledOrcaLrsPropagation = originalVal;
26872688
}
26882689

26892690
@Test

‎xds/src/test/java/io/grpc/xds/LoadReportClientTest.java‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -51,13 +51,15 @@
5151
import io.grpc.internal.FakeClock;
5252
import io.grpc.stub.StreamObserver;
5353
import io.grpc.testing.GrpcCleanupRule;
54+
import io.grpc.xds.client.BackendMetricPropagation;
5455
import io.grpc.xds.client.EnvoyProtoData;
5556
import io.grpc.xds.client.LoadReportClient;
5657
import io.grpc.xds.client.LoadStatsManager2;
5758
import io.grpc.xds.client.LoadStatsManager2.ClusterDropStats;
5859
import io.grpc.xds.client.LoadStatsManager2.ClusterLocalityStats;
5960
import io.grpc.xds.client.Locality;
6061
import java.util.ArrayDeque;
62+
import java.util.Arrays;
6163
import java.util.List;
6264
import java.util.concurrent.TimeUnit;
6365
import java.util.concurrent.atomic.AtomicBoolean;
@@ -91,6 +93,8 @@ public class LoadReportClientTest {
9193
private static final String EDS_SERVICE_NAME2 = "backend-service-bar.googleapis.com";
9294
private static final Locality LOCALITY1 = Locality.create("region1", "zone1", "subZone1");
9395
private static final Locality LOCALITY2 = Locality.create("region2", "zone2", "subZone2");
96+
private static final BackendMetricPropagation PROPAGATE_ALL =
97+
BackendMetricPropagation.fromMetricSpecs(Arrays.asList("named_metrics.*"));
9498
private static final FakeClock.TaskFilter LOAD_REPORTING_TASK_FILTER =
9599
new FakeClock.TaskFilter() {
96100
@Override
@@ -205,15 +209,17 @@ private void addFakeStatsData() {
205209
dropStats2.recordDroppedRequest("throttle");
206210
}
207211
ClusterLocalityStats localityStats1 =
208-
loadStatsManager.getClusterLocalityStats(CLUSTER1, EDS_SERVICE_NAME1, LOCALITY1);
212+
loadStatsManager.getClusterLocalityStats(
213+
CLUSTER1, EDS_SERVICE_NAME1, LOCALITY1, PROPAGATE_ALL);
209214
for (int i = 0; i < 31; i++) {
210215
localityStats1.recordCallStarted();
211216
}
212217
localityStats1.recordBackendLoadMetricStats(ImmutableMap.of("named1", 3.14159));
213218
localityStats1.recordBackendLoadMetricStats(ImmutableMap.of("named1", 1.618));
214219
localityStats1.recordBackendLoadMetricStats(ImmutableMap.of("named1", -2.718));
215220
ClusterLocalityStats localityStats2 =
216-
loadStatsManager.getClusterLocalityStats(CLUSTER2, EDS_SERVICE_NAME2, LOCALITY2);
221+
loadStatsManager.getClusterLocalityStats(
222+
CLUSTER2, EDS_SERVICE_NAME2, LOCALITY2, PROPAGATE_ALL);
217223
for (int i = 0; i < 45; i++) {
218224
localityStats2.recordCallStarted();
219225
}
@@ -263,7 +269,7 @@ public void periodicLoadReporting() {
263269
assertThat(localityStats.getLoadMetricStatsCount()).isEqualTo(1);
264270
EndpointLoadMetricStats loadMetricStats = Iterables.getOnlyElement(
265271
localityStats.getLoadMetricStatsList());
266-
assertThat(loadMetricStats.getMetricName()).isEqualTo("named1");
272+
assertThat(loadMetricStats.getMetricName()).isEqualTo("named_metrics.named1");
267273
assertThat(loadMetricStats.getNumRequestsFinishedWithMetric()).isEqualTo(3L);
268274
assertThat(loadMetricStats.getTotalMetricValue()).isEqualTo(3.14159 + 1.618 - 2.718);
269275

@@ -353,7 +359,7 @@ public void periodicLoadReporting() {
353359
assertThat(localityStats2.getLoadMetricStatsCount()).isEqualTo(1);
354360
EndpointLoadMetricStats loadMetricStats2 = Iterables.getOnlyElement(
355361
localityStats2.getLoadMetricStatsList());
356-
assertThat(loadMetricStats2.getMetricName()).isEqualTo("named2");
362+
assertThat(loadMetricStats2.getMetricName()).isEqualTo("named_metrics.named2");
357363
assertThat(loadMetricStats2.getNumRequestsFinishedWithMetric()).isEqualTo(1L);
358364
assertThat(loadMetricStats2.getTotalMetricValue()).isEqualTo(1.414);
359365

@@ -530,7 +536,7 @@ public void lrsStreamClosedAndRetried() {
530536
assertThat(localityStats.getLoadMetricStatsCount()).isEqualTo(1);
531537
EndpointLoadMetricStats loadMetricStats = Iterables.getOnlyElement(
532538
localityStats.getLoadMetricStatsList());
533-
assertThat(loadMetricStats.getMetricName()).isEqualTo("named1");
539+
assertThat(loadMetricStats.getMetricName()).isEqualTo("named_metrics.named1");
534540
assertThat(loadMetricStats.getNumRequestsFinishedWithMetric()).isEqualTo(3L);
535541
assertThat(loadMetricStats.getTotalMetricValue()).isEqualTo(3.14159 + 1.618 - 2.718);
536542

‎xds/src/test/java/io/grpc/xds/client/LoadStatsManager2Test.java‎

Lines changed: 39 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,9 @@ public class LoadStatsManager2Test {
5353
private static final Locality LOCALITY3 =
5454
Locality.create("test_region3", "test_zone3", "test_subzone3");
5555

56+
private static final BackendMetricPropagation PROPAGATE_ALL =
57+
BackendMetricPropagation.fromMetricSpecs(Arrays.asList("named_metrics.*"));
58+
5659
private final FakeClock fakeClock = new FakeClock();
5760
private final LoadStatsManager2 loadStatsManager =
5861
new LoadStatsManager2(fakeClock.getStopwatchSupplier());
@@ -64,11 +67,11 @@ public void recordAndGetReport() {
6467
ClusterDropStats dropCounter2 = loadStatsManager.getClusterDropStats(
6568
CLUSTER_NAME1, EDS_SERVICE_NAME2);
6669
ClusterLocalityStats loadCounter1 = loadStatsManager.getClusterLocalityStats(
67-
CLUSTER_NAME1, EDS_SERVICE_NAME1, LOCALITY1);
70+
CLUSTER_NAME1, EDS_SERVICE_NAME1, LOCALITY1, PROPAGATE_ALL);
6871
ClusterLocalityStats loadCounter2 = loadStatsManager.getClusterLocalityStats(
69-
CLUSTER_NAME1, EDS_SERVICE_NAME1, LOCALITY2);
72+
CLUSTER_NAME1, EDS_SERVICE_NAME1, LOCALITY2, PROPAGATE_ALL);
7073
ClusterLocalityStats loadCounter3 = loadStatsManager.getClusterLocalityStats(
71-
CLUSTER_NAME2, null, LOCALITY3);
74+
CLUSTER_NAME2, null, LOCALITY3, PROPAGATE_ALL);
7275
dropCounter1.recordDroppedRequest("lb");
7376
dropCounter1.recordDroppedRequest("throttle");
7477
for (int i = 0; i < 19; i++) {
@@ -106,31 +109,31 @@ public void recordAndGetReport() {
106109
assertThat(loadStats1.totalSuccessfulRequests()).isEqualTo(1L);
107110
assertThat(loadStats1.totalErrorRequests()).isEqualTo(0L);
108111
assertThat(loadStats1.totalRequestsInProgress()).isEqualTo(19L - 1L);
109-
assertThat(loadStats1.loadMetricStatsMap().containsKey("named1")).isTrue();
110-
assertThat(loadStats1.loadMetricStatsMap().containsKey("named2")).isTrue();
112+
assertThat(loadStats1.loadMetricStatsMap().containsKey("named_metrics.named1")).isTrue();
113+
assertThat(loadStats1.loadMetricStatsMap().containsKey("named_metrics.named2")).isTrue();
111114
assertThat(
112-
loadStats1.loadMetricStatsMap().get("named1").numRequestsFinishedWithMetric()).isEqualTo(
113-
4L);
114-
assertThat(loadStats1.loadMetricStatsMap().get("named1").totalMetricValue()).isWithin(TOLERANCE)
115-
.of(3.14159 + 1.618 + 99 - 97.23);
115+
loadStats1.loadMetricStatsMap().get("named_metrics.named1")
116+
.numRequestsFinishedWithMetric()).isEqualTo(4L);
117+
assertThat(loadStats1.loadMetricStatsMap().get("named_metrics.named1").totalMetricValue())
118+
.isWithin(TOLERANCE).of(3.14159 + 1.618 + 99 - 97.23);
116119
assertThat(
117-
loadStats1.loadMetricStatsMap().get("named2").numRequestsFinishedWithMetric()).isEqualTo(
118-
1L);
119-
assertThat(loadStats1.loadMetricStatsMap().get("named2").totalMetricValue()).isWithin(TOLERANCE)
120-
.of(-2.718);
120+
loadStats1.loadMetricStatsMap().get("named_metrics.named2")
121+
.numRequestsFinishedWithMetric()).isEqualTo(1L);
122+
assertThat(loadStats1.loadMetricStatsMap().get("named_metrics.named2").totalMetricValue())
123+
.isWithin(TOLERANCE).of(-2.718);
121124

122125
UpstreamLocalityStats loadStats2 =
123126
findLocalityStats(stats1.upstreamLocalityStatsList(), LOCALITY2);
124127
assertThat(loadStats2.totalIssuedRequests()).isEqualTo(9L);
125128
assertThat(loadStats2.totalSuccessfulRequests()).isEqualTo(0L);
126129
assertThat(loadStats2.totalErrorRequests()).isEqualTo(1L);
127130
assertThat(loadStats2.totalRequestsInProgress()).isEqualTo(9L - 1L);
128-
assertThat(loadStats2.loadMetricStatsMap().containsKey("named3")).isTrue();
131+
assertThat(loadStats2.loadMetricStatsMap().containsKey("named_metrics.named3")).isTrue();
129132
assertThat(
130-
loadStats2.loadMetricStatsMap().get("named3").numRequestsFinishedWithMetric()).isEqualTo(
131-
1L);
132-
assertThat(loadStats2.loadMetricStatsMap().get("named3").totalMetricValue()).isWithin(TOLERANCE)
133-
.of(0.0009);
133+
loadStats2.loadMetricStatsMap().get("named_metrics.named3")
134+
.numRequestsFinishedWithMetric()).isEqualTo(1L);
135+
assertThat(loadStats2.loadMetricStatsMap().get("named_metrics.named3").totalMetricValue())
136+
.isWithin(TOLERANCE).of(0.0009);
134137

135138
ClusterStats stats2 = findClusterStats(allStats, CLUSTER_NAME1, EDS_SERVICE_NAME2);
136139
assertThat(stats2.loadReportIntervalNano()).isEqualTo(TimeUnit.SECONDS.toNanos(5L + 10L));
@@ -221,9 +224,9 @@ public void dropCounterDelayedDeletionAfterReported() {
221224
@Test
222225
public void sharedLoadCounterStatsAggregation() {
223226
ClusterLocalityStats ref1 = loadStatsManager.getClusterLocalityStats(
224-
CLUSTER_NAME1, EDS_SERVICE_NAME1, LOCALITY1);
227+
CLUSTER_NAME1, EDS_SERVICE_NAME1, LOCALITY1, PROPAGATE_ALL);
225228
ClusterLocalityStats ref2 = loadStatsManager.getClusterLocalityStats(
226-
CLUSTER_NAME1, EDS_SERVICE_NAME1, LOCALITY1);
229+
CLUSTER_NAME1, EDS_SERVICE_NAME1, LOCALITY1, PROPAGATE_ALL);
227230
ref1.recordCallStarted();
228231
ref1.recordBackendLoadMetricStats(ImmutableMap.of("named1", 1.618));
229232
ref1.recordBackendLoadMetricStats(ImmutableMap.of("named1", 3.14159));
@@ -241,18 +244,18 @@ public void sharedLoadCounterStatsAggregation() {
241244
assertThat(localityStats.totalSuccessfulRequests()).isEqualTo(1L);
242245
assertThat(localityStats.totalErrorRequests()).isEqualTo(1L);
243246
assertThat(localityStats.totalRequestsInProgress()).isEqualTo(1L + 2L - 1L - 1L);
244-
assertThat(localityStats.loadMetricStatsMap().containsKey("named1")).isTrue();
245-
assertThat(localityStats.loadMetricStatsMap().containsKey("named2")).isTrue();
247+
assertThat(localityStats.loadMetricStatsMap().containsKey("named_metrics.named1")).isTrue();
248+
assertThat(localityStats.loadMetricStatsMap().containsKey("named_metrics.named2")).isTrue();
246249
assertThat(
247-
localityStats.loadMetricStatsMap().get("named1").numRequestsFinishedWithMetric()).isEqualTo(
248-
3L);
249-
assertThat(localityStats.loadMetricStatsMap().get("named1").totalMetricValue()).isWithin(
250-
TOLERANCE).of(1.618 + 3.14159 - 1);
250+
localityStats.loadMetricStatsMap().get("named_metrics.named1")
251+
.numRequestsFinishedWithMetric()).isEqualTo(3L);
252+
assertThat(localityStats.loadMetricStatsMap().get("named_metrics.named1")
253+
.totalMetricValue()).isWithin(TOLERANCE).of(1.618 + 3.14159 - 1);
251254
assertThat(
252-
localityStats.loadMetricStatsMap().get("named2").numRequestsFinishedWithMetric()).isEqualTo(
253-
1L);
254-
assertThat(localityStats.loadMetricStatsMap().get("named2").totalMetricValue()).isEqualTo(
255-
2.718);
255+
localityStats.loadMetricStatsMap().get("named_metrics.named2")
256+
.numRequestsFinishedWithMetric()).isEqualTo(1L);
257+
assertThat(localityStats.loadMetricStatsMap().get("named_metrics.named2")
258+
.totalMetricValue()).isEqualTo(2.718);
256259
}
257260

258261
@Test
@@ -311,7 +314,7 @@ public void recordMetrics_orcaLrsPropagationEnabled_wildcardNamedMetrics() {
311314
@Test
312315
public void loadCounterDelayedDeletionAfterAllInProgressRequestsReported() {
313316
ClusterLocalityStats counter = loadStatsManager.getClusterLocalityStats(
314-
CLUSTER_NAME1, EDS_SERVICE_NAME1, LOCALITY1);
317+
CLUSTER_NAME1, EDS_SERVICE_NAME1, LOCALITY1, PROPAGATE_ALL);
315318
counter.recordCallStarted();
316319
counter.recordCallStarted();
317320
counter.recordBackendLoadMetricStats(ImmutableMap.of("named1", 2.718));
@@ -325,12 +328,12 @@ public void loadCounterDelayedDeletionAfterAllInProgressRequestsReported() {
325328
assertThat(localityStats.totalSuccessfulRequests()).isEqualTo(0L);
326329
assertThat(localityStats.totalErrorRequests()).isEqualTo(0L);
327330
assertThat(localityStats.totalRequestsInProgress()).isEqualTo(2L);
328-
assertThat(localityStats.loadMetricStatsMap().containsKey("named1")).isTrue();
331+
assertThat(localityStats.loadMetricStatsMap().containsKey("named_metrics.named1")).isTrue();
329332
assertThat(
330-
localityStats.loadMetricStatsMap().get("named1").numRequestsFinishedWithMetric()).isEqualTo(
331-
2L);
332-
assertThat(localityStats.loadMetricStatsMap().get("named1").totalMetricValue()).isEqualTo(
333-
2.718 + 1.414);
333+
localityStats.loadMetricStatsMap().get("named_metrics.named1")
334+
.numRequestsFinishedWithMetric()).isEqualTo(2L);
335+
assertThat(localityStats.loadMetricStatsMap().get("named_metrics.named1")
336+
.totalMetricValue()).isEqualTo(2.718 + 1.414);
334337

335338
// release the counter, but requests still in-flight
336339
counter.release();

0 commit comments

Comments
 (0)