all: use a proper log id which can reference channels, subchannels, and transports

Author: carl-mastrangelo

core/src/main/java/io/grpc/inprocess/InProcessTransport.java

@@ -44,7 +44,7 @@
 import io.grpc.internal.ClientStream;
 import io.grpc.internal.ClientStreamListener;
 import io.grpc.internal.ConnectionClientTransport;
-import io.grpc.internal.GrpcUtil;
+import io.grpc.internal.LogId;
 import io.grpc.internal.ManagedClientTransport;
 import io.grpc.internal.NoopClientStream;
 import io.grpc.internal.ServerStream;
@@ -71,6 +71,7 @@
 class InProcessTransport implements ServerTransport, ConnectionClientTransport {
   private static final Logger log = Logger.getLogger(InProcessTransport.class.getName());
 
+  private final LogId logId = LogId.allocate(getClass().getName());
   private final String name;
   private ServerTransportListener serverTransportListener;
   private Attributes serverStreamAttributes;
@@ -203,8 +204,8 @@ public String toString() {
   }
 
   @Override
-  public String getLogId() {
-    return GrpcUtil.getLogId(this);
+  public LogId getLogId() {
+    return logId;
   }
 
   @Override

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

@@ -63,6 +63,9 @@
  * streams are transferred to the given transport, thus this transport won't own any stream.
  */
 class DelayedClientTransport implements ManagedClientTransport {
+
+  private final LogId lodId = LogId.allocate(getClass().getName());
+
   private final Object lock = new Object();
 
   private final Executor streamCreationExecutor;
@@ -455,9 +458,10 @@ public void run() {
     }
   }
 
+  // TODO(carl-mastrangelo): remove this once the Subchannel change is in.
   @Override
-  public final String getLogId() {
-    return GrpcUtil.getLogId(this);
+  public LogId getLogId() {
+    return lodId;
   }
 
   @VisibleForTesting

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

@@ -73,7 +73,7 @@ public void ping(PingCallback callback, Executor executor) {
   }
 
   @Override
-  public String getLogId() {
+  public LogId getLogId() {
     return delegate().getLogId();
   }
 

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

@@ -535,13 +535,6 @@ public Long parseAsciiString(String serialized) {
     }
   }
 
-  /**
-   * The canonical implementation of {@link WithLogId#getLogId}.
-   */
-  public static String getLogId(WithLogId subject) {
-    return subject.getClass().getSimpleName() + "@" + Integer.toHexString(subject.hashCode());
-  }
-
   private GrpcUtil() {}
 
   private static String getImplementationVersion() {

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

@@ -72,7 +72,9 @@
 final class InternalSubchannel implements WithLogId {
   private static final Logger log = Logger.getLogger(InternalSubchannel.class.getName());
 
+
   private final Object lock = new Object();
+  private final LogId logId = LogId.allocate(getClass().getName());
   private final EquivalentAddressGroup addressGroup;
   private final String authority;
   private final String userAgent;
@@ -119,12 +121,12 @@ final class InternalSubchannel implements WithLogId {
    * also be present.
    */
   @GuardedBy("lock")
-  private final Collection<ManagedClientTransport> transports =
-      new ArrayList<ManagedClientTransport>();
+  private final Collection<ConnectionClientTransport> transports =
+      new ArrayList<ConnectionClientTransport>();
 
   // Must only be used from channelExecutor
-  private final InUseStateAggregator2<ManagedClientTransport> inUseStateAggregator =
-      new InUseStateAggregator2<ManagedClientTransport>() {
+  private final InUseStateAggregator2<ConnectionClientTransport> inUseStateAggregator =
+      new InUseStateAggregator2<ConnectionClientTransport>() {
         @Override
         void handleInUse() {
           callback.onInUse(InternalSubchannel.this);
@@ -329,7 +331,7 @@ public void run() {
   }
 
   private void handleTransportInUseState(
-      final ManagedClientTransport transport, final boolean inUse) {
+      final ConnectionClientTransport transport, final boolean inUse) {
     channelExecutor.execute(new Runnable() {
         @Override
         public void run() {
@@ -358,8 +360,8 @@ private void cancelReconnectTask() {
   }
 
   @Override
-  public String getLogId() {
-    return GrpcUtil.getLogId(this);
+  public LogId getLogId() {
+    return logId;
   }
 
   @VisibleForTesting
@@ -371,10 +373,10 @@ ConnectivityState getState() {
 
   /** Listener for real transports. */
   private class TransportListener implements ManagedClientTransport.Listener {
-    final ManagedClientTransport transport;
+    final ConnectionClientTransport transport;
     final SocketAddress address;
 
-    TransportListener(ManagedClientTransport transport, SocketAddress address) {
+    TransportListener(ConnectionClientTransport transport, SocketAddress address) {
       this.transport = transport;
       this.address = address;
     }

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

@@ -0,0 +1,69 @@
+/*
+ * Copyright 2016, Google Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ *
+ *    * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *    * Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following disclaimer
+ * in the documentation and/or other materials provided with the
+ * distribution.
+ *
+ *    * Neither the name of Google Inc. nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package io.grpc.internal;
+
+import java.util.concurrent.atomic.AtomicLong;
+
+/**
+ * A loggable ID, unique for the duration of the program.
+ */
+public final class LogId {
+  private static final AtomicLong idAlloc = new AtomicLong();
+
+  /**
+   * @param tag a loggable tag associated with this ID.
+   */
+  public static LogId allocate(String tag) {
+    return new LogId(tag, idAlloc.incrementAndGet());
+  }
+
+  private final String tag;
+  private final long id;
+
+  private LogId(String tag, long id) {
+    this.tag = tag;
+    this.id = id;
+  }
+
+  public long getId() {
+    return id;
+  }
+
+  public String getTag() {
+    return tag;
+  }
+
+  @Override
+  public String toString() {
+    return tag + "-" + id;
+  }
+}

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

@@ -130,6 +130,7 @@ public final class ManagedChannelImpl extends ManagedChannel implements WithLogI
   private final Executor executor;
   private final boolean usingSharedExecutor;
   private final Object lock = new Object();
+  private final LogId logId = LogId.allocate(getClass().getName());
 
   private final DecompressorRegistry decompressorRegistry;
   private final CompressorRegistry compressorRegistry;
@@ -745,8 +746,8 @@ public OobTransportProvider<ClientTransport> createOobTransportProvider(
   };
 
   @Override
-  public String getLogId() {
-    return GrpcUtil.getLogId(this);
+  public LogId getLogId() {
+    return logId;
   }
 
   private static class NameResolverListenerImpl implements NameResolver.Listener {

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

@@ -73,6 +73,7 @@ final class TransportSet extends ManagedChannel implements WithLogId {
 
   private final CountDownLatch terminatedLatch = new CountDownLatch(1);
   private final Object lock = new Object();
+  private final LogId logId = LogId.allocate(getClass().getName());
   private final EquivalentAddressGroup addressGroup;
   private final String authority;
   private final String userAgent;
@@ -352,8 +353,8 @@ private void cancelReconnectTask() {
   }
 
   @Override
-  public String getLogId() {
-    return GrpcUtil.getLogId(this);
+  public LogId getLogId() {
+    return logId;
   }
 
   @Override

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

@@ -43,5 +43,5 @@ public interface WithLogId {
    * <p>The subclasses of this interface usually want to include the log ID in their {@link
    * #toString} results.
    */
-  String getLogId();
+  LogId getLogId();
 }

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

@@ -422,7 +422,7 @@ public void connectIsLazy() {
     // No scheduled tasks that would ever try to reconnect ...
     assertEquals(0, fakeClock.numPendingTasks());
     assertEquals(0, fakeExecutor.numPendingTasks());
-    
+
     // ... until it's requested.
     internalSubchannel.obtainActiveTransport();
     assertExactCallbackInvokes("onStateChange:CONNECTING");
@@ -434,7 +434,7 @@ public void connectIsLazy() {
   public void shutdownWhenReady() throws Exception {
     SocketAddress addr = mock(SocketAddress.class);
     createInternalSubchannel(addr);
-    
+
     internalSubchannel.obtainActiveTransport();
     MockClientTransportInfo transportInfo = transports.poll();
     transportInfo.listener.transportReady();
@@ -528,7 +528,7 @@ public void shutdownBeforeTransportReady() throws Exception {
   public void shutdownNow() throws Exception {
     SocketAddress addr = mock(SocketAddress.class);
     createInternalSubchannel(addr);
-    
+
     internalSubchannel.obtainActiveTransport();
     MockClientTransportInfo t1 = transports.poll();
     t1.listener.transportReady();
@@ -565,8 +565,8 @@ public void obtainTransportAfterShutdown() throws Exception {
   @Test
   public void logId() {
     createInternalSubchannel(mock(SocketAddress.class));
-    assertEquals("InternalSubchannel@" + Integer.toHexString(internalSubchannel.hashCode()),
-        internalSubchannel.getLogId());
+
+    assertNotNull(internalSubchannel.getLogId());
   }
 
   @Test