From caf03a53e913a4226b68b935d1657b9ff04af262 Mon Sep 17 00:00:00 2001 From: "Penn (Dapeng) Zhang" Date: Tue, 24 Jul 2018 09:17:31 -0700 Subject: [PATCH 1/2] core: let channel tracing log service config changes --- .../io/grpc/internal/ManagedChannelImpl.java | 17 +++++- .../grpc/internal/ManagedChannelImplTest.java | 56 +++++++++++++++++++ 2 files changed, 71 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java b/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java index ff46cca6385..bd478e18fa3 100644 --- a/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java +++ b/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java @@ -211,7 +211,10 @@ void handleUncaughtThrowable(Throwable t) { @CheckForNull private final ChannelTracer channelTracer; private final Channelz channelz; + @CheckForNull private Boolean zeroBackends; // a flag for doing channel tracing when flipped + @Nullable + private Map lastServiceConfig; // used for channel tracing when value changed // One instance per channel. private final ChannelBufferMeter channelBufferUsed = new ChannelBufferMeter(); @@ -1258,6 +1261,17 @@ public void onAddresses(final List servers, final Attrib .build()); zeroBackends = false; } + final Map serviceConfig = + config.get(GrpcAttributes.NAME_RESOLVER_SERVICE_CONFIG); + if (channelTracer != null && serviceConfig != null + && !serviceConfig.equals(lastServiceConfig)) { + channelTracer.reportEvent(new ChannelTrace.Event.Builder() + .setDescription("Service config changed") + .setSeverity(ChannelTrace.Event.Severity.CT_INFO) + .setTimestampNanos(timeProvider.currentTimeNanos()) + .build()); + lastServiceConfig = serviceConfig; + } final class NamesResolved implements Runnable { @Override @@ -1269,8 +1283,7 @@ public void run() { nameResolverBackoffPolicy = null; - Map serviceConfig = - config.get(GrpcAttributes.NAME_RESOLVER_SERVICE_CONFIG); + if (serviceConfig != null) { try { serviceConfigInterceptor.handleUpdate(serviceConfig); diff --git a/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java b/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java index f8e72e63c3b..6c883463ca4 100644 --- a/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java +++ b/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java @@ -2216,6 +2216,62 @@ public void channelTracing_nameResolvedEvent_zeorAndNonzeroBackends() throws Exc assertThat(getStats(channel).channelTrace.events).hasSize(prevSize + 1); } + @Test + public void channelTracing_serviceConfigChange() throws Exception { + timer.forwardNanos(1234); + channelBuilder.maxTraceEvents(10); + List servers = new ArrayList(); + servers.add(new EquivalentAddressGroup(socketAddress)); + FakeNameResolverFactory nameResolverFactory = + new FakeNameResolverFactory.Builder(expectedUri).setServers(servers).build(); + channelBuilder.nameResolverFactory(nameResolverFactory); + createChannel(); + + int prevSize = getStats(channel).channelTrace.events.size(); + Attributes attributes = + Attributes.newBuilder() + .set(GrpcAttributes.NAME_RESOLVER_SERVICE_CONFIG, new HashMap()) + .build(); + nameResolverFactory.resolvers.get(0).listener.onAddresses( + Collections.singletonList(new EquivalentAddressGroup( + Arrays.asList(new SocketAddress() {}, new SocketAddress() {}))), + attributes); + assertThat(getStats(channel).channelTrace.events).hasSize(prevSize + 1); + assertThat(getStats(channel).channelTrace.events.get(prevSize)) + .isEqualTo(new ChannelTrace.Event.Builder() + .setDescription("Service config changed") + .setSeverity(ChannelTrace.Event.Severity.CT_INFO) + .setTimestampNanos(timer.getTicker().read()) + .build()); + + prevSize = getStats(channel).channelTrace.events.size(); + nameResolverFactory.resolvers.get(0).listener.onAddresses( + Collections.singletonList(new EquivalentAddressGroup( + Arrays.asList(new SocketAddress() {}, new SocketAddress() {}))), + attributes); + assertThat(getStats(channel).channelTrace.events).hasSize(prevSize); + + prevSize = getStats(channel).channelTrace.events.size(); + Map serviceConfig = new HashMap(); + serviceConfig.put("methodConfig", new HashMap()); + attributes = + Attributes.newBuilder() + .set(GrpcAttributes.NAME_RESOLVER_SERVICE_CONFIG, serviceConfig) + .build(); + timer.forwardNanos(1234); + nameResolverFactory.resolvers.get(0).listener.onAddresses( + Collections.singletonList(new EquivalentAddressGroup( + Arrays.asList(new SocketAddress() {}, new SocketAddress() {}))), + attributes); + assertThat(getStats(channel).channelTrace.events).hasSize(prevSize + 1); + assertThat(getStats(channel).channelTrace.events.get(prevSize)) + .isEqualTo(new ChannelTrace.Event.Builder() + .setDescription("Service config changed") + .setSeverity(ChannelTrace.Event.Severity.CT_INFO) + .setTimestampNanos(timer.getTicker().read()) + .build()); + } + @Test public void channelTracing_stateChangeEvent() throws Exception { channelBuilder.maxTraceEvents(10); From 73258125b654013d54d2ca611def485ab3159d8d Mon Sep 17 00:00:00 2001 From: ZHANG Dapeng Date: Wed, 25 Jul 2018 09:53:09 -0700 Subject: [PATCH 2/2] fix negation --- .../java/io/grpc/internal/ManagedChannelImpl.java | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java b/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java index bd478e18fa3..52eeef97542 100644 --- a/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java +++ b/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java @@ -212,7 +212,7 @@ void handleUncaughtThrowable(Throwable t) { private final ChannelTracer channelTracer; private final Channelz channelz; @CheckForNull - private Boolean zeroBackends; // a flag for doing channel tracing when flipped + private Boolean haveBackends; // a flag for doing channel tracing when flipped @Nullable private Map lastServiceConfig; // used for channel tracing when value changed @@ -1253,13 +1253,13 @@ public void onAddresses(final List servers, final Attrib new Object[]{getLogId(), servers, config}); } - if (channelTracer != null && (zeroBackends == null || zeroBackends)) { + if (channelTracer != null && (haveBackends == null || !haveBackends)) { channelTracer.reportEvent(new ChannelTrace.Event.Builder() .setDescription("Address resolved: " + servers) .setSeverity(ChannelTrace.Event.Severity.CT_INFO) .setTimestampNanos(timeProvider.currentTimeNanos()) .build()); - zeroBackends = false; + haveBackends = true; } final Map serviceConfig = config.get(GrpcAttributes.NAME_RESOLVER_SERVICE_CONFIG); @@ -1283,7 +1283,6 @@ public void run() { nameResolverBackoffPolicy = null; - if (serviceConfig != null) { try { serviceConfigInterceptor.handleUpdate(serviceConfig); @@ -1310,13 +1309,13 @@ public void onError(final Status error) { checkArgument(!error.isOk(), "the error status must not be OK"); logger.log(Level.WARNING, "[{0}] Failed to resolve name. status={1}", new Object[] {getLogId(), error}); - if (channelTracer != null && (zeroBackends == null || !zeroBackends)) { + if (channelTracer != null && (haveBackends == null || haveBackends)) { channelTracer.reportEvent(new ChannelTrace.Event.Builder() .setDescription("Failed to resolve name") .setSeverity(ChannelTrace.Event.Severity.CT_WARNING) .setTimestampNanos(timeProvider.currentTimeNanos()) .build()); - zeroBackends = true; + haveBackends = false; } channelExecutor .executeLater(