add protocol arg to be used to switch of GRPC (#1723)

* add protocol arg to be used to switch of GRPC, a string arg was introduced instead of bool, because protocol might change in the future Signed-off-by: Sandor Szücs <[email protected]> * switch only on http Signed-off-by: Sandor Szücs <[email protected]> * use switch block to be explicit and add failure case in test Signed-off-by: Sandor Szücs <[email protected]>
zalando · Feb 18, 2021 · 7de16ef · 7de16ef
1 parent 76fb0a7
commit 7de16ef
Show file tree

Hide file tree

Showing 3 changed files with 71 additions and 8 deletions.
diff --git a/tracing/tracers/lightstep/README.md b/tracing/tracers/lightstep/README.md
@@ -1,10 +1,11 @@
 # lightstep tracer
 
-As with [other tracers](https://pkg.go.dev/github.com/zalando/skipper/tracing), the lightstep tracer is configured by setting 
+As with [other tracers](https://pkg.go.dev/github.com/zalando/skipper/tracing), the lightstep tracer is configured by setting
 `-opentracing="lightstep OPTIONS"`. Valid options are:
 
 * `component-name` - set component name instead of `skipper`
 * `token` - Access token for the lightstep satellites (REQUIRED)
+* `protocol` - sets `UseGRPC` option to true if set to `"grpc"`, defaults to `"grpc"`
 * `grpc-max-msg-size` - maximum size for gRPC messages
 * `min-period` - minimum time to wait before sending spans to the satellites, string with value parseable by `time.ParseDuration`
 * `max-period` - maximum time to wait before sending spans to the satellites, string with value parseable by `time.ParseDuration`

diff --git a/tracing/tracers/lightstep/lightstep.go b/tracing/tracers/lightstep/lightstep.go
@@ -33,6 +33,7 @@ func parseOptions(opts []string) (lightstep.Options, error) {
 		minReportingPeriod = lightstep.DefaultMinReportingPeriod
 		maxReportingPeriod = lightstep.DefaultMaxReportingPeriod
 		propagators        = make(map[opentracing.BuiltinFormat]lightstep.Propagator)
+		useGRPC            = true
 	)
 
 	componentName := defComponentName
@@ -99,6 +100,15 @@ func parseOptions(opts []string) (lightstep.Options, error) {
 		case "cmd-line":
 			cmdLine = parts[1]
 			logCmdLine = true
+		case "protocol":
+			switch parts[1] {
+			case "http":
+				useGRPC = false
+			case "grpc":
+				useGRPC = true
+			default:
+				return lightstep.Options{}, fmt.Errorf("failed to parse protocol allowed 'http' or 'grpc', got: %s", parts[1])
+			}
 		case "log-events":
 			logEvents = true
 		case "max-buffered-spans":
@@ -162,7 +172,7 @@ func parseOptions(opts []string) (lightstep.Options, error) {
 			Port:      port,
 			Plaintext: plaintext,
 		},
-		UseGRPC:                     true,
+		UseGRPC:                     useGRPC,
 		Tags:                        tags,
 		MaxBufferedSpans:            maxBufferedSpans,
 		GRPCMaxCallSendMsgSizeBytes: grpcMaxMsgSize,

diff --git a/tracing/tracers/lightstep/lightstep_test.go b/tracing/tracers/lightstep/lightstep_test.go
@@ -97,11 +97,10 @@ func TestParseOptions(t *testing.T) {
 			propagators: map[opentracing.BuiltinFormat]lightstep.Propagator{opentracing.HTTPHeaders: defPropagator},
 		},
 		{
-			name: "test with token set tags",
+			name: "test with token set component name",
 			opts: []string{
 				"token=" + token,
-				"tag=cluster=my-test",
-				"tag=foo=bar",
+				"component-name=skipper-ingress",
 			},
 			want: lightstep.Options{
 				AccessToken: token,
@@ -110,10 +109,63 @@ func TestParseOptions(t *testing.T) {
 					Port: lightstep.DefaultSecurePort,
 				},
 				UseGRPC: true,
+				Tags: map[string]interface{}{
+					lightstep.ComponentNameKey: "skipper-ingress",
+				},
+				GRPCMaxCallSendMsgSizeBytes: defaultGRPMaxMsgSize,
+				ReportingPeriod:             lightstep.DefaultMaxReportingPeriod,
+				MinReportingPeriod:          lightstep.DefaultMinReportingPeriod,
+			},
+			wantErr:     false,
+			propagators: map[opentracing.BuiltinFormat]lightstep.Propagator{opentracing.HTTPHeaders: defPropagator},
+		},
+		{
+			name: "test with token set protocol to grpc use grpc",
+			opts: []string{
+				"token=" + token,
+				"protocol=grpc",
+			},
+			want: lightstep.Options{
+				AccessToken: token,
+				Collector: lightstep.Endpoint{
+					Host: lightstep.DefaultGRPCCollectorHost,
+					Port: lightstep.DefaultSecurePort,
+				},
+				UseGRPC: true,
+				Tags: map[string]interface{}{
+					lightstep.ComponentNameKey: defComponentName,
+				},
+				GRPCMaxCallSendMsgSizeBytes: defaultGRPMaxMsgSize,
+				ReportingPeriod:             lightstep.DefaultMaxReportingPeriod,
+				MinReportingPeriod:          lightstep.DefaultMinReportingPeriod,
+			},
+			wantErr:     false,
+			propagators: map[opentracing.BuiltinFormat]lightstep.Propagator{opentracing.HTTPHeaders: defPropagator},
+		},
+		{
+			name: "test with token set protocol to foo returns error",
+			opts: []string{
+				"token=" + token,
+				"protocol=foo",
+			},
+			want:    lightstep.Options{},
+			wantErr: true,
+		},
+		{
+			name: "test with token set protocol to http does not use grpc",
+			opts: []string{
+				"token=" + token,
+				"protocol=http",
+			},
+			want: lightstep.Options{
+				AccessToken: token,
+				Collector: lightstep.Endpoint{
+					Host: lightstep.DefaultGRPCCollectorHost,
+					Port: lightstep.DefaultSecurePort,
+				},
+				UseGRPC: false,
 				Tags: map[string]interface{}{
 					lightstep.ComponentNameKey: defComponentName,
-					"cluster":                  "my-test",
-					"foo":                      "bar",
 				},
 				GRPCMaxCallSendMsgSizeBytes: defaultGRPMaxMsgSize,
 				ReportingPeriod:             lightstep.DefaultMaxReportingPeriod,
@@ -194,7 +246,7 @@ func TestParseOptions(t *testing.T) {
 			propagators: map[opentracing.BuiltinFormat]lightstep.Propagator{opentracing.HTTPHeaders: defPropagator},
 		},
 		{
-			name: "test with token and wront reporting period values should fail",
+			name: "test with token and wrong reporting period values should fail",
 			opts: []string{
 				"token=" + token,
 				"min-period=2100ms",