fix(lifecycle-operator): fix app deployment span structure (keptn#2352)

Signed-off-by: Florian Bacher <[email protected]>
bacherfl · Nov 2, 2023 · 64c1919 · 64c1919
1 parent 045b359
commit 64c1919
Show file tree

Hide file tree

Showing 10 changed files with 43 additions and 108 deletions.
diff --git a/lifecycle-operator/apis/lifecycle/v1alpha3/keptnapp_test.go b/lifecycle-operator/apis/lifecycle/v1alpha3/keptnapp_test.go
@@ -24,12 +24,11 @@ func TestKeptnApp(t *testing.T) {
 	appVersionName := app.GetAppVersionName()
 	require.Equal(t, "app-version-6b86b273", appVersionName)
 
-	appVersion := app.GenerateAppVersion("prev", map[string]string{})
+	appVersion := app.GenerateAppVersion("prev")
 	require.Equal(t, KeptnAppVersion{
 		ObjectMeta: metav1.ObjectMeta{
-			Annotations: map[string]string{},
-			Name:        "app-version-6b86b273",
-			Namespace:   "namespace",
+			Name:      "app-version-6b86b273",
+			Namespace: "namespace",
 		},
 		Spec: KeptnAppVersionSpec{
 			KeptnAppSpec: KeptnAppSpec{

diff --git a/lifecycle-operator/apis/lifecycle/v1alpha3/keptnapp_types.go b/lifecycle-operator/apis/lifecycle/v1alpha3/keptnapp_types.go
@@ -109,12 +109,11 @@ func (a KeptnApp) SetSpanAttributes(span trace.Span) {
 	span.SetAttributes(a.GetSpanAttributes()...)
 }
 
-func (a KeptnApp) GenerateAppVersion(previousVersion string, traceContextCarrier map[string]string) KeptnAppVersion {
+func (a KeptnApp) GenerateAppVersion(previousVersion string) KeptnAppVersion {
 	return KeptnAppVersion{
 		ObjectMeta: metav1.ObjectMeta{
-			Annotations: traceContextCarrier,
-			Name:        a.GetAppVersionName(),
-			Namespace:   a.Namespace,
+			Name:      a.GetAppVersionName(),
+			Namespace: a.Namespace,
 		},
 		Spec: KeptnAppVersionSpec{
 			KeptnAppSpec:    a.Spec,

diff --git a/lifecycle-operator/controllers/common/telemetry/spanhandler.go b/lifecycle-operator/controllers/common/telemetry/spanhandler.go
@@ -45,9 +45,11 @@ func (r *SpanHandler) GetSpan(ctx context.Context, tracer trace.Tracer, reconcil
 	childCtx, span := tracer.Start(ctx, spanName, trace.WithSpanKind(trace.SpanKindConsumer))
 	piWrapper.SetSpanAttributes(span)
 
-	traceContextCarrier := propagation.MapCarrier{}
-	otel.GetTextMapPropagator().Inject(childCtx, traceContextCarrier)
-	piWrapper.SetPhaseTraceID(phase, traceContextCarrier)
+	if phase != "" {
+		traceContextCarrier := propagation.MapCarrier{}
+		otel.GetTextMapPropagator().Inject(childCtx, traceContextCarrier)
+		piWrapper.SetPhaseTraceID(phase, traceContextCarrier)
+	}
 
 	r.bindCRDSpan[spanKey] = keptnSpanCtx{
 		Span: span,

diff --git a/lifecycle-operator/controllers/lifecycle/keptnapp/controller.go b/lifecycle-operator/controllers/lifecycle/keptnapp/controller.go
@@ -25,11 +25,9 @@ import (
 	"github.com/keptn/lifecycle-toolkit/lifecycle-operator/apis/lifecycle/v1alpha3/common"
 	operatorcommon "github.com/keptn/lifecycle-toolkit/lifecycle-operator/common"
 	controllercommon "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common"
-	"github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common/telemetry"
 	controllererrors "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/errors"
 	"go.opentelemetry.io/otel"
 	"go.opentelemetry.io/otel/propagation"
-	"go.opentelemetry.io/otel/trace"
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
@@ -41,15 +39,12 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 )
 
-const traceComponentName = "keptn/lifecycle-operator/app"
-
 // KeptnAppReconciler reconciles a KeptnApp object
 type KeptnAppReconciler struct {
 	client.Client
-	Scheme        *runtime.Scheme
-	EventSender   controllercommon.IEvent
-	Log           logr.Logger
-	TracerFactory telemetry.TracerFactory
+	Scheme      *runtime.Scheme
+	EventSender controllercommon.IEvent
+	Log         logr.Logger
 }
 
 // +kubebuilder:rbac:groups=lifecycle.keptn.sh,resources=keptnapps,verbs=get;list;watch;create;update;patch;delete
@@ -129,25 +124,14 @@ func (r *KeptnAppReconciler) SetupWithManager(mgr ctrl.Manager) error {
 }
 
 func (r *KeptnAppReconciler) createAppVersion(ctx context.Context, app *klcv1alpha3.KeptnApp) (*klcv1alpha3.KeptnAppVersion, error) {
-	ctxAppTrace, spanAppTrace := r.getTracer().Start(ctx, app.GetAppVersionName(), trace.WithNewRoot(), trace.WithSpanKind(trace.SpanKindServer))
-	defer spanAppTrace.End()
-
-	app.SetSpanAttributes(spanAppTrace)
-
-	// create TraceContext
-	// follow up with a Keptn propagator that JSON-encoded the OTel map into our own key
-	traceContextCarrier := propagation.MapCarrier{}
-	otel.GetTextMapPropagator().Inject(ctx, traceContextCarrier)
-	appTraceContextCarrier := propagation.MapCarrier{}
-	otel.GetTextMapPropagator().Inject(ctxAppTrace, appTraceContextCarrier)
 
 	previousVersion := ""
 	if app.Spec.Version != app.Status.CurrentVersion {
 		previousVersion = app.Status.CurrentVersion
 	}
 
-	appVersion := app.GenerateAppVersion(previousVersion, traceContextCarrier)
-	appVersion.Spec.TraceId = appTraceContextCarrier
+	appVersion := app.GenerateAppVersion(previousVersion)
+
 	err := controllerutil.SetControllerReference(app, &appVersion, r.Scheme)
 	if err != nil {
 		r.Log.Error(err, "could not set controller reference for AppVersion: "+appVersion.Name)
@@ -188,7 +172,3 @@ func (r *KeptnAppReconciler) deprecateAppVersions(ctx context.Context, app *klcv
 	}
 	return lastResultErr
 }
-
-func (r *KeptnAppReconciler) getTracer() telemetry.ITracer {
-	return r.TracerFactory.GetTracer(traceComponentName)
-}
diff --git a/lifecycle-operator/controllers/lifecycle/keptnapp/controller_test.go b/lifecycle-operator/controllers/lifecycle/keptnapp/controller_test.go
@@ -12,7 +12,6 @@ import (
 	"github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common/fake"
 	"github.com/magiconair/properties/assert"
 	"github.com/stretchr/testify/require"
-	"go.opentelemetry.io/otel/trace"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/kubernetes/scheme"
@@ -37,7 +36,7 @@ func TestKeptnAppReconciler_createAppVersionSuccess(t *testing.T) {
 		},
 		Status: lfcv1alpha3.KeptnAppStatus{},
 	}
-	r, _, _ := setupReconciler()
+	r, _ := setupReconciler()
 
 	appVersion, err := r.createAppVersion(context.TODO(), app)
 	if err != nil {
@@ -62,7 +61,7 @@ func TestKeptnAppReconciler_createAppVersionWithLongName(t *testing.T) {
 			Version: "version",
 		},
 	}
-	r, _, _ := setupReconciler()
+	r, _ := setupReconciler()
 
 	appVersion, err := r.createAppVersion(context.Background(), app)
 	if err != nil {
@@ -117,7 +116,7 @@ func TestKeptnAppReconciler_reconcile(t *testing.T) {
 	app := controllercommon.GetApp("myapp")
 	appfin := controllercommon.GetApp("myfinishedapp")
 	appver := controllercommon.ReturnAppVersion("default", "myfinishedapp", "1.0.0-6b86b273", nil, lfcv1alpha3.KeptnAppVersionStatus{Status: apicommon.StateSucceeded})
-	r, _, tracer := setupReconciler(app, appfin, appver)
+	r, _ := setupReconciler(app, appfin, appver)
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -135,10 +134,6 @@ func TestKeptnAppReconciler_reconcile(t *testing.T) {
 
 		})
 	}
-
-	// check correct traces
-	assert.Equal(t, len(tracer.StartCalls()), 1)
-	assert.Equal(t, tracer.StartCalls()[0].SpanName, "myapp-1.0.0-6b86b273")
 }
 
 func TestKeptnAppReconciler_deprecateAppVersions(t *testing.T) {
@@ -152,7 +147,7 @@ func TestKeptnAppReconciler_deprecateAppVersions(t *testing.T) {
 			Namespace: "default",
 		},
 	}
-	r, _, _ := setupReconciler(app, appVersion)
+	r, _ := setupReconciler(app, appVersion)
 
 	_, err := r.Reconcile(context.TODO(), ctrl.Request{
 		NamespacedName: types.NamespacedName{
@@ -172,31 +167,21 @@ func TestKeptnAppReconciler_deprecateAppVersions(t *testing.T) {
 	require.Equal(t, apicommon.StateDeprecated, keptnappversion.Status.Status)
 }
 
-func setupReconciler(objs ...client.Object) (*KeptnAppReconciler, chan string, *fake.ITracerMock) {
+func setupReconciler(objs ...client.Object) (*KeptnAppReconciler, chan string) {
 	// setup logger
 	opts := zap.Options{
 		Development: true,
 	}
 	ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts)))
 
-	// fake a tracer
-	tr := &fake.ITracerMock{StartFunc: func(ctx context.Context, spanName string, opts ...trace.SpanStartOption) (context.Context, trace.Span) {
-		return ctx, trace.SpanFromContext(ctx)
-	}}
-
-	tf := &fake.TracerFactoryMock{GetTracerFunc: func(name string) trace.Tracer {
-		return tr
-	}}
-
 	fakeClient := fake.NewClient(objs...)
 
 	recorder := record.NewFakeRecorder(100)
 	r := &KeptnAppReconciler{
-		Client:        fakeClient,
-		Scheme:        scheme.Scheme,
-		EventSender:   controllercommon.NewK8sSender(recorder),
-		Log:           ctrl.Log.WithName("test-appController"),
-		TracerFactory: tf,
+		Client:      fakeClient,
+		Scheme:      scheme.Scheme,
+		EventSender: controllercommon.NewK8sSender(recorder),
+		Log:         ctrl.Log.WithName("test-appController"),
 	}
-	return r, recorder.Events, tr
+	return r, recorder.Events
 }
diff --git a/lifecycle-operator/main.go b/lifecycle-operator/main.go
@@ -235,11 +235,10 @@ func main() {
 	appLogger := ctrl.Log.WithName("KeptnApp Controller").V(env.KeptnAppControllerLogLevel)
 	appRecorder := mgr.GetEventRecorderFor("keptnapp-controller")
 	appReconciler := &keptnapp.KeptnAppReconciler{
-		Client:        mgr.GetClient(),
-		Scheme:        mgr.GetScheme(),
-		Log:           appLogger,
-		EventSender:   controllercommon.NewEventMultiplexer(appLogger, appRecorder, ceClient),
-		TracerFactory: telemetry.GetOtelInstance(),
+		Client:      mgr.GetClient(),
+		Scheme:      mgr.GetScheme(),
+		Log:         appLogger,
+		EventSender: controllercommon.NewEventMultiplexer(appLogger, appRecorder, ceClient),
 	}
 	if err = (appReconciler).SetupWithManager(mgr); err != nil {
 		setupLog.Error(err, "unable to create controller", "controller", "KeptnApp")

diff --git a/lifecycle-operator/test/component/app/app_suite_test.go b/lifecycle-operator/test/component/app/app_suite_test.go
@@ -11,8 +11,6 @@ import (
 	"github.com/keptn/lifecycle-toolkit/lifecycle-operator/test/component/common"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
-	otelsdk "go.opentelemetry.io/otel/sdk/trace"
-	sdktest "go.opentelemetry.io/otel/sdk/trace/tracetest"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	// nolint:gci
@@ -25,24 +23,21 @@ func TestApp(t *testing.T) {
 }
 
 var (
-	k8sManager   ctrl.Manager
-	tracer       *otelsdk.TracerProvider
-	k8sClient    client.Client
-	ctx          context.Context
-	spanRecorder *sdktest.SpanRecorder
+	k8sManager ctrl.Manager
+	k8sClient  client.Client
+	ctx        context.Context
 )
 
 var _ = BeforeSuite(func() {
 	var readyToStart chan struct{}
-	ctx, k8sManager, tracer, spanRecorder, k8sClient, readyToStart = common.InitSuite()
+	ctx, k8sManager, _, _, k8sClient, readyToStart = common.InitSuite()
 
 	// //setup controllers here
 	controller := &keptnapp.KeptnAppReconciler{
-		Client:        k8sManager.GetClient(),
-		Scheme:        k8sManager.GetScheme(),
-		EventSender:   controllercommon.NewK8sSender(k8sManager.GetEventRecorderFor("test-app-controller")),
-		Log:           GinkgoLogr,
-		TracerFactory: &common.TracerFactory{Tracer: tracer},
+		Client:      k8sManager.GetClient(),
+		Scheme:      k8sManager.GetScheme(),
+		EventSender: controllercommon.NewK8sSender(k8sManager.GetEventRecorderFor("test-app-controller")),
+		Log:         GinkgoLogr,
 	}
 	Eventually(controller.SetupWithManager(k8sManager)).WithTimeout(30 * time.Second).WithPolling(time.Second).Should(Succeed())
 	close(readyToStart)

diff --git a/lifecycle-operator/test/component/app/app_test.go b/lifecycle-operator/test/component/app/app_test.go
@@ -4,12 +4,9 @@ import (
 	"fmt"
 
 	klcv1alpha3 "github.com/keptn/lifecycle-toolkit/lifecycle-operator/apis/lifecycle/v1alpha3"
-	apicommon "github.com/keptn/lifecycle-toolkit/lifecycle-operator/apis/lifecycle/v1alpha3/common"
 	"github.com/keptn/lifecycle-toolkit/lifecycle-operator/test/component/common"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
-	otelsdk "go.opentelemetry.io/otel/sdk/trace"
-	sdktest "go.opentelemetry.io/otel/sdk/trace/tracetest"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apiserver/pkg/storage/names"
 )
@@ -41,34 +38,17 @@ var _ = Describe("App", Ordered, func() {
 			It("should update the spans", func() {
 				By("creating a new app version")
 				common.AssertResourceUpdated(ctx, k8sClient, instance)
-				assertAppSpan(instance, spanRecorder)
-				fmt.Println("spanned ", instance.Name)
 			})
 
 		})
 		AfterEach(func() {
 			// Remember to clean up the cluster after each test
 			common.DeleteAppInCluster(ctx, k8sClient, instance)
-			// Reset span recorder after each spec
-			common.ResetSpanRecords(tracer, spanRecorder)
 		})
 
 	})
 })
 
-func assertAppSpan(instance *klcv1alpha3.KeptnApp, spanRecorder *sdktest.SpanRecorder) {
-	By("Comparing spans")
-	var spans []otelsdk.ReadOnlySpan
-	Eventually(func() bool {
-		spans = spanRecorder.Ended()
-		return len(spans) >= 1
-	}, "10s").Should(BeTrue())
-
-	Expect(spans[0].Name()).To(Equal(instance.GetAppVersionName()))
-	Expect(spans[0].Attributes()).To(ContainElement(apicommon.AppName.String(instance.Name)))
-	Expect(spans[0].Attributes()).To(ContainElement(apicommon.AppVersion.String(instance.Spec.Version)))
-}
-
 func createInstanceInCluster(name string, namespace string, version string) *klcv1alpha3.KeptnApp {
 	instance := &klcv1alpha3.KeptnApp{
 		ObjectMeta: metav1.ObjectMeta{

diff --git a/lifecycle-operator/test/component/load/load_suite_test.go b/lifecycle-operator/test/component/load/load_suite_test.go
@@ -11,7 +11,6 @@ import (
 	"github.com/keptn/lifecycle-toolkit/lifecycle-operator/test/component/common"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
-	otelsdk "go.opentelemetry.io/otel/sdk/trace"
 	sdktest "go.opentelemetry.io/otel/sdk/trace/tracetest"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -26,23 +25,21 @@ func TestLoad(t *testing.T) {
 
 var (
 	k8sManager   ctrl.Manager
-	tracer       *otelsdk.TracerProvider
 	k8sClient    client.Client
 	ctx          context.Context
 	spanRecorder *sdktest.SpanRecorder
 )
 
 var _ = BeforeSuite(func() {
 	var readyToStart chan struct{}
-	ctx, k8sManager, tracer, spanRecorder, k8sClient, readyToStart = common.InitSuite()
+	ctx, k8sManager, _, spanRecorder, k8sClient, readyToStart = common.InitSuite()
 
 	// //setup controllers here
 	controller := &keptnapp.KeptnAppReconciler{
-		Client:        k8sManager.GetClient(),
-		Scheme:        k8sManager.GetScheme(),
-		EventSender:   controllercommon.NewK8sSender(k8sManager.GetEventRecorderFor("load-app-controller")),
-		Log:           GinkgoLogr,
-		TracerFactory: &common.TracerFactory{Tracer: tracer},
+		Client:      k8sManager.GetClient(),
+		Scheme:      k8sManager.GetScheme(),
+		EventSender: controllercommon.NewK8sSender(k8sManager.GetEventRecorderFor("load-app-controller")),
+		Log:         GinkgoLogr,
 	}
 	Eventually(controller.SetupWithManager(k8sManager)).WithTimeout(30 * time.Second).WithPolling(time.Second).Should(Succeed())
 	close(readyToStart)

diff --git a/lifecycle-operator/test/component/load/load_test.go b/lifecycle-operator/test/component/load/load_test.go
@@ -58,7 +58,6 @@ var _ = Describe("Load", Ordered, func() {
 		for _, app := range apps {
 			// Remember to clean up the cluster after each test
 			common.DeleteAppInCluster(ctx, k8sClient, app)
-			common.ResetSpanRecords(tracer, spanRecorder)
 		}
 	})
 	JustAfterEach(func() { // this is an example of how to add logs to report