Remove explicitly setting agent's reporter type (#1168)

Tchannel reporter has been removed in 1.18.0 The GRPC reporter was set as default in 1.11.0 Signed-off-by: Pavol Loffay <[email protected]>
jaegertracing · Aug 25, 2020 · 78ecb22 · 78ecb22
1 parent b2fcc57
commit 78ecb22
Show file tree

Hide file tree

Showing 6 changed files with 19 additions and 37 deletions.
diff --git a/deploy/examples/statefulset-manual-sidecar.yaml b/deploy/examples/statefulset-manual-sidecar.yaml
@@ -43,4 +43,3 @@ spec:
               protocol: TCP
           args:
             - --reporter.grpc.host-port=dns:///jaeger-collector-headless.example-ns:14250
-            - --reporter.type=grpc
diff --git a/deploy/examples/tracegen.yaml b/deploy/examples/tracegen.yaml
@@ -28,7 +28,6 @@ spec:
         image: jaegertracing/jaeger-agent:1.17
         args:
         - --reporter.grpc.host-port=dns:///simple-prod-collector-headless.default:14250
-        - --reporter.type=grpc
         env:
         - name: POD_NAME
           valueFrom:

diff --git a/pkg/deployment/agent.go b/pkg/deployment/agent.go
@@ -40,18 +40,14 @@ func (a *Agent) Get() *appsv1.DaemonSet {
 
 	args := append(a.jaeger.Spec.Agent.Options.ToArgs())
 
-	if len(util.FindItem("--reporter.type=", args)) == 0 {
-		args = append(args, "--reporter.type=grpc")
-
-		// we only add the grpc host if we are adding the reporter type and there's no explicit value yet
-		if len(util.FindItem("--reporter.grpc.host-port=", args)) == 0 {
-			args = append(args, fmt.Sprintf("--reporter.grpc.host-port=dns:///%s.%s:14250", service.GetNameForHeadlessCollectorService(a.jaeger), a.jaeger.Namespace))
-		}
+	// we only add the grpc host if we are adding the reporter type and there's no explicit value yet
+	if len(util.FindItem("--reporter.grpc.host-port=", args)) == 0 {
+		args = append(args, fmt.Sprintf("--reporter.grpc.host-port=dns:///%s.%s:14250", service.GetNameForHeadlessCollectorService(a.jaeger), a.jaeger.Namespace))
 	}
 
 	// Enable tls by default for openshift platform
 	if viper.GetString("platform") == v1.FlagPlatformOpenShift {
-		if len(util.FindItem("--reporter.type=grpc", args)) > 0 && len(util.FindItem("--reporter.grpc.tls=true", args)) == 0 {
+		if len(util.FindItem("--reporter.grpc.tls=true", args)) == 0 {
 			args = append(args, "--reporter.grpc.tls.enabled=true")
 			args = append(args, fmt.Sprintf("--reporter.grpc.tls.ca=%s", ca.ServiceCAPath))
 			args = append(args, fmt.Sprintf("--reporter.grpc.tls.server-name=%s.%s.svc.cluster.local", service.GetNameForHeadlessCollectorService(a.jaeger), a.jaeger.Namespace))

diff --git a/pkg/deployment/agent_test.go b/pkg/deployment/agent_test.go
@@ -175,29 +175,26 @@ func TestAgentOrderOfArguments(t *testing.T) {
 	dep := a.Get()
 
 	assert.Len(t, dep.Spec.Template.Spec.Containers, 1)
-	assert.Len(t, dep.Spec.Template.Spec.Containers[0].Args, 5)
+	assert.Len(t, dep.Spec.Template.Spec.Containers[0].Args, 4)
 	assert.True(t, strings.HasPrefix(dep.Spec.Template.Spec.Containers[0].Args[0], "--a-option"))
 	assert.True(t, strings.HasPrefix(dep.Spec.Template.Spec.Containers[0].Args[1], "--b-option"))
 	assert.True(t, strings.HasPrefix(dep.Spec.Template.Spec.Containers[0].Args[2], "--c-option"))
 
 	// the following are added automatically
 	assert.True(t, strings.HasPrefix(dep.Spec.Template.Spec.Containers[0].Args[3], "--reporter.grpc.host-port"))
-	assert.True(t, strings.HasPrefix(dep.Spec.Template.Spec.Containers[0].Args[4], "--reporter.type"))
 }
 
-func TestAgentOverrideReporterType(t *testing.T) {
+func TestAgentCustomReporterPort(t *testing.T) {
 	jaeger := v1.NewJaeger(types.NamespacedName{Name: "my-instance"})
 	jaeger.Spec.Agent.Strategy = "daemonset"
 	jaeger.Spec.Agent.Options = v1.NewOptions(map[string]interface{}{
-		"reporter.type":             "thrift",
-		"reporter.thrift.host-port": "collector:14267",
+		"reporter.grpc.host-port": "collector:5000",
 	})
 
 	a := NewAgent(jaeger)
 	dep := a.Get()
 
-	assert.Equal(t, "--reporter.thrift.host-port=collector:14267", dep.Spec.Template.Spec.Containers[0].Args[0])
-	assert.Equal(t, "--reporter.type=thrift", dep.Spec.Template.Spec.Containers[0].Args[1])
+	assert.Equal(t, "--reporter.grpc.host-port=collector:5000", dep.Spec.Template.Spec.Containers[0].Args[0])
 }
 
 func TestAgentArgumentsOpenshiftTLS(t *testing.T) {
@@ -217,11 +214,10 @@ func TestAgentArgumentsOpenshiftTLS(t *testing.T) {
 	dep := a.Get()
 
 	assert.Len(t, dep.Spec.Template.Spec.Containers, 1)
-	assert.Len(t, dep.Spec.Template.Spec.Containers[0].Args, 6)
+	assert.Len(t, dep.Spec.Template.Spec.Containers[0].Args, 5)
 	assert.Greater(t, len(util.FindItem("--a-option=a-value", dep.Spec.Template.Spec.Containers[0].Args)), 0)
 
 	// the following are added automatically
-	assert.Greater(t, len(util.FindItem("--reporter.type=grpc", dep.Spec.Template.Spec.Containers[0].Args)), 0)
 	assert.Greater(t, len(util.FindItem("--reporter.grpc.host-port=dns:///my-instance-collector-headless.test:14250", dep.Spec.Template.Spec.Containers[0].Args)), 0)
 	assert.Greater(t, len(util.FindItem("--reporter.grpc.tls.enabled=true", dep.Spec.Template.Spec.Containers[0].Args)), 0)
 	assert.Greater(t, len(util.FindItem("--reporter.grpc.tls.ca="+ca.ServiceCAPath, dep.Spec.Template.Spec.Containers[0].Args)), 0)

diff --git a/pkg/inject/sidecar.go b/pkg/inject/sidecar.go
@@ -167,18 +167,14 @@ func getJaeger(name string, jaegers *v1.JaegerList) *v1.Jaeger {
 func container(jaeger *v1.Jaeger, dep *appsv1.Deployment) corev1.Container {
 	args := append(jaeger.Spec.Agent.Options.ToArgs())
 
-	if len(util.FindItem("--reporter.type=", args)) == 0 {
-		args = append(args, "--reporter.type=grpc")
-
-		// we only add the grpc host if we are adding the reporter type and there's no explicit value yet
-		if len(util.FindItem("--reporter.grpc.host-port=", args)) == 0 {
-			args = append(args, fmt.Sprintf("--reporter.grpc.host-port=dns:///%s.%s.svc:14250", service.GetNameForHeadlessCollectorService(jaeger), jaeger.Namespace))
-		}
+	// we only add the grpc host if we are adding the reporter type and there's no explicit value yet
+	if len(util.FindItem("--reporter.grpc.host-port=", args)) == 0 {
+		args = append(args, fmt.Sprintf("--reporter.grpc.host-port=dns:///%s.%s.svc:14250", service.GetNameForHeadlessCollectorService(jaeger), jaeger.Namespace))
 	}
 
 	// Enable tls by default for openshift platform
 	if viper.GetString("platform") == v1.FlagPlatformOpenShift {
-		if len(util.FindItem("--reporter.type=grpc", args)) > 0 && len(util.FindItem("--reporter.grpc.tls.enabled=true", args)) == 0 {
+		if len(util.FindItem("--reporter.grpc.tls.enabled=true", args)) == 0 {
 			args = append(args, "--reporter.grpc.tls.enabled=true")
 			args = append(args, fmt.Sprintf("--reporter.grpc.tls.ca=%s", ca.ServiceCAPath))
 		}

diff --git a/pkg/inject/sidecar_test.go b/pkg/inject/sidecar_test.go
@@ -502,13 +502,12 @@ func TestSidecarOrderOfArguments(t *testing.T) {
 	dep = Sidecar(jaeger, dep)
 
 	assert.Len(t, dep.Spec.Template.Spec.Containers, 2)
-	assert.Len(t, dep.Spec.Template.Spec.Containers[1].Args, 6)
+	assert.Len(t, dep.Spec.Template.Spec.Containers[1].Args, 5)
 	containsOptionWithPrefix(t, dep.Spec.Template.Spec.Containers[1].Args, "--a-option")
 	containsOptionWithPrefix(t, dep.Spec.Template.Spec.Containers[1].Args, "--b-option")
 	containsOptionWithPrefix(t, dep.Spec.Template.Spec.Containers[1].Args, "--c-option")
 	containsOptionWithPrefix(t, dep.Spec.Template.Spec.Containers[1].Args, "--jaeger.tags")
 	containsOptionWithPrefix(t, dep.Spec.Template.Spec.Containers[1].Args, "--reporter.grpc.host-port")
-	containsOptionWithPrefix(t, dep.Spec.Template.Spec.Containers[1].Args, "--reporter.type")
 	agentTags := agentTags(dep.Spec.Template.Spec.Containers[1].Args)
 	assert.Contains(t, agentTags, "container.name=only_container")
 }
@@ -528,20 +527,18 @@ func TestSidecarExplicitTags(t *testing.T) {
 	assert.Equal(t, []string{"key=val"}, agentTags)
 }
 
-func TestSidecarOverrideReporter(t *testing.T) {
+func TestSidecarCustomReporterPort(t *testing.T) {
 	jaeger := v1.NewJaeger(types.NamespacedName{Name: "my-instance"})
 	jaeger.Spec.Agent.Options = v1.NewOptions(map[string]interface{}{
-		"reporter.type":             "thrift",
-		"reporter.thrift.host-port": "collector:14267",
+		"reporter.grpc.host-port": "collector:5000",
 	})
 
 	dep := dep(map[string]string{}, map[string]string{})
 	dep = Sidecar(jaeger, dep)
 
 	assert.Len(t, dep.Spec.Template.Spec.Containers, 2)
-	assert.Len(t, dep.Spec.Template.Spec.Containers[1].Args, 3)
-	assert.Contains(t, dep.Spec.Template.Spec.Containers[1].Args, "--reporter.thrift.host-port=collector:14267")
-	assert.Contains(t, dep.Spec.Template.Spec.Containers[1].Args, "--reporter.type=thrift")
+	assert.Len(t, dep.Spec.Template.Spec.Containers[1].Args, 2)
+	assert.Contains(t, dep.Spec.Template.Spec.Containers[1].Args, "--reporter.grpc.host-port=collector:5000")
 }
 
 func TestSidecarAgentResources(t *testing.T) {
@@ -762,10 +759,9 @@ func TestSidecarArgumentsOpenshiftTLS(t *testing.T) {
 	dep = Sidecar(jaeger, dep)
 
 	assert.Len(t, dep.Spec.Template.Spec.Containers, 2)
-	assert.Len(t, dep.Spec.Template.Spec.Containers[1].Args, 6)
+	assert.Len(t, dep.Spec.Template.Spec.Containers[1].Args, 5)
 	assert.Greater(t, len(util.FindItem("--a-option=a-value", dep.Spec.Template.Spec.Containers[1].Args)), 0)
 	assert.Greater(t, len(util.FindItem("--jaeger.tags", dep.Spec.Template.Spec.Containers[1].Args)), 0)
-	assert.Greater(t, len(util.FindItem("--reporter.type=grpc", dep.Spec.Template.Spec.Containers[1].Args)), 0)
 	assert.Greater(t, len(util.FindItem("--reporter.grpc.host-port=dns:///my-instance-collector-headless.test.svc:14250", dep.Spec.Template.Spec.Containers[1].Args)), 0)
 	assert.Greater(t, len(util.FindItem("--reporter.grpc.tls.enabled=true", dep.Spec.Template.Spec.Containers[1].Args)), 0)
 	assert.Greater(t, len(util.FindItem("--reporter.grpc.tls.ca="+ca.ServiceCAPath, dep.Spec.Template.Spec.Containers[1].Args)), 0)