Replace grpc.Dial function that is going to be deprecated in grpc-go@…

…v1.64 (jaegertracing#5336) ## Which problem is this PR solving? - Related jaegertracing#5330 ## Description of the changes - Partial replace deprecated function in `grpc-go@v1.64` - `grpc.Dial` => `grpc.NewClient` - `grpc.DialContext` => `grpc.NewClient` - FYI: jaegertracing#5330 (comment) - Refactor configmanager GetSamplingStrategy business logic - Fixed related test ## How was this change tested? - make lint - make test - Try & Error ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `yarn lint` and `yarn test` --------- Signed-off-by: tico88612 <17496418+tico88612@users.noreply.github.com>
akagami-harsh · Apr 9, 2024 · aa9cefc · aa9cefc
1 parent 7be2e34
commit aa9cefc
Show file tree

Hide file tree

Showing 16 changed files with 27 additions and 25 deletions.
diff --git a/cmd/agent/app/configmanager/grpc/manager.go b/cmd/agent/app/configmanager/grpc/manager.go
@@ -17,6 +17,7 @@ package grpc
 import (
 	"context"
 	"errors"
+	"fmt"
 
 	"google.golang.org/grpc"
 
@@ -38,7 +39,11 @@ func NewConfigManager(conn *grpc.ClientConn) *ConfigManagerProxy {
 
 // GetSamplingStrategy returns sampling strategies from collector.
 func (s *ConfigManagerProxy) GetSamplingStrategy(ctx context.Context, serviceName string) (*api_v2.SamplingStrategyResponse, error) {
-	return s.client.GetSamplingStrategy(ctx, &api_v2.SamplingStrategyParameters{ServiceName: serviceName})
+	resp, err := s.client.GetSamplingStrategy(ctx, &api_v2.SamplingStrategyParameters{ServiceName: serviceName})
+	if err != nil {
+		return nil, fmt.Errorf("failed to get sampling strategy: %w", err)
+	}
+	return resp, nil
 }
 
 // GetBaggageRestrictions returns baggage restrictions from collector.

diff --git a/cmd/agent/app/configmanager/grpc/manager_test.go b/cmd/agent/app/configmanager/grpc/manager_test.go
@@ -37,7 +37,7 @@ func TestSamplingManager_GetSamplingStrategy(t *testing.T) {
 	s, addr := initializeGRPCTestServer(t, func(s *grpc.Server) {
 		api_v2.RegisterSamplingManagerServer(s, &mockSamplingHandler{})
 	})
-	conn, err := grpc.Dial(addr.String(), grpc.WithTransportCredentials(insecure.NewCredentials()))
+	conn, err := grpc.NewClient(addr.String(), grpc.WithTransportCredentials(insecure.NewCredentials()))
 	defer close(t, conn)
 	require.NoError(t, err)
 	defer s.GracefulStop()
@@ -48,14 +48,14 @@ func TestSamplingManager_GetSamplingStrategy(t *testing.T) {
 }
 
 func TestSamplingManager_GetSamplingStrategy_error(t *testing.T) {
-	conn, err := grpc.Dial("foo", grpc.WithTransportCredentials(insecure.NewCredentials()))
+	conn, err := grpc.NewClient("foo", grpc.WithTransportCredentials(insecure.NewCredentials()))
 	defer close(t, conn)
 	require.NoError(t, err)
 	manager := NewConfigManager(conn)
 	resp, err := manager.GetSamplingStrategy(context.Background(), "any")
 	require.Nil(t, resp)
 	require.Error(t, err)
-	assert.Contains(t, err.Error(), "Error while dialing: dial tcp: address foo: missing port in address")
+	assert.Contains(t, err.Error(), "failed to get sampling strategy")
 }
 
 func TestSamplingManager_GetBaggageRestrictions(t *testing.T) {

diff --git a/cmd/agent/app/processors/thrift_processor_test.go b/cmd/agent/app/processors/thrift_processor_test.go
@@ -81,7 +81,7 @@ func createProcessor(t *testing.T, mFactory metrics.Factory, tFactory thrift.TPr
 
 func initCollectorAndReporter(t *testing.T) (*metricstest.Factory, *testutils.GrpcCollector, reporter.Reporter, *grpc.ClientConn) {
 	grpcCollector := testutils.StartGRPCCollector(t)
-	conn, err := grpc.Dial(grpcCollector.Listener().Addr().String(), grpc.WithTransportCredentials(insecure.NewCredentials()))
+	conn, err := grpc.NewClient(grpcCollector.Listener().Addr().String(), grpc.WithTransportCredentials(insecure.NewCredentials()))
 	require.NoError(t, err)
 	rep := grpcrep.NewReporter(conn, map[string]string{}, zaptest.NewLogger(t))
 	metricsFactory := metricstest.NewFactory(0)

diff --git a/cmd/agent/app/reporter/grpc/builder.go b/cmd/agent/app/reporter/grpc/builder.go
@@ -102,6 +102,7 @@ func (b *ConnBuilder) CreateConnection(ctx context.Context, logger *zap.Logger,
 	dialOptions = append(dialOptions, grpc.WithUnaryInterceptor(grpc_retry.UnaryClientInterceptor(grpc_retry.WithMax(b.MaxRetry))))
 	dialOptions = append(dialOptions, b.AdditionalDialOptions...)
 
+	// TODO: Need to replace grpc.Dial with grpc.NewClient and pass test
 	conn, err := grpc.Dial(dialTarget, dialOptions...)
 	if err != nil {
 		return nil, err

diff --git a/cmd/agent/app/reporter/grpc/reporter_test.go b/cmd/agent/app/reporter/grpc/reporter_test.go
@@ -59,7 +59,7 @@ func TestReporter_EmitZipkinBatch(t *testing.T) {
 		api_v2.RegisterCollectorServiceServer(s, handler)
 	})
 	defer s.Stop()
-	conn, err := grpc.Dial(addr.String(), grpc.WithTransportCredentials(insecure.NewCredentials()))
+	conn, err := grpc.NewClient(addr.String(), grpc.WithTransportCredentials(insecure.NewCredentials()))
 	//nolint:staticcheck // don't care about errors
 	require.NoError(t, err)
 	defer conn.Close()
@@ -102,7 +102,7 @@ func TestReporter_EmitBatch(t *testing.T) {
 		api_v2.RegisterCollectorServiceServer(s, handler)
 	})
 	defer s.Stop()
-	conn, err := grpc.Dial(addr.String(), grpc.WithTransportCredentials(insecure.NewCredentials()))
+	conn, err := grpc.NewClient(addr.String(), grpc.WithTransportCredentials(insecure.NewCredentials()))
 	//nolint:staticcheck // don't care about errors
 	require.NoError(t, err)
 	defer conn.Close()
@@ -131,7 +131,7 @@ func TestReporter_EmitBatch(t *testing.T) {
 }
 
 func TestReporter_SendFailure(t *testing.T) {
-	conn, err := grpc.Dial("invalid-host-name-blah:12345", grpc.WithTransportCredentials(insecure.NewCredentials()))
+	conn, err := grpc.NewClient("invalid-host-name-blah:12345", grpc.WithTransportCredentials(insecure.NewCredentials()))
 	require.NoError(t, err)
 	defer conn.Close()
 	rep := NewReporter(conn, nil, zap.NewNop())
@@ -207,7 +207,7 @@ func TestReporter_MultitenantEmitBatch(t *testing.T) {
 		api_v2.RegisterCollectorServiceServer(s, handler)
 	})
 	defer s.Stop()
-	conn, err := grpc.Dial(addr.String(), grpc.WithTransportCredentials(insecure.NewCredentials()))
+	conn, err := grpc.NewClient(addr.String(), grpc.WithTransportCredentials(insecure.NewCredentials()))
 	require.NoError(t, err)
 	defer func() { require.NoError(t, conn.Close()) }()
 	rep := NewReporter(conn, nil, zap.NewNop())

diff --git a/cmd/anonymizer/app/query/query.go b/cmd/anonymizer/app/query/query.go
@@ -19,7 +19,6 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"time"
 
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/credentials/insecure"
@@ -38,10 +37,7 @@ type Query struct {
 
 // New creates a Query object
 func New(addr string) (*Query, error) {
-	ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second)
-	defer cancel()
-
-	conn, err := grpc.DialContext(ctx, addr, grpc.WithTransportCredentials(insecure.NewCredentials()))
+	conn, err := grpc.NewClient(addr, grpc.WithTransportCredentials(insecure.NewCredentials()))
 	if err != nil {
 		return nil, fmt.Errorf("failed to connect with the jaeger-query service: %w", err)
 	}

diff --git a/cmd/collector/app/handler/grpc_handler_test.go b/cmd/collector/app/handler/grpc_handler_test.go
@@ -108,7 +108,7 @@ func initializeGRPCTestServer(t *testing.T, beforeServe func(s *grpc.Server)) (*
 }
 
 func newClient(t *testing.T, addr net.Addr) (api_v2.CollectorServiceClient, *grpc.ClientConn) {
-	conn, err := grpc.Dial(addr.String(), grpc.WithTransportCredentials(insecure.NewCredentials()))
+	conn, err := grpc.NewClient(addr.String(), grpc.WithTransportCredentials(insecure.NewCredentials()))
 	require.NoError(t, err)
 	return api_v2.NewCollectorServiceClient(conn), conn
 }

diff --git a/cmd/collector/app/server/grpc_test.go b/cmd/collector/app/server/grpc_test.go
@@ -84,7 +84,7 @@ func TestSpanCollector(t *testing.T) {
 	require.NoError(t, err)
 	defer server.Stop()
 
-	conn, err := grpc.Dial(
+	conn, err := grpc.NewClient(
 		params.HostPortActual,
 		grpc.WithTransportCredentials(insecure.NewCredentials()))
 	require.NoError(t, err)

diff --git a/cmd/query/app/apiv3/grpc_handler_test.go b/cmd/query/app/apiv3/grpc_handler_test.go
@@ -77,8 +77,7 @@ func newTestServerClient(t *testing.T) *testServerClient {
 	}
 	tsc.server, tsc.address = newGrpcServer(t, h)
 
-	conn, err := grpc.DialContext(
-		context.Background(),
+	conn, err := grpc.NewClient(
 		tsc.address.String(),
 		grpc.WithTransportCredentials(insecure.NewCredentials()),
 	)

diff --git a/cmd/query/app/grpc_handler_test.go b/cmd/query/app/grpc_handler_test.go
@@ -176,6 +176,7 @@ func newGRPCServer(t *testing.T, q *querysvc.QueryService, mq querysvc.MetricsQu
 func newGRPCClient(t *testing.T, addr string) *grpcClient {
 	ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second)
 	defer cancel()
+	// TODO: Need to replace grpc.DialContext with grpc.NewClient and pass test
 	conn, err := grpc.DialContext(ctx, addr, grpc.WithTransportCredentials(insecure.NewCredentials()))
 	require.NoError(t, err)
 

diff --git a/cmd/query/app/server_test.go b/cmd/query/app/server_test.go
@@ -421,8 +421,10 @@ func newGRPCClientWithTLS(t *testing.T, addr string, creds credentials.Transport
 	var err error
 
 	if creds != nil {
+		// TODO: Need to replace grpc.DialContext with grpc.NewClient and pass test
 		conn, err = grpc.DialContext(ctx, addr, grpc.WithTransportCredentials(creds))
 	} else {
+		// TODO: Need to replace grpc.DialContext with grpc.NewClient and pass test
 		conn, err = grpc.DialContext(ctx, addr, grpc.WithTransportCredentials(insecure.NewCredentials()))
 	}
 

diff --git a/cmd/remote-storage/app/server_test.go b/cmd/remote-storage/app/server_test.go
@@ -294,9 +294,6 @@ type grpcClient struct {
 }
 
 func newGRPCClient(t *testing.T, addr string, creds credentials.TransportCredentials, tm *tenancy.Manager) *grpcClient {
-	ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second)
-	defer cancel()
-
 	dialOpts := []grpc.DialOption{
 		grpc.WithUnaryInterceptor(tenancy.NewClientUnaryInterceptor(tm)),
 	}
@@ -306,7 +303,7 @@ func newGRPCClient(t *testing.T, addr string, creds credentials.TransportCredent
 		dialOpts = append(dialOpts, grpc.WithTransportCredentials(insecure.NewCredentials()))
 	}
 
-	conn, err := grpc.DialContext(ctx, addr, dialOpts...)
+	conn, err := grpc.NewClient(addr, dialOpts...)
 	require.NoError(t, err)
 
 	return &grpcClient{

diff --git a/examples/hotrod/services/driver/client.go b/examples/hotrod/services/driver/client.go
@@ -36,7 +36,7 @@ type Client struct {
 
 // NewClient creates a new driver.Client
 func NewClient(tracerProvider trace.TracerProvider, logger log.Factory, hostPort string) *Client {
-	conn, err := grpc.Dial(hostPort,
+	conn, err := grpc.NewClient(hostPort,
 		grpc.WithTransportCredentials(insecure.NewCredentials()),
 		grpc.WithStatsHandler(otelgrpc.NewClientHandler(otelgrpc.WithTracerProvider(tracerProvider))),
 	)

diff --git a/internal/grpctest/reflection.go b/internal/grpctest/reflection.go
@@ -34,7 +34,7 @@ type ReflectionServiceValidator struct {
 
 // Execute performs validation.
 func (v ReflectionServiceValidator) Execute(t *testing.T) {
-	conn, err := grpc.Dial(
+	conn, err := grpc.NewClient(
 		v.HostPort,
 		grpc.WithTransportCredentials(insecure.NewCredentials()))
 	require.NoError(t, err)

diff --git a/pkg/discovery/grpcresolver/grpc_resolver_test.go b/pkg/discovery/grpcresolver/grpc_resolver_test.go
@@ -148,7 +148,7 @@ func TestGRPCResolverRoundRobin(t *testing.T) {
 		t.Run(fmt.Sprintf("%+v", test), func(t *testing.T) {
 			res := New(notifier, discoverer, zap.NewNop(), test.minPeers)
 
-			cc, err := grpc.Dial(res.Scheme()+":///round_robin", grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithDefaultServiceConfig(GRPCServiceConfig))
+			cc, err := grpc.NewClient(res.Scheme()+":///round_robin", grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithDefaultServiceConfig(GRPCServiceConfig))
 			require.NoError(t, err, "could not dial using resolver's scheme")
 			defer cc.Close()
 

diff --git a/plugin/storage/grpc/config/config.go b/plugin/storage/grpc/config/config.go
@@ -111,6 +111,7 @@ func (c *Configuration) buildRemote(logger *zap.Logger, tracerProvider trace.Tra
 		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...)
 	if err != nil {
 		return nil, fmt.Errorf("error connecting to remote storage: %w", err)