Refactor flags

Signed-off-by: Pavol Loffay <ploffay@redhat.com>
jaegertracing · May 14, 2020 · 6935b60 · 6935b60
1 parent 786e311
commit 6935b60
Show file tree

Hide file tree

Showing 14 changed files with 287 additions and 66 deletions.
diff --git a/cmd/agent/app/reporter/grpc/flags.go b/cmd/agent/app/reporter/grpc/flags.go
@@ -39,9 +39,14 @@ var tlsFlagsConfig = tlscfg.ClientFlagsConfig{
 
 // AddFlags adds flags for Options.
 func AddFlags(flags *flag.FlagSet) {
-	flags.String(collectorHostPort, "", "Comma-separated string representing host:port of a static list of collectors to connect to directly")
 	flags.Uint(retry, defaultMaxRetry, "Sets the maximum number of retries for a call")
 	flags.Int(discoveryMinPeers, 3, "Max number of collectors to which the agent will try to connect at any given time")
+	AddOTELFlags(flags)
+}
+
+// AddOTELFlags adds flags that are exposed by OTEL collector
+func AddOTELFlags(flags *flag.FlagSet) {
+	flags.String(collectorHostPort, "", "Comma-separated string representing host:port of a static list of collectors to connect to directly")
 	tlsFlagsConfig.AddFlags(flags)
 }
 

diff --git a/cmd/collector/app/builder_flags.go b/cmd/collector/app/builder_flags.go
@@ -27,12 +27,14 @@ import (
 )
 
 const (
-	collectorDynQueueSizeMemory   = "collector.queue-size-memory"
-	collectorQueueSize            = "collector.queue-size"
-	collectorNumWorkers           = "collector.num-workers"
-	collectorHTTPPort             = "collector.http-port"
-	collectorGRPCPort             = "collector.grpc-port"
-	CollectorHTTPHostPort         = "collector.http-server.host-port"
+	collectorDynQueueSizeMemory = "collector.queue-size-memory"
+	collectorQueueSize          = "collector.queue-size"
+	collectorNumWorkers         = "collector.num-workers"
+	collectorHTTPPort           = "collector.http-port"
+	collectorGRPCPort           = "collector.grpc-port"
+	// CollectorHTTPHostPort is a flag for collector HTTP port
+	CollectorHTTPHostPort = "collector.http-server.host-port"
+	// CollectorGRPCHostPort is a flag for collector gRPC port
 	CollectorGRPCHostPort         = "collector.grpc-server.host-port"
 	collectorZipkinHTTPPort       = "collector.zipkin.http-port"
 	collectorZipkinHTTPHostPort   = "collector.zipkin.host-port"
@@ -82,13 +84,18 @@ func AddFlags(flags *flag.FlagSet) {
 	flags.Int(collectorHTTPPort, 0, collectorHTTPPortWarning+" see --"+CollectorHTTPHostPort)
 	flags.Int(collectorGRPCPort, 0, collectorGRPCPortWarning+" see --"+CollectorGRPCHostPort)
 	flags.Int(collectorZipkinHTTPPort, 0, collectorZipkinHTTPPortWarning+" see --"+collectorZipkinHTTPHostPort)
-	flags.String(CollectorHTTPHostPort, ports.PortToHostPort(ports.CollectorHTTP), "The host:port (e.g. 127.0.0.1:5555 or :5555) of the collector's HTTP server")
-	flags.String(CollectorGRPCHostPort, ports.PortToHostPort(ports.CollectorGRPC), "The host:port (e.g. 127.0.0.1:5555 or :5555) of the collector's GRPC server")
 	flags.String(collectorZipkinHTTPHostPort, ports.PortToHostPort(0), "The host:port (e.g. 127.0.0.1:5555 or :5555) of the collector's Zipkin server")
 	flags.Uint(collectorDynQueueSizeMemory, 0, "(experimental) The max memory size in MiB to use for the dynamic queue.")
 	flags.String(collectorTags, "", "One or more tags to be added to the Process tags of all spans passing through this collector. Ex: key1=value1,key2=${envVar:defaultValue}")
 	flags.String(collectorZipkinAllowedOrigins, "*", "Comma separated list of allowed origins for the Zipkin collector service, default accepts all")
 	flags.String(collectorZipkinAllowedHeaders, "content-type", "Comma separated list of allowed headers for the Zipkin collector service, default content-type")
+	AddOTELFlags(flags)
+}
+
+// AddOTELFlags adds flags that are exposed by OTEL collector
+func AddOTELFlags(flags *flag.FlagSet) {
+	flags.String(CollectorHTTPHostPort, ports.PortToHostPort(ports.CollectorHTTP), "The host:port (e.g. 127.0.0.1:5555 or :5555) of the collector's HTTP server")
+	flags.String(CollectorGRPCHostPort, ports.PortToHostPort(ports.CollectorGRPC), "The host:port (e.g. 127.0.0.1:5555 or :5555) of the collector's GRPC server")
 	tlsFlagsConfig.AddFlags(flags)
 }
 

diff --git a/cmd/opentelemetry-collector/app/defaults/default_config_test.go b/cmd/opentelemetry-collector/app/defaults/default_config_test.go
@@ -23,13 +23,15 @@ import (
 	"github.com/open-telemetry/opentelemetry-collector/config/configmodels"
 	"github.com/open-telemetry/opentelemetry-collector/exporter/jaegerexporter"
 	"github.com/open-telemetry/opentelemetry-collector/processor/resourceprocessor"
+	"github.com/open-telemetry/opentelemetry-collector/receiver"
 	"github.com/open-telemetry/opentelemetry-collector/receiver/jaegerreceiver"
+	"github.com/open-telemetry/opentelemetry-collector/receiver/zipkinreceiver"
 	"github.com/spf13/viper"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"go.uber.org/zap"
 
-	"github.com/jaegertracing/jaeger/cmd/agent/app/reporter/grpc"
+	"github.com/jaegertracing/jaeger/cmd/opentelemetry-collector/app"
 	"github.com/jaegertracing/jaeger/cmd/opentelemetry-collector/app/exporter/cassandra"
 	"github.com/jaegertracing/jaeger/cmd/opentelemetry-collector/app/exporter/elasticsearch"
 	"github.com/jaegertracing/jaeger/cmd/opentelemetry-collector/app/exporter/grpcplugin"
@@ -118,7 +120,7 @@ func TestDefaultCollectorConfig(t *testing.T) {
 	}
 	for _, test := range tests {
 		t.Run(test.storageType, func(t *testing.T) {
-			v, _ := jConfig.Viperize(grpc.AddFlags)
+			v, _ := jConfig.Viperize(app.AddComponentFlags)
 			factories := Components(v)
 			for key, val := range test.config {
 				v.Set(key, val)
@@ -162,6 +164,87 @@ func TestDefaultCollectorConfig(t *testing.T) {
 	}
 }
 
+func TestCreateCollectorReceivers(t *testing.T) {
+	tests := []struct {
+		name           string
+		args           []string
+		zipkinHostPort string
+		receivers      configmodels.Receivers
+	}{
+		{
+			name:           "defaultWithoutZipkin",
+			args:           []string{},
+			zipkinHostPort: ":0",
+			receivers: configmodels.Receivers{
+				"jaeger": &jaegerreceiver.Config{
+					TypeVal: "jaeger",
+					NameVal: "jaeger",
+					Protocols: map[string]*receiver.SecureReceiverSettings{
+						"grpc": {
+							ReceiverSettings: configmodels.ReceiverSettings{
+								Endpoint: gRPCEndpoint,
+							},
+						},
+						"thrift_http": {
+							ReceiverSettings: configmodels.ReceiverSettings{
+								Endpoint: httpThriftBinaryEndpoint,
+							},
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "configurationViaFlags",
+			args: []string{
+				"--collector.grpc-server.host-port=host:11",
+				"--collector.grpc.tls.cert=cacert.crt",
+				"--collector.grpc.tls.key=keycert.crt",
+				"--collector.http-server.host-port=host2:22",
+			},
+			zipkinHostPort: "localhost:55",
+			receivers: configmodels.Receivers{
+				"jaeger": &jaegerreceiver.Config{
+					TypeVal: "jaeger",
+					NameVal: "jaeger",
+					Protocols: map[string]*receiver.SecureReceiverSettings{
+						"grpc": {
+							ReceiverSettings: configmodels.ReceiverSettings{
+								Endpoint: "host:11",
+							},
+							TLSCredentials: &receiver.TLSCredentials{
+								CertFile: "cacert.crt",
+								KeyFile:  "keycert.crt",
+							},
+						},
+						"thrift_http": {
+							ReceiverSettings: configmodels.ReceiverSettings{
+								Endpoint: "host2:22",
+							},
+						},
+					},
+				},
+				"zipkin": &zipkinreceiver.Config{
+					ReceiverSettings: configmodels.ReceiverSettings{
+						NameVal:  "zipkin",
+						TypeVal:  "zipkin",
+						Endpoint: "localhost:55",
+					},
+				},
+			},
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			v, c := jConfig.Viperize(app.AddComponentFlags)
+			require.NoError(t, c.ParseFlags(test.args))
+			factories := Components(v)
+			recvs := createCollectorReceivers(test.zipkinHostPort, factories)
+			assert.Equal(t, test.receivers, recvs)
+		})
+	}
+}
+
 func TestDefaultAgentConfig(t *testing.T) {
 	tests := []struct {
 		config  map[string]interface{}
@@ -196,7 +279,7 @@ func TestDefaultAgentConfig(t *testing.T) {
 	}
 	for _, test := range tests {
 		t.Run(fmt.Sprintf("%v", test.config), func(t *testing.T) {
-			v, _ := jConfig.Viperize(grpc.AddFlags)
+			v, _ := jConfig.Viperize(app.AddComponentFlags)
 			for key, val := range test.config {
 				v.Set(key, val)
 			}
@@ -221,6 +304,63 @@ func TestDefaultAgentConfig(t *testing.T) {
 	}
 }
 
+func TestCreateAgentReceivers(t *testing.T) {
+	tests := []struct {
+		args      []string
+		receivers configmodels.Receivers
+	}{
+		{
+			args: []string{""},
+			receivers: configmodels.Receivers{
+				"jaeger": &jaegerreceiver.Config{
+					TypeVal: "jaeger",
+					NameVal: "jaeger",
+					Protocols: map[string]*receiver.SecureReceiverSettings{
+						"thrift_compact": {
+							ReceiverSettings: configmodels.ReceiverSettings{
+								Endpoint: udpThriftCompactEndpoint,
+							},
+						},
+						"thrift_binary": {
+							ReceiverSettings: configmodels.ReceiverSettings{
+								Endpoint: udpThriftBinaryEndpoint,
+							},
+						},
+					},
+				},
+			},
+		},
+		{
+			args: []string{"--processor.jaeger-binary.server-host-port=host:1", "--processor.jaeger-compact.server-host-port=host:2"},
+			receivers: configmodels.Receivers{
+				"jaeger": &jaegerreceiver.Config{
+					TypeVal: "jaeger",
+					NameVal: "jaeger",
+					Protocols: map[string]*receiver.SecureReceiverSettings{
+						"thrift_binary": {
+							ReceiverSettings: configmodels.ReceiverSettings{
+								Endpoint: "host:1",
+							},
+						},
+						"thrift_compact": {
+							ReceiverSettings: configmodels.ReceiverSettings{
+								Endpoint: "host:2",
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+	for _, test := range tests {
+		v, c := jConfig.Viperize(app.AddComponentFlags)
+		require.NoError(t, c.ParseFlags(test.args))
+		factories := Components(v)
+		recvs := createAgentReceivers(factories)
+		assert.Equal(t, test.receivers, recvs)
+	}
+}
+
 func TestDefaultIngesterConfig(t *testing.T) {
 	tests := []struct {
 		storageType string

diff --git a/cmd/opentelemetry-collector/app/exporter/jaegerexporter/jaeger_exporter_test.go b/cmd/opentelemetry-collector/app/exporter/jaegerexporter/jaeger_exporter_test.go
@@ -1,23 +1,38 @@
+// Copyright (c) 2020 The Jaeger Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
 package jaegerexporter
 
 import (
 	"context"
-	jConfig "github.com/jaegertracing/jaeger/pkg/config"
+	"path"
+	"testing"
+
 	"github.com/open-telemetry/opentelemetry-collector/component"
 	"github.com/open-telemetry/opentelemetry-collector/config"
 	"github.com/open-telemetry/opentelemetry-collector/config/configerror"
 	"github.com/open-telemetry/opentelemetry-collector/config/configmodels"
 	"github.com/open-telemetry/opentelemetry-collector/exporter/jaegerexporter"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	"path"
-	"testing"
 
-	grpcRep "github.com/jaegertracing/jaeger/cmd/agent/app/reporter/grpc"
+	"github.com/jaegertracing/jaeger/cmd/opentelemetry-collector/app/receiver/jaegerreceiver"
+	jConfig "github.com/jaegertracing/jaeger/pkg/config"
 )
 
 func TestDefaultValues(t *testing.T) {
-	v, c := jConfig.Viperize(grpcRep.AddFlags)
+	v, c := jConfig.Viperize(jaegerreceiver.AddFlags)
 	err := c.ParseFlags([]string{})
 	require.NoError(t, err)
 
@@ -33,7 +48,7 @@ func TestDefaultValues(t *testing.T) {
 }
 
 func TestDefaultValueFromViper(t *testing.T) {
-	v, c := jConfig.Viperize(grpcRep.AddFlags)
+	v, c := jConfig.Viperize(jaegerreceiver.AddFlags)
 	err := c.ParseFlags([]string{"--reporter.grpc.host-port=foo", "--reporter.grpc.tls.enabled=true", "--reporter.grpc.tls.ca=ca.crt"})
 	require.NoError(t, err)
 
@@ -53,7 +68,7 @@ func TestLoadConfigAndFlags(t *testing.T) {
 	factories, err := config.ExampleComponents()
 	require.NoError(t, err)
 
-	v, c := jConfig.Viperize(grpcRep.AddFlags)
+	v, c := jConfig.Viperize(jaegerreceiver.AddFlags)
 	err = c.ParseFlags([]string{"--reporter.grpc.host-port=foo"})
 	require.NoError(t, err)
 

diff --git a/cmd/opentelemetry-collector/app/util.go → cmd/opentelemetry-collector/app/flags.go b/cmd/opentelemetry-collector/app/util.go → cmd/opentelemetry-collector/app/flags.go
@@ -19,15 +19,26 @@ import (
 	"fmt"
 	"strings"
 
+	jConfigFile "github.com/jaegertracing/jaeger/cmd/flags"
 	"github.com/jaegertracing/jaeger/cmd/opentelemetry-collector/app/exporter/cassandra"
 	"github.com/jaegertracing/jaeger/cmd/opentelemetry-collector/app/exporter/elasticsearch"
 	"github.com/jaegertracing/jaeger/cmd/opentelemetry-collector/app/exporter/grpcplugin"
 	"github.com/jaegertracing/jaeger/cmd/opentelemetry-collector/app/exporter/kafka"
+	"github.com/jaegertracing/jaeger/cmd/opentelemetry-collector/app/processor/resourceprocessor"
+	"github.com/jaegertracing/jaeger/cmd/opentelemetry-collector/app/receiver/jaegerreceiver"
 )
 
-// StorageFlags return a function that adds storage flags.
+// AddComponentFlags adds all flags exposed by components
+func AddComponentFlags(flags *flag.FlagSet) {
+	// Jaeger receiver (via sampling strategies receiver) exposes the same flags as exporter.
+	jaegerreceiver.AddFlags(flags)
+	resourceprocessor.AddFlags(flags)
+	jConfigFile.AddConfigFileFlag(flags)
+}
+
+// AddStorageFlags return a function that adds storage flags.
 // storage parameter can contain a comma separated list of supported Jaeger storage backends.
-func StorageFlags(storage string) (func(*flag.FlagSet), error) {
+func AddStorageFlags(storage string) (func(*flag.FlagSet), error) {
 	var flagFn []func(*flag.FlagSet)
 	for _, s := range strings.Split(storage, ",") {
 		switch s {

diff --git a/cmd/opentelemetry-collector/app/processor/resourceprocessor/flags.go b/cmd/opentelemetry-collector/app/processor/resourceprocessor/flags.go
@@ -0,0 +1,28 @@
+// Copyright (c) 2020 The Jaeger Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package resourceprocessor
+
+import (
+	"flag"
+	"fmt"
+
+	"github.com/jaegertracing/jaeger/cmd/agent/app/reporter"
+)
+
+// AddFlags adds flags for Options.
+func AddFlags(flags *flag.FlagSet) {
+	flags.String(reporter.AgentTagsDeprecated, "", fmt.Sprintf("(deprecated, use --%s) One or more tags to be added to the Process tags of all spans passing through this agent. Ex: key1=value1,key2=${envVar:defaultValue}", resourceLabels))
+	flags.String(resourceLabels, "", "One or more tags to be added to the Process tags of all spans passing through this agent. Ex: key1=value1,key2=${envVar:defaultValue}")
+}
diff --git a/cmd/opentelemetry-collector/app/processor/resourceprocessor/resource_processor.go b/cmd/opentelemetry-collector/app/processor/resourceprocessor/resource_processor.go
@@ -15,9 +15,6 @@
 package resourceprocessor
 
 import (
-	"flag"
-	"fmt"
-
 	"github.com/open-telemetry/opentelemetry-collector/component"
 	"github.com/open-telemetry/opentelemetry-collector/config/configmodels"
 	"github.com/open-telemetry/opentelemetry-collector/consumer"
@@ -88,9 +85,3 @@ func (f Factory) CreateMetricsProcessor(
 ) (component.MetricsProcessorOld, error) {
 	return f.Wrapped.CreateMetricsProcessor(logger, nextConsumer, cfg)
 }
-
-// AddFlags adds flags for Options.
-func AddFlags(flags *flag.FlagSet) {
-	flags.String(reporter.AgentTagsDeprecated, "", fmt.Sprintf("(deprecated, use --%s) One or more tags to be added to the Process tags of all spans passing through this agent. Ex: key1=value1,key2=${envVar:defaultValue}", resourceLabels))
-	flags.String(resourceLabels, "", "One or more tags to be added to the Process tags of all spans passing through this agent. Ex: key1=value1,key2=${envVar:defaultValue}")
-}