Remove context timeout for subsequent calls

Signed-off-by: Bernardo Tolosa <bernardotolosajr@gmail.com>
jaegertracing · Sep 11, 2020 · f08962a · f08962a
1 parent 6a4730f
commit f08962a
Show file tree

Hide file tree

Showing 2 changed files with 56 additions and 66 deletions.
diff --git a/cmd/agent/app/reporter/grpc/builder.go b/cmd/agent/app/reporter/grpc/builder.go
@@ -19,12 +19,10 @@ import (
 	"errors"
 	"fmt"
 	"strings"
-	"time"
 
 	grpc_retry "github.com/grpc-ecosystem/go-grpc-middleware/retry"
 	"go.uber.org/zap"
 	"google.golang.org/grpc"
-	"google.golang.org/grpc/connectivity"
 	"google.golang.org/grpc/credentials"
 	"google.golang.org/grpc/resolver"
 	"google.golang.org/grpc/resolver/manual"
@@ -101,18 +99,10 @@ func (b *ConnBuilder) CreateConnection(logger *zap.Logger) (*grpc.ClientConn, er
 
 	go func(cc *grpc.ClientConn) {
 		logger.Info("Checking connection to collector")
-		ctx, cancel := context.WithTimeout(context.Background(), time.Second)
-		defer cancel()
 		for {
 			s := cc.GetState()
 			logger.Info("Agent collector connection state change", zap.String("dialTarget", dialTarget), zap.Stringer("status", s))
-			if s == connectivity.Ready {
-				return
-			}
-			if !cc.WaitForStateChange(ctx, s) {
-				logger.Error("Could not get collector connection state", zap.String("error", ctx.Err().Error()))
-				return
-			}
+			cc.WaitForStateChange(context.Background(), s)
 		}
 	}(conn)
 

diff --git a/cmd/agent/app/reporter/grpc/builder_test.go b/cmd/agent/app/reporter/grpc/builder_test.go
@@ -72,72 +72,72 @@ func TestBuilderWithCollectors(t *testing.T) {
 	defer s1.Stop()
 
 	tests := []struct {
-		target              string
-		name                string
-		hostPorts           []string
-		checkSuffixOnly     bool
-		notifier            discovery.Notifier
-		discoverer          discovery.Discoverer
-		expectedError       string
-		checkConnectioState bool
-		expectedState       string
+		target               string
+		name                 string
+		hostPorts            []string
+		checkSuffixOnly      bool
+		notifier             discovery.Notifier
+		discoverer           discovery.Discoverer
+		expectedError        string
+		checkConnectionState bool
+		expectedState        string
 	}{
 		{
-			target:              "///round_robin",
-			name:                "with roundrobin schema",
-			hostPorts:           []string{"127.0.0.1:9876", "127.0.0.1:9877", "127.0.0.1:9878"},
-			checkSuffixOnly:     true,
-			notifier:            nil,
-			discoverer:          nil,
-			checkConnectioState: false,
+			target:               "///round_robin",
+			name:                 "with roundrobin schema",
+			hostPorts:            []string{"127.0.0.1:9876", "127.0.0.1:9877", "127.0.0.1:9878"},
+			checkSuffixOnly:      true,
+			notifier:             nil,
+			discoverer:           nil,
+			checkConnectionState: false,
 		},
 		{
-			target:              "127.0.0.1:9876",
-			name:                "with single host",
-			hostPorts:           []string{"127.0.0.1:9876"},
-			checkSuffixOnly:     false,
-			notifier:            nil,
-			discoverer:          nil,
-			checkConnectioState: false,
+			target:               "127.0.0.1:9876",
+			name:                 "with single host",
+			hostPorts:            []string{"127.0.0.1:9876"},
+			checkSuffixOnly:      false,
+			notifier:             nil,
+			discoverer:           nil,
+			checkConnectionState: false,
 		},
 		{
-			target:              "///round_robin",
-			name:                "with custom resolver and fixed discoverer",
-			hostPorts:           []string{"dns://random_stuff"},
-			checkSuffixOnly:     true,
-			notifier:            noopNotifier{},
-			discoverer:          discovery.FixedDiscoverer{},
-			checkConnectioState: false,
+			target:               "///round_robin",
+			name:                 "with custom resolver and fixed discoverer",
+			hostPorts:            []string{"dns://random_stuff"},
+			checkSuffixOnly:      true,
+			notifier:             noopNotifier{},
+			discoverer:           discovery.FixedDiscoverer{},
+			checkConnectionState: false,
 		},
 		{
-			target:              "",
-			name:                "without collectorPorts and resolver",
-			hostPorts:           nil,
-			checkSuffixOnly:     false,
-			notifier:            nil,
-			discoverer:          nil,
-			expectedError:       "at least one collector hostPort address is required when resolver is not available",
-			checkConnectioState: false,
+			target:               "",
+			name:                 "without collectorPorts and resolver",
+			hostPorts:            nil,
+			checkSuffixOnly:      false,
+			notifier:             nil,
+			discoverer:           nil,
+			expectedError:        "at least one collector hostPort address is required when resolver is not available",
+			checkConnectionState: false,
 		},
 		{
-			target:              addr1.String(),
-			name:                "with collector connection status ready",
-			hostPorts:           []string{addr1.String()},
-			checkSuffixOnly:     false,
-			notifier:            nil,
-			discoverer:          nil,
-			checkConnectioState: true,
-			expectedState:       "READY",
+			target:               addr1.String(),
+			name:                 "with collector connection status ready",
+			hostPorts:            []string{addr1.String()},
+			checkSuffixOnly:      false,
+			notifier:             nil,
+			discoverer:           nil,
+			checkConnectionState: true,
+			expectedState:        "READY",
 		},
 		{
-			target:              "random_stuff",
-			name:                "with collector connection status failure",
-			hostPorts:           []string{"random_stuff"},
-			checkSuffixOnly:     false,
-			notifier:            nil,
-			discoverer:          nil,
-			checkConnectioState: true,
-			expectedState:       "TRANSIENT_FAILURE",
+			target:               "random_stuff",
+			name:                 "with collector connection status failure",
+			hostPorts:            []string{"random_stuff"},
+			checkSuffixOnly:      false,
+			notifier:             nil,
+			discoverer:           nil,
+			checkConnectionState: true,
+			expectedState:        "TRANSIENT_FAILURE",
 		},
 	}
 
@@ -153,7 +153,7 @@ func TestBuilderWithCollectors(t *testing.T) {
 			if test.expectedError == "" {
 				require.NoError(t, err)
 				require.NotNil(t, conn)
-				if test.checkConnectioState {
+				if test.checkConnectionState {
 					assertConnectionState(t, conn, test.expectedState)
 				}
 				if test.checkSuffixOnly {