core,netty,okhttp: make toString more consistent for channelz (#4434)

Use MoreObjects.toStringHelper and use only the log id's long value,
because the class name is already present in the toStringHelper.

9 files changed, 61 insertions, 6 deletions

diff --git a/core/src/main/java/io/grpc/inprocess/InProcessTransport.java b/core/src/main/java/io/grpc/inprocess/InProcessTransport.java
index 013c65832..77f94ad7d 100644
--- a/core/src/main/java/io/grpc/inprocess/InProcessTransport.java
+++ b/core/src/main/java/io/grpc/inprocess/InProcessTransport.java
@@ -20,6 +20,7 @@ import static com.google.common.base.Preconditions.checkNotNull;
 import static io.grpc.internal.GrpcUtil.TIMEOUT_KEY;
 import static java.lang.Math.max;
 
+import com.google.common.base.MoreObjects;
 import com.google.common.util.concurrent.ListenableFuture;
 import com.google.common.util.concurrent.SettableFuture;
 import io.grpc.Attributes;
@@ -210,7 +211,10 @@ final class InProcessTransport implements ServerTransport, ConnectionClientTrans
 
   @Override
   public String toString() {
-    return getLogId() + "(" + name + ")";
+    return MoreObjects.toStringHelper(this)
+        .add("logId", logId.getId())
+        .add("name", name)
+        .toString();
   }
 
   @Override
diff --git a/core/src/main/java/io/grpc/internal/InternalSubchannel.java b/core/src/main/java/io/grpc/internal/InternalSubchannel.java
index c8103dbc1..0664aa6dd 100644
--- a/core/src/main/java/io/grpc/internal/InternalSubchannel.java
+++ b/core/src/main/java/io/grpc/internal/InternalSubchannel.java
@@ -23,6 +23,7 @@ import static io.grpc.ConnectivityState.SHUTDOWN;
 import static io.grpc.ConnectivityState.TRANSIENT_FAILURE;
 
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.MoreObjects;
 import com.google.common.base.Preconditions;
 import com.google.common.base.Stopwatch;
 import com.google.common.base.Supplier;
@@ -390,6 +391,20 @@ final class InternalSubchannel implements Instrumented<ChannelStats> {
     }
   }
 
+  @Override
+  public String toString() {
+    // addressGroupCopy being a little stale is fine, just avoid calling toString with the lock
+    // since there may be many addresses.
+    Object addressGroupCopy;
+    synchronized (lock) {
+      addressGroupCopy = addressGroup;
+    }
+    return MoreObjects.toStringHelper(this)
+          .add("logId", logId.getId())
+          .add("addressGroup", addressGroupCopy)
+          .toString();
+  }
+
   @GuardedBy("lock")
   private void handleTermination() {
     channelExecutor.executeLater(new Runnable() {
diff --git a/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java b/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java
index d21ad7f25..c2e0ba465 100644
--- a/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java
+++ b/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java
@@ -1322,7 +1322,7 @@ final class ManagedChannelImpl extends ManagedChannel implements Instrumented<Ch
   @Override
   public String toString() {
     return MoreObjects.toStringHelper(this)
-        .add("logId", logId)
+        .add("logId", logId.getId())
         .add("target", target)
         .toString();
   }
diff --git a/core/src/main/java/io/grpc/internal/OobChannel.java b/core/src/main/java/io/grpc/internal/OobChannel.java
index 578bf92bb..cda226a8f 100644
--- a/core/src/main/java/io/grpc/internal/OobChannel.java
+++ b/core/src/main/java/io/grpc/internal/OobChannel.java
@@ -19,6 +19,7 @@ package io.grpc.internal;
 import static com.google.common.base.Preconditions.checkNotNull;
 
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.MoreObjects;
 import com.google.common.base.Preconditions;
 import com.google.common.util.concurrent.ListenableFuture;
 import com.google.common.util.concurrent.SettableFuture;
@@ -275,6 +276,14 @@ final class OobChannel extends ManagedChannel implements Instrumented<ChannelSta
   }
 
   @Override
+  public String toString() {
+    return MoreObjects.toStringHelper(this)
+        .add("logId", logId.getId())
+        .add("authority", authority)
+        .toString();
+  }
+
+  @Override
   public void resetConnectBackoff() {
     subchannel.resetConnectBackoff();
   }
diff --git a/core/src/main/java/io/grpc/internal/ServerImpl.java b/core/src/main/java/io/grpc/internal/ServerImpl.java
index b3650aa6e..7ea2754d6 100644
--- a/core/src/main/java/io/grpc/internal/ServerImpl.java
+++ b/core/src/main/java/io/grpc/internal/ServerImpl.java
@@ -586,7 +586,7 @@ public final class ServerImpl extends io.grpc.Server implements Instrumented<Ser
   @Override
   public String toString() {
     return MoreObjects.toStringHelper(this)
-        .add("logId", logId)
+        .add("logId", logId.getId())
         .add("transportServer", transportServer)
         .toString();
   }
diff --git a/netty/src/main/java/io/grpc/netty/NettyClientTransport.java b/netty/src/main/java/io/grpc/netty/NettyClientTransport.java
index e46ff4ba0..a5aa771a2 100644
--- a/netty/src/main/java/io/grpc/netty/NettyClientTransport.java
+++ b/netty/src/main/java/io/grpc/netty/NettyClientTransport.java
@@ -20,6 +20,7 @@ import static io.grpc.internal.GrpcUtil.KEEPALIVE_TIME_NANOS_DISABLED;
 import static io.netty.channel.ChannelOption.SO_KEEPALIVE;
 
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.MoreObjects;
 import com.google.common.base.Preconditions;
 import com.google.common.util.concurrent.ListenableFuture;
 import com.google.common.util.concurrent.SettableFuture;
@@ -298,7 +299,11 @@ class NettyClientTransport implements ConnectionClientTransport {
 
   @Override
   public String toString() {
-    return getLogId() + "(" + address + ")";
+    return MoreObjects.toStringHelper(this)
+        .add("logId", logId.getId())
+        .add("address", address)
+        .add("channel", channel)
+        .toString();
   }
 
   @Override
diff --git a/netty/src/main/java/io/grpc/netty/NettyServer.java b/netty/src/main/java/io/grpc/netty/NettyServer.java
index 0f23d88f7..d85b13d5e 100644
--- a/netty/src/main/java/io/grpc/netty/NettyServer.java
+++ b/netty/src/main/java/io/grpc/netty/NettyServer.java
@@ -308,7 +308,7 @@ class NettyServer implements InternalServer, WithLogId {
   @Override
   public String toString() {
     return MoreObjects.toStringHelper(this)
-        .add("logId", logId)
+        .add("logId", logId.getId())
         .add("address", address)
         .toString();
   }
@@ -392,5 +392,13 @@ class NettyServer implements InternalServer, WithLogId {
     public LogId getLogId() {
       return id;
     }
+
+    @Override
+    public String toString() {
+      return MoreObjects.toStringHelper(this)
+          .add("logId", id.getId())
+          .add("channel", ch)
+          .toString();
+    }
   }
 }
diff --git a/netty/src/main/java/io/grpc/netty/NettyServerTransport.java b/netty/src/main/java/io/grpc/netty/NettyServerTransport.java
index fafc4a508..3b573393c 100644
--- a/netty/src/main/java/io/grpc/netty/NettyServerTransport.java
+++ b/netty/src/main/java/io/grpc/netty/NettyServerTransport.java
@@ -17,6 +17,7 @@
 package io.grpc.netty;
 
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.MoreObjects;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import com.google.common.util.concurrent.ListenableFuture;
@@ -234,6 +235,15 @@ class NettyServerTransport implements ServerTransport {
         channel.remoteAddress(),
         Utils.getSocketOptions(ch),
         grpcHandler == null ? null : grpcHandler.getSecurityInfo());
+
+  }
+
+  @Override
+  public String toString() {
+    return MoreObjects.toStringHelper(this)
+        .add("logId", logId.getId())
+        .add("channel", channel)
+        .toString();
   }
 
   /**
diff --git a/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java b/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java
index dbe5999f8..e223fc348 100644
--- a/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java
+++ b/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java
@@ -20,6 +20,7 @@ import static com.google.common.base.Preconditions.checkState;
 import static io.grpc.internal.GrpcUtil.TIMER_SERVICE;
 
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.MoreObjects;
 import com.google.common.base.Preconditions;
 import com.google.common.base.Stopwatch;
 import com.google.common.base.Supplier;
@@ -618,7 +619,10 @@ class OkHttpClientTransport implements ConnectionClientTransport {
 
   @Override
   public String toString() {
-    return getLogId() + "(" + address + ")";
+    return MoreObjects.toStringHelper(this)
+        .add("logId", logId.getId())
+        .add("address", address)
+        .toString();
   }
 
   @Override