Skip to content

Commit 20d09ce

Browse files
authored
xds: Add counter and gauge metrics (#11661)
Adds the following xDS client metrics defined in [A78](https://github.com/grpc/proposal/blob/master/A78-grpc-metrics-wrr-pf-xds.md#xdsclient). Counters - grpc.xds_client.server_failure - grpc.xds_client.resource_updates_valid - grpc.xds_client.resource_updates_invalid Gauges - grpc.xds_client.connected - grpc.xds_client.resources
1 parent 92de2f3 commit 20d09ce

23 files changed

+1381
-184
lines changed

api/src/main/java/io/grpc/NameResolver.java

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,7 @@ public static final class Args {
290290
@Nullable private final ChannelLogger channelLogger;
291291
@Nullable private final Executor executor;
292292
@Nullable private final String overrideAuthority;
293+
@Nullable private final MetricRecorder metricRecorder;
293294

294295
private Args(
295296
Integer defaultPort,
@@ -299,7 +300,8 @@ private Args(
299300
@Nullable ScheduledExecutorService scheduledExecutorService,
300301
@Nullable ChannelLogger channelLogger,
301302
@Nullable Executor executor,
302-
@Nullable String overrideAuthority) {
303+
@Nullable String overrideAuthority,
304+
@Nullable MetricRecorder metricRecorder) {
303305
this.defaultPort = checkNotNull(defaultPort, "defaultPort not set");
304306
this.proxyDetector = checkNotNull(proxyDetector, "proxyDetector not set");
305307
this.syncContext = checkNotNull(syncContext, "syncContext not set");
@@ -308,6 +310,7 @@ private Args(
308310
this.channelLogger = channelLogger;
309311
this.executor = executor;
310312
this.overrideAuthority = overrideAuthority;
313+
this.metricRecorder = metricRecorder;
311314
}
312315

313316
/**
@@ -405,6 +408,14 @@ public String getOverrideAuthority() {
405408
return overrideAuthority;
406409
}
407410

411+
/**
412+
* Returns the {@link MetricRecorder} that the channel uses to record metrics.
413+
*/
414+
@Nullable
415+
public MetricRecorder getMetricRecorder() {
416+
return metricRecorder;
417+
}
418+
408419

409420
@Override
410421
public String toString() {
@@ -417,6 +428,7 @@ public String toString() {
417428
.add("channelLogger", channelLogger)
418429
.add("executor", executor)
419430
.add("overrideAuthority", overrideAuthority)
431+
.add("metricRecorder", metricRecorder)
420432
.toString();
421433
}
422434

@@ -435,6 +447,7 @@ public Builder toBuilder() {
435447
builder.setChannelLogger(channelLogger);
436448
builder.setOffloadExecutor(executor);
437449
builder.setOverrideAuthority(overrideAuthority);
450+
builder.setMetricRecorder(metricRecorder);
438451
return builder;
439452
}
440453

@@ -461,6 +474,7 @@ public static final class Builder {
461474
private ChannelLogger channelLogger;
462475
private Executor executor;
463476
private String overrideAuthority;
477+
private MetricRecorder metricRecorder;
464478

465479
Builder() {
466480
}
@@ -547,6 +561,14 @@ public Builder setOverrideAuthority(String authority) {
547561
return this;
548562
}
549563

564+
/**
565+
* See {@link Args#getMetricRecorder()}. This is an optional field.
566+
*/
567+
public Builder setMetricRecorder(MetricRecorder metricRecorder) {
568+
this.metricRecorder = metricRecorder;
569+
return this;
570+
}
571+
550572
/**
551573
* Builds an {@link Args}.
552574
*
@@ -556,7 +578,8 @@ public Args build() {
556578
return
557579
new Args(
558580
defaultPort, proxyDetector, syncContext, serviceConfigParser,
559-
scheduledExecutorService, channelLogger, executor, overrideAuthority);
581+
scheduledExecutorService, channelLogger, executor, overrideAuthority,
582+
metricRecorder);
560583
}
561584
}
562585
}

api/src/test/java/io/grpc/NameResolverTest.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ public class NameResolverTest {
6464
private final ChannelLogger channelLogger = mock(ChannelLogger.class);
6565
private final Executor executor = Executors.newSingleThreadExecutor();
6666
private final String overrideAuthority = "grpc.io";
67+
private final MetricRecorder metricRecorder = new MetricRecorder() {};
6768
@Mock NameResolver.Listener mockListener;
6869

6970
@Test
@@ -77,6 +78,7 @@ public void args() {
7778
assertThat(args.getChannelLogger()).isSameInstanceAs(channelLogger);
7879
assertThat(args.getOffloadExecutor()).isSameInstanceAs(executor);
7980
assertThat(args.getOverrideAuthority()).isSameInstanceAs(overrideAuthority);
81+
assertThat(args.getMetricRecorder()).isSameInstanceAs(metricRecorder);
8082

8183
NameResolver.Args args2 = args.toBuilder().build();
8284
assertThat(args2.getDefaultPort()).isEqualTo(defaultPort);
@@ -87,6 +89,7 @@ public void args() {
8789
assertThat(args2.getChannelLogger()).isSameInstanceAs(channelLogger);
8890
assertThat(args2.getOffloadExecutor()).isSameInstanceAs(executor);
8991
assertThat(args2.getOverrideAuthority()).isSameInstanceAs(overrideAuthority);
92+
assertThat(args.getMetricRecorder()).isSameInstanceAs(metricRecorder);
9093

9194
assertThat(args2).isNotSameInstanceAs(args);
9295
assertThat(args2).isNotEqualTo(args);
@@ -102,6 +105,7 @@ private NameResolver.Args createArgs() {
102105
.setChannelLogger(channelLogger)
103106
.setOffloadExecutor(executor)
104107
.setOverrideAuthority(overrideAuthority)
108+
.setMetricRecorder(metricRecorder)
105109
.build();
106110
}
107111

core/src/main/java/io/grpc/internal/ManagedChannelImpl.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -589,6 +589,8 @@ ClientStream newSubstream(
589589
builder.maxHedgedAttempts,
590590
loadBalancerFactory);
591591
this.authorityOverride = builder.authorityOverride;
592+
this.metricRecorder = new MetricRecorderImpl(builder.metricSinks,
593+
MetricInstrumentRegistry.getDefaultRegistry());
592594
this.nameResolverArgs =
593595
NameResolver.Args.newBuilder()
594596
.setDefaultPort(builder.getDefaultPort())
@@ -599,6 +601,7 @@ ClientStream newSubstream(
599601
.setChannelLogger(channelLogger)
600602
.setOffloadExecutor(this.offloadExecutorHolder)
601603
.setOverrideAuthority(this.authorityOverride)
604+
.setMetricRecorder(this.metricRecorder)
602605
.build();
603606
this.nameResolver = getNameResolver(
604607
targetUri, authorityOverride, nameResolverProvider, nameResolverArgs);
@@ -671,8 +674,6 @@ public CallTracer create() {
671674
}
672675
serviceConfigUpdated = true;
673676
}
674-
this.metricRecorder = new MetricRecorderImpl(builder.metricSinks,
675-
MetricInstrumentRegistry.getDefaultRegistry());
676677
}
677678

678679
@VisibleForTesting

core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -687,6 +687,30 @@ public void metricRecorder_recordsToMetricSink() {
687687
eq(optionalLabelValues));
688688
}
689689

690+
@Test
691+
public void metricRecorder_fromNameResolverArgs_recordsToMetricSink() {
692+
MetricSink mockSink1 = mock(MetricSink.class);
693+
MetricSink mockSink2 = mock(MetricSink.class);
694+
channelBuilder.addMetricSink(mockSink1);
695+
channelBuilder.addMetricSink(mockSink2);
696+
createChannel();
697+
698+
LongCounterMetricInstrument counter = metricInstrumentRegistry.registerLongCounter(
699+
"test_counter", "Time taken by metric recorder", "s",
700+
ImmutableList.of("grpc.method"), Collections.emptyList(), false);
701+
List<String> requiredLabelValues = ImmutableList.of("testMethod");
702+
List<String> optionalLabelValues = Collections.emptyList();
703+
704+
NameResolver.Args args = helper.getNameResolverArgs();
705+
assertThat(args.getMetricRecorder()).isNotNull();
706+
args.getMetricRecorder()
707+
.addLongCounter(counter, 10, requiredLabelValues, optionalLabelValues);
708+
verify(mockSink1).addLongCounter(eq(counter), eq(10L), eq(requiredLabelValues),
709+
eq(optionalLabelValues));
710+
verify(mockSink2).addLongCounter(eq(counter), eq(10L), eq(requiredLabelValues),
711+
eq(optionalLabelValues));
712+
}
713+
690714
@Test
691715
public void shutdownWithNoTransportsEverCreated() {
692716
channelBuilder.nameResolverFactory(
@@ -2240,6 +2264,7 @@ public void lbHelper_getNameResolverArgs() {
22402264
assertThat(args.getSynchronizationContext())
22412265
.isSameInstanceAs(helper.getSynchronizationContext());
22422266
assertThat(args.getServiceConfigParser()).isNotNull();
2267+
assertThat(args.getMetricRecorder()).isNotNull();
22432268
}
22442269

22452270
@Test

xds/src/main/java/io/grpc/xds/InternalSharedXdsClientPoolProvider.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package io.grpc.xds;
1818

1919
import io.grpc.Internal;
20+
import io.grpc.MetricRecorder;
2021
import io.grpc.internal.ObjectPool;
2122
import io.grpc.xds.client.XdsClient;
2223
import io.grpc.xds.client.XdsInitializationException;
@@ -36,6 +37,11 @@ public static void setDefaultProviderBootstrapOverride(Map<String, ?> bootstrap)
3637

3738
public static ObjectPool<XdsClient> getOrCreate(String target)
3839
throws XdsInitializationException {
39-
return SharedXdsClientPoolProvider.getDefaultProvider().getOrCreate(target);
40+
return getOrCreate(target, new MetricRecorder() {});
41+
}
42+
43+
public static ObjectPool<XdsClient> getOrCreate(String target, MetricRecorder metricRecorder)
44+
throws XdsInitializationException {
45+
return SharedXdsClientPoolProvider.getDefaultProvider().getOrCreate(target, metricRecorder);
4046
}
4147
}

xds/src/main/java/io/grpc/xds/SharedXdsClientPoolProvider.java

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
import com.google.common.annotations.VisibleForTesting;
2323
import com.google.common.collect.ImmutableList;
24+
import io.grpc.MetricRecorder;
2425
import io.grpc.internal.ExponentialBackoffPolicy;
2526
import io.grpc.internal.GrpcUtil;
2627
import io.grpc.internal.ObjectPool;
@@ -51,6 +52,8 @@ final class SharedXdsClientPoolProvider implements XdsClientPoolFactory {
5152
private static final boolean LOG_XDS_NODE_ID = Boolean.parseBoolean(
5253
System.getenv("GRPC_LOG_XDS_NODE_ID"));
5354
private static final Logger log = Logger.getLogger(XdsClientImpl.class.getName());
55+
private static final ExponentialBackoffPolicy.Provider BACKOFF_POLICY_PROVIDER =
56+
new ExponentialBackoffPolicy.Provider();
5457

5558
private final Bootstrapper bootstrapper;
5659
private final Object lock = new Object();
@@ -82,7 +85,8 @@ public ObjectPool<XdsClient> get(String target) {
8285
}
8386

8487
@Override
85-
public ObjectPool<XdsClient> getOrCreate(String target) throws XdsInitializationException {
88+
public ObjectPool<XdsClient> getOrCreate(String target, MetricRecorder metricRecorder)
89+
throws XdsInitializationException {
8690
ObjectPool<XdsClient> ref = targetToXdsClientMap.get(target);
8791
if (ref == null) {
8892
synchronized (lock) {
@@ -98,7 +102,7 @@ public ObjectPool<XdsClient> getOrCreate(String target) throws XdsInitialization
98102
if (bootstrapInfo.servers().isEmpty()) {
99103
throw new XdsInitializationException("No xDS server provided");
100104
}
101-
ref = new RefCountedXdsClientObjectPool(bootstrapInfo, target);
105+
ref = new RefCountedXdsClientObjectPool(bootstrapInfo, target, metricRecorder);
102106
targetToXdsClientMap.put(target, ref);
103107
}
104108
}
@@ -111,31 +115,33 @@ public ImmutableList<String> getTargets() {
111115
return ImmutableList.copyOf(targetToXdsClientMap.keySet());
112116
}
113117

114-
115118
private static class SharedXdsClientPoolProviderHolder {
116119
private static final SharedXdsClientPoolProvider instance = new SharedXdsClientPoolProvider();
117120
}
118121

119122
@ThreadSafe
120123
@VisibleForTesting
121-
static class RefCountedXdsClientObjectPool implements ObjectPool<XdsClient> {
124+
class RefCountedXdsClientObjectPool implements ObjectPool<XdsClient> {
122125

123-
private static final ExponentialBackoffPolicy.Provider BACKOFF_POLICY_PROVIDER =
124-
new ExponentialBackoffPolicy.Provider();
125126
private final BootstrapInfo bootstrapInfo;
126127
private final String target; // The target associated with the xDS client.
128+
private final MetricRecorder metricRecorder;
127129
private final Object lock = new Object();
128130
@GuardedBy("lock")
129131
private ScheduledExecutorService scheduler;
130132
@GuardedBy("lock")
131133
private XdsClient xdsClient;
132134
@GuardedBy("lock")
133135
private int refCount;
136+
@GuardedBy("lock")
137+
private XdsClientMetricReporterImpl metricReporter;
134138

135139
@VisibleForTesting
136-
RefCountedXdsClientObjectPool(BootstrapInfo bootstrapInfo, String target) {
140+
RefCountedXdsClientObjectPool(BootstrapInfo bootstrapInfo, String target,
141+
MetricRecorder metricRecorder) {
137142
this.bootstrapInfo = checkNotNull(bootstrapInfo);
138143
this.target = target;
144+
this.metricRecorder = metricRecorder;
139145
}
140146

141147
@Override
@@ -146,6 +152,7 @@ public XdsClient getObject() {
146152
log.log(Level.INFO, "xDS node ID: {0}", bootstrapInfo.node().getId());
147153
}
148154
scheduler = SharedResourceHolder.get(GrpcUtil.TIMER_SERVICE);
155+
metricReporter = new XdsClientMetricReporterImpl(metricRecorder, target);
149156
xdsClient = new XdsClientImpl(
150157
DEFAULT_XDS_TRANSPORT_FACTORY,
151158
bootstrapInfo,
@@ -154,7 +161,9 @@ public XdsClient getObject() {
154161
GrpcUtil.STOPWATCH_SUPPLIER,
155162
TimeProvider.SYSTEM_TIME_PROVIDER,
156163
MessagePrinter.INSTANCE,
157-
new TlsContextManagerImpl(bootstrapInfo));
164+
new TlsContextManagerImpl(bootstrapInfo),
165+
metricReporter);
166+
metricReporter.setXdsClient(xdsClient);
158167
}
159168
refCount++;
160169
return xdsClient;
@@ -168,6 +177,9 @@ public XdsClient returnObject(Object object) {
168177
if (refCount == 0) {
169178
xdsClient.shutdown();
170179
xdsClient = null;
180+
metricReporter.close();
181+
metricReporter = null;
182+
targetToXdsClientMap.remove(target);
171183
scheduler = SharedResourceHolder.release(GrpcUtil.TIMER_SERVICE, scheduler);
172184
}
173185
return null;

0 commit comments

Comments
 (0)