chore(lifecycle-operator): remove spans for reconciliation loops, adj…

…ust log levels (#2310)

Signed-off-by: Florian Bacher <florian.bacher@dynatrace.com>

lifecycle-operator/controllers/common/evaluationhandler.go

@@ -111,7 +111,7 @@ func (r EvaluationHandler) ReconcileEvaluations(ctx context.Context, phaseCtx co
 }
 
 //nolint:dupl
-func (r EvaluationHandler) CreateKeptnEvaluation(ctx context.Context, namespace string, reconcileObject client.Object, evaluationCreateAttributes CreateEvaluationAttributes) (string, error) {
+func (r EvaluationHandler) CreateKeptnEvaluation(ctx context.Context, reconcileObject client.Object, evaluationCreateAttributes CreateEvaluationAttributes) (string, error) {
 	piWrapper, err := interfaces.NewPhaseItemWrapperFromClientObject(reconcileObject)
 	if err != nil {
 		return "", err
@@ -163,7 +163,7 @@ func (r EvaluationHandler) setupEvaluations(evaluationCreateAttributes CreateEva
 
 func (r EvaluationHandler) handleEvaluationNotExists(ctx context.Context, phaseCtx context.Context, evaluationCreateAttributes CreateEvaluationAttributes, evaluationName string, piWrapper *interfaces.PhaseItemWrapper, reconcileObject client.Object, evaluation *klcv1alpha3.KeptnEvaluation, evaluationStatus *klcv1alpha3.ItemStatus) error {
 	evaluationCreateAttributes.Definition.Name = evaluationName
-	evaluationName, err := r.CreateKeptnEvaluation(ctx, piWrapper.GetNamespace(), reconcileObject, evaluationCreateAttributes)
+	evaluationName, err := r.CreateKeptnEvaluation(ctx, reconcileObject, evaluationCreateAttributes)
 	if err != nil {
 		return err
 	}

lifecycle-operator/controllers/common/evaluationhandler_test.go

@@ -354,7 +354,7 @@ func TestEvaluationHandler_createEvaluation(t *testing.T) {
 				Tracer:      trace.NewNoopTracerProvider().Tracer("tracer"),
 				Scheme:      scheme.Scheme,
 			}
-			name, err := handler.CreateKeptnEvaluation(context.TODO(), "namespace", tt.object, tt.createAttr)
+			name, err := handler.CreateKeptnEvaluation(context.TODO(), tt.object, tt.createAttr)
 			require.True(t, strings.Contains(name, tt.wantName))
 			require.Equal(t, tt.wantErr, err)
 		})

lifecycle-operator/controllers/common/helperfunctions.go

@@ -94,10 +94,10 @@ func GetEvaluationDefinition(k8sclient client.Client, log logr.Logger, ctx conte
 func getObject(k8sclient client.Client, log logr.Logger, ctx context.Context, definitionName string, namespace string, definition client.Object) error {
 	err := k8sclient.Get(ctx, types.NamespacedName{Name: definitionName, Namespace: namespace}, definition)
 	if err != nil {
-		log.Info("Failed to get resource from application namespace", "resource type", fmt.Sprintf("%T", definition), "Definition name", definitionName, "namespace", namespace)
+		log.Info("Could not find resource in application namespace", "resource type", fmt.Sprintf("%T", definition), "Definition name", definitionName, "namespace", namespace)
 		if k8serrors.IsNotFound(err) {
 			if err := k8sclient.Get(ctx, types.NamespacedName{Name: definitionName, Namespace: config.Instance().GetDefaultNamespace()}, definition); err != nil {
-				log.Info("Failed to get resource from default KLT namespace", "resource type", fmt.Sprintf("%T", definition), "definition name", definitionName)
+				log.Info("Could not find resource in default Keptn namespace", "resource type", fmt.Sprintf("%T", definition), "definition name", definitionName)
 				return err
 			}
 			return nil

lifecycle-operator/controllers/common/phasehandler.go

@@ -27,7 +27,7 @@ type PhaseResult struct {
 	ctrl.Result
 }
 
-func (r PhaseHandler) HandlePhase(ctx context.Context, ctxTrace context.Context, tracer trace.Tracer, reconcileObject client.Object, phase apicommon.KeptnPhaseType, span trace.Span, reconcilePhase func(phaseCtx context.Context) (apicommon.KeptnState, error)) (*PhaseResult, error) {
+func (r PhaseHandler) HandlePhase(ctx context.Context, ctxTrace context.Context, tracer trace.Tracer, reconcileObject client.Object, phase apicommon.KeptnPhaseType, reconcilePhase func(phaseCtx context.Context) (apicommon.KeptnState, error)) (*PhaseResult, error) {
 	requeueResult := ctrl.Result{Requeue: true, RequeueAfter: 5 * time.Second}
 	piWrapper, err := interfaces.NewPhaseItemWrapperFromClientObject(reconcileObject)
 	if err != nil {
@@ -53,7 +53,6 @@ func (r PhaseHandler) HandlePhase(ctx context.Context, ctxTrace context.Context,
 	if err != nil {
 		spanPhaseTrace.AddEvent(phase.LongName + " could not get reconciled")
 		r.EventSender.Emit(phase, "Warning", reconcileObject, apicommon.PhaseStateReconcileError, "could not get reconciled", piWrapper.GetVersion())
-		span.SetStatus(codes.Error, err.Error())
 		return &PhaseResult{Continue: false, Result: requeueResult}, err
 	}
 

lifecycle-operator/controllers/common/phasehandler_test.go

@@ -216,7 +216,7 @@ func TestPhaseHandler(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			result, err := tt.handler.HandlePhase(context.TODO(), context.TODO(), trace.NewNoopTracerProvider().Tracer("tracer"), tt.object, tt.phase, trace.SpanFromContext(context.TODO()), tt.reconcilePhase)
+			result, err := tt.handler.HandlePhase(context.TODO(), context.TODO(), trace.NewNoopTracerProvider().Tracer("tracer"), tt.object, tt.phase, tt.reconcilePhase)
 			require.Equal(t, tt.want, result)
 			require.Equal(t, tt.wantErr, err)
 			require.Equal(t, tt.wantObject.Status.Status, tt.object.Status.Status)

lifecycle-operator/controllers/common/taskhandler.go

@@ -91,17 +91,24 @@ func (r TaskHandler) ReconcileTasks(ctx context.Context, phaseCtx context.Contex
 				&taskStatus,
 			)
 			if err != nil {
-				// log the error, but continue to proceed with other tasks that may be created
-				r.Log.Error(err, "Could not create task", "task", taskDefinitionName)
+				if errors.IsNotFound(err) {
+					r.Log.Info("TaskDefinition for Task not found",
+						"task", taskStatus.Name,
+						"taskDefinition", taskDefinitionName,
+						"namespace", piWrapper.GetNamespace(),
+					)
+				} else {
+					// log the error, but continue to proceed with other tasks that may be created
+					r.Log.Error(err, "Could not create task",
+						"task", taskStatus.Name,
+						"taskDefinition", taskDefinitionName,
+						"namespace", piWrapper.GetNamespace(),
+					)
+				}
 				continue
 			}
 		} else {
-			r.handleTaskExists(
-				phaseCtx,
-				piWrapper,
-				task,
-				&taskStatus,
-			)
+			r.handleTaskExists(phaseCtx, task, &taskStatus)
 		}
 		// Update state of the Check
 		newStatus = append(newStatus, taskStatus)
@@ -160,7 +167,6 @@ func (r TaskHandler) setupTasks(taskCreateAttributes CreateTaskAttributes, piWra
 func (r TaskHandler) handleTaskNotExists(ctx context.Context, phaseCtx context.Context, taskCreateAttributes CreateTaskAttributes, taskName string, piWrapper *interfaces.PhaseItemWrapper, reconcileObject client.Object, task *klcv1alpha3.KeptnTask, taskStatus *klcv1alpha3.ItemStatus) error {
 	definition, err := GetTaskDefinition(r.Client, r.Log, ctx, taskName, piWrapper.GetNamespace())
 	if err != nil {
-		r.Log.Error(err, "could not find KeptnTaskDefinition")
 		return controllererrors.ErrCannotGetKeptnTaskDefinition
 	}
 	taskCreateAttributes.Definition = *definition
@@ -178,7 +184,7 @@ func (r TaskHandler) handleTaskNotExists(ctx context.Context, phaseCtx context.C
 	return nil
 }
 
-func (r TaskHandler) handleTaskExists(phaseCtx context.Context, piWrapper *interfaces.PhaseItemWrapper, task *klcv1alpha3.KeptnTask, taskStatus *klcv1alpha3.ItemStatus) {
+func (r TaskHandler) handleTaskExists(phaseCtx context.Context, task *klcv1alpha3.KeptnTask, taskStatus *klcv1alpha3.ItemStatus) {
 	_, spanTaskTrace, err := r.SpanHandler.GetSpan(phaseCtx, r.Tracer, task, "")
 	if err != nil {
 		r.Log.Error(err, "could not get span")

lifecycle-operator/controllers/errors/errors.go

@@ -1,6 +1,10 @@
 package errors
 
-import "fmt"
+import (
+	"fmt"
+
+	"github.com/pkg/errors"
+)
 
 var ErrCannotWrapToPhaseItem = fmt.Errorf("provided object does not implement PhaseItem interface")
 var ErrCannotWrapToListItem = fmt.Errorf("provided object does not implement ListItem interface")
@@ -15,6 +19,7 @@ var ErrCannotMarshalParams = fmt.Errorf("could not marshal parameters")
 var ErrNoTaskDefinitionSpec = fmt.Errorf("the TaskDefinition specs are empty")
 var ErrUnsupportedWorkloadVersionResourceReference = fmt.Errorf("unsupported Resource Reference")
 var ErrCannotGetKeptnTaskDefinition = fmt.Errorf("cannot retrieve KeptnTaskDefinition")
+var ErrNoMatchingAppVersionFound = fmt.Errorf("no matching KeptnAppVersion found")
 
 var ErrCannotRetrieveConfigMsg = "could not retrieve KeptnConfig: %w"
 var ErrCannotRetrieveInstancesMsg = "could not retrieve instances: %w"
@@ -27,3 +32,12 @@ var ErrNoConfigMapMsg = "no ConfigMap specified or HTTP source specified in Task
 var ErrCannotGetFunctionConfigMap = "could not get function configMap: %w"
 var ErrCannotFetchAppVersionForWorkloadVersionMsg = "could not fetch AppVersion for KeptnWorkloadVersion: "
 var ErrCouldNotUnbindSpan = "could not unbind span for %s"
+
+// IgnoreReferencedResourceNotFound returns nil on NotFound errors.
+// All other values that are not NotFound errors or nil are returned unmodified.
+func IgnoreReferencedResourceNotFound(err error) error {
+	if errors.Is(err, ErrNoMatchingAppVersionFound) {
+		return nil
+	}
+	return err
+}

lifecycle-operator/controllers/lifecycle/keptnapp/controller.go

@@ -28,7 +28,6 @@ import (
 	"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/codes"
 	"go.opentelemetry.io/otel/propagation"
 	"go.opentelemetry.io/otel/trace"
 	"k8s.io/apimachinery/pkg/api/errors"
@@ -85,11 +84,6 @@ func (r *KeptnAppReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
 	traceContextCarrier := propagation.MapCarrier(app.Annotations)
 	ctx = otel.GetTextMapPropagator().Extract(ctx, traceContextCarrier)
 
-	ctx, span := r.getTracer().Start(ctx, "reconcile_app", trace.WithSpanKind(trace.SpanKindConsumer))
-	defer span.End()
-
-	app.SetSpanAttributes(span)
-
 	r.Log.Info("Reconciling Keptn App", "app", app.Name)
 
 	appVersion := &klcv1alpha3.KeptnAppVersion{}
@@ -100,13 +94,11 @@ func (r *KeptnAppReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
 	if errors.IsNotFound(err) {
 		appVersion, err := r.createAppVersion(ctx, app)
 		if err != nil {
-			span.SetStatus(codes.Error, err.Error())
 			return reconcile.Result{}, err
 		}
 		err = r.Client.Create(ctx, appVersion)
 		if err != nil {
 			r.Log.Error(err, "could not create AppVersion")
-			span.SetStatus(codes.Error, err.Error())
 			r.EventSender.Emit(common.PhaseCreateAppVersion, "Warning", appVersion, common.PhaseStateFailed, "Could not create KeptnAppVersion", appVersion.Spec.Version)
 			return ctrl.Result{}, err
 		}
@@ -123,7 +115,6 @@ func (r *KeptnAppReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
 	}
 	if err != nil {
 		r.Log.Error(err, "could not get AppVersion")
-		span.SetStatus(codes.Error, err.Error())
 		return ctrl.Result{}, err
 	}
 
@@ -138,13 +129,9 @@ func (r *KeptnAppReconciler) SetupWithManager(mgr ctrl.Manager) error {
 }
 
 func (r *KeptnAppReconciler) createAppVersion(ctx context.Context, app *klcv1alpha3.KeptnApp) (*klcv1alpha3.KeptnAppVersion, error) {
-	ctx, span := r.getTracer().Start(ctx, "create_app_version", trace.WithSpanKind(trace.SpanKindProducer))
-	defer span.End()
-
 	ctxAppTrace, spanAppTrace := r.getTracer().Start(ctx, app.GetAppVersionName(), trace.WithNewRoot(), trace.WithSpanKind(trace.SpanKindServer))
 	defer spanAppTrace.End()
 
-	app.SetSpanAttributes(span)
 	app.SetSpanAttributes(spanAppTrace)
 
 	// create TraceContext

lifecycle-operator/controllers/lifecycle/keptnapp/controller_test.go

@@ -137,14 +137,8 @@ func TestKeptnAppReconciler_reconcile(t *testing.T) {
 	}
 
 	// check correct traces
-	assert.Equal(t, len(tracer.StartCalls()), 4)
-	// case 1 reconcile and create app ver
-	assert.Equal(t, tracer.StartCalls()[0].SpanName, "reconcile_app")
-	assert.Equal(t, tracer.StartCalls()[1].SpanName, "create_app_version")
-	assert.Equal(t, tracer.StartCalls()[2].SpanName, "myapp-1.0.0-6b86b273")
-	// case 2 creates no span because notfound
-	// case 3 reconcile finished crd
-	assert.Equal(t, tracer.StartCalls()[3].SpanName, "reconcile_app")
+	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) {

lifecycle-operator/controllers/lifecycle/keptnappversion/controller.go

@@ -84,8 +84,8 @@ func (r *KeptnAppVersionReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 		return reconcile.Result{}, fmt.Errorf(controllererrors.ErrCannotFetchAppVersionMsg, err)
 	}
 
-	ctx, ctxAppTrace, span, endSpan := r.setupSpansContexts(ctx, appVersion)
-	defer endSpan()
+	ctxAppTrace, completionFunc := r.setupSpansContexts(ctx, appVersion)
+	defer completionFunc()
 
 	phase := apicommon.PhaseAppPreDeployment
 	phaseHandler := controllercommon.PhaseHandler{
@@ -109,7 +109,7 @@ func (r *KeptnAppVersionReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 		reconcilePreDep := func(phaseCtx context.Context) (apicommon.KeptnState, error) {
 			return r.reconcilePrePostDeployment(ctx, phaseCtx, appVersion, apicommon.PreDeploymentCheckType)
 		}
-		result, err := phaseHandler.HandlePhase(ctx, ctxAppTrace, r.getTracer(), appVersion, phase, span, reconcilePreDep)
+		result, err := phaseHandler.HandlePhase(ctx, ctxAppTrace, r.getTracer(), appVersion, phase, reconcilePreDep)
 		if !result.Continue {
 			return result.Result, err
 		}
@@ -120,7 +120,7 @@ func (r *KeptnAppVersionReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 		reconcilePreEval := func(phaseCtx context.Context) (apicommon.KeptnState, error) {
 			return r.reconcilePrePostEvaluation(ctx, phaseCtx, appVersion, apicommon.PreDeploymentEvaluationCheckType)
 		}
-		result, err := phaseHandler.HandlePhase(ctx, ctxAppTrace, r.getTracer(), appVersion, phase, span, reconcilePreEval)
+		result, err := phaseHandler.HandlePhase(ctx, ctxAppTrace, r.getTracer(), appVersion, phase, reconcilePreEval)
 		if !result.Continue {
 			return result.Result, err
 		}
@@ -131,7 +131,7 @@ func (r *KeptnAppVersionReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 		reconcileAppDep := func(phaseCtx context.Context) (apicommon.KeptnState, error) {
 			return r.reconcileWorkloads(ctx, appVersion)
 		}
-		result, err := phaseHandler.HandlePhase(ctx, ctxAppTrace, r.getTracer(), appVersion, phase, span, reconcileAppDep)
+		result, err := phaseHandler.HandlePhase(ctx, ctxAppTrace, r.getTracer(), appVersion, phase, reconcileAppDep)
 		if !result.Continue {
 			return result.Result, err
 		}
@@ -142,7 +142,7 @@ func (r *KeptnAppVersionReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 		reconcilePostDep := func(phaseCtx context.Context) (apicommon.KeptnState, error) {
 			return r.reconcilePrePostDeployment(ctx, phaseCtx, appVersion, apicommon.PostDeploymentCheckType)
 		}
-		result, err := phaseHandler.HandlePhase(ctx, ctxAppTrace, r.getTracer(), appVersion, phase, span, reconcilePostDep)
+		result, err := phaseHandler.HandlePhase(ctx, ctxAppTrace, r.getTracer(), appVersion, phase, reconcilePostDep)
 		if !result.Continue {
 			return result.Result, err
 		}
@@ -153,17 +153,17 @@ func (r *KeptnAppVersionReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 		reconcilePostEval := func(phaseCtx context.Context) (apicommon.KeptnState, error) {
 			return r.reconcilePrePostEvaluation(ctx, phaseCtx, appVersion, apicommon.PostDeploymentEvaluationCheckType)
 		}
-		result, err := phaseHandler.HandlePhase(ctx, ctxAppTrace, r.getTracer(), appVersion, phase, span, reconcilePostEval)
+		result, err := phaseHandler.HandlePhase(ctx, ctxAppTrace, r.getTracer(), appVersion, phase, reconcilePostEval)
 		if !result.Continue {
 			return result.Result, err
 		}
 	}
 
 	// AppVersion is completed at this place
-	return r.finishKeptnAppVersionReconcile(ctx, appVersion, spanAppTrace, span)
+	return r.finishKeptnAppVersionReconcile(ctx, appVersion, spanAppTrace)
 }
 
-func (r *KeptnAppVersionReconciler) finishKeptnAppVersionReconcile(ctx context.Context, appVersion *klcv1alpha3.KeptnAppVersion, spanAppTrace, span trace.Span) (ctrl.Result, error) {
+func (r *KeptnAppVersionReconciler) finishKeptnAppVersionReconcile(ctx context.Context, appVersion *klcv1alpha3.KeptnAppVersion, spanAppTrace trace.Span) (ctrl.Result, error) {
 
 	if !appVersion.IsEndTimeSet() {
 		appVersion.Status.CurrentPhase = apicommon.PhaseCompleted.ShortName
@@ -173,7 +173,6 @@ func (r *KeptnAppVersionReconciler) finishKeptnAppVersionReconcile(ctx context.C
 
 	err := r.Client.Status().Update(ctx, appVersion)
 	if err != nil {
-		span.SetStatus(codes.Error, err.Error())
 		return ctrl.Result{Requeue: true}, err
 	}
 
@@ -195,28 +194,21 @@ func (r *KeptnAppVersionReconciler) finishKeptnAppVersionReconcile(ctx context.C
 	return ctrl.Result{}, nil
 }
 
-func (r *KeptnAppVersionReconciler) setupSpansContexts(ctx context.Context, appVersion *klcv1alpha3.KeptnAppVersion) (context.Context, context.Context, trace.Span, func()) {
+func (r *KeptnAppVersionReconciler) setupSpansContexts(ctx context.Context, appVersion *klcv1alpha3.KeptnAppVersion) (context.Context, func()) {
 	appVersion.SetStartTime()
 
-	traceContextCarrier := propagation.MapCarrier(appVersion.Annotations)
-	ctx = otel.GetTextMapPropagator().Extract(ctx, traceContextCarrier)
-
 	appTraceContextCarrier := propagation.MapCarrier(appVersion.Spec.TraceId)
 	ctxAppTrace := otel.GetTextMapPropagator().Extract(context.TODO(), appTraceContextCarrier)
 
-	ctx, span := r.getTracer().Start(ctx, "reconcile_app_version", trace.WithSpanKind(trace.SpanKindConsumer))
-
 	endFunc := func() {
 		if appVersion.IsEndTimeSet() {
 			r.Log.Info("Increasing app count")
 			attrs := appVersion.GetMetricsAttributes()
 			r.Meters.AppCount.Add(ctx, 1, metric.WithAttributes(attrs...))
 		}
-		span.End()
 	}
 
-	appVersion.SetSpanAttributes(span)
-	return ctx, ctxAppTrace, span, endFunc
+	return ctxAppTrace, endFunc
 }
 
 // SetupWithManager sets up the controller with the Manager.