[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())