fix(operator): use correct parent/child span relationship (#418)

Co-authored-by: RealAnna <anna.reale@dynatrace.com>
Fixes #336

operator/controllers/common/ITracer.go

@@ -2,7 +2,5 @@ package common
 
 import "go.opentelemetry.io/otel/trace"
 
-// TODO Autogenerated mock uses pointers, I changed it manually
-
 //go:generate moq -pkg fake -skip-ensure -out ./fake/tracer.go . ITracer
 type ITracer = trace.Tracer

operator/controllers/common/phasehandler.go

@@ -18,7 +18,7 @@ type PhaseHandler struct {
 	client.Client
 	Recorder    record.EventRecorder
 	Log         logr.Logger
-	SpanHandler *SpanHandler
+	SpanHandler ISpanHandler
 }
 
 type PhaseResult struct {
@@ -44,14 +44,14 @@ func (r PhaseHandler) HandlePhase(ctx context.Context, ctxTrace context.Context,
 	piWrapper.SetCurrentPhase(phase.ShortName)
 
 	r.Log.Info(phase.LongName + " not finished")
-	ctxTrace, spanTrace, err := r.SpanHandler.GetSpan(ctxTrace, tracer, reconcileObject, phase.ShortName)
+	_, spanAppTrace, err := r.SpanHandler.GetSpan(ctxTrace, tracer, reconcileObject, phase.ShortName)
 	if err != nil {
 		r.Log.Error(err, "could not get span")
 	}
 
 	state, err := reconcilePhase()
 	if err != nil {
-		spanTrace.AddEvent(phase.LongName + " could not get reconciled")
+		spanAppTrace.AddEvent(phase.LongName + " could not get reconciled")
 		RecordEvent(r.Recorder, phase, "Warning", reconcileObject, "ReconcileErrored", "could not get reconciled", piWrapper.GetVersion())
 		span.SetStatus(codes.Error, err.Error())
 		return &PhaseResult{Continue: false, Result: requeueResult}, err
@@ -64,7 +64,7 @@ func (r PhaseHandler) HandlePhase(ctx context.Context, ctxTrace context.Context,
 	defer func(oldStatus common.KeptnState, oldPhase string, reconcileObject client.Object) {
 		piWrapper, _ := NewPhaseItemWrapperFromClientObject(reconcileObject)
 		if oldStatus != piWrapper.GetState() || oldPhase != piWrapper.GetCurrentPhase() {
-			ctx, spanTrace, err = r.SpanHandler.GetSpan(ctxTrace, tracer, reconcileObject, piWrapper.GetCurrentPhase())
+			ctx, spanAppTrace, err = r.SpanHandler.GetSpan(ctxTrace, tracer, reconcileObject, piWrapper.GetCurrentPhase())
 			if err != nil {
 				r.Log.Error(err, "could not get span")
 			}
@@ -78,9 +78,9 @@ func (r PhaseHandler) HandlePhase(ctx context.Context, ctxTrace context.Context,
 		if state.IsFailed() {
 			piWrapper.Complete()
 			piWrapper.SetState(common.StateFailed)
-			spanTrace.AddEvent(phase.LongName + " has failed")
-			spanTrace.SetStatus(codes.Error, "Failed")
-			spanTrace.End()
+			spanAppTrace.AddEvent(phase.LongName + " has failed")
+			spanAppTrace.SetStatus(codes.Error, "Failed")
+			spanAppTrace.End()
 			if err := r.SpanHandler.UnbindSpan(reconcileObject, phase.ShortName); err != nil {
 				r.Log.Error(err, "cannot unbind span")
 			}
@@ -90,9 +90,9 @@ func (r PhaseHandler) HandlePhase(ctx context.Context, ctxTrace context.Context,
 		}
 
 		piWrapper.SetState(common.StateSucceeded)
-		spanTrace.AddEvent(phase.LongName + " has succeeded")
-		spanTrace.SetStatus(codes.Ok, "Succeeded")
-		spanTrace.End()
+		spanAppTrace.AddEvent(phase.LongName + " has succeeded")
+		spanAppTrace.SetStatus(codes.Ok, "Succeeded")
+		spanAppTrace.End()
 		if err := r.SpanHandler.UnbindSpan(reconcileObject, phase.ShortName); err != nil {
 			r.Log.Error(err, "cannot unbind span")
 		}

operator/controllers/common/spanhandler.go

@@ -8,6 +8,12 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
+//go:generate moq -pkg fake -skip-ensure -out ./fake/spanhandler_mock.go . ISpanHandler
+type ISpanHandler interface {
+	GetSpan(ctx context.Context, tracer trace.Tracer, reconcileObject client.Object, phase string) (context.Context, trace.Span, error)
+	UnbindSpan(reconcileObject client.Object, phase string) error
+}
+
 type SpanHandler struct {
 	bindCRDSpan map[string]trace.Span
 	mtx         sync.Mutex

operator/controllers/common/test_utils.go

@@ -0,0 +1,38 @@
+package common
+
+import (
+	"context"
+	lfcv1alpha1 "github.com/keptn/lifecycle-toolkit/operator/api/v1alpha1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+func AddApp(c client.Client, name string) error {
+	app := &lfcv1alpha1.KeptnApp{
+		TypeMeta: metav1.TypeMeta{},
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      name,
+			Namespace: "default",
+		},
+		Spec: lfcv1alpha1.KeptnAppSpec{
+			Version: "1.0.0",
+		},
+		Status: lfcv1alpha1.KeptnAppStatus{},
+	}
+	return c.Create(context.TODO(), app)
+
+}
+
+func AddAppVersion(c client.Client, name string, status lfcv1alpha1.KeptnAppVersionStatus) error {
+	app := &lfcv1alpha1.KeptnAppVersion{
+		TypeMeta: metav1.TypeMeta{},
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      name,
+			Namespace: "default",
+		},
+		Spec:   lfcv1alpha1.KeptnAppVersionSpec{KeptnAppSpec: lfcv1alpha1.KeptnAppSpec{Version: "1.0.0"}},
+		Status: status,
+	}
+	return c.Create(context.TODO(), app)
+
+}

operator/controllers/keptnapp/controller_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	lfcv1alpha1 "github.com/keptn/lifecycle-toolkit/operator/api/v1alpha1"
 	keptncommon "github.com/keptn/lifecycle-toolkit/operator/api/v1alpha1/common"
+	utils "github.com/keptn/lifecycle-toolkit/operator/controllers/common"
 	"github.com/keptn/lifecycle-toolkit/operator/controllers/common/fake"
 	"github.com/magiconair/properties/assert"
 	"go.opentelemetry.io/otel/trace"
@@ -43,7 +44,7 @@ func TestKeptnAppReconciler_createAppVersionSuccess(t *testing.T) {
 
 }
 
-func TestKeptnAppReconciler_Reconcile(t *testing.T) {
+func TestKeptnAppReconciler_reconcile(t *testing.T) {
 
 	r, eventChannel, tracer := setupReconciler(t)
 
@@ -88,9 +89,9 @@ func TestKeptnAppReconciler_Reconcile(t *testing.T) {
 
 	//setting up fakeclient CRD data
 
-	addApp(r, "myapp")
-	addApp(r, "myfinishedapp")
-	addAppVersion(r, "myfinishedapp-1.0.0", keptncommon.StateSucceeded)
+	utils.AddApp(r.Client, "myapp")
+	utils.AddApp(r.Client, "myfinishedapp")
+	utils.AddAppVersion(r.Client, "myfinishedapp-1.0.0", lfcv1alpha1.KeptnAppVersionStatus{Status: keptncommon.StateSucceeded})
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -146,35 +147,3 @@ func setupReconciler(t *testing.T) (*KeptnAppReconciler, chan string, *fake.ITra
 	}
 	return r, recorder.Events, tr
 }
-
-func addApp(r *KeptnAppReconciler, name string) error {
-	app := &lfcv1alpha1.KeptnApp{
-		TypeMeta: metav1.TypeMeta{},
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      name,
-			Namespace: "default",
-		},
-		Spec: lfcv1alpha1.KeptnAppSpec{
-			Version: "1.0.0",
-		},
-		Status: lfcv1alpha1.KeptnAppStatus{},
-	}
-	return r.Client.Create(context.TODO(), app)
-
-}
-
-func addAppVersion(r *KeptnAppReconciler, name string, status keptncommon.KeptnState) error {
-	app := &lfcv1alpha1.KeptnAppVersion{
-		TypeMeta: metav1.TypeMeta{},
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      name,
-			Namespace: "default",
-		},
-		Spec: lfcv1alpha1.KeptnAppVersionSpec{},
-		Status: lfcv1alpha1.KeptnAppVersionStatus{
-			Status: status,
-		},
-	}
-	return r.Client.Create(context.TODO(), app)
-
-}