[grpc-storage] Use grpc.NewClient

plugin/storage/grpc/config/config.go

@@ -15,7 +15,6 @@
 package config
 
 import (
- "context"
  "fmt"
  "os/exec"
  "time"
@@ -77,7 +76,7 @@ func (c *Configuration) Build(logger *zap.Logger, tracerProvider trace.TracerPro
  if c.PluginBinary != "" {
  return c.buildPlugin(logger, tracerProvider)
  } else {
- return c.buildRemote(logger, tracerProvider)
+ return c.buildRemote(logger, tracerProvider, grpc.NewClient)
  }
 }
 
@@ -93,7 +92,9 @@ func (c *Configuration) Close() error {
  return c.RemoteTLS.Close()
 }
 
-func (c *Configuration) buildRemote(logger *zap.Logger, tracerProvider trace.TracerProvider) (*ClientPluginServices, error) {
+type newClientFn func(target string, opts ...grpc.DialOption) (conn *grpc.ClientConn, err error)
+
+func (c *Configuration) buildRemote(logger *zap.Logger, tracerProvider trace.TracerProvider, newClient newClientFn) (*ClientPluginServices, error) {
  opts := []grpc.DialOption{
  grpc.WithStatsHandler(otelgrpc.NewClientHandler(otelgrpc.WithTracerProvider(tracerProvider))),
  grpc.WithBlock(),
@@ -109,19 +110,15 @@ func (c *Configuration) buildRemote(logger *zap.Logger, tracerProvider trace.Tra
  opts = append(opts, grpc.WithTransportCredentials(insecure.NewCredentials()))
  }
 
- ctx, cancel := context.WithTimeout(context.Background(), c.RemoteConnectTimeout)
- defer cancel()
-
  tenancyMgr := tenancy.NewManager(&c.TenancyOpts)
  if tenancyMgr.Enabled {
  opts = append(opts, grpc.WithUnaryInterceptor(tenancy.NewClientUnaryInterceptor(tenancyMgr)))
  opts = append(opts, grpc.WithStreamInterceptor(tenancy.NewClientStreamInterceptor(tenancyMgr)))
  }
  var err error
- // TODO: Need to replace grpc.DialContext with grpc.NewClient and pass test
- c.remoteConn, err = grpc.DialContext(ctx, c.RemoteServerAddr, opts...)
+ c.remoteConn, err = newClient(c.RemoteServerAddr, opts...)
  if err != nil {
- return nil, fmt.Errorf("error connecting to remote storage: %w", err)
+ return nil, fmt.Errorf("error creating remote storage client: %w", err)
  }
 
  grpcClient := shared.NewGRPCClient(c.remoteConn)

plugin/storage/grpc/config/config_test.go

@@ -0,0 +1,23 @@
+// Copyright (c) 2024 The Jaeger Authors.
+// SPDX-License-Identifier: Apache-2.0
+
+package config
+
+import (
+ "errors"
+ "testing"
+
+ "github.com/stretchr/testify/require"
+ "go.uber.org/zap"
+ "google.golang.org/grpc"
+)
+
+func TestBuildRemoteNewClientError(t *testing.T) {
+ // this is a silly test to verify handling of error from grpc.NewClient, which cannot be induced via params.
+ c := &Configuration{}
+ _, err := c.buildRemote(zap.NewNop(), nil, func(target string, opts ...grpc.DialOption) (conn *grpc.ClientConn, err error) {
+ return nil, errors.New("test error")
+ })
+ require.Error(t, err)
+ require.Contains(t, err.Error(), "error creating remote storage client")
+}

plugin/storage/grpc/factory_test.go

@@ -148,10 +148,6 @@ func TestGRPCStorageFactory(t *testing.T) {
 }
 
 func TestGRPCStorageFactoryWithConfig(t *testing.T) {
- cfg := grpcConfig.Configuration{}
- _, err := NewFactoryWithConfig(cfg, metrics.NullFactory, zap.NewNop())
- require.ErrorContains(t, err, "grpc-plugin builder failed to create a store: error connecting to remote storage")
-
  lis, err := net.Listen("tcp", ":0")
  require.NoError(t, err, "failed to listen")
 
@@ -163,8 +159,10 @@ func TestGRPCStorageFactoryWithConfig(t *testing.T) {
  }()
  defer s.Stop()
 
- cfg.RemoteServerAddr = lis.Addr().String()
- cfg.RemoteConnectTimeout = 1 * time.Second
+ cfg := grpcConfig.Configuration{
+ RemoteServerAddr: lis.Addr().String(),
+ RemoteConnectTimeout: 1 * time.Second,
+ }
  f, err := NewFactoryWithConfig(cfg, metrics.NullFactory, zap.NewNop())
  require.NoError(t, err)
  require.NoError(t, f.Close())