From 4e8409929ae7e37c49d533577adea81516d506a3 Mon Sep 17 00:00:00 2001 From: Melchior Moulin Date: Mon, 27 Jun 2022 18:51:56 +0200 Subject: [PATCH] exporterTracing string to define the exporter instead of bool exporterTracing string to define the exporter instead of bool make tracing exporter secure if the right env variables are set remove from dynamicconfig as it is not working Signed-off-by: Melchior Moulin --- cmd/main.go | 18 +++++------ internal/config/canary_config.go | 54 +++++++++----------------------- internal/services/consumer.go | 3 +- 3 files changed, 24 insertions(+), 51 deletions(-) diff --git a/cmd/main.go b/cmd/main.go index a182065..f18b735 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -45,14 +45,14 @@ var ( var saramaLogger = log.New(io.Discard, "[Sarama] ", log.LstdFlags) -func initTracerProvider(ActivateTracing bool, JaegerExporterTracing bool, OtlpExporterTracing bool) *sdktrace.TracerProvider { +func initTracerProvider(exporterType string) *sdktrace.TracerProvider { resources, _ := resource.New(context.Background(), resource.WithFromEnv(), // pull attributes from OTEL_RESOURCE_ATTRIBUTES and OTEL_SERVICE_NAME environment variables resource.WithProcess(), // This option configures a set of Detectors that discover process information ) otel.SetTextMapPropagator(propagation.NewCompositeTextMapPropagator(propagation.TraceContext{})) - if ActivateTracing { - exporter := exporterTracing(JaegerExporterTracing, OtlpExporterTracing) + if exporterType != "" { + exporter := exporterTracing(exporterType) tp := sdktrace.NewTracerProvider( sdktrace.WithResource(resources), sdktrace.WithBatcher(exporter), @@ -65,14 +65,14 @@ func initTracerProvider(ActivateTracing bool, JaegerExporterTracing bool, OtlpEx return nil } -func exporterTracing(JaegerExporterTracing bool, OtlpExporterTracing bool) sdktrace.SpanExporter { +func exporterTracing(exporterType string) sdktrace.SpanExporter { //Could not use OTEL_TRACES_EXPORTER https://github.com/open-telemetry/opentelemetry-go/issues/2310 var exporter sdktrace.SpanExporter //If the env variables needed are defined we set the exporter to Jaeger else it is an opentelemetry exporter - if JaegerExporterTracing { + if exporterType == "jaeger" { exporter, _ = jaeger.New(jaeger.WithAgentEndpoint()) //from env variable https://github.com/open-telemetry/opentelemetry-go/tree/main/exporters/jaeger#environment-variables - } else if OtlpExporterTracing { - exporter, _ = otlptracegrpc.New(context.Background(), otlptracegrpc.WithInsecure()) //from env variable https://github.com/open-telemetry/opentelemetry-go/blob/main/exporters/otlp/otlptrace/README.md + } else if exporterType == "otlp" { + exporter, _ = otlptracegrpc.New(context.Background()) //from env variable https://github.com/open-telemetry/opentelemetry-go/blob/main/exporters/otlp/otlptrace/README.md } return exporter } @@ -92,7 +92,7 @@ func main() { glog.Infof("Starting Strimzi canary tool [%s] with config: %+v", version, canaryConfig) - tp := initTracerProvider(canaryConfig.ActivateTracing, canaryConfig.JaegerExporterTracing, canaryConfig.OtlpExporterTracing) + tp := initTracerProvider(canaryConfig.ExporterTypeTracing) defer func() { if tp != nil { if err := tp.Shutdown(context.Background()); err != nil { @@ -100,7 +100,6 @@ func main() { } } }() - dynamicConfigWatcher, err := config.NewDynamicConfigWatcher(canaryConfig, applyDynamicConfig, config.NewDynamicCanaryConfig) if err != nil { glog.Fatalf("Failed to create dynamic config watcher: %v", err) @@ -191,7 +190,6 @@ func applyDynamicConfig(dynamicCanaryConfig *config.DynamicCanaryConfig) { } else { saramaLogger.SetOutput(io.Discard) } - initTracerProvider(*dynamicCanaryConfig.ActivateTracing, *dynamicCanaryConfig.JaegerExporterTracing, *dynamicCanaryConfig.OtlpExporterTracing) glog.Warningf("Applied dynamic config %s", dynamicCanaryConfig) } diff --git a/internal/config/canary_config.go b/internal/config/canary_config.go index 8299d56..838081d 100644 --- a/internal/config/canary_config.go +++ b/internal/config/canary_config.go @@ -48,11 +48,7 @@ const ( DynamicConfigFileEnvVar = "DYNAMIC_CONFIG_FILE" DynamicConfigWatcherIntervalEnvVar = "DYNAMIC_CONFIG_WATCHER_INTERVAL" //TODO: This will be removed when Support the OTEL_TRACES_EXPORTER env var is available in the SDK see: https://github.com/open-telemetry/opentelemetry-go/issues/2310 - JaegerEndpointEnvVar = "OTEL_EXPORTER_JAEGER_ENDPOINT" - JaegerAgentHostEnvVar = "OTEL_EXPORTER_JAEGER_AGENT_HOST" - OtlpEndpointEnvVar = "OTEL_EXPORTER_OTLP_ENDPOINT" - OtlpTracesEndpointEnvVar = "OTEL_EXPORTER_OTLP_TRACES_ENDPOINT" - ActivateTracingEnvVar = "ACTIVATE_TRACING" + ExporterTypeTracing = "EXPORTER_TYPE_TRACING" // default values for environment variables BootstrapServersDefault = "localhost:9092" BootstrapBackoffMaxAttemptsDefault = 10 @@ -82,19 +78,12 @@ const ( StatusTimeWindowDefault = 300000 DynamicConfigFileDefault = "" DynamicConfigWatcherIntervalDefault = 30000 - JaegerEndpointEnvVarDefault = "" - JaegerAgentHostEnvVarDefault = "" - OtlpEndpointEnvVarDefault = "" - OtlpTracesEndpointEnvVarDefault = "" - ActivateTracingEnvVarDefault = false + ExporterTypeTracingDefault = "" //if empty no tracing for now, possible values : "otlp" or "jaeger" ) type DynamicCanaryConfig struct { - SaramaLogEnabled *bool `json:"saramaLogEnabled"` - VerbosityLogLevel *int `json:"verbosityLogLevel"` - OtlpExporterTracing *bool - JaegerExporterTracing *bool - ActivateTracing *bool `json:"activateTracing"` + SaramaLogEnabled *bool `json:"saramaLogEnabled"` + VerbosityLogLevel *int `json:"verbosityLogLevel"` } // CanaryConfig defines the canary tool configuration @@ -126,24 +115,16 @@ type CanaryConfig struct { StatusCheckInterval time.Duration StatusTimeWindow time.Duration DynamicConfigWatcherInterval time.Duration - OtlpExporterTracing bool - JaegerExporterTracing bool - ActivateTracing bool + ExporterTypeTracing string } func NewDynamicCanaryConfig() *DynamicCanaryConfig { saramaLogEnabled := lookupBoolEnv(SaramaLogEnabledEnvVar, SaramaLogEnabledDefault) verbosityLogLevel := lookupIntEnv(VerbosityLogLevelEnvVar, VerbosityLogLevelDefault) - activateTracing := lookupBoolEnv(ActivateTracingEnvVar, ActivateTracingEnvVarDefault) - jaegerExporterTracing := isJaegerTracingExporter() - otlpExporterTracing := isOtlpTracingExporter() dynamicCanaryConfig := DynamicCanaryConfig{ - SaramaLogEnabled: &saramaLogEnabled, - VerbosityLogLevel: &verbosityLogLevel, - OtlpExporterTracing: &otlpExporterTracing, - JaegerExporterTracing: &jaegerExporterTracing, - ActivateTracing: &activateTracing, + SaramaLogEnabled: &saramaLogEnabled, + VerbosityLogLevel: &verbosityLogLevel, } return &dynamicCanaryConfig } @@ -167,16 +148,6 @@ func (c DynamicCanaryConfig) String() string { return str } -//If the env variables needed are defined we set the exporter to Jaeger else it is an OTLP exporter -func isJaegerTracingExporter() bool { - return lookupStringEnv(JaegerEndpointEnvVar, JaegerEndpointEnvVarDefault) != "" || lookupStringEnv(JaegerAgentHostEnvVar, JaegerAgentHostEnvVarDefault) != "" -} - -//If the env variables needed are defined we set the exporter to OTLP else it is an Jaeger exporter -func isOtlpTracingExporter() bool { - return lookupStringEnv(OtlpEndpointEnvVar, OtlpEndpointEnvVarDefault) != "" || lookupStringEnv(OtlpTracesEndpointEnvVar, OtlpTracesEndpointEnvVarDefault) != "" -} - // NewCanaryConfig returns an configuration instance from environment variables func NewCanaryConfig() *CanaryConfig { dynamicCanaryConfig := NewDynamicCanaryConfig() @@ -209,9 +180,7 @@ func NewCanaryConfig() *CanaryConfig { StatusTimeWindow: time.Duration(lookupIntEnv(StatusTimeWindowEnvVar, StatusTimeWindowDefault)), DynamicConfigFile: lookupStringEnv(DynamicConfigFileEnvVar, DynamicConfigFileDefault), DynamicConfigWatcherInterval: time.Duration(lookupIntEnv(DynamicConfigWatcherIntervalEnvVar, DynamicConfigWatcherIntervalDefault)), - OtlpExporterTracing: isOtlpTracingExporter(), - JaegerExporterTracing: isJaegerTracingExporter(), - ActivateTracing: lookupBoolEnv(ActivateTracingEnvVar, ActivateTracingEnvVarDefault), + ExporterTypeTracing: exporterTypeTracing(), } return &config } @@ -317,3 +286,10 @@ func (c CanaryConfig) String() string { c.ConnectionCheckInterval, c.ConnectionCheckLatencyBuckets, c.StatusCheckInterval, c.StatusTimeWindow, c.DynamicConfigFile, c.DynamicCanaryConfig, c.DynamicConfigWatcherInterval) } +func exporterTypeTracing() string { + exporterType := lookupStringEnv(ExporterTypeTracing, ExporterTypeTracingDefault) + if exporterType != "jaeger" && exporterType != "otlp" && exporterType != "" { + panic(fmt.Errorf("%s env variable possible values are : '' or 'jaeger' or 'otlp'", ExporterTypeTracing)) + } + return exporterType +} diff --git a/internal/services/consumer.go b/internal/services/consumer.go index 1dcece4..1a1fd7a 100644 --- a/internal/services/consumer.go +++ b/internal/services/consumer.go @@ -107,11 +107,10 @@ func NewConsumerService(canaryConfig *config.CanaryConfig, client sarama.Client) func (cs *ConsumerService) Consume() { backoff := NewBackoff(maxConsumeAttempts, 5000*time.Millisecond, MaxDefault) for { - var h sarama.ConsumerGroupHandler cgh := &consumerGroupHandler{ consumerService: cs, } - h = otelsarama.WrapConsumerGroupHandler(cgh) + h := otelsarama.WrapConsumerGroupHandler(cgh) // creating new context with cancellation, for exiting Consume when metadata refresh is needed ctx, cancel := context.WithCancel(context.Background())