Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 5 commits into from
Nov 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions operator/controllers/common/ITracer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
141 changes: 141 additions & 0 deletions operator/controllers/common/fake/spanhandler_mock.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

20 changes: 10 additions & 10 deletions operator/controllers/common/phasehandler.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ type PhaseHandler struct {
client.Client
Recorder record.EventRecorder
Log logr.Logger
SpanHandler *SpanHandler
SpanHandler ISpanHandler
}

type PhaseResult struct {
Expand All @@ -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
Expand All @@ -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")
}
Expand All @@ -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")
}
Expand All @@ -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")
}
Expand Down
6 changes: 6 additions & 0 deletions operator/controllers/common/spanhandler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
38 changes: 38 additions & 0 deletions operator/controllers/common/test_utils.go
Original file line number Diff line number Diff line change
@@ -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)

}
41 changes: 5 additions & 36 deletions operator/controllers/keptnapp/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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)

}
Loading