fix(operator): Use pointer receiver for SpanHandler methods to ensure…

… span map is populated; thread safety via mutex (#288)

operator/api/v1alpha1/keptnappversion_types.go

@@ -234,3 +234,12 @@ func (v KeptnAppVersion) GetVersion() string {
 func (v KeptnAppVersion) GetSpanName(phase string) string {
 	return fmt.Sprintf("%s.%s.%s.%s", v.Spec.TraceId, v.Spec.AppName, v.Spec.Version, phase)
 }
+
+func (v KeptnAppVersion) GetSpanAttributes() []attribute.KeyValue {
+	return []attribute.KeyValue{
+		common.AppName.String(v.Spec.AppName),
+		common.AppVersion.String(v.Spec.Version),
+		common.WorkloadVersion.String(v.Spec.PreviousVersion),
+		common.WorkloadVersion.String(v.Namespace),
+	}
+}

operator/api/v1alpha1/keptnworkloadinstance_types.go

@@ -271,3 +271,12 @@ func (i KeptnWorkloadInstance) GetVersion() string {
 func (v KeptnWorkloadInstance) GetSpanName(phase string) string {
 	return fmt.Sprintf("%s.%s.%s.%s", v.Spec.TraceId, v.Spec.AppName, v.Spec.Version, phase)
 }
+
+func (i KeptnWorkloadInstance) GetSpanAttributes() []attribute.KeyValue {
+	return []attribute.KeyValue{
+		common.AppName.String(i.Spec.AppName),
+		common.WorkloadName.String(i.Spec.WorkloadName),
+		common.WorkloadVersion.String(i.Spec.Version),
+		common.WorkloadNamespace.String(i.Namespace),
+	}
+}

operator/controllers/common/interfaces.go

@@ -16,6 +16,7 @@ type PhaseItem interface {
 	SetCurrentPhase(string)
 	GetVersion() string
 	GetMetricsAttributes() []attribute.KeyValue
+	GetSpanAttributes() []attribute.KeyValue
 	GetSpanName(phase string) string
 	Complete()
 }
@@ -63,3 +64,7 @@ func (pw PhaseItemWrapper) GetVersion() string {
 func (pw PhaseItemWrapper) GetSpanName(phase string) string {
 	return pw.Obj.GetSpanName(phase)
 }
+
+func (pw PhaseItemWrapper) GetSpanAttributes() []attribute.KeyValue {
+	return pw.Obj.GetSpanAttributes()
+}

operator/controllers/common/phasehandler.go

@@ -18,7 +18,7 @@ type PhaseHandler struct {
 	client.Client
 	Recorder    record.EventRecorder
 	Log         logr.Logger
-	SpanHandler SpanHandler
+	SpanHandler *SpanHandler
 }
 
 type PhaseResult struct {

operator/controllers/common/spanhandler.go

@@ -2,37 +2,47 @@ package common
 
 import (
 	"context"
+	"sync"
 
 	"go.opentelemetry.io/otel/trace"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
 type SpanHandler struct {
 	bindCRDSpan map[string]trace.Span
+	mtx         sync.Mutex
 }
 
-func (r SpanHandler) GetSpan(ctx context.Context, tracer trace.Tracer, reconcileObject client.Object, phase string) (context.Context, trace.Span, error) {
+func (r *SpanHandler) GetSpan(ctx context.Context, tracer trace.Tracer, reconcileObject client.Object, phase string) (context.Context, trace.Span, error) {
 	piWrapper, err := NewPhaseItemWrapperFromClientObject(reconcileObject)
 	if err != nil {
 		return nil, nil, err
 	}
 	appvName := piWrapper.GetSpanName(phase)
+	r.mtx.Lock()
+	defer r.mtx.Unlock()
 	if r.bindCRDSpan == nil {
 		r.bindCRDSpan = make(map[string]trace.Span)
 	}
 	if span, ok := r.bindCRDSpan[appvName]; ok {
 		return ctx, span, nil
 	}
 	ctx, span := tracer.Start(ctx, phase, trace.WithSpanKind(trace.SpanKindConsumer))
+	attributes := piWrapper.GetSpanAttributes()
+	for _, attribute := range attributes {
+		span.SetAttributes(attribute)
+	}
 	r.bindCRDSpan[appvName] = span
 	return ctx, span, nil
 }
 
-func (r SpanHandler) UnbindSpan(reconcileObject client.Object, phase string) error {
+func (r *SpanHandler) UnbindSpan(reconcileObject client.Object, phase string) error {
 	piWrapper, err := NewPhaseItemWrapperFromClientObject(reconcileObject)
 	if err != nil {
 		return err
 	}
+	r.mtx.Lock()
+	defer r.mtx.Unlock()
 	delete(r.bindCRDSpan, piWrapper.GetSpanName(phase))
 	return nil
 }

operator/controllers/common/spanhandler_test.go

@@ -0,0 +1,30 @@
+package common
+
+import (
+	"context"
+	"github.com/keptn/lifecycle-toolkit/operator/api/v1alpha1"
+	"github.com/stretchr/testify/require"
+	"go.opentelemetry.io/otel"
+	"testing"
+)
+
+func TestSpanHandler_GetAndUnbindSpan(t *testing.T) {
+	r := SpanHandler{}
+
+	tracer := otel.Tracer("keptn/operator/workloadinstance")
+
+	wi := &v1alpha1.KeptnWorkloadInstance{}
+	ctx, span, err := r.GetSpan(context.TODO(), tracer, wi, "pre")
+
+	require.Nil(t, err)
+	require.NotNil(t, t, span)
+	require.NotNil(t, ctx)
+
+	require.Len(t, r.bindCRDSpan, 1)
+
+	err = r.UnbindSpan(wi, "pre")
+
+	require.Nil(t, err)
+
+	require.Empty(t, r.bindCRDSpan)
+}

operator/controllers/keptnappversion/controller.go

@@ -53,7 +53,7 @@ type KeptnAppVersionReconciler struct {
 	Recorder    record.EventRecorder
 	Tracer      trace.Tracer
 	Meters      common.KeptnMeters
-	SpanHandler controllercommon.SpanHandler
+	SpanHandler *controllercommon.SpanHandler
 }
 
 //+kubebuilder:rbac:groups=lifecycle.keptn.sh,resources=keptnappversions,verbs=get;list;watch;create;update;patch;delete

operator/controllers/keptnworkloadinstance/controller.go

@@ -53,7 +53,7 @@ type KeptnWorkloadInstanceReconciler struct {
 	Log         logr.Logger
 	Meters      common.KeptnMeters
 	Tracer      trace.Tracer
-	SpanHandler controllercommon.SpanHandler
+	SpanHandler *controllercommon.SpanHandler
 }
 
 //+kubebuilder:rbac:groups=lifecycle.keptn.sh,resources=keptnworkloadinstances,verbs=get;list;watch;create;update;patch;delete

operator/main.go

@@ -255,7 +255,7 @@ func main() {
 		os.Exit(1)
 	}
 
-	spanHandler := controllercommon.SpanHandler{}
+	spanHandler := &controllercommon.SpanHandler{}
 
 	if !disableWebhook {
 		mgr.GetWebhookServer().Register("/mutate-v1-pod", &webhook.Admission{