From c4577ecc65f38b6cdf4e5457b439840758bef785 Mon Sep 17 00:00:00 2001 From: Spencer Fang Date: Wed, 27 Dec 2017 16:17:13 -0800 Subject: respond to comments --- .../java/io/grpc/binarylog/BinaryLogProto.java | 35 +++--- .../main/java/io/grpc/binarylog/GrpcLogEntry.java | 122 ++++++++++++-------- .../main/java/io/grpc/binarylog/MetadataEntry.java | 124 ++++++--------------- .../io/grpc/binarylog/MetadataEntryOrBuilder.java | 13 +-- .../main/java/io/grpc/binarylog/Peer.java | 51 ++++++--- .../src/main/java/io/grpc/services/BinaryLog.java | 24 ++-- services/src/main/proto/io/grpc/binarylog.proto | 27 +++-- .../test/java/io/grpc/services/BinaryLogTest.java | 93 ++++++++++------ 8 files changed, 255 insertions(+), 234 deletions(-) (limited to 'services') diff --git a/services/src/generated/main/java/io/grpc/binarylog/BinaryLogProto.java b/services/src/generated/main/java/io/grpc/binarylog/BinaryLogProto.java index f493a500f..19a7979e5 100644 --- a/services/src/generated/main/java/io/grpc/binarylog/BinaryLogProto.java +++ b/services/src/generated/main/java/io/grpc/binarylog/BinaryLogProto.java @@ -54,7 +54,7 @@ public final class BinaryLogProto { static { java.lang.String[] descriptorData = { "\n\027io/grpc/binarylog.proto\022\026grpc.binarylo" + - "g.v1alpha\"\224\004\n\014GrpcLogEntry\0227\n\004type\030\001 \001(\016" + + "g.v1alpha\"\272\004\n\014GrpcLogEntry\0227\n\004type\030\001 \001(\016" + "2).grpc.binarylog.v1alpha.GrpcLogEntry.T" + "ype\022;\n\006logger\030\002 \001(\0162+.grpc.binarylog.v1a" + "lpha.GrpcLogEntry.Logger\0220\n\007call_id\030\003 \001(" + @@ -62,22 +62,23 @@ public final class BinaryLogProto { "tadata\030\004 \001(\0132 .grpc.binarylog.v1alpha.Me" + "tadataH\000\0222\n\007message\030\005 \001(\0132\037.grpc.binaryl" + "og.v1alpha.MessageH\000\022*\n\004peer\030\006 \001(\0132\034.grp" + - "c.binarylog.v1alpha.Peer\"\230\001\n\004Type\022\031\n\025SEN" + - "D_INITIAL_METADATA\020\000\022\032\n\026SEND_TRAILING_ME" + - "TADATA\020\001\022\020\n\014SEND_MESSAGE\020\002\022\031\n\025RECV_INITI" + - "AL_METADATA\020\003\022\032\n\026RECV_TRAILING_METADATA\020" + - "\004\022\020\n\014RECV_MESSAGE\020\005\" \n\006Logger\022\n\n\006CLIENT\020" + - "\000\022\n\n\006SERVER\020\001B\t\n\007payload\"6\n\007Message\022\r\n\005f" + - "lags\030\001 \001(\r\022\016\n\006length\030\002 \001(\r\022\014\n\004data\030\003 \001(\014" + - "\"@\n\010Metadata\0224\n\005entry\030\001 \003(\0132%.grpc.binar" + - "ylog.v1alpha.MetadataEntry\"+\n\rMetadataEn" + - "try\022\013\n\003key\030\001 \001(\t\022\r\n\005value\030\002 \001(\014\"\207\001\n\004Peer" + - "\0228\n\tpeer_type\030\001 \001(\0162%.grpc.binarylog.v1a" + - "lpha.Peer.PeerType\022\014\n\004peer\030\002 \001(\014\"7\n\010Peer" + - "Type\022\r\n\tPEER_IPV4\020\000\022\r\n\tPEER_IPV6\020\001\022\r\n\tPE" + - "ER_UNIX\020\002\"$\n\007Uint128\022\014\n\004high\030\001 \001(\006\022\013\n\003lo" + - "w\030\002 \001(\006B%\n\021io.grpc.binarylogB\016BinaryLogP" + - "rotoP\001b\006proto3" + "c.binarylog.v1alpha.Peer\"\252\001\n\004Type\022\020\n\014UNK" + + "NOWN_TYPE\020\000\022\031\n\025SEND_INITIAL_METADATA\020\001\022\032" + + "\n\026SEND_TRAILING_METADATA\020\002\022\020\n\014SEND_MESSA" + + "GE\020\003\022\031\n\025RECV_INITIAL_METADATA\020\004\022\032\n\026RECV_" + + "TRAILING_METADATA\020\005\022\020\n\014RECV_MESSAGE\020\006\"4\n" + + "\006Logger\022\022\n\016UNKNOWN_LOGGER\020\000\022\n\n\006CLIENT\020\001\022" + + "\n\n\006SERVER\020\002B\t\n\007payload\"6\n\007Message\022\r\n\005fla" + + "gs\030\001 \001(\r\022\016\n\006length\030\002 \001(\r\022\014\n\004data\030\003 \001(\014\"@" + + "\n\010Metadata\0224\n\005entry\030\001 \003(\0132%.grpc.binaryl" + + "og.v1alpha.MetadataEntry\"+\n\rMetadataEntr" + + "y\022\013\n\003key\030\001 \001(\014\022\r\n\005value\030\002 \001(\014\"\235\001\n\004Peer\0228" + + "\n\tpeer_type\030\001 \001(\0162%.grpc.binarylog.v1alp" + + "ha.Peer.PeerType\022\014\n\004peer\030\002 \001(\014\"M\n\010PeerTy" + + "pe\022\024\n\020UNKNOWN_PEERTYPE\020\000\022\r\n\tPEER_IPV4\020\001\022" + + "\r\n\tPEER_IPV6\020\002\022\r\n\tPEER_UNIX\020\003\"$\n\007Uint128" + + "\022\014\n\004high\030\001 \001(\006\022\013\n\003low\030\002 \001(\006B%\n\021io.grpc.b" + + "inarylogB\016BinaryLogProtoP\001b\006proto3" }; com.google.protobuf.Descriptors.FileDescriptor.InternalDescriptorAssigner assigner = new com.google.protobuf.Descriptors.FileDescriptor. InternalDescriptorAssigner() { diff --git a/services/src/generated/main/java/io/grpc/binarylog/GrpcLogEntry.java b/services/src/generated/main/java/io/grpc/binarylog/GrpcLogEntry.java index 6cd5974c6..2bdfd7db2 100644 --- a/services/src/generated/main/java/io/grpc/binarylog/GrpcLogEntry.java +++ b/services/src/generated/main/java/io/grpc/binarylog/GrpcLogEntry.java @@ -155,56 +155,72 @@ private static final long serialVersionUID = 0L; public enum Type implements com.google.protobuf.ProtocolMessageEnum { /** - * SEND_INITIAL_METADATA = 0; + * 
+     * TODO(zpencer): upstream this
+     *
+ * + * UNKNOWN_TYPE = 0; + */ + UNKNOWN_TYPE(0), + /** + * SEND_INITIAL_METADATA = 1; */ - SEND_INITIAL_METADATA(0), + SEND_INITIAL_METADATA(1), /** - * SEND_TRAILING_METADATA = 1; + * SEND_TRAILING_METADATA = 2; */ - SEND_TRAILING_METADATA(1), + SEND_TRAILING_METADATA(2), /** - * SEND_MESSAGE = 2; + * SEND_MESSAGE = 3; */ - SEND_MESSAGE(2), + SEND_MESSAGE(3), /** - * RECV_INITIAL_METADATA = 3; + * RECV_INITIAL_METADATA = 4; */ - RECV_INITIAL_METADATA(3), + RECV_INITIAL_METADATA(4), /** - * RECV_TRAILING_METADATA = 4; + * RECV_TRAILING_METADATA = 5; */ - RECV_TRAILING_METADATA(4), + RECV_TRAILING_METADATA(5), /** - * RECV_MESSAGE = 5; + * RECV_MESSAGE = 6; */ - RECV_MESSAGE(5), + RECV_MESSAGE(6), UNRECOGNIZED(-1), ; /** - * SEND_INITIAL_METADATA = 0; + * 
+     * TODO(zpencer): upstream this
+     *
+ * + * UNKNOWN_TYPE = 0; + */ + public static final int UNKNOWN_TYPE_VALUE = 0; + /** + * SEND_INITIAL_METADATA = 1; */ - public static final int SEND_INITIAL_METADATA_VALUE = 0; + public static final int SEND_INITIAL_METADATA_VALUE = 1; /** - * SEND_TRAILING_METADATA = 1; + * SEND_TRAILING_METADATA = 2; */ - public static final int SEND_TRAILING_METADATA_VALUE = 1; + public static final int SEND_TRAILING_METADATA_VALUE = 2; /** - * SEND_MESSAGE = 2; + * SEND_MESSAGE = 3; */ - public static final int SEND_MESSAGE_VALUE = 2; + public static final int SEND_MESSAGE_VALUE = 3; /** - * RECV_INITIAL_METADATA = 3; + * RECV_INITIAL_METADATA = 4; */ - public static final int RECV_INITIAL_METADATA_VALUE = 3; + public static final int RECV_INITIAL_METADATA_VALUE = 4; /** - * RECV_TRAILING_METADATA = 4; + * RECV_TRAILING_METADATA = 5; */ - public static final int RECV_TRAILING_METADATA_VALUE = 4; + public static final int RECV_TRAILING_METADATA_VALUE = 5; /** - * RECV_MESSAGE = 5; + * RECV_MESSAGE = 6; */ - public static final int RECV_MESSAGE_VALUE = 5; + public static final int RECV_MESSAGE_VALUE = 6; public final int getNumber() { @@ -225,12 +241,13 @@ private static final long serialVersionUID = 0L; public static Type forNumber(int value) { switch (value) { - case 0: return SEND_INITIAL_METADATA; - case 1: return SEND_TRAILING_METADATA; - case 2: return SEND_MESSAGE; - case 3: return RECV_INITIAL_METADATA; - case 4: return RECV_TRAILING_METADATA; - case 5: return RECV_MESSAGE; + case 0: return UNKNOWN_TYPE; + case 1: return SEND_INITIAL_METADATA; + case 2: return SEND_TRAILING_METADATA; + case 3: return SEND_MESSAGE; + case 4: return RECV_INITIAL_METADATA; + case 5: return RECV_TRAILING_METADATA; + case 6: return RECV_MESSAGE; default: return null; } } @@ -293,24 +310,40 @@ private static final long serialVersionUID = 0L; public enum Logger implements com.google.protobuf.ProtocolMessageEnum { /** - * CLIENT = 0; + * 
+     * TODO(zpencer): upstream this
+     *
+ * + * UNKNOWN_LOGGER = 0; + */ + UNKNOWN_LOGGER(0), + /** + * CLIENT = 1; */ - CLIENT(0), + CLIENT(1), /** - * SERVER = 1; + * SERVER = 2; */ - SERVER(1), + SERVER(2), UNRECOGNIZED(-1), ; /** - * CLIENT = 0; + * 
+     * TODO(zpencer): upstream this
+     *
+ * + * UNKNOWN_LOGGER = 0; + */ + public static final int UNKNOWN_LOGGER_VALUE = 0; + /** + * CLIENT = 1; */ - public static final int CLIENT_VALUE = 0; + public static final int CLIENT_VALUE = 1; /** - * SERVER = 1; + * SERVER = 2; */ - public static final int SERVER_VALUE = 1; + public static final int SERVER_VALUE = 2; public final int getNumber() { @@ -331,8 +364,9 @@ private static final long serialVersionUID = 0L; public static Logger forNumber(int value) { switch (value) { - case 0: return CLIENT; - case 1: return SERVER; + case 0: return UNKNOWN_LOGGER; + case 1: return CLIENT; + case 2: return SERVER; default: return null; } } @@ -631,10 +665,10 @@ private static final long serialVersionUID = 0L; public void writeTo(com.google.protobuf.CodedOutputStream output) throws java.io.IOException { - if (type_ != io.grpc.binarylog.GrpcLogEntry.Type.SEND_INITIAL_METADATA.getNumber()) { + if (type_ != io.grpc.binarylog.GrpcLogEntry.Type.UNKNOWN_TYPE.getNumber()) { output.writeEnum(1, type_); } - if (logger_ != io.grpc.binarylog.GrpcLogEntry.Logger.CLIENT.getNumber()) { + if (logger_ != io.grpc.binarylog.GrpcLogEntry.Logger.UNKNOWN_LOGGER.getNumber()) { output.writeEnum(2, logger_); } if (callId_ != null) { @@ -657,11 +691,11 @@ private static final long serialVersionUID = 0L; if (size != -1) return size; size = 0; - if (type_ != io.grpc.binarylog.GrpcLogEntry.Type.SEND_INITIAL_METADATA.getNumber()) { + if (type_ != io.grpc.binarylog.GrpcLogEntry.Type.UNKNOWN_TYPE.getNumber()) { size += com.google.protobuf.CodedOutputStream .computeEnumSize(1, type_); } - if (logger_ != io.grpc.binarylog.GrpcLogEntry.Logger.CLIENT.getNumber()) { + if (logger_ != io.grpc.binarylog.GrpcLogEntry.Logger.UNKNOWN_LOGGER.getNumber()) { size += com.google.protobuf.CodedOutputStream .computeEnumSize(2, logger_); } diff --git a/services/src/generated/main/java/io/grpc/binarylog/MetadataEntry.java b/services/src/generated/main/java/io/grpc/binarylog/MetadataEntry.java index 8ecdd2b35..b04500f8c 100644 --- a/services/src/generated/main/java/io/grpc/binarylog/MetadataEntry.java +++ b/services/src/generated/main/java/io/grpc/binarylog/MetadataEntry.java @@ -20,7 +20,7 @@ private static final long serialVersionUID = 0L; super(builder); } private MetadataEntry() { - key_ = ""; + key_ = com.google.protobuf.ByteString.EMPTY; value_ = com.google.protobuf.ByteString.EMPTY; } @@ -56,9 +56,8 @@ private static final long serialVersionUID = 0L; break; } case 10: { - java.lang.String s = input.readStringRequireUtf8(); - key_ = s; + key_ = input.readBytes(); break; } case 18: { @@ -91,37 +90,16 @@ private static final long serialVersionUID = 0L; } public static final int KEY_FIELD_NUMBER = 1; - private volatile java.lang.Object key_; + private com.google.protobuf.ByteString key_; /** - * string key = 1; + * 
+   * TODO(zpencer): upstream this
+   *
+ * + * bytes key = 1; */ - public java.lang.String getKey() { - java.lang.Object ref = key_; - if (ref instanceof java.lang.String) { - return (java.lang.String) ref; - } else { - com.google.protobuf.ByteString bs = - (com.google.protobuf.ByteString) ref; - java.lang.String s = bs.toStringUtf8(); - key_ = s; - return s; - } - } - /** - * string key = 1; - */ - public com.google.protobuf.ByteString - getKeyBytes() { - java.lang.Object ref = key_; - if (ref instanceof java.lang.String) { - com.google.protobuf.ByteString b = - com.google.protobuf.ByteString.copyFromUtf8( - (java.lang.String) ref); - key_ = b; - return b; - } else { - return (com.google.protobuf.ByteString) ref; - } + public com.google.protobuf.ByteString getKey() { + return key_; } public static final int VALUE_FIELD_NUMBER = 2; @@ -145,8 +123,8 @@ private static final long serialVersionUID = 0L; public void writeTo(com.google.protobuf.CodedOutputStream output) throws java.io.IOException { - if (!getKeyBytes().isEmpty()) { - com.google.protobuf.GeneratedMessageV3.writeString(output, 1, key_); + if (!key_.isEmpty()) { + output.writeBytes(1, key_); } if (!value_.isEmpty()) { output.writeBytes(2, value_); @@ -159,8 +137,9 @@ private static final long serialVersionUID = 0L; if (size != -1) return size; size = 0; - if (!getKeyBytes().isEmpty()) { - size += com.google.protobuf.GeneratedMessageV3.computeStringSize(1, key_); + if (!key_.isEmpty()) { + size += com.google.protobuf.CodedOutputStream + .computeBytesSize(1, key_); } if (!value_.isEmpty()) { size += com.google.protobuf.CodedOutputStream @@ -334,7 +313,7 @@ private static final long serialVersionUID = 0L; } public Builder clear() { super.clear(); - key_ = ""; + key_ = com.google.protobuf.ByteString.EMPTY; value_ = com.google.protobuf.ByteString.EMPTY; @@ -403,9 +382,8 @@ private static final long serialVersionUID = 0L; public Builder mergeFrom(io.grpc.binarylog.MetadataEntry other) { if (other == io.grpc.binarylog.MetadataEntry.getDefaultInstance()) return this; - if (!other.getKey().isEmpty()) { - key_ = other.key_; - onChanged(); + if (other.getKey() != com.google.protobuf.ByteString.EMPTY) { + setKey(other.getKey()); } if (other.getValue() != com.google.protobuf.ByteString.EMPTY) { setValue(other.getValue()); @@ -437,43 +415,25 @@ private static final long serialVersionUID = 0L; return this; } - private java.lang.Object key_ = ""; - /** - * string key = 1; - */ - public java.lang.String getKey() { - java.lang.Object ref = key_; - if (!(ref instanceof java.lang.String)) { - com.google.protobuf.ByteString bs = - (com.google.protobuf.ByteString) ref; - java.lang.String s = bs.toStringUtf8(); - key_ = s; - return s; - } else { - return (java.lang.String) ref; - } - } + private com.google.protobuf.ByteString key_ = com.google.protobuf.ByteString.EMPTY; /** - * string key = 1; + * 
+     * TODO(zpencer): upstream this
+     *
+ * + * bytes key = 1; */ - public com.google.protobuf.ByteString - getKeyBytes() { - java.lang.Object ref = key_; - if (ref instanceof String) { - com.google.protobuf.ByteString b = - com.google.protobuf.ByteString.copyFromUtf8( - (java.lang.String) ref); - key_ = b; - return b; - } else { - return (com.google.protobuf.ByteString) ref; - } + public com.google.protobuf.ByteString getKey() { + return key_; } /** - * string key = 1; + * 
+     * TODO(zpencer): upstream this
+     *
+ * + * bytes key = 1; */ - public Builder setKey( - java.lang.String value) { + public Builder setKey(com.google.protobuf.ByteString value) { if (value == null) { throw new NullPointerException(); } @@ -483,7 +443,11 @@ private static final long serialVersionUID = 0L; return this; } /** - * string key = 1; + * 
+     * TODO(zpencer): upstream this
+     *
+ * + * bytes key = 1; */ public Builder clearKey() { @@ -491,20 +455,6 @@ private static final long serialVersionUID = 0L; onChanged(); return this; } - /** - * string key = 1; - */ - public Builder setKeyBytes( - com.google.protobuf.ByteString value) { - if (value == null) { - throw new NullPointerException(); - } - checkByteStringIsUtf8(value); - - key_ = value; - onChanged(); - return this; - } private com.google.protobuf.ByteString value_ = com.google.protobuf.ByteString.EMPTY; /** diff --git a/services/src/generated/main/java/io/grpc/binarylog/MetadataEntryOrBuilder.java b/services/src/generated/main/java/io/grpc/binarylog/MetadataEntryOrBuilder.java index 177cd9d8a..9271efcb7 100644 --- a/services/src/generated/main/java/io/grpc/binarylog/MetadataEntryOrBuilder.java +++ b/services/src/generated/main/java/io/grpc/binarylog/MetadataEntryOrBuilder.java @@ -8,14 +8,13 @@ public interface MetadataEntryOrBuilder extends com.google.protobuf.MessageOrBuilder { /** - * string key = 1; + * 
+   * TODO(zpencer): upstream this
+   *
+ * + * bytes key = 1; */ - java.lang.String getKey(); - /** - * string key = 1; - */ - com.google.protobuf.ByteString - getKeyBytes(); + com.google.protobuf.ByteString getKey(); /** * bytes value = 2; diff --git a/services/src/generated/main/java/io/grpc/binarylog/Peer.java b/services/src/generated/main/java/io/grpc/binarylog/Peer.java index 549b61946..6aa963f61 100644 --- a/services/src/generated/main/java/io/grpc/binarylog/Peer.java +++ b/services/src/generated/main/java/io/grpc/binarylog/Peer.java @@ -95,57 +95,73 @@ private static final long serialVersionUID = 0L; */ public enum PeerType implements com.google.protobuf.ProtocolMessageEnum { + /** + * 
+     * TODO(zpencer): upstream this
+     *
+ * + * UNKNOWN_PEERTYPE = 0; + */ + UNKNOWN_PEERTYPE(0), /** * 
      * peer is struct sockaddr_in
      *
* - * PEER_IPV4 = 0; + * PEER_IPV4 = 1; */ - PEER_IPV4(0), + PEER_IPV4(1), /** * 
      * peer is struct sockaddr_in6
      *
* - * PEER_IPV6 = 1; + * PEER_IPV6 = 2; */ - PEER_IPV6(1), + PEER_IPV6(2), /** * 
      * peer is struct sockaddr_un
      *
* - * PEER_UNIX = 2; + * PEER_UNIX = 3; */ - PEER_UNIX(2), + PEER_UNIX(3), UNRECOGNIZED(-1), ; + /** + * 
+     * TODO(zpencer): upstream this
+     *
+ * + * UNKNOWN_PEERTYPE = 0; + */ + public static final int UNKNOWN_PEERTYPE_VALUE = 0; /** * 
      * peer is struct sockaddr_in
      *
* - * PEER_IPV4 = 0; + * PEER_IPV4 = 1; */ - public static final int PEER_IPV4_VALUE = 0; + public static final int PEER_IPV4_VALUE = 1; /** * 
      * peer is struct sockaddr_in6
      *
* - * PEER_IPV6 = 1; + * PEER_IPV6 = 2; */ - public static final int PEER_IPV6_VALUE = 1; + public static final int PEER_IPV6_VALUE = 2; /** * 
      * peer is struct sockaddr_un
      *
* - * PEER_UNIX = 2; + * PEER_UNIX = 3; */ - public static final int PEER_UNIX_VALUE = 2; + public static final int PEER_UNIX_VALUE = 3; public final int getNumber() { @@ -166,9 +182,10 @@ private static final long serialVersionUID = 0L; public static PeerType forNumber(int value) { switch (value) { - case 0: return PEER_IPV4; - case 1: return PEER_IPV6; - case 2: return PEER_UNIX; + case 0: return UNKNOWN_PEERTYPE; + case 1: return PEER_IPV4; + case 2: return PEER_IPV6; + case 3: return PEER_UNIX; default: return null; } } @@ -262,7 +279,7 @@ private static final long serialVersionUID = 0L; public void writeTo(com.google.protobuf.CodedOutputStream output) throws java.io.IOException { - if (peerType_ != io.grpc.binarylog.Peer.PeerType.PEER_IPV4.getNumber()) { + if (peerType_ != io.grpc.binarylog.Peer.PeerType.UNKNOWN_PEERTYPE.getNumber()) { output.writeEnum(1, peerType_); } if (!peer_.isEmpty()) { @@ -276,7 +293,7 @@ private static final long serialVersionUID = 0L; if (size != -1) return size; size = 0; - if (peerType_ != io.grpc.binarylog.Peer.PeerType.PEER_IPV4.getNumber()) { + if (peerType_ != io.grpc.binarylog.Peer.PeerType.UNKNOWN_PEERTYPE.getNumber()) { size += com.google.protobuf.CodedOutputStream .computeEnumSize(1, peerType_); } diff --git a/services/src/main/java/io/grpc/services/BinaryLog.java b/services/src/main/java/io/grpc/services/BinaryLog.java index f974e18f1..6e1f74b39 100644 --- a/services/src/main/java/io/grpc/services/BinaryLog.java +++ b/services/src/main/java/io/grpc/services/BinaryLog.java @@ -272,7 +272,8 @@ final class BinaryLog { */ // TODO(zpencer): verify int64 representation with other gRPC languages static Uint128 callIdToProto(byte[] bytes) { - Preconditions.checkArgument(bytes.length == 16); + Preconditions.checkArgument( + bytes.length == 16, "can only convert from 16 byte input, actual length =" + bytes.length); ByteBuffer bb = ByteBuffer.wrap(bytes); long high = bb.getLong(); long low = bb.getLong(); @@ -282,7 +283,7 @@ final class BinaryLog { @VisibleForTesting // TODO(zpencer): the binlog design does not specify how to actually express the peer bytes static Peer socketToProto(SocketAddress address) { - PeerType peerType = null; + PeerType peerType = PeerType.UNKNOWN_PEERTYPE; byte[] peerAddress = null; if (address instanceof InetSocketAddress) { @@ -296,23 +297,20 @@ final class BinaryLog { } int port = ((InetSocketAddress) address).getPort(); byte[] portBytes = new byte[IP_PORT_BYTES]; - portBytes[0] = (byte) (port & IP_PORT_UPPER_MASK); + portBytes[0] = (byte) ((port & IP_PORT_UPPER_MASK) >> 8); portBytes[1] = (byte) (port & IP_PORT_LOWER_MASK); peerAddress = Bytes.concat(inetAddress.getAddress(), portBytes); } 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; - // DomainSocketAddress.toString() is equivalent to DomainSocketAdddress.path() - peerAddress = address.toString().getBytes(Charset.defaultCharset()); - } - Peer.Builder builder = Peer.newBuilder(); - if (peerType != null) { - builder.setPeerType(peerType); } - if (peerAddress != null) { - builder.setPeer(ByteString.copyFrom(peerAddress)); + if (peerAddress == null) { + peerAddress = address.toString().getBytes(Charset.defaultCharset()); } - return builder.build(); + return Peer.newBuilder() + .setPeerType(peerType) + .setPeer(ByteString.copyFrom(peerAddress)) + .build(); } @VisibleForTesting @@ -330,7 +328,7 @@ final class BinaryLog { builder.addEntry( MetadataEntry .newBuilder() - .setKeyBytes(ByteString.copyFrom(key)) + .setKey(ByteString.copyFrom(key)) .setValue(ByteString.copyFrom(value)) .build()); written += key.length; diff --git a/services/src/main/proto/io/grpc/binarylog.proto b/services/src/main/proto/io/grpc/binarylog.proto index 4c9229168..cbc0945af 100644 --- a/services/src/main/proto/io/grpc/binarylog.proto +++ b/services/src/main/proto/io/grpc/binarylog.proto @@ -26,18 +26,20 @@ option java_outer_classname = "BinaryLogProto"; message GrpcLogEntry { // Enumerates the type of logs enum Type { - SEND_INITIAL_METADATA = 0; - SEND_TRAILING_METADATA = 1; - SEND_MESSAGE = 2; - RECV_INITIAL_METADATA = 3; - RECV_TRAILING_METADATA = 4; - RECV_MESSAGE = 5; + UNKNOWN_TYPE = 0; // TODO(zpencer): upstream this + SEND_INITIAL_METADATA = 1; + SEND_TRAILING_METADATA = 2; + SEND_MESSAGE = 3; + RECV_INITIAL_METADATA = 4; + RECV_TRAILING_METADATA = 5; + RECV_MESSAGE = 6; }; // Enumerates the entity that generates the log entry enum Logger { - CLIENT = 0; - SERVER = 1; + UNKNOWN_LOGGER = 0; // TODO(zpencer): upstream this + CLIENT = 1; + SERVER = 2; } Type type = 1; // One of the above Type enum @@ -87,16 +89,17 @@ message Metadata { // A metadata key value pair message MetadataEntry { - string key = 1; + bytes key = 1; // TODO(zpencer): upstream this bytes value = 2; } // Peer information message Peer { enum PeerType { - PEER_IPV4 = 0; // peer is struct sockaddr_in - PEER_IPV6 = 1; // peer is struct sockaddr_in6 - PEER_UNIX = 2; // peer is struct sockaddr_un + UNKNOWN_PEERTYPE = 0; // TODO(zpencer): upstream this + PEER_IPV4 = 1; // peer is struct sockaddr_in + PEER_IPV6 = 2; // peer is struct sockaddr_in6 + PEER_UNIX = 3; // peer is struct sockaddr_un }; PeerType peer_type = 1; bytes peer = 2; // value depends on peer_type diff --git a/services/src/test/java/io/grpc/services/BinaryLogTest.java b/services/src/test/java/io/grpc/services/BinaryLogTest.java index a24cdac1a..fa5997716 100644 --- a/services/src/test/java/io/grpc/services/BinaryLogTest.java +++ b/services/src/test/java/io/grpc/services/BinaryLogTest.java @@ -18,6 +18,8 @@ package io.grpc.services; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import com.google.common.primitives.Bytes; import com.google.protobuf.ByteString; @@ -25,13 +27,17 @@ import io.grpc.Metadata; import io.grpc.binarylog.Message; import io.grpc.binarylog.MetadataEntry; import io.grpc.binarylog.Peer; +import io.grpc.binarylog.Peer.PeerType; import io.grpc.binarylog.Uint128; import io.grpc.services.BinaryLog.FactoryImpl; 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 org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -48,49 +54,43 @@ public final class BinaryLogTest { private static final BinaryLog BOTH_FULL = new Builder().header(Integer.MAX_VALUE).msg(Integer.MAX_VALUE).build(); - private static final byte[] CALL_ID = new byte[] { - 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18, - 0x19, 0x10, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f }; - + private static final String dataA = "aaaaaaaaa"; + private static final String dataB = "bbbbbbbbb"; + private static final String dataC = "ccccccccc"; private static final Metadata.Key KEY_A = Metadata.Key.of("a", Metadata.ASCII_STRING_MARSHALLER); private static final Metadata.Key KEY_B = Metadata.Key.of("b", Metadata.ASCII_STRING_MARSHALLER); private static final Metadata.Key KEY_C = Metadata.Key.of("c", Metadata.ASCII_STRING_MARSHALLER); - private static final MetadataEntry ENTRY_A; - private static final MetadataEntry ENTRY_B; - private static final MetadataEntry ENTRY_C; - private static final Metadata metadata; - private static final boolean IS_COMPRESSED = true; - private static final boolean IS_UNCOMPRESSED = false; - - static { - String dataA = "aaaaaaaaa"; - String dataB = "bbbbbbbbb"; - String dataC = "ccccccccc"; - metadata = new Metadata(); - metadata.put(KEY_A, dataA); - metadata.put(KEY_B, dataB); - metadata.put(KEY_C, dataC); - ENTRY_A = + private static final MetadataEntry ENTRY_A = MetadataEntry .newBuilder() - .setKey(KEY_A.name()) + .setKey(ByteString.copyFrom(KEY_A.name(), US_ASCII)) .setValue(ByteString.copyFrom(dataA.getBytes(US_ASCII))) .build(); - ENTRY_B = + private static final MetadataEntry ENTRY_B = MetadataEntry .newBuilder() - .setKey(KEY_B.name()) + .setKey(ByteString.copyFrom(KEY_B.name(), US_ASCII)) .setValue(ByteString.copyFrom(dataB.getBytes(US_ASCII))) .build(); - ENTRY_C = + private static final MetadataEntry ENTRY_C = MetadataEntry .newBuilder() - .setKey(KEY_C.name()) + .setKey(ByteString.copyFrom(KEY_C.name(), US_ASCII)) .setValue(ByteString.copyFrom(dataC.getBytes(US_ASCII))) .build(); + private static final boolean IS_COMPRESSED = true; + private static final boolean IS_UNCOMPRESSED = false; + + private final Metadata metadata = new Metadata(); + + @Before + public void setUp() throws Exception { + metadata.put(KEY_A, dataA); + metadata.put(KEY_B, dataB); + metadata.put(KEY_C, dataC); } @Test @@ -262,24 +262,39 @@ public final class BinaryLogTest { @Test public void callIdToProto() { + byte[] callId = new byte[] { + 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18, + 0x19, 0x10, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f }; assertEquals( Uint128 .newBuilder() .setHigh(0x1112131415161718L) .setLow(0x19101a1b1c1d1e1fL) .build(), - BinaryLog.callIdToProto(CALL_ID)); + BinaryLog.callIdToProto(callId)); } - @Test(expected = IllegalArgumentException.class) + @Test public void callIdToProto_invalid_shorter_len() { - BinaryLog.callIdToProto(new byte[14]); + try { + BinaryLog.callIdToProto(new byte[14]); + fail(); + } catch (IllegalArgumentException expected) { + assertTrue( + expected.getMessage().startsWith("can only convert from 16 byte input, actual length")); + } } - @Test(expected = IllegalArgumentException.class) + @Test public void callIdToProto_invalid_longer_len() { - BinaryLog.callIdToProto(new byte[18]); + try { + BinaryLog.callIdToProto(new byte[18]); + fail(); + } catch (IllegalArgumentException expected) { + assertTrue( + expected.getMessage().startsWith("can only convert from 16 byte input, actual length")); + } } @Test @@ -288,12 +303,13 @@ public final class BinaryLogTest { int port = 12345; InetSocketAddress socketAddress = new InetSocketAddress(address, port); byte[] addressBytes = address.getAddress(); - byte[] portBytes = new byte[] {(byte) (port & 0xff00), (byte) (port & 0xff)}; + 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, portBytes))) + .setPeer(ByteString.copyFrom(Bytes.concat(addressBytes, portUnsignedBytes))) .build(), BinaryLog.socketToProto(socketAddress)); } @@ -305,12 +321,13 @@ public final class BinaryLogTest { int port = 12345; InetSocketAddress socketAddress = new InetSocketAddress(address, port); byte[] addressBytes = address.getAddress(); - byte[] portBytes = new byte[] {(byte) (port & 0xff00), (byte) (port & 0xff)}; + 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, portBytes))) + .setPeer(ByteString.copyFrom(Bytes.concat(addressBytes, portUnsignedBytes))) .build(), BinaryLog.socketToProto(socketAddress)); } @@ -333,9 +350,11 @@ public final class BinaryLogTest { public void socketToProto_unknown() throws Exception { SocketAddress unknownSocket = new SocketAddress() { }; assertEquals( - Peer.newBuilder().build(), - BinaryLog.socketToProto(unknownSocket) - ); + Peer.newBuilder() + .setPeerType(PeerType.UNKNOWN_PEERTYPE) + .setPeer(ByteString.copyFrom(unknownSocket.toString(), US_ASCII)) + .build(), + BinaryLog.socketToProto(unknownSocket)); } @Test -- cgit v1.2.3