services: use Peer.address proto (#4502)

address+ip_port is the new way; peer is deprecated

3 files changed, 119 insertions, 38 deletions

diff --git a/services/src/main/java/io/grpc/services/BinlogHelper.java b/services/src/main/java/io/grpc/services/BinlogHelper.java
index 79a9a4e65..9d06cd5f9 100644
--- a/services/src/main/java/io/grpc/services/BinlogHelper.java
+++ b/services/src/main/java/io/grpc/services/BinlogHelper.java
@@ -21,7 +21,6 @@ import static io.grpc.BinaryLogProvider.BYTEARRAY_MARSHALLER;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
 import com.google.common.base.Splitter;
-import com.google.common.primitives.Bytes;
 import com.google.protobuf.ByteString;
 import io.grpc.Attributes;
 import io.grpc.BinaryLog.CallId;
@@ -56,7 +55,6 @@ import java.net.Inet6Address;
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
 import java.net.SocketAddress;
-import java.nio.charset.Charset;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
@@ -73,9 +71,6 @@ import javax.annotation.concurrent.ThreadSafe;
 @ThreadSafe
 final class BinlogHelper {
   private static final Logger logger = Logger.getLogger(BinlogHelper.class.getName());
-  private static final int IP_PORT_BYTES = 2;
-  private static final int IP_PORT_UPPER_MASK = 0xff00;
-  private static final int IP_PORT_LOWER_MASK = 0xff;
   private static final boolean SERVER = true;
   private static final boolean CLIENT = false;
 
@@ -525,37 +520,31 @@ final class BinlogHelper {
   }
 
   @VisibleForTesting
-  // TODO(zpencer): the binlog design does not specify how to actually express the peer bytes
   static Peer socketToProto(SocketAddress address) {
     Preconditions.checkNotNull(address);
-    PeerType peerType = PeerType.UNKNOWN_PEERTYPE;
-    byte[] peerAddress = null;
 
+    Peer.Builder builder = Peer.newBuilder();
     if (address instanceof InetSocketAddress) {
       InetAddress inetAddress = ((InetSocketAddress) address).getAddress();
       if (inetAddress instanceof Inet4Address) {
-        peerType = PeerType.PEER_IPV4;
+        builder.setPeerType(PeerType.PEER_IPV4)
+            .setAddress(InetAddressUtil.toAddrString(inetAddress));
       } else if (inetAddress instanceof Inet6Address) {
-        peerType = PeerType.PEER_IPV6;
+        builder.setPeerType(PeerType.PEER_IPV6)
+            .setAddress(InetAddressUtil.toAddrString(inetAddress));
       } else {
         logger.log(Level.SEVERE, "unknown type of InetSocketAddress: {}", address);
+        builder.setAddress(address.toString());
       }
-      int port = ((InetSocketAddress) address).getPort();
-      byte[] portBytes = new byte[IP_PORT_BYTES];
-      portBytes[0] = (byte) ((port & IP_PORT_UPPER_MASK) >> 8);
-      portBytes[1] = (byte) (port & IP_PORT_LOWER_MASK);
-      peerAddress = Bytes.concat(inetAddress.getAddress(), portBytes);
+      builder.setIpPort(((InetSocketAddress) address).getPort());
     } else if (address.getClass().getName().equals("io.netty.channel.unix.DomainSocketAddress")) {
       // To avoid a compile time dependency on grpc-netty, we check against the runtime class name.
-      peerType = PeerType.PEER_UNIX;
+      builder.setPeerType(PeerType.PEER_UNIX)
+          .setAddress(address.toString());
+    } else {
+      builder.setPeerType(PeerType.UNKNOWN_PEERTYPE).setAddress(address.toString());
     }
-    if (peerAddress == null) {
-      peerAddress = address.toString().getBytes(Charset.defaultCharset());
-    }
-    return Peer.newBuilder()
-        .setPeerType(peerType)
-        .setPeer(ByteString.copyFrom(peerAddress))
-        .build();
+    return builder.build();
   }
 
   @VisibleForTesting
diff --git a/services/src/main/java/io/grpc/services/InetAddressUtil.java b/services/src/main/java/io/grpc/services/InetAddressUtil.java
new file mode 100644
index 000000000..057a8ccb5
--- /dev/null
+++ b/services/src/main/java/io/grpc/services/InetAddressUtil.java
@@ -0,0 +1,94 @@
+/*
+ * Copyright 2018 The gRPC Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.grpc.services;
+
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkNotNull;
+
+import com.google.common.primitives.Ints;
+import java.net.Inet4Address;
+import java.net.Inet6Address;
+import java.net.InetAddress;
+import java.util.Arrays;
+
+// This is copied from guava 20.0 because it is a @Beta api
+final class InetAddressUtil {
+  private static final int IPV6_PART_COUNT = 8;
+
+  public static String toAddrString(InetAddress ip) {
+    checkNotNull(ip);
+    if (ip instanceof Inet4Address) {
+      // For IPv4, Java's formatting is good enough.
+      return ip.getHostAddress();
+    }
+    checkArgument(ip instanceof Inet6Address);
+    byte[] bytes = ip.getAddress();
+    int[] hextets = new int[IPV6_PART_COUNT];
+    for (int i = 0; i < hextets.length; i++) {
+      hextets[i] = Ints.fromBytes((byte) 0, (byte) 0, bytes[2 * i], bytes[2 * i + 1]);
+    }
+    compressLongestRunOfZeroes(hextets);
+    return hextetsToIPv6String(hextets);
+  }
+
+  private static void compressLongestRunOfZeroes(int[] hextets) {
+    int bestRunStart = -1;
+    int bestRunLength = -1;
+    int runStart = -1;
+    for (int i = 0; i < hextets.length + 1; i++) {
+      if (i < hextets.length && hextets[i] == 0) {
+        if (runStart < 0) {
+          runStart = i;
+        }
+      } else if (runStart >= 0) {
+        int runLength = i - runStart;
+        if (runLength > bestRunLength) {
+          bestRunStart = runStart;
+          bestRunLength = runLength;
+        }
+        runStart = -1;
+      }
+    }
+    if (bestRunLength >= 2) {
+      Arrays.fill(hextets, bestRunStart, bestRunStart + bestRunLength, -1);
+    }
+  }
+
+  private static String hextetsToIPv6String(int[] hextets) {
+    // While scanning the array, handle these state transitions:
+    //   start->num => "num"     start->gap => "::"
+    //   num->num   => ":num"    num->gap   => "::"
+    //   gap->num   => "num"     gap->gap   => ""
+    StringBuilder buf = new StringBuilder(39);
+    boolean lastWasNumber = false;
+    for (int i = 0; i < hextets.length; i++) {
+      boolean thisIsNumber = hextets[i] >= 0;
+      if (thisIsNumber) {
+        if (lastWasNumber) {
+          buf.append(':');
+        }
+        buf.append(Integer.toHexString(hextets[i]));
+      } else {
+        if (i == 0 || lastWasNumber) {
+          buf.append("::");
+        }
+      }
+      lastWasNumber = thisIsNumber;
+    }
+    return buf.toString();
+  }
+}
\ No newline at end of file
diff --git a/services/src/test/java/io/grpc/services/BinlogHelperTest.java b/services/src/test/java/io/grpc/services/BinlogHelperTest.java
index 3141e7a4f..a11502a49 100644
--- a/services/src/test/java/io/grpc/services/BinlogHelperTest.java
+++ b/services/src/test/java/io/grpc/services/BinlogHelperTest.java
@@ -28,7 +28,6 @@ import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoMoreInteractions;
 
-import com.google.common.primitives.Bytes;
 import com.google.protobuf.ByteString;
 import io.grpc.Attributes;
 import io.grpc.BinaryLog.CallId;
@@ -58,9 +57,7 @@ import io.netty.channel.unix.DomainSocketAddress;
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
 import java.net.SocketAddress;
-import java.nio.ByteBuffer;
 import java.nio.charset.Charset;
-import java.util.Arrays;
 import java.util.concurrent.atomic.AtomicReference;
 import org.junit.Before;
 import org.junit.Test;
@@ -316,14 +313,12 @@ public final class BinlogHelperTest {
     InetAddress address = InetAddress.getByName("127.0.0.1");
     int port = 12345;
     InetSocketAddress socketAddress = new InetSocketAddress(address, port);
-    byte[] addressBytes = address.getAddress();
-    byte[] portBytes = ByteBuffer.allocate(4).putInt(port).array();
-    byte[] portUnsignedBytes = Arrays.copyOfRange(portBytes, 2, 4);
     assertEquals(
         Peer
             .newBuilder()
             .setPeerType(Peer.PeerType.PEER_IPV4)
-            .setPeer(ByteString.copyFrom(Bytes.concat(addressBytes, portUnsignedBytes)))
+            .setAddress("127.0.0.1")
+            .setIpPort(12345)
             .build(),
         BinlogHelper.socketToProto(socketAddress));
   }
@@ -331,17 +326,15 @@ public final class BinlogHelperTest {
   @Test
   public void socketToProto_ipv6() throws Exception {
     // this is a ipv6 link local address
-    InetAddress address = InetAddress.getByName("fe:80:12:34:56:78:90:ab");
+    InetAddress address = InetAddress.getByName("2001:db8:0:0:0:0:2:1");
     int port = 12345;
     InetSocketAddress socketAddress = new InetSocketAddress(address, port);
-    byte[] addressBytes = address.getAddress();
-    byte[] portBytes = ByteBuffer.allocate(4).putInt(port).array();
-    byte[] portUnsignedBytes = Arrays.copyOfRange(portBytes, 2, 4);
     assertEquals(
         Peer
             .newBuilder()
             .setPeerType(Peer.PeerType.PEER_IPV6)
-            .setPeer(ByteString.copyFrom(Bytes.concat(addressBytes, portUnsignedBytes)))
+            .setAddress("2001:db8::2:1") // RFC 5952 section 4: ipv6 canonical form required
+            .setIpPort(12345)
             .build(),
         BinlogHelper.socketToProto(socketAddress));
   }
@@ -354,7 +347,7 @@ public final class BinlogHelperTest {
         Peer
             .newBuilder()
             .setPeerType(Peer.PeerType.PEER_UNIX)
-            .setPeer(ByteString.copyFrom(path.getBytes(US_ASCII)))
+            .setAddress("/some/path")
             .build(),
         BinlogHelper.socketToProto(socketAddress)
     );
@@ -362,11 +355,16 @@ public final class BinlogHelperTest {
 
   @Test
   public void socketToProto_unknown() throws Exception {
-    SocketAddress unknownSocket = new SocketAddress() { };
+    SocketAddress unknownSocket = new SocketAddress() {
+      @Override
+      public String toString() {
+        return "some-socket-address";
+      }
+    };
     assertEquals(
         Peer.newBuilder()
             .setPeerType(PeerType.UNKNOWN_PEERTYPE)
-            .setPeer(ByteString.copyFrom(unknownSocket.toString(), US_ASCII))
+            .setAddress("some-socket-address")
             .build(),
         BinlogHelper.socketToProto(unknownSocket));
   }