fix review comments

channelz/service/service.go

@@ -32,12 +32,16 @@ import (
 	"google.golang.org/grpc/channelz"
 	pb "google.golang.org/grpc/channelz/service_proto"
 	"google.golang.org/grpc/connectivity"
+	"google.golang.org/grpc/credentials"
 )
 
-const (
-	secToNano  = 1e9
-	usecToNano = 1e3
-)
+func secToNano(i int64) int64 {
+	return i * 1e9
+}
+
+func usecToNano(i int64) int64 {
+	return i * 1e3
+}
 
 // RegisterChannelzServiceToServer registers the channelz service to the given server.
 func RegisterChannelzServiceToServer(s *grpc.Server) {
@@ -135,51 +139,45 @@ func subChannelMetricToProto(cm *channelz.SubChannelMetric) *pb.Subchannel {
 	return sc
 }
 
-func securityToProto(se channelz.SecurityValue) *pb.Security {
+func securityToProto(se credentials.SecurityValue) *pb.Security {
 	switch v := se.(type) {
-	case *channelz.TLSSecurityValue:
+	case *credentials.TLSSecurityValue:
 		return &pb.Security{Model: &pb.Security_Tls_{Tls: &pb.Security_Tls{
 			CipherSuite:       &pb.Security_Tls_StandardName{StandardName: v.StandardName},
 			LocalCertificate:  v.LocalCertificate,
 			RemoteCertificate: v.RemoteCertificate,
-		},
-		},
-		}
-	case *channelz.OtherSecurityValue:
+		}}}
+	case *credentials.OtherSecurityValue:
 		anyval, err := ptypes.MarshalAny(v.Value)
 		if err != nil {
 			return &pb.Security{Model: &pb.Security_Other{Other: &pb.Security_OtherSecurity{
 				Name: v.Name,
-			},
-			},
-			}
+			}}}
 		}
 		return &pb.Security{Model: &pb.Security_Other{Other: &pb.Security_OtherSecurity{
 			Name:  v.Name,
 			Value: anyval,
-		},
-		},
-		}
+		}}}
 	}
 	return nil
 }
 
 func sockoptToProto(skopts *channelz.SocketOptionData) []*pb.SocketOption {
 	var opts []*pb.SocketOption
 	if skopts.Linger != nil {
-		additional, err := ptypes.MarshalAny(&pb.SocketOptionLinger{Active: skopts.Linger.Onoff != 0, Duration: ptypes.DurationProto(time.Duration(int64(skopts.Linger.Linger) * secToNano))})
+		additional, err := ptypes.MarshalAny(&pb.SocketOptionLinger{Active: skopts.Linger.Onoff != 0, Duration: ptypes.DurationProto(time.Duration(secToNano(int64(skopts.Linger.Linger))))})
 		if err == nil {
 			opts = append(opts, &pb.SocketOption{Name: "SO_LINGER", Additional: additional})
 		}
 	}
 	if skopts.RecvTimeout != nil {
-		additional, err := ptypes.MarshalAny(&pb.SocketOptionTimeout{Duration: ptypes.DurationProto(time.Duration(skopts.RecvTimeout.Sec*secToNano + skopts.RecvTimeout.Usec*usecToNano))})
+		additional, err := ptypes.MarshalAny(&pb.SocketOptionTimeout{Duration: ptypes.DurationProto(time.Duration(secToNano(int64(skopts.RecvTimeout.Sec)) + usecToNano(int64(skopts.RecvTimeout.Usec))))})
 		if err == nil {
 			opts = append(opts, &pb.SocketOption{Name: "SO_RCVTIMEO", Additional: additional})
 		}
 	}
 	if skopts.SendTimeout != nil {
-		additional, err := ptypes.MarshalAny(&pb.SocketOptionTimeout{Duration: ptypes.DurationProto(time.Duration(skopts.SendTimeout.Sec*secToNano + skopts.SendTimeout.Usec*usecToNano))})
+		additional, err := ptypes.MarshalAny(&pb.SocketOptionTimeout{Duration: ptypes.DurationProto(time.Duration(secToNano(int64(skopts.SendTimeout.Sec)) + usecToNano(int64(skopts.SendTimeout.Usec))))})
 		if err == nil {
 			opts = append(opts, &pb.SocketOption{Name: "SO_SNDTIMEO", Additional: additional})
 		}

channelz/service/service_test.go

@@ -34,10 +34,11 @@ import (
 	"google.golang.org/grpc/channelz"
 	pb "google.golang.org/grpc/channelz/service_proto"
 	"google.golang.org/grpc/connectivity"
+	"google.golang.org/grpc/credentials"
 )
 
 func init() {
-	proto.RegisterType((*OtherSecurityValue)(nil), "grpc.channelz.OtherSecurityValue")
+	proto.RegisterType((*OtherSecurityValue)(nil), "grpc.credentials.OtherSecurityValue")
 	channelz.TurnOn()
 }
 
@@ -100,7 +101,7 @@ type dummySocket struct {
 	SocketOptions                    *channelz.SocketOptionData
 	localAddr                        net.Addr
 	remoteAddr                       net.Addr
-	Security                         channelz.SecurityValue
+	Security                         credentials.SecurityValue
 	remoteName                       string
 }
 
@@ -193,12 +194,12 @@ func protoToTime(protoTime *pb.SocketOptionTimeout) *unix.Timeval {
 	return timeout
 }
 
-func protoToSecurity(protoSecurity *pb.Security) channelz.SecurityValue {
+func protoToSecurity(protoSecurity *pb.Security) credentials.SecurityValue {
 	switch v := protoSecurity.Model.(type) {
 	case *pb.Security_Tls_:
-		return &channelz.TLSSecurityValue{StandardName: v.Tls.GetStandardName(), LocalCertificate: v.Tls.GetLocalCertificate(), RemoteCertificate: v.Tls.GetRemoteCertificate()}
+		return &credentials.TLSSecurityValue{StandardName: v.Tls.GetStandardName(), LocalCertificate: v.Tls.GetLocalCertificate(), RemoteCertificate: v.Tls.GetRemoteCertificate()}
 	case *pb.Security_Other:
-		sv := &channelz.OtherSecurityValue{Name: v.Other.GetName()}
+		sv := &credentials.OtherSecurityValue{Name: v.Other.GetName()}
 		var x ptypes.DynamicAny
 		if err := ptypes.UnmarshalAny(v.Other.GetValue(), &x); err == nil {
 			sv.Value = x.Message
@@ -588,18 +589,18 @@ func TestGetSocket(t *testing.T) {
 			},
 		},
 		{
-			Security: &channelz.TLSSecurityValue{
+			Security: &credentials.TLSSecurityValue{
 				StandardName:      "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256",
 				RemoteCertificate: []byte{48, 130, 2, 156, 48, 130, 2, 5, 160},
 			},
 		},
 		{
-			Security: &channelz.OtherSecurityValue{
+			Security: &credentials.OtherSecurityValue{
 				Name: "XXXX",
 			},
 		},
 		{
-			Security: &channelz.OtherSecurityValue{
+			Security: &credentials.OtherSecurityValue{
 				Name:  "YYYY",
 				Value: &OtherSecurityValue{LocalCertificate: []byte{1, 2, 3}, RemoteCertificate: []byte{4, 5, 6}},
 			},

channelz/types.go

@@ -22,8 +22,8 @@ import (
 	"net"
 	"time"
 
-	"github.com/golang/protobuf/proto"
 	"google.golang.org/grpc/connectivity"
+	"google.golang.org/grpc/credentials"
 	"google.golang.org/grpc/grpclog"
 )
 
@@ -284,7 +284,7 @@ type SocketInternalMetric struct {
 	// the original target name.
 	RemoteName    string
 	SocketOptions *SocketOptionData
-	Security      SecurityValue
+	Security      credentials.SecurityValue
 }
 
 // Socket is the interface that should be satisfied in order to be tracked by
@@ -417,35 +417,3 @@ func (s *server) deleteSelfIfReady() {
 	}
 	s.cm.deleteEntry(s.id)
 }
-
-// Security defines the interface that security protocols should implement in order
-// to provide security info to channelz.
-type Security interface {
-	GetSecurityValue() SecurityValue
-}
-
-// SecurityValue defines the interface that GetSecurityValue() return value should
-// satisfy. This interface should only be satisfied by *TLSSecurityValue and
-// *OtherSecurityValue.
-type SecurityValue interface {
-	isSecurityValue()
-}
-
-// TLSSecurityValue defines the struct that TLS protocol should return from GetSecurityValue(),
-// containing security info like cipher and certificate used.
-type TLSSecurityValue struct {
-	StandardName      string
-	LocalCertificate  []byte
-	RemoteCertificate []byte
-}
-
-func (*TLSSecurityValue) isSecurityValue() {}
-
-// OtherSecurityValue defines the struct that non-TLS protocol should return from
-// GetSecurityValue(), which contains protocol specific security info.
-type OtherSecurityValue struct {
-	Name  string
-	Value proto.Message
-}
-
-func (*OtherSecurityValue) isSecurityValue() {}

credentials/credentials.go

@@ -31,8 +31,7 @@ import (
 	"net"
 	"strings"
 
-	"google.golang.org/grpc/channelz"
-
+	"github.com/golang/protobuf/proto"
 	"golang.org/x/net/context"
 )
 
@@ -121,8 +120,8 @@ func (t TLSInfo) AuthType() string {
 }
 
 // GetSecurityValue returns security info requested by channelz.
-func (t TLSInfo) GetSecurityValue() channelz.SecurityValue {
-	v := &channelz.TLSSecurityValue{
+func (t TLSInfo) GetSecurityValue() SecurityValue {
+	v := &TLSSecurityValue{
 		StandardName: cipherSuiteLookup[t.State.CipherSuite],
 	}
 	// Currently there's no way to get LocalCertificate info from tls package.
@@ -169,15 +168,15 @@ func (c *tlsCreds) ClientHandshake(ctx context.Context, authority string, rawCon
 	case <-ctx.Done():
 		return nil, nil, ctx.Err()
 	}
-	return tlsConn{conn, rawConn}, TLSInfo{conn.ConnectionState()}, nil
+	return tlsConn{Conn: conn, rawConn: rawConn}, TLSInfo{conn.ConnectionState()}, nil
 }
 
 func (c *tlsCreds) ServerHandshake(rawConn net.Conn) (net.Conn, AuthInfo, error) {
 	conn := tls.Server(rawConn, c.config)
 	if err := conn.Handshake(); err != nil {
 		return nil, nil, err
 	}
-	return tlsConn{conn, rawConn}, TLSInfo{conn.ConnectionState()}, nil
+	return tlsConn{Conn: conn, rawConn: rawConn}, TLSInfo{conn.ConnectionState()}, nil
 }
 
 func (c *tlsCreds) Clone() TransportCredentials {
@@ -232,3 +231,35 @@ func NewServerTLSFromFile(certFile, keyFile string) (TransportCredentials, error
 	}
 	return NewTLS(&tls.Config{Certificates: []tls.Certificate{cert}}), nil
 }
+
+// Security defines the interface that security protocols should implement in order
+// to provide security info to channelz.
+type Security interface {
+	GetSecurityValue() SecurityValue
+}
+
+// SecurityValue defines the interface that GetSecurityValue() return value should
+// satisfy. This interface should only be satisfied by *TLSSecurityValue and
+// *OtherSecurityValue.
+type SecurityValue interface {
+	isSecurityValue()
+}
+
+// TLSSecurityValue defines the struct that TLS protocol should return from GetSecurityValue(),
+// containing security info like cipher and certificate used.
+type TLSSecurityValue struct {
+	StandardName      string
+	LocalCertificate  []byte
+	RemoteCertificate []byte
+}
+
+func (*TLSSecurityValue) isSecurityValue() {}
+
+// OtherSecurityValue defines the struct that non-TLS protocol should return from
+// GetSecurityValue(), which contains protocol specific security info.
+type OtherSecurityValue struct {
+	Name  string
+	Value proto.Message
+}
+
+func (*OtherSecurityValue) isSecurityValue() {}

credentials/credentials_util_go19.go

@@ -2,7 +2,7 @@
 
 /*
  *
- * Copyright 2014 gRPC authors.
+ * Copyright 2018 gRPC authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.

credentials/credentials_util_pre_go19.go

@@ -2,7 +2,7 @@
 
 /*
  *
- * Copyright 2014 gRPC authors.
+ * Copyright 2018 gRPC authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.