merge from master

Author: dfawley

.github/workflows/codeql-analysis.yml

@@ -29,9 +29,9 @@ jobs:
 
     # Initializes the CodeQL tools for scanning.
     - name: Initialize CodeQL
-      uses: github/codeql-action/init@2f93e4319b2f04a2efc38fa7f78bd681bc3f7b2f # v2.23.2
+      uses: github/codeql-action/init@928ff8c822d966a999092a6a35e32177899afb7c # v2.24.6
       with:
         languages: go
 
     - name: Perform CodeQL Analysis
-      uses: github/codeql-action/analyze@2f93e4319b2f04a2efc38fa7f78bd681bc3f7b2f # v2.23.2
+      uses: github/codeql-action/analyze@928ff8c822d966a999092a6a35e32177899afb7c # v2.24.6

.github/workflows/testing.yml

@@ -26,7 +26,7 @@ jobs:
       - name: Setup Go
         uses: actions/setup-go@0c52d547c9bc32b1aa3301fd7a9cb496313a4491 # v5.0.0
         with:
-          go-version: '1.21'
+          go-version: '1.22'
       - name: Checkout repo
         uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
 
@@ -46,32 +46,31 @@ jobs:
       matrix:
         include:
           - type: vet
-            goversion: '1.21'
+            goversion: '1.22'
+
+          - type: extras
+            goversion: '1.22'
 
           - type: tests
-            goversion: '1.21'
+            goversion: '1.22'
 
           - type: tests
-            goversion: '1.21'
+            goversion: '1.22'
             testflags: -race
 
           - type: tests
-            goversion: '1.21'
+            goversion: '1.22'
             goarch: 386
 
           - type: tests
-            goversion: '1.21'
+            goversion: '1.22'
             goarch: arm64
 
           - type: tests
-            goversion: '1.20'
-
-          - type: tests
-            goversion: '1.19'
-
-          - type: extras
             goversion: '1.21'
 
+          - type: tests
+            goversion: '1.20'
     steps:
       # Setup the environment.
       - name: Setup GOARCH

balancer/leastrequest/balancer_test.go

@@ -170,7 +170,7 @@ func checkRoundRobinRPCs(ctx context.Context, client testgrpc.TestServiceClient,
 			}
 			iterations = append(iterations, iteration)
 		}
-		// Ensure the the first iteration contains all addresses in addrs.
+		// Ensure the first iteration contains all addresses in addrs.
 		for _, addr := range iterations[0] {
 			gotAddrCount[addr]++
 		}

balancer/weightedroundrobin/scheduler.go

@@ -125,7 +125,7 @@ func (s *edfScheduler) nextIndex() int {
 }
 
 // A simple RR scheduler to use for fallback when fewer than two backends have
-// non-zero weights, or all backends have the the same weight, or when only one
+// non-zero weights, or all backends have the same weight, or when only one
 // subconn exists.
 type rrScheduler struct {
 	inc    func() uint32

clientconn.go

@@ -67,7 +67,7 @@ var (
 	errConnDrain = errors.New("grpc: the connection is drained")
 	// errConnClosing indicates that the connection is closing.
 	errConnClosing = errors.New("grpc: the connection is closing")
-	// errConnIdling indicates the the connection is being closed as the channel
+	// errConnIdling indicates the connection is being closed as the channel
 	// is moving to an idle mode due to inactivity.
 	errConnIdling = errors.New("grpc: the connection is closing due to channel idleness")
 	// invalidDefaultServiceConfigErrPrefix is used to prefix the json parsing error for the default
@@ -101,11 +101,6 @@ const (
 	defaultReadBufSize  = 32 * 1024
 )
 
-// Dial creates a client connection to the given target.
-func Dial(target string, opts ...DialOption) (*ClientConn, error) {
-	return DialContext(context.Background(), target, opts...)
-}
-
 type defaultConfigSelector struct {
 	sc *ServiceConfig
 }
@@ -117,8 +112,18 @@ func (dcs *defaultConfigSelector) SelectConfig(rpcInfo iresolver.RPCInfo) (*ires
 	}, nil
 }
 
-// newClient returns a new client in idle mode.
-func newClient(target string, opts ...DialOption) (conn *ClientConn, err error) {
+// NewClient creates a new gRPC "channel" for the target URI provided.  No I/O
+// is performed.  Use of the ClientConn for RPCs will automatically cause it to
+// connect.  Connect may be used to manually create a connection, but for most
+// users this is unnecessary.
+//
+// The target name syntax is defined in
+// https://github.com/grpc/grpc/blob/master/doc/naming.md.  e.g. to use dns
+// resolver, a "dns:///" prefix should be applied to the target.
+//
+// The DialOptions returned by WithBlock, WithTimeout, and
+// WithReturnConnectionError are ignored by this function.
+func NewClient(target string, opts ...DialOption) (conn *ClientConn, err error) {
 	cc := &ClientConn{
 		target: target,
 		conns:  make(map[*addrConn]struct{}),
@@ -190,39 +195,36 @@ func newClient(target string, opts ...DialOption) (conn *ClientConn, err error)
 	return cc, nil
 }
 
-// DialContext creates a client connection to the given target. By default, it's
-// a non-blocking dial (the function won't wait for connections to be
-// established, and connecting happens in the background). To make it a blocking
-// dial, use WithBlock() dial option.
+// Dial calls DialContext(context.Background(), target, opts...).
 //
-// In the non-blocking case, the ctx does not act against the connection. It
-// only controls the setup steps.
+// Deprecated: use NewClient instead.  Will be supported throughout 1.x.
+func Dial(target string, opts ...DialOption) (*ClientConn, error) {
+	return DialContext(context.Background(), target, opts...)
+}
+
+// DialContext calls NewClient and then exits idle mode.  If WithBlock(true) is
+// used, it calls Connect and WaitForStateChange until either the context
+// expires or the state of the ClientConn is Ready.
 //
-// In the blocking case, ctx can be used to cancel or expire the pending
-// connection. Once this function returns, the cancellation and expiration of
-// ctx will be noop. Users should call ClientConn.Close to terminate all the
-// pending operations after this function returns.
+// One subtle difference between NewClient and Dial and DialContext is that the
+// former uses "dns" as the default name resolver, while the latter use
+// "passthrough" for backward compatibility.  This distinction should not matter
+// to most users, but could matter to legacy users that specify a custom dialer
+// and expect it to receive the target string directly.
 //
-// The target name syntax is defined in
-// https://github.com/grpc/grpc/blob/master/doc/naming.md.
-// e.g. to use dns resolver, a "dns:///" prefix should be applied to the target.
+// Deprecated: use NewClient instead.  Will be supported throughout 1.x.
 func DialContext(ctx context.Context, target string, opts ...DialOption) (conn *ClientConn, err error) {
-	cc, err := newClient(target, opts...)
+	// At the end of this method, we kick the channel out of idle, rather than
+	// waiting for the first rpc.
+	opts = append([]DialOption{withDefaultScheme("passthrough")}, opts...)
+	cc, err := NewClient(target, opts...)
 	if err != nil {
 		return nil, err
 	}
 
 	// We start the channel off in idle mode, but kick it out of idle now,
-	// instead of waiting for the first RPC. Other gRPC implementations do wait
-	// for the first RPC to kick the channel out of idle. But doing so would be
-	// a major behavior change for our users who are used to seeing the channel
-	// active after Dial.
-	//
-	// Taking this approach of kicking it out of idle at the end of this method
-	// allows us to share the code between channel creation and exiting idle
-	// mode. This will also make it easy for us to switch to starting the
-	// channel off in idle, i.e. by making newClient exported.
-
+	// instead of waiting for the first RPC.  This is the legacy behavior of
+	// Dial.
 	defer func() {
 		if err != nil {
 			cc.Close()
@@ -706,15 +708,15 @@ func init() {
 	}
 }
 
-func (cc *ClientConn) maybeApplyDefaultServiceConfig(addrs []resolver.Address) {
+func (cc *ClientConn) maybeApplyDefaultServiceConfig() {
 	if cc.sc != nil {
-		cc.applyServiceConfigAndBalancer(cc.sc, nil, addrs)
+		cc.applyServiceConfigAndBalancer(cc.sc, nil)
 		return
 	}
 	if cc.dopts.defaultServiceConfig != nil {
-		cc.applyServiceConfigAndBalancer(cc.dopts.defaultServiceConfig, &defaultConfigSelector{cc.dopts.defaultServiceConfig}, addrs)
+		cc.applyServiceConfigAndBalancer(cc.dopts.defaultServiceConfig, &defaultConfigSelector{cc.dopts.defaultServiceConfig})
 	} else {
-		cc.applyServiceConfigAndBalancer(emptyServiceConfig, &defaultConfigSelector{emptyServiceConfig}, addrs)
+		cc.applyServiceConfigAndBalancer(emptyServiceConfig, &defaultConfigSelector{emptyServiceConfig})
 	}
 }
 
@@ -732,7 +734,7 @@ func (cc *ClientConn) updateResolverStateAndUnlock(s resolver.State, err error)
 		// May need to apply the initial service config in case the resolver
 		// doesn't support service configs, or doesn't provide a service config
 		// with the new addresses.
-		cc.maybeApplyDefaultServiceConfig(nil)
+		cc.maybeApplyDefaultServiceConfig()
 
 		cc.balancerWrapper.resolverError(err)
 
@@ -744,9 +746,9 @@ func (cc *ClientConn) updateResolverStateAndUnlock(s resolver.State, err error)
 	var ret error
 	if cc.dopts.disableServiceConfig {
 		channelz.Infof(logger, cc.channelz, "ignoring service config from resolver (%v) and applying the default because service config is disabled", s.ServiceConfig)
-		cc.maybeApplyDefaultServiceConfig(s.Addresses)
+		cc.maybeApplyDefaultServiceConfig()
 	} else if s.ServiceConfig == nil {
-		cc.maybeApplyDefaultServiceConfig(s.Addresses)
+		cc.maybeApplyDefaultServiceConfig()
 		// TODO: do we need to apply a failing LB policy if there is no
 		// default, per the error handling design?
 	} else {
@@ -759,7 +761,7 @@ func (cc *ClientConn) updateResolverStateAndUnlock(s resolver.State, err error)
 			} else {
 				configSelector = &defaultConfigSelector{sc}
 			}
-			cc.applyServiceConfigAndBalancer(sc, configSelector, s.Addresses)
+			cc.applyServiceConfigAndBalancer(sc, configSelector)
 		} else {
 			ret = balancer.ErrBadResolverState
 			if cc.sc == nil {
@@ -867,15 +869,15 @@ func (cc *ClientConn) removeAddrConn(ac *addrConn, err error) {
 }
 
 // Target returns the target string of the ClientConn.
-//
-// # Experimental
-//
-// Notice: This API is EXPERIMENTAL and may be changed or removed in a
-// later release.
 func (cc *ClientConn) Target() string {
 	return cc.target
 }
 
+// CanonicalTarget returns the canonical target string of the ClientConn.
+func (cc *ClientConn) CanonicalTarget() string {
+	return cc.parsedTarget.String()
+}
+
 func (cc *ClientConn) incrCallsStarted() {
 	cc.channelz.ChannelMetrics.CallsStarted.Add(1)
 	cc.channelz.ChannelMetrics.LastCallStartedTimestamp.Store(time.Now().UnixNano())
@@ -1050,7 +1052,7 @@ func (cc *ClientConn) getTransport(ctx context.Context, failfast bool, method st
 	})
 }
 
-func (cc *ClientConn) applyServiceConfigAndBalancer(sc *ServiceConfig, configSelector iresolver.ConfigSelector, addrs []resolver.Address) {
+func (cc *ClientConn) applyServiceConfigAndBalancer(sc *ServiceConfig, configSelector iresolver.ConfigSelector) {
 	if sc == nil {
 		// should never reach here.
 		return
@@ -1694,8 +1696,13 @@ func (cc *ClientConn) parseTargetAndFindResolver() error {
 	// We are here because the user's dial target did not contain a scheme or
 	// specified an unregistered scheme. We should fallback to the default
 	// scheme, except when a custom dialer is specified in which case, we should
-	// always use passthrough scheme.
-	defScheme := resolver.GetDefaultScheme()
+	// always use passthrough scheme. For either case, we need to respect any overridden
+	// global defaults set by the user.
+	defScheme := cc.dopts.defaultScheme
+	if internal.UserSetDefaultScheme {
+		defScheme = resolver.GetDefaultScheme()
+	}
+
 	channelz.Infof(logger, cc.channelz, "fallback to scheme %q", defScheme)
 	canonicalTarget := defScheme + ":///" + cc.target
 
@@ -1726,6 +1733,8 @@ func parseTarget(target string) (resolver.Target, error) {
 	return resolver.Target{URL: *u}, nil
 }
 
+// encodeAuthority escapes the authority string based on valid chars defined in
+// https://datatracker.ietf.org/doc/html/rfc3986#section-3.2.
 func encodeAuthority(authority string) string {
 	const upperhex = "0123456789ABCDEF"