fix(operator): fix otel collector URL setup (#1262)

Signed-off-by: Florian Bacher <florian.bacher@dynatrace.com> Co-authored-by: odubajDT <93584209+odubajDT@users.noreply.github.com>
keptn · Apr 24, 2023 · c3754b7 · c3754b7
1 parent c8131b6
commit c3754b7
Show file tree

Hide file tree

Showing 4 changed files with 101 additions and 10 deletions.
diff --git a/operator/controllers/common/otel_utils.go b/operator/controllers/common/otel_utils.go
@@ -39,6 +39,8 @@ type otelConfig struct {
 	TracerProvider *trace.TracerProvider
 	OtelExporter   *trace.SpanExporter
 
+	lastAppliedCollectorURL string
+
 	mtx     sync.RWMutex
 	tracers map[string]ITracer
 }
@@ -58,6 +60,9 @@ func GetOtelInstance() *otelConfig {
 }
 
 func (o *otelConfig) InitOtelCollector(otelCollectorUrl string) error {
+	if o.lastAppliedCollectorURL == otelCollectorUrl {
+		return nil
+	}
 	tpOptions, otelExporter, err := GetOTelTracerProviderOptions(otelCollectorUrl)
 	if err != nil {
 		return err
@@ -68,6 +73,7 @@ func (o *otelConfig) InitOtelCollector(otelCollectorUrl string) error {
 	otel.SetTextMapPropagator(propagation.NewCompositeTextMapPropagator(propagation.TraceContext{}, propagation.Baggage{}))
 	o.OtelExporter = &otelExporter
 	o.cleanTracers()
+	o.lastAppliedCollectorURL = otelCollectorUrl
 	logger.Info("Successfully initialized OTel collector")
 	return nil
 }

diff --git a/operator/controllers/common/otel_utils_test.go b/operator/controllers/common/otel_utils_test.go
@@ -34,6 +34,8 @@ func TestGetOTelTracerProviderOptions(t *testing.T) {
 		}
 	}()
 
+	defer s.Stop()
+
 	type args struct {
 		oTelCollectorUrl string
 	}
@@ -240,3 +242,92 @@ func Test_otelConfig_GetTracer(t *testing.T) {
 
 	require.Empty(t, otelConfig.tracers)
 }
+
+func Test_otelConfig_InitOtelCollector_ReInitWithSameURL(t *testing.T) {
+
+	listener, err := net.Listen("tcp", ":9000")
+	if err != nil {
+		panic(err)
+	}
+
+	s := grpc.NewServer()
+	go func() {
+		err := s.Serve(listener)
+		if err != nil {
+			panic(err)
+		}
+	}()
+
+	defer s.Stop()
+
+	o := GetOtelInstance()
+	err = o.InitOtelCollector("localhost:9000")
+
+	require.Nil(t, err)
+
+	require.Equal(t, "localhost:9000", o.lastAppliedCollectorURL)
+
+	tracer := o.GetTracer("my-tracer")
+	require.NotNil(t, tracer)
+	require.Len(t, o.tracers, 1)
+
+	// init with the same URL again
+	err = o.InitOtelCollector("localhost:9000")
+
+	require.Nil(t, err)
+
+	// in this case the init function should NOT have changed any internal state,
+	// i.e. the tracers should NOT have been cleaned up
+	require.Len(t, o.tracers, 1)
+}
+
+func Test_otelConfig_InitOtelCollector_ReInitWithDifferentURL(t *testing.T) {
+
+	listener, err := net.Listen("tcp", ":9000")
+	if err != nil {
+		panic(err)
+	}
+
+	listener2, err := net.Listen("tcp", ":9001")
+	if err != nil {
+		panic(err)
+	}
+
+	s := grpc.NewServer()
+	go func() {
+		err := s.Serve(listener)
+		if err != nil {
+			panic(err)
+		}
+	}()
+
+	s2 := grpc.NewServer()
+	go func() {
+		err := s2.Serve(listener2)
+		if err != nil {
+			panic(err)
+		}
+	}()
+
+	defer s.Stop()
+
+	o := GetOtelInstance()
+	err = o.InitOtelCollector("localhost:9000")
+
+	require.Nil(t, err)
+
+	require.Equal(t, "localhost:9000", o.lastAppliedCollectorURL)
+
+	tracer := o.GetTracer("my-tracer")
+	require.NotNil(t, tracer)
+	require.Len(t, o.tracers, 1)
+
+	// init with a different URL
+	err = o.InitOtelCollector("localhost:9001")
+
+	require.Nil(t, err)
+
+	// in this case the init function should have changed any internal state,
+	// i.e. the tracers should have been cleaned up
+	require.Empty(t, o.tracers)
+}
diff --git a/operator/controllers/options/keptnconfig_controller.go b/operator/controllers/options/keptnconfig_controller.go
@@ -76,12 +76,9 @@ func (r *KeptnConfigReconciler) reconcileOtelCollectorUrl(config *optionsv1alpha
 	r.Log.Info(fmt.Sprintf("reconciling Keptn Config: %s", config.Name))
 	otelConfig := controllercommon.GetOtelInstance()
 
-	// collector URL changed, so we need to re-initialize the exporter
-	if r.LastAppliedSpec.OTelCollectorUrl != config.Spec.OTelCollectorUrl {
-		if err := otelConfig.InitOtelCollector(config.Spec.OTelCollectorUrl); err != nil {
-			r.Log.Error(err, "unable to initialize OTel tracer options")
-			return ctrl.Result{Requeue: true, RequeueAfter: 10 * time.Second}, err
-		}
+	if err := otelConfig.InitOtelCollector(config.Spec.OTelCollectorUrl); err != nil {
+		r.Log.Error(err, "unable to initialize OTel tracer options")
+		return ctrl.Result{Requeue: true, RequeueAfter: 10 * time.Second}, err
 	}
 	return ctrl.Result{}, nil
 }

diff --git a/operator/controllers/options/keptnconfig_controller_test.go b/operator/controllers/options/keptnconfig_controller_test.go
@@ -198,17 +198,14 @@ func TestKeptnConfigReconciler_reconcileOtelCollectorUrl(t *testing.T) {
 				Client: nil,
 				Scheme: nil,
 				Log:    ctrl.Log.WithName("test-keptn-config-controller"),
-				LastAppliedSpec: &optionsv1alpha1.KeptnConfigSpec{
-					OTelCollectorUrl: "some-url",
-				},
 			},
 			args: args{
 				config: &optionsv1alpha1.KeptnConfig{
 					ObjectMeta: metav1.ObjectMeta{
 						Name: "test-config",
 					},
 					Spec: optionsv1alpha1.KeptnConfigSpec{
-						OTelCollectorUrl: "some-url",
+						OTelCollectorUrl: "",
 					},
 				},
 			},