xdsclient: only include nodeID in error strings, not the whole nodePr…

…oto (#5461)
grpc · Jun 23, 2022 · c6ee1c7 · c6ee1c7
1 parent 06ad0b8
commit c6ee1c7
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 8 deletions.
diff --git a/xds/internal/xdsclient/authority.go b/xds/internal/xdsclient/authority.go
@@ -21,6 +21,8 @@ import (
 	"errors"
 	"fmt"
 
+	v2corepb "github.com/envoyproxy/go-control-plane/envoy/api/v2/core"
+	v3corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
 	"google.golang.org/grpc/xds/internal/xdsclient/bootstrap"
 	"google.golang.org/grpc/xds/internal/xdsclient/load"
 	"google.golang.org/grpc/xds/internal/xdsclient/pubsub"
@@ -102,7 +104,13 @@ func (c *clientImpl) newAuthorityLocked(config *bootstrap.ServerConfig) (_ *auth
 	}
 
 	// Make a new authority since there's no existing authority for this config.
-	ret := &authority{config: config, pubsub: pubsub.New(c.watchExpiryTimeout, c.config.XDSServer.NodeProto, c.logger)}
+	nodeID := ""
+	if v3, ok := c.config.XDSServer.NodeProto.(*v3corepb.Node); ok {
+		nodeID = v3.GetId()
+	} else if v2, ok := c.config.XDSServer.NodeProto.(*v2corepb.Node); ok {
+		nodeID = v2.GetId()
+	}
+	ret := &authority{config: config, pubsub: pubsub.New(c.watchExpiryTimeout, nodeID, c.logger)}
 	defer func() {
 		if retErr != nil {
 			ret.close()

diff --git a/xds/internal/xdsclient/pubsub/pubsub.go b/xds/internal/xdsclient/pubsub/pubsub.go
@@ -23,11 +23,9 @@
 package pubsub
 
 import (
-	"fmt"
 	"sync"
 	"time"
 
-	"github.com/golang/protobuf/proto"
 	"google.golang.org/grpc/internal/buffer"
 	"google.golang.org/grpc/internal/grpclog"
 	"google.golang.org/grpc/internal/grpcsync"
@@ -43,7 +41,7 @@ type Pubsub struct {
 	done               *grpcsync.Event
 	logger             *grpclog.PrefixLogger
 	watchExpiryTimeout time.Duration
-	nodeIDJSON         string
+	nodeID             string
 
 	updateCh *buffer.Unbounded // chan *watcherInfoWithUpdate
 	// All the following maps are to keep the updates/metadata in a cache.
@@ -65,12 +63,12 @@ type Pubsub struct {
 // New creates a new Pubsub.
 //
 // The passed in nodeID will be attached to all errors sent to the watchers.
-func New(watchExpiryTimeout time.Duration, nodeID proto.Message, logger *grpclog.PrefixLogger) *Pubsub {
+func New(watchExpiryTimeout time.Duration, nodeID string, logger *grpclog.PrefixLogger) *Pubsub {
 	pb := &Pubsub{
 		done:               grpcsync.NewEvent(),
 		logger:             logger,
 		watchExpiryTimeout: watchExpiryTimeout,
-		nodeIDJSON:         fmt.Sprint(nodeID),
+		nodeID:             nodeID,
 
 		updateCh:    buffer.NewUnbounded(),
 		ldsWatchers: make(map[string]map[*watchInfo]bool),

diff --git a/xds/internal/xdsclient/pubsub/watch.go b/xds/internal/xdsclient/pubsub/watch.go
@@ -115,9 +115,9 @@ func (wi *watchInfo) sendErrorLocked(err error) {
 	errMsg := err.Error()
 	errTyp := xdsresource.ErrType(err)
 	if errTyp == xdsresource.ErrorTypeUnknown {
-		err = fmt.Errorf("%v, xDS client nodeID: %s", errMsg, wi.c.nodeIDJSON)
+		err = fmt.Errorf("%v, xDS client nodeID: %s", errMsg, wi.c.nodeID)
 	} else {
-		err = xdsresource.NewErrorf(errTyp, "%v, xDS client nodeID: %s", errMsg, wi.c.nodeIDJSON)
+		err = xdsresource.NewErrorf(errTyp, "%v, xDS client nodeID: %s", errMsg, wi.c.nodeID)
 	}
 
 	wi.c.scheduleCallback(wi, u, err)