From 4911a62e06ed94afeab48c3fe7989db0a567657d Mon Sep 17 00:00:00 2001 From: yafeiaa Date: Mon, 10 Mar 2025 15:22:30 +0800 Subject: [PATCH 1/8] feat: add opentelemetry trace to record the sync trace Signed-off-by: yafeiaa --- go.mod | 2 ++ pkg/engine/engine.go | 3 +- pkg/sync/sync_context.go | 46 +++++++++++++++++++++++- pkg/utils/tracing/api.go | 3 ++ pkg/utils/tracing/logging.go | 14 ++++++++ pkg/utils/tracing/nop.go | 12 +++++++ pkg/utils/tracing/opentelemetry.go | 56 ++++++++++++++++++++++++++++++ 7 files changed, 134 insertions(+), 2 deletions(-) create mode 100644 pkg/utils/tracing/opentelemetry.go diff --git a/go.mod b/go.mod index d34c8b8fd..4072b6fb8 100644 --- a/go.mod +++ b/go.mod @@ -18,6 +18,8 @@ require ( k8s.io/apimachinery v0.32.2 k8s.io/cli-runtime v0.32.2 k8s.io/client-go v0.32.2 + go.opentelemetry.io/otel v1.28.0 + go.opentelemetry.io/otel/trace v1.28.0 k8s.io/klog/v2 v2.130.1 k8s.io/kube-aggregator v0.32.2 k8s.io/kube-openapi v0.0.0-20241212222426-2c72e554b1e7 diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index 0347fa1c9..7744cc76f 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -23,6 +23,7 @@ import ( "github.com/argoproj/gitops-engine/pkg/sync" "github.com/argoproj/gitops-engine/pkg/sync/common" "github.com/argoproj/gitops-engine/pkg/utils/kube" + "github.com/argoproj/gitops-engine/pkg/utils/tracing" ) const ( @@ -84,7 +85,7 @@ func (e *gitOpsEngine) Sync(ctx context.Context, return nil, err } opts = append(opts, sync.WithSkipHooks(!diffRes.Modified)) - syncCtx, cleanup, err := sync.NewSyncContext(revision, result, e.config, e.config, e.kubectl, namespace, e.cache.GetOpenAPISchema(), opts...) + syncCtx, cleanup, err := sync.NewSyncContext(revision, result, e.config, e.config, e.kubectl, namespace, e.cache.GetOpenAPISchema(), tracing.NopTracer{}, "", "", opts...) if err != nil { return nil, err } diff --git a/pkg/sync/sync_context.go b/pkg/sync/sync_context.go index 7d43899c9..f743f4049 100644 --- a/pkg/sync/sync_context.go +++ b/pkg/sync/sync_context.go @@ -32,6 +32,7 @@ import ( "github.com/argoproj/gitops-engine/pkg/sync/hook" resourceutil "github.com/argoproj/gitops-engine/pkg/sync/resource" kubeutil "github.com/argoproj/gitops-engine/pkg/utils/kube" + "github.com/argoproj/gitops-engine/pkg/utils/tracing" ) type reconciledResource struct { @@ -209,6 +210,8 @@ func NewSyncContext( kubectl kubeutil.Kubectl, namespace string, openAPISchema openapi.Resources, + syncTracer tracing.Tracer, + syncTraceID, syncTraceRootSpanID string, opts ...SyncOpt, ) (SyncContext, func(), error) { dynamicIf, err := dynamic.NewForConfig(restConfig) @@ -246,6 +249,9 @@ func NewSyncContext( permissionValidator: func(_ *unstructured.Unstructured, _ *metav1.APIResource) error { return nil }, + syncTracer: syncTracer, + syncTraceID: syncTraceID, + syncTraceRootSpanID: syncTraceRootSpanID, } for _, opt := range opts { opt(ctx) @@ -357,6 +363,11 @@ type syncContext struct { // lock to protect concurrent updates of the result list lock sync.Mutex + // tracer for tracing the sync operation + syncTraceID string + syncTraceRootSpanID string + syncTracer tracing.Tracer + // syncNamespace is a function that will determine if the managed // namespace should be synced syncNamespace func(*unstructured.Unstructured, *unstructured.Unstructured) (bool, error) @@ -1262,6 +1273,8 @@ func (sc *syncContext) runTasks(tasks syncTasks, dryRun bool) runState { ss.Go(func(state runState) runState { logCtx := sc.log.WithValues("dryRun", dryRun, "task", t) logCtx.V(1).Info("Pruning") + span := sc.syncTracer.StartSpanFromTraceParent("pruneObject", sc.syncTraceID, sc.syncTraceRootSpanID) + defer span.Finish() result, message := sc.pruneObject(t.liveObj, sc.prune, dryRun) if result == common.ResultCodeSyncFailed { state = failed @@ -1270,6 +1283,7 @@ func (sc *syncContext) runTasks(tasks syncTasks, dryRun bool) runState { if !dryRun || sc.dryRun || result == common.ResultCodeSyncFailed { sc.setResourceResult(t, result, operationPhases[result], message) } + sc.setBaggageItemForTasks(&span, t, message, result, operationPhases[result]) return state }) } @@ -1289,19 +1303,27 @@ func (sc *syncContext) runTasks(tasks syncTasks, dryRun bool) runState { ss.Go(func(state runState) runState { sc.log.WithValues("dryRun", dryRun, "task", t).V(1).Info("Deleting") if !dryRun { + span := sc.syncTracer.StartSpanFromTraceParent("hooksDeletion", sc.syncTraceID, sc.syncTraceRootSpanID) + defer span.Finish() err := sc.deleteResource(t) + message := "deleted" + operationPhase := common.OperationRunning if err != nil { // it is possible to get a race condition here, such that the resource does not exist when // delete is requested, we treat this as a nop if !apierrors.IsNotFound(err) { state = failed - sc.setResourceResult(t, "", common.OperationError, fmt.Sprintf("failed to delete resource: %v", err)) + message = fmt.Sprintf("failed to delete resource: %v", err) + operationPhase = common.OperationError + sc.setResourceResult(t, "", operationPhase, message) } } else { // if there is anything that needs deleting, we are at best now in pending and // want to return and wait for sync to be invoked again state = pending + operationPhase = common.OperationSucceeded } + sc.setBaggageItemForTasks(&span, t, message, "", operationPhase) } return state }) @@ -1330,6 +1352,24 @@ func (sc *syncContext) runTasks(tasks syncTasks, dryRun bool) runState { return state } +func (sc *syncContext) createSpan(operation string, dryrun bool) tracing.Span { + // skip tracing if dryrun + if dryrun { + return tracing.NopTracer{}.StartSpan(operation) + } + return sc.syncTracer.StartSpanFromTraceParent(operation, sc.syncTraceID, sc.syncTraceRootSpanID) +} + +func (sc *syncContext) setBaggageItemForTasks(span *tracing.Span, t *syncTask, message string, result common.ResultCode, operationPhase common.OperationPhase) { + resourceKey := t.resourceKey() + (*span).SetBaggageItem("resource", resourceKey.String()) + (*span).SetBaggageItem("result", string(result)) + (*span).SetBaggageItem("operationPhase", string(operationPhase)) + (*span).SetBaggageItem("message", message) + (*span).SetBaggageItem("phase", string(t.phase)) + (*span).SetBaggageItem("wave", fmt.Sprint(t.wave())) +} + func (sc *syncContext) processCreateTasks(state runState, tasks syncTasks, dryRun bool) runState { ss := newStateSync(state) for _, task := range tasks { @@ -1341,11 +1381,14 @@ func (sc *syncContext) processCreateTasks(state runState, tasks syncTasks, dryRu logCtx := sc.log.WithValues("dryRun", dryRun, "task", t) logCtx.V(1).Info("Applying") validate := sc.validate && !resourceutil.HasAnnotationOption(t.targetObj, common.AnnotationSyncOptions, common.SyncOptionsDisableValidation) + span := sc.syncTracer.StartSpanFromTraceParent("applyObject", sc.syncTraceID, sc.syncTraceRootSpanID) + defer span.Finish() result, message := sc.applyObject(t, dryRun, validate) if result == common.ResultCodeSyncFailed { logCtx.WithValues("message", message).Info("Apply failed") state = failed } + var phase common.OperationPhase if !dryRun || sc.dryRun || result == common.ResultCodeSyncFailed { phase := operationPhases[result] // no resources are created in dry-run, so running phase means validation was @@ -1355,6 +1398,7 @@ func (sc *syncContext) processCreateTasks(state runState, tasks syncTasks, dryRu } sc.setResourceResult(t, result, phase, message) } + sc.setBaggageItemForTasks(&span, t, message, result, phase) return state }) } diff --git a/pkg/utils/tracing/api.go b/pkg/utils/tracing/api.go index 89670a67b..29148962f 100644 --- a/pkg/utils/tracing/api.go +++ b/pkg/utils/tracing/api.go @@ -8,9 +8,12 @@ package tracing type Tracer interface { StartSpan(operationName string) Span + StartSpanFromTraceParent(operationName string, parentTraceId, parentSpanId string) Span } type Span interface { SetBaggageItem(key string, value any) Finish() + SpanID() string + TraceID() string } diff --git a/pkg/utils/tracing/logging.go b/pkg/utils/tracing/logging.go index fd0619f99..59481bf49 100644 --- a/pkg/utils/tracing/logging.go +++ b/pkg/utils/tracing/logging.go @@ -30,6 +30,12 @@ func (l LoggingTracer) StartSpan(operationName string) Span { } } +// loggingSpan is not a real distributed tracing system. +// so no need to implement real StartSpanFromTraceParent method. +func (l LoggingTracer) StartSpanFromTraceParent(operationName string, parentTraceId, parentSpanId string) Span { + return l.StartSpan(operationName) +} + type loggingSpan struct { logger logr.Logger operationName string @@ -54,3 +60,11 @@ func baggageToVals(baggage map[string]any) []any { } return result } + +func (s loggingSpan) TraceID() string { + return "" +} + +func (s loggingSpan) SpanID() string { + return "" +} diff --git a/pkg/utils/tracing/nop.go b/pkg/utils/tracing/nop.go index e39b67b99..6bafc4d2c 100644 --- a/pkg/utils/tracing/nop.go +++ b/pkg/utils/tracing/nop.go @@ -11,6 +11,10 @@ func (n NopTracer) StartSpan(_ string) Span { return nopSpan{} } +func (n NopTracer) StartSpanFromTraceParent(_, _, _ string) Span { + return nopSpan{} +} + type nopSpan struct{} func (n nopSpan) SetBaggageItem(_ string, _ any) { @@ -18,3 +22,11 @@ func (n nopSpan) SetBaggageItem(_ string, _ any) { func (n nopSpan) Finish() { } + +func (s nopSpan) TraceID() string { + return "" +} + +func (s nopSpan) SpanID() string { + return "" +} diff --git a/pkg/utils/tracing/opentelemetry.go b/pkg/utils/tracing/opentelemetry.go new file mode 100644 index 000000000..6803cb839 --- /dev/null +++ b/pkg/utils/tracing/opentelemetry.go @@ -0,0 +1,56 @@ +package tracing + +import ( + "context" + + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/trace" +) + +type OpenTelemetryTracer struct { + realTracer trace.Tracer +} + +func NewOpenTelemetryTracer(t trace.Tracer) Tracer { + return &OpenTelemetryTracer{ + realTracer: t, + } +} + +func (t OpenTelemetryTracer) StartSpan(operationName string) Span { + _, realspan := t.realTracer.Start(context.Background(), operationName) + return openTelemetrySpan{realSpan: realspan} +} + +func (t OpenTelemetryTracer) StartSpanFromTraceParent(operationName string, parentTraceId, parentSpanId string) Span { + traceID, _ := trace.TraceIDFromHex(parentTraceId) + parentSpanID, _ := trace.SpanIDFromHex(parentSpanId) + spanCtx := trace.NewSpanContext(trace.SpanContextConfig{ + TraceID: traceID, + SpanID: parentSpanID, + TraceFlags: trace.FlagsSampled, + }) + ctx := trace.ContextWithSpanContext(context.Background(), spanCtx) + _, realSpan := t.realTracer.Start(ctx, operationName) + return openTelemetrySpan{realSpan: realSpan} +} + +type openTelemetrySpan struct { + realSpan trace.Span +} + +func (s openTelemetrySpan) SetBaggageItem(key string, value interface{}) { + s.realSpan.SetAttributes(attribute.Key(key).String(value.(string))) +} + +func (s openTelemetrySpan) Finish() { + s.realSpan.End() +} + +func (s openTelemetrySpan) TraceID() string { + return s.realSpan.SpanContext().TraceID().String() +} + +func (s openTelemetrySpan) SpanID() string { + return s.realSpan.SpanContext().SpanID().String() +} From f9672c8b584e3082578bc6881ddbb9818b305441 Mon Sep 17 00:00:00 2001 From: yafeiaa Date: Mon, 10 Mar 2025 15:43:50 +0800 Subject: [PATCH 2/8] fix unused Signed-off-by: yafeiaa --- go.mod | 6 ++---- pkg/sync/sync_context.go | 6 +++--- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/go.mod b/go.mod index 4072b6fb8..9c08ba032 100644 --- a/go.mod +++ b/go.mod @@ -11,6 +11,8 @@ require ( github.com/google/uuid v1.6.0 github.com/spf13/cobra v1.9.1 github.com/stretchr/testify v1.10.0 + go.opentelemetry.io/otel v1.28.0 + go.opentelemetry.io/otel/trace v1.28.0 golang.org/x/sync v0.11.0 google.golang.org/protobuf v1.36.5 k8s.io/api v0.32.2 @@ -18,8 +20,6 @@ require ( k8s.io/apimachinery v0.32.2 k8s.io/cli-runtime v0.32.2 k8s.io/client-go v0.32.2 - go.opentelemetry.io/otel v1.28.0 - go.opentelemetry.io/otel/trace v1.28.0 k8s.io/klog/v2 v2.130.1 k8s.io/kube-aggregator v0.32.2 k8s.io/kube-openapi v0.0.0-20241212222426-2c72e554b1e7 @@ -80,8 +80,6 @@ require ( github.com/stretchr/objx v0.5.2 // indirect github.com/x448/float16 v0.8.4 // indirect github.com/xlab/treeprint v1.2.0 // indirect - go.opentelemetry.io/otel v1.28.0 // indirect - go.opentelemetry.io/otel/trace v1.28.0 // indirect golang.org/x/net v0.33.0 // indirect golang.org/x/oauth2 v0.23.0 // indirect golang.org/x/sys v0.28.0 // indirect diff --git a/pkg/sync/sync_context.go b/pkg/sync/sync_context.go index f743f4049..d3b574105 100644 --- a/pkg/sync/sync_context.go +++ b/pkg/sync/sync_context.go @@ -1273,7 +1273,7 @@ func (sc *syncContext) runTasks(tasks syncTasks, dryRun bool) runState { ss.Go(func(state runState) runState { logCtx := sc.log.WithValues("dryRun", dryRun, "task", t) logCtx.V(1).Info("Pruning") - span := sc.syncTracer.StartSpanFromTraceParent("pruneObject", sc.syncTraceID, sc.syncTraceRootSpanID) + span := sc.createSpan("pruneObject", dryRun) defer span.Finish() result, message := sc.pruneObject(t.liveObj, sc.prune, dryRun) if result == common.ResultCodeSyncFailed { @@ -1303,7 +1303,7 @@ func (sc *syncContext) runTasks(tasks syncTasks, dryRun bool) runState { ss.Go(func(state runState) runState { sc.log.WithValues("dryRun", dryRun, "task", t).V(1).Info("Deleting") if !dryRun { - span := sc.syncTracer.StartSpanFromTraceParent("hooksDeletion", sc.syncTraceID, sc.syncTraceRootSpanID) + span := sc.createSpan("hooksDeletion", dryRun) defer span.Finish() err := sc.deleteResource(t) message := "deleted" @@ -1381,7 +1381,7 @@ func (sc *syncContext) processCreateTasks(state runState, tasks syncTasks, dryRu logCtx := sc.log.WithValues("dryRun", dryRun, "task", t) logCtx.V(1).Info("Applying") validate := sc.validate && !resourceutil.HasAnnotationOption(t.targetObj, common.AnnotationSyncOptions, common.SyncOptionsDisableValidation) - span := sc.syncTracer.StartSpanFromTraceParent("applyObject", sc.syncTraceID, sc.syncTraceRootSpanID) + span := sc.createSpan("applyObject", dryRun) defer span.Finish() result, message := sc.applyObject(t, dryRun, validate) if result == common.ResultCodeSyncFailed { From da16a9731961d69d214d5cfc3c89279b72fc721b Mon Sep 17 00:00:00 2001 From: yafeiaa Date: Mon, 10 Mar 2025 15:47:56 +0800 Subject: [PATCH 3/8] fix panic Signed-off-by: yafeiaa --- pkg/sync/sync_context.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/sync/sync_context.go b/pkg/sync/sync_context.go index d3b574105..ba00129ba 100644 --- a/pkg/sync/sync_context.go +++ b/pkg/sync/sync_context.go @@ -1354,7 +1354,7 @@ func (sc *syncContext) runTasks(tasks syncTasks, dryRun bool) runState { func (sc *syncContext) createSpan(operation string, dryrun bool) tracing.Span { // skip tracing if dryrun - if dryrun { + if dryrun || sc.syncTracer == nil { return tracing.NopTracer{}.StartSpan(operation) } return sc.syncTracer.StartSpanFromTraceParent(operation, sc.syncTraceID, sc.syncTraceRootSpanID) From 11d70e38728aecbb98af1b7dcf5ed85c62f286e7 Mon Sep 17 00:00:00 2001 From: yafeiaa Date: Mon, 10 Mar 2025 15:52:50 +0800 Subject: [PATCH 4/8] fix golang ci lint Signed-off-by: yafeiaa --- pkg/sync/sync_context.go | 3 ++- pkg/utils/tracing/logging.go | 2 +- pkg/utils/tracing/nop.go | 4 ++-- pkg/utils/tracing/opentelemetry.go | 2 +- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/pkg/sync/sync_context.go b/pkg/sync/sync_context.go index ba00129ba..c0d9151af 100644 --- a/pkg/sync/sync_context.go +++ b/pkg/sync/sync_context.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "sort" + "strconv" "strings" "sync" "time" @@ -1367,7 +1368,7 @@ func (sc *syncContext) setBaggageItemForTasks(span *tracing.Span, t *syncTask, m (*span).SetBaggageItem("operationPhase", string(operationPhase)) (*span).SetBaggageItem("message", message) (*span).SetBaggageItem("phase", string(t.phase)) - (*span).SetBaggageItem("wave", fmt.Sprint(t.wave())) + (*span).SetBaggageItem("wave", strconv.Itoa(t.wave())) } func (sc *syncContext) processCreateTasks(state runState, tasks syncTasks, dryRun bool) runState { diff --git a/pkg/utils/tracing/logging.go b/pkg/utils/tracing/logging.go index 59481bf49..2c4c30f83 100644 --- a/pkg/utils/tracing/logging.go +++ b/pkg/utils/tracing/logging.go @@ -32,7 +32,7 @@ func (l LoggingTracer) StartSpan(operationName string) Span { // loggingSpan is not a real distributed tracing system. // so no need to implement real StartSpanFromTraceParent method. -func (l LoggingTracer) StartSpanFromTraceParent(operationName string, parentTraceId, parentSpanId string) Span { +func (l LoggingTracer) StartSpanFromTraceParent(operationName string, _, _ string) Span { return l.StartSpan(operationName) } diff --git a/pkg/utils/tracing/nop.go b/pkg/utils/tracing/nop.go index 6bafc4d2c..3af4c725d 100644 --- a/pkg/utils/tracing/nop.go +++ b/pkg/utils/tracing/nop.go @@ -23,10 +23,10 @@ func (n nopSpan) SetBaggageItem(_ string, _ any) { func (n nopSpan) Finish() { } -func (s nopSpan) TraceID() string { +func (n nopSpan) TraceID() string { return "" } -func (s nopSpan) SpanID() string { +func (n nopSpan) SpanID() string { return "" } diff --git a/pkg/utils/tracing/opentelemetry.go b/pkg/utils/tracing/opentelemetry.go index 6803cb839..d4fcb643c 100644 --- a/pkg/utils/tracing/opentelemetry.go +++ b/pkg/utils/tracing/opentelemetry.go @@ -39,7 +39,7 @@ type openTelemetrySpan struct { realSpan trace.Span } -func (s openTelemetrySpan) SetBaggageItem(key string, value interface{}) { +func (s openTelemetrySpan) SetBaggageItem(key string, value any) { s.realSpan.SetAttributes(attribute.Key(key).String(value.(string))) } From cdbe33d1525acd54a4a304dd8a6aeaa66dcee433 Mon Sep 17 00:00:00 2001 From: yafeiaa Date: Tue, 13 May 2025 14:27:27 +0800 Subject: [PATCH 5/8] Update pkg/sync/sync_context.go Co-authored-by: sivchari --- pkg/sync/sync_context.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/sync/sync_context.go b/pkg/sync/sync_context.go index c0d9151af..65deb0e25 100644 --- a/pkg/sync/sync_context.go +++ b/pkg/sync/sync_context.go @@ -1391,7 +1391,7 @@ func (sc *syncContext) processCreateTasks(state runState, tasks syncTasks, dryRu } var phase common.OperationPhase if !dryRun || sc.dryRun || result == common.ResultCodeSyncFailed { - phase := operationPhases[result] + phase = operationPhases[result] // no resources are created in dry-run, so running phase means validation was // successful and sync operation succeeded if sc.dryRun && phase == common.OperationRunning { From c9dd69fac56f299dca8f7685417907bf3541f34a Mon Sep 17 00:00:00 2001 From: yovafeng Date: Tue, 13 May 2025 14:48:13 +0800 Subject: [PATCH 6/8] opmize opentelemetry interface & struct --- pkg/sync/sync_context.go | 23 ++++++++++++++--------- pkg/utils/tracing/api.go | 6 ++++-- pkg/utils/tracing/logging.go | 7 ++++--- pkg/utils/tracing/nop.go | 6 ++++-- pkg/utils/tracing/opentelemetry.go | 8 ++++---- 5 files changed, 30 insertions(+), 20 deletions(-) diff --git a/pkg/sync/sync_context.go b/pkg/sync/sync_context.go index 65deb0e25..b6115b72a 100644 --- a/pkg/sync/sync_context.go +++ b/pkg/sync/sync_context.go @@ -1303,12 +1303,12 @@ func (sc *syncContext) runTasks(tasks syncTasks, dryRun bool) runState { t := task ss.Go(func(state runState) runState { sc.log.WithValues("dryRun", dryRun, "task", t).V(1).Info("Deleting") + span := sc.createSpan("hooksDeletion", dryRun) + defer span.Finish() + message := "deleted" + operationPhase := common.OperationRunning if !dryRun { - span := sc.createSpan("hooksDeletion", dryRun) - defer span.Finish() err := sc.deleteResource(t) - message := "deleted" - operationPhase := common.OperationRunning if err != nil { // it is possible to get a race condition here, such that the resource does not exist when // delete is requested, we treat this as a nop @@ -1319,6 +1319,7 @@ func (sc *syncContext) runTasks(tasks syncTasks, dryRun bool) runState { sc.setResourceResult(t, "", operationPhase, message) } } else { + message = "deleted(dry-run)" // if there is anything that needs deleting, we are at best now in pending and // want to return and wait for sync to be invoked again state = pending @@ -1354,11 +1355,15 @@ func (sc *syncContext) runTasks(tasks syncTasks, dryRun bool) runState { } func (sc *syncContext) createSpan(operation string, dryrun bool) tracing.Span { - // skip tracing if dryrun - if dryrun || sc.syncTracer == nil { - return tracing.NopTracer{}.StartSpan(operation) - } - return sc.syncTracer.StartSpanFromTraceParent(operation, sc.syncTraceID, sc.syncTraceRootSpanID) + // NOTE: we use context.Background() here because we don't want to inherit any trace context from the parent + var span tracing.Span + ctx := context.Background() + if sc.syncTracer == nil { + span = tracing.NopTracer{}.StartSpan(ctx, operation) + } + span = sc.syncTracer.StartSpanFromTraceParent(ctx, operation, sc.syncTraceID, sc.syncTraceRootSpanID) + span.SetBaggageItem("dryrun", strconv.FormatBool(dryrun)) + return span } func (sc *syncContext) setBaggageItemForTasks(span *tracing.Span, t *syncTask, message string, result common.ResultCode, operationPhase common.OperationPhase) { diff --git a/pkg/utils/tracing/api.go b/pkg/utils/tracing/api.go index 29148962f..7585e2db7 100644 --- a/pkg/utils/tracing/api.go +++ b/pkg/utils/tracing/api.go @@ -1,5 +1,7 @@ package tracing +import "context" + /* Poor Mans OpenTracing. @@ -7,8 +9,8 @@ package tracing */ type Tracer interface { - StartSpan(operationName string) Span - StartSpanFromTraceParent(operationName string, parentTraceId, parentSpanId string) Span + StartSpan(_ context.Context, operationName string) Span + StartSpanFromTraceParent(ctx context.Context, operationName string, parentTraceId, parentSpanId string) Span } type Span interface { diff --git a/pkg/utils/tracing/logging.go b/pkg/utils/tracing/logging.go index 2c4c30f83..779ae65c3 100644 --- a/pkg/utils/tracing/logging.go +++ b/pkg/utils/tracing/logging.go @@ -1,6 +1,7 @@ package tracing import ( + "context" "time" "github.com/go-logr/logr" @@ -21,7 +22,7 @@ func NewLoggingTracer(logger logr.Logger) *LoggingTracer { } } -func (l LoggingTracer) StartSpan(operationName string) Span { +func (l LoggingTracer) StartSpan(_ context.Context, operationName string) Span { return loggingSpan{ logger: l.logger, operationName: operationName, @@ -32,8 +33,8 @@ func (l LoggingTracer) StartSpan(operationName string) Span { // loggingSpan is not a real distributed tracing system. // so no need to implement real StartSpanFromTraceParent method. -func (l LoggingTracer) StartSpanFromTraceParent(operationName string, _, _ string) Span { - return l.StartSpan(operationName) +func (l LoggingTracer) StartSpanFromTraceParent(ctx context.Context, operationName string, _, _ string) Span { + return l.StartSpan(ctx, operationName) } type loggingSpan struct { diff --git a/pkg/utils/tracing/nop.go b/pkg/utils/tracing/nop.go index 3af4c725d..7a28567ac 100644 --- a/pkg/utils/tracing/nop.go +++ b/pkg/utils/tracing/nop.go @@ -1,5 +1,7 @@ package tracing +import "context" + var ( _ Tracer = NopTracer{} _ Span = nopSpan{} @@ -7,11 +9,11 @@ var ( type NopTracer struct{} -func (n NopTracer) StartSpan(_ string) Span { +func (n NopTracer) StartSpan(_ context.Context, _ string) Span { return nopSpan{} } -func (n NopTracer) StartSpanFromTraceParent(_, _, _ string) Span { +func (n NopTracer) StartSpanFromTraceParent(_ context.Context, _, _, _ string) Span { return nopSpan{} } diff --git a/pkg/utils/tracing/opentelemetry.go b/pkg/utils/tracing/opentelemetry.go index d4fcb643c..c4ab5a2d3 100644 --- a/pkg/utils/tracing/opentelemetry.go +++ b/pkg/utils/tracing/opentelemetry.go @@ -17,12 +17,12 @@ func NewOpenTelemetryTracer(t trace.Tracer) Tracer { } } -func (t OpenTelemetryTracer) StartSpan(operationName string) Span { - _, realspan := t.realTracer.Start(context.Background(), operationName) +func (t OpenTelemetryTracer) StartSpan(ctx context.Context, operationName string) Span { + _, realspan := t.realTracer.Start(ctx, operationName) return openTelemetrySpan{realSpan: realspan} } -func (t OpenTelemetryTracer) StartSpanFromTraceParent(operationName string, parentTraceId, parentSpanId string) Span { +func (t OpenTelemetryTracer) StartSpanFromTraceParent(ctx context.Context, operationName string, parentTraceId, parentSpanId string) Span { traceID, _ := trace.TraceIDFromHex(parentTraceId) parentSpanID, _ := trace.SpanIDFromHex(parentSpanId) spanCtx := trace.NewSpanContext(trace.SpanContextConfig{ @@ -30,7 +30,7 @@ func (t OpenTelemetryTracer) StartSpanFromTraceParent(operationName string, pare SpanID: parentSpanID, TraceFlags: trace.FlagsSampled, }) - ctx := trace.ContextWithSpanContext(context.Background(), spanCtx) + ctx = trace.ContextWithSpanContext(ctx, spanCtx) _, realSpan := t.realTracer.Start(ctx, operationName) return openTelemetrySpan{realSpan: realSpan} } From 033a986cd9ad96164f1700a341edfac10d50bbc4 Mon Sep 17 00:00:00 2001 From: yovafeng Date: Tue, 13 May 2025 15:18:51 +0800 Subject: [PATCH 7/8] fix go test --- pkg/utils/kube/ctl.go | 14 +++++++------- pkg/utils/kube/resource_ops.go | 10 +++++----- pkg/utils/tracing/logging_test.go | 3 ++- 3 files changed, 14 insertions(+), 13 deletions(-) diff --git a/pkg/utils/kube/ctl.go b/pkg/utils/kube/ctl.go index 8af205ad4..8c5ac364b 100644 --- a/pkg/utils/kube/ctl.go +++ b/pkg/utils/kube/ctl.go @@ -163,7 +163,7 @@ func (k *KubectlCmd) newGVKParser(oapiGetter discovery.OpenAPISchemaInterface) ( } func (k *KubectlCmd) GetAPIResources(config *rest.Config, preferred bool, resourceFilter ResourceFilter) ([]APIResourceInfo, error) { - span := k.Tracer.StartSpan("GetAPIResources") + span := k.Tracer.StartSpan(context.Background(), "GetAPIResources") defer span.Finish() apiResIfs, err := k.filterAPIResources(config, preferred, resourceFilter, func(apiResource *metav1.APIResource) bool { return isSupportedVerb(apiResource, listVerb) && isSupportedVerb(apiResource, watchVerb) @@ -176,7 +176,7 @@ func (k *KubectlCmd) GetAPIResources(config *rest.Config, preferred bool, resour // GetResource returns resource func (k *KubectlCmd) GetResource(ctx context.Context, config *rest.Config, gvk schema.GroupVersionKind, name string, namespace string) (*unstructured.Unstructured, error) { - span := k.Tracer.StartSpan("GetResource") + span := k.Tracer.StartSpan(context.Background(), "GetResource") span.SetBaggageItem("kind", gvk.Kind) span.SetBaggageItem("name", name) defer span.Finish() @@ -199,7 +199,7 @@ func (k *KubectlCmd) GetResource(ctx context.Context, config *rest.Config, gvk s // CreateResource creates resource func (k *KubectlCmd) CreateResource(ctx context.Context, config *rest.Config, gvk schema.GroupVersionKind, name string, namespace string, obj *unstructured.Unstructured, createOptions metav1.CreateOptions, subresources ...string) (*unstructured.Unstructured, error) { - span := k.Tracer.StartSpan("CreateResource") + span := k.Tracer.StartSpan(context.Background(), "CreateResource") span.SetBaggageItem("kind", gvk.Kind) span.SetBaggageItem("name", name) defer span.Finish() @@ -222,7 +222,7 @@ func (k *KubectlCmd) CreateResource(ctx context.Context, config *rest.Config, gv // PatchResource patches resource func (k *KubectlCmd) PatchResource(ctx context.Context, config *rest.Config, gvk schema.GroupVersionKind, name string, namespace string, patchType types.PatchType, patchBytes []byte, subresources ...string) (*unstructured.Unstructured, error) { - span := k.Tracer.StartSpan("PatchResource") + span := k.Tracer.StartSpan(context.Background(), "PatchResource") span.SetBaggageItem("kind", gvk.Kind) span.SetBaggageItem("name", name) defer span.Finish() @@ -245,7 +245,7 @@ func (k *KubectlCmd) PatchResource(ctx context.Context, config *rest.Config, gvk // DeleteResource deletes resource func (k *KubectlCmd) DeleteResource(ctx context.Context, config *rest.Config, gvk schema.GroupVersionKind, name string, namespace string, deleteOptions metav1.DeleteOptions) error { - span := k.Tracer.StartSpan("DeleteResource") + span := k.Tracer.StartSpan(context.Background(), "DeleteResource") span.SetBaggageItem("kind", gvk.Kind) span.SetBaggageItem("name", name) defer span.Finish() @@ -323,7 +323,7 @@ func ManageServerSideDiffDryRuns(config *rest.Config, openAPISchema openapi.Reso // ConvertToVersion converts an unstructured object into the specified group/version func (k *KubectlCmd) ConvertToVersion(obj *unstructured.Unstructured, group string, version string) (*unstructured.Unstructured, error) { - span := k.Tracer.StartSpan("ConvertToVersion") + span := k.Tracer.StartSpan(context.Background(), "ConvertToVersion") from := obj.GroupVersionKind().GroupVersion() span.SetBaggageItem("from", from.String()) span.SetBaggageItem("to", schema.GroupVersion{Group: group, Version: version}.String()) @@ -335,7 +335,7 @@ func (k *KubectlCmd) ConvertToVersion(obj *unstructured.Unstructured, group stri } func (k *KubectlCmd) GetServerVersion(config *rest.Config) (string, error) { - span := k.Tracer.StartSpan("GetServerVersion") + span := k.Tracer.StartSpan(context.Background(), "GetServerVersion") defer span.Finish() client, err := discovery.NewDiscoveryClientForConfig(config) if err != nil { diff --git a/pkg/utils/kube/resource_ops.go b/pkg/utils/kube/resource_ops.go index 039749ac3..5427dec4d 100644 --- a/pkg/utils/kube/resource_ops.go +++ b/pkg/utils/kube/resource_ops.go @@ -221,7 +221,7 @@ func kubeCmdFactory(kubeconfig, ns string, config *rest.Config) cmdutil.Factory } func (k *kubectlResourceOperations) ReplaceResource(ctx context.Context, obj *unstructured.Unstructured, dryRunStrategy cmdutil.DryRunStrategy, force bool) (string, error) { - span := k.tracer.StartSpan("ReplaceResource") + span := k.tracer.StartSpan(context.Background(), "ReplaceResource") span.SetBaggageItem("kind", obj.GetKind()) span.SetBaggageItem("name", obj.GetName()) defer span.Finish() @@ -243,7 +243,7 @@ func (k *kubectlResourceOperations) ReplaceResource(ctx context.Context, obj *un func (k *kubectlResourceOperations) CreateResource(ctx context.Context, obj *unstructured.Unstructured, dryRunStrategy cmdutil.DryRunStrategy, validate bool) (string, error) { gvk := obj.GroupVersionKind() - span := k.tracer.StartSpan("CreateResource") + span := k.tracer.StartSpan(context.Background(), "CreateResource") span.SetBaggageItem("kind", gvk.Kind) span.SetBaggageItem("name", obj.GetName()) defer span.Finish() @@ -273,7 +273,7 @@ func (k *kubectlResourceOperations) CreateResource(ctx context.Context, obj *uns func (k *kubectlResourceOperations) UpdateResource(ctx context.Context, obj *unstructured.Unstructured, dryRunStrategy cmdutil.DryRunStrategy) (*unstructured.Unstructured, error) { gvk := obj.GroupVersionKind() - span := k.tracer.StartSpan("UpdateResource") + span := k.tracer.StartSpan(context.Background(), "UpdateResource") span.SetBaggageItem("kind", gvk.Kind) span.SetBaggageItem("name", obj.GetName()) defer span.Finish() @@ -302,7 +302,7 @@ func (k *kubectlResourceOperations) UpdateResource(ctx context.Context, obj *uns // ApplyResource performs an apply of a unstructured resource func (k *kubectlServerSideDiffDryRunApplier) ApplyResource(_ context.Context, obj *unstructured.Unstructured, dryRunStrategy cmdutil.DryRunStrategy, force bool, validate bool, serverSideApply bool, manager string) (string, error) { - span := k.tracer.StartSpan("ApplyResource") + span := k.tracer.StartSpan(context.Background(), "ApplyResource") span.SetBaggageItem("kind", obj.GetKind()) span.SetBaggageItem("name", obj.GetName()) defer span.Finish() @@ -328,7 +328,7 @@ func (k *kubectlServerSideDiffDryRunApplier) ApplyResource(_ context.Context, ob // ApplyResource performs an apply of a unstructured resource func (k *kubectlResourceOperations) ApplyResource(ctx context.Context, obj *unstructured.Unstructured, dryRunStrategy cmdutil.DryRunStrategy, force, validate, serverSideApply bool, manager string) (string, error) { - span := k.tracer.StartSpan("ApplyResource") + span := k.tracer.StartSpan(context.Background(), "ApplyResource") span.SetBaggageItem("kind", obj.GetKind()) span.SetBaggageItem("name", obj.GetName()) defer span.Finish() diff --git a/pkg/utils/tracing/logging_test.go b/pkg/utils/tracing/logging_test.go index f9346550f..c16b91fb7 100644 --- a/pkg/utils/tracing/logging_test.go +++ b/pkg/utils/tracing/logging_test.go @@ -1,6 +1,7 @@ package tracing import ( + "context" "testing" "github.com/go-logr/logr" @@ -22,7 +23,7 @@ func TestLoggingTracer(t *testing.T) { tr := NewLoggingTracer(logr.New(l)) - span := tr.StartSpan("my-operation") + span := tr.StartSpan(context.Background(), "my-operation") span.SetBaggageItem("my-key", "my-value") span.Finish() } From 859ca42ac1d724cd1757750bcf7bd0b8b255fd8a Mon Sep 17 00:00:00 2001 From: yovafeng Date: Tue, 13 May 2025 15:27:03 +0800 Subject: [PATCH 8/8] fix go test --- pkg/sync/sync_context.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/sync/sync_context.go b/pkg/sync/sync_context.go index da5b8e34c..2a94ec6ad 100644 --- a/pkg/sync/sync_context.go +++ b/pkg/sync/sync_context.go @@ -1389,8 +1389,9 @@ func (sc *syncContext) createSpan(operation string, dryrun bool) tracing.Span { ctx := context.Background() if sc.syncTracer == nil { span = tracing.NopTracer{}.StartSpan(ctx, operation) + } else { + span = sc.syncTracer.StartSpanFromTraceParent(ctx, operation, sc.syncTraceID, sc.syncTraceRootSpanID) } - span = sc.syncTracer.StartSpanFromTraceParent(ctx, operation, sc.syncTraceID, sc.syncTraceRootSpanID) span.SetBaggageItem("dryrun", strconv.FormatBool(dryrun)) return span }