review comments

Author: mbissa

balancer/pickfirst/pickfirst_ext_test.go

@@ -2024,8 +2024,7 @@ func (s) TestPickFirstLeaf_HappyEyeballs_TriggerConnectionDelay(t *testing.T) {
 
 func waitForMetric(ctx context.Context, t *testing.T, tmr *stats.TestMetricsRecorder, metricName string) {
 	for {
-		_, ok := tmr.Metric(metricName)
-		if ok {
+		if _, ok := tmr.Metric(metricName); ok {
 			break
 		}
 		select {

clientconn.go

@@ -1336,8 +1336,11 @@ func (ac *addrConn) resetTransportAndUnlock() {
 		if !errors.Is(err, context.Canceled) {
 			connectionAttemptsFailedMetric.Record(ac.cc.metricsRecorderList, 1, ac.cc.target, ac.backendServiceLabel, ac.localityLabel)
 		} else {
-			// This records cancelled connection attempts which can be later replaced by a metric.
-			logger.Infof("Context cancellation detected; not recording this as a failed connection attempt.")
+			if logger.V(2) {
+				// This records cancelled connection attempts which can be later
+				// replaced by a metric.
+				logger.Infof("Context cancellation detected; not recording this as a failed connection attempt.")
+			}
 		}
 		// TODO: #7534 - Move re-resolution requests into the pick_first LB policy
 		// to ensure one resolution request per pass instead of per subconn failure.
@@ -1387,11 +1390,11 @@ func (ac *addrConn) resetTransportAndUnlock() {
 // updateTelemetryLabelsLocked calculates and caches the telemetry labels based on the
 // first address in addrConn.
 func (ac *addrConn) updateTelemetryLabelsLocked() {
-	// Reset defaults
-	ac.localityLabel = ""
-	ac.backendServiceLabel = ""
 	labelsFunc, ok := internal.AddressToTelemetryLabels.(func(resolver.Address) map[string]string)
 	if !ok || len(ac.addrs) == 0 {
+		// Reset defaults
+		ac.localityLabel = ""
+		ac.backendServiceLabel = ""
 		return
 	}
 	labels := labelsFunc(ac.addrs[0])
@@ -1409,7 +1412,7 @@ func (ac *addrConn) securityLevelLocked() string {
 		secLevel, _ = ac.curAddr.Attributes.Value(securityLevelKey{}).(string)
 		return secLevel
 	}
-	authInfo := ac.transport.GetPeer().AuthInfo
+	authInfo := ac.transport.Peer().AuthInfo
 	if ci, ok := authInfo.(interface {
 		GetCommonAuthInfo() credentials.CommonAuthInfo
 	}); ok {

internal/transport/http2_client.go

@@ -370,7 +370,7 @@ func NewHTTP2Client(connectCtx, ctx context.Context, addr resolver.Address, opts
 		})
 	t.logger = prefixLoggerForClientTransport(t)
 	// Add peer information to the http2client context.
-	t.ctx = peer.NewContext(t.ctx, t.GetPeer())
+	t.ctx = peer.NewContext(t.ctx, t.Peer())
 
 	if md, ok := addr.Metadata.(*metadata.MD); ok {
 		t.md = *md
@@ -510,7 +510,7 @@ func (t *http2Client) newStream(ctx context.Context, callHdr *CallHdr) *ClientSt
 	return s
 }
 
-func (t *http2Client) GetPeer() *peer.Peer {
+func (t *http2Client) Peer() *peer.Peer {
 	return &peer.Peer{
 		Addr:      t.remoteAddr,
 		AuthInfo:  t.authInfo, // Can be nil
@@ -742,7 +742,7 @@ func (e NewStreamError) Error() string {
 // NewStream creates a stream and registers it into the transport as "active"
 // streams.  All non-nil errors returned will be *NewStreamError.
 func (t *http2Client) NewStream(ctx context.Context, callHdr *CallHdr) (*ClientStream, error) {
-	ctx = peer.NewContext(ctx, t.GetPeer())
+	ctx = peer.NewContext(ctx, t.Peer())
 
 	// ServerName field of the resolver returned address takes precedence over
 	// Host field of CallHdr to determine the :authority header. This is because,
@@ -1807,10 +1807,6 @@ func (t *http2Client) socketMetrics() *channelz.EphemeralSocketMetrics {
 	}
 }
 
-func (t *http2Client) RemoteAddr() net.Addr { return t.remoteAddr }
-
-func (t *http2Client) AuthInfo() credentials.AuthInfo { return t.authInfo }
-
 func (t *http2Client) incrMsgSent() {
 	if channelz.IsOn() {
 		t.channelz.SocketMetrics.MessagesSent.Add(1)

internal/transport/transport.go

@@ -608,9 +608,9 @@ type ClientTransport interface {
 	// with a human readable string with debug info.
 	GetGoAwayReason() (GoAwayReason, string)
 
-	// GetPeer return peer information that includes auth information and remote
-	// network address
-	GetPeer() *peer.Peer
+	// Peer returns information about the peer associated with the Transport.
+	// The returned information includes authentication and network address details.
+	Peer() *peer.Peer
 }
 
 // ServerTransport is the common interface for all gRPC server-side transport

stream.go

@@ -484,7 +484,7 @@ func (a *csAttempt) getTransport() error {
 		return err
 	}
 	if a.trInfo != nil {
-		a.trInfo.firstLine.SetRemoteAddr(a.transport.GetPeer().Addr)
+		a.trInfo.firstLine.SetRemoteAddr(a.transport.Peer().Addr)
 	}
 	if pick.blocked && a.statsHandler != nil {
 		a.statsHandler.HandleRPC(a.ctx, &stats.DelayedPickComplete{})