From 384c75b5506fbcda81354941da5aef923c9ea9d7 Mon Sep 17 00:00:00 2001 From: thc1006 <84045975+thc1006@users.noreply.github.com> Date: Tue, 23 Jun 2026 00:48:20 +0800 Subject: [PATCH] feat(tracing): record reconcile.write_outcome span attribute Add a reconcile.write_outcome attribute (write, status-only, no-op) to the PipelineRun and TaskRun ReconcileKind spans so operators can see how many reconciliations actually write to etcd versus being short-circuited. The status write is decided by the generated reconciler after ReconcileKind returns, so the outcome is computed inside ReconcileKind: the status and labels/annotations are captured at the start, before initTracing (which can persist span context into the status), and compared against their final values with the same equality the framework uses. A shared RecordWriteOutcome helper in pkg/reconciler does the comparison, which runs only when the span is recording. Implements deliverable 2 of #10322. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com> --- pkg/reconciler/pipelinerun/pipelinerun.go | 12 +++ pkg/reconciler/taskrun/taskrun.go | 12 +++ pkg/reconciler/write_outcome.go | 59 +++++++++++ pkg/reconciler/write_outcome_test.go | 119 ++++++++++++++++++++++ 4 files changed, 202 insertions(+) create mode 100644 pkg/reconciler/write_outcome.go create mode 100644 pkg/reconciler/write_outcome_test.go diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index 38667abd57a..7f5ea616f66 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -184,9 +184,21 @@ var ( // resource with the current status of the resource. func (c *Reconciler) ReconcileKind(ctx context.Context, pr *v1.PipelineRun) pkgreconciler.Event { logger := logging.FromContext(ctx) + + // Capture the write-outcome baseline before initTracing, which can persist + // span context into the status (a real write that must not look like a no-op). + oldStatus := pr.Status.DeepCopy() + oldLabels := maps.Clone(pr.Labels) + oldAnnotations := maps.Clone(pr.Annotations) + ctx = initTracing(ctx, c.tracerProvider, pr) ctx, span := c.tracerProvider.Tracer(TracerName).Start(ctx, "PipelineRun:ReconcileKind") defer span.End() + defer func() { + if span.IsRecording() { + tknreconciler.RecordWriteOutcome(span, *oldStatus, pr.Status, oldLabels, pr.Labels, oldAnnotations, pr.Annotations) + } + }() span.SetAttributes( attribute.String("pipelinerun", pr.Name), attribute.String("namespace", pr.Namespace), diff --git a/pkg/reconciler/taskrun/taskrun.go b/pkg/reconciler/taskrun/taskrun.go index db18f0f9070..52dc8b6176f 100644 --- a/pkg/reconciler/taskrun/taskrun.go +++ b/pkg/reconciler/taskrun/taskrun.go @@ -131,9 +131,21 @@ var ( // resource with the current status of the resource. func (c *Reconciler) ReconcileKind(ctx context.Context, tr *v1.TaskRun) pkgreconciler.Event { logger := logging.FromContext(ctx) + + // Capture the write-outcome baseline before initTracing, which can persist + // span context into the status (a real write that must not look like a no-op). + oldStatus := tr.Status.DeepCopy() + oldLabels := maps.Clone(tr.Labels) + oldAnnotations := maps.Clone(tr.Annotations) + ctx = initTracing(ctx, c.tracerProvider, tr) ctx, span := c.tracerProvider.Tracer(TracerName).Start(ctx, "TaskRun:ReconcileKind") defer span.End() + defer func() { + if span.IsRecording() { + tknreconciler.RecordWriteOutcome(span, *oldStatus, tr.Status, oldLabels, tr.Labels, oldAnnotations, tr.Annotations) + } + }() span.SetAttributes(attribute.String("taskrun", tr.Name), attribute.String("namespace", tr.Namespace)) if spanCtx := span.SpanContext(); spanCtx.IsValid() { diff --git a/pkg/reconciler/write_outcome.go b/pkg/reconciler/write_outcome.go new file mode 100644 index 00000000000..55bccf7c2ed --- /dev/null +++ b/pkg/reconciler/write_outcome.go @@ -0,0 +1,59 @@ +/* +Copyright 2026 The Tekton Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package reconciler + +import ( + "maps" + + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/trace" + "k8s.io/apimachinery/pkg/api/equality" +) + +// WriteOutcomeAttributeKey is the span attribute that records whether a +// reconcile pass changed anything that gets written to etcd. It lets operators +// see how many reconciliations cost a write versus being short-circuited. +const WriteOutcomeAttributeKey = "reconcile.write_outcome" + +const ( + writeOutcomeWrite = "write" + writeOutcomeStatusOnly = "status-only" + writeOutcomeNoOp = "no-op" +) + +// ClassifyWriteOutcome maps what changed during a reconcile to a write outcome. +// A labels/annotations change means the object itself is updated, so it counts +// as a full write and takes precedence over a status-only change. +func ClassifyWriteOutcome(metadataChanged, statusChanged bool) string { + switch { + case metadataChanged: + return writeOutcomeWrite + case statusChanged: + return writeOutcomeStatusOnly + default: + return writeOutcomeNoOp + } +} + +// RecordWriteOutcome tags span with what a reconcile pass changed on the object, +// comparing the status and labels/annotations captured on entry against their +// current values. Status uses the same equality the generated reconciler uses. +func RecordWriteOutcome(span trace.Span, oldStatus, newStatus any, oldLabels, newLabels, oldAnnotations, newAnnotations map[string]string) { + statusChanged := !equality.Semantic.DeepEqual(oldStatus, newStatus) + metadataChanged := !maps.Equal(oldLabels, newLabels) || !maps.Equal(oldAnnotations, newAnnotations) + span.SetAttributes(attribute.String(WriteOutcomeAttributeKey, ClassifyWriteOutcome(metadataChanged, statusChanged))) +} diff --git a/pkg/reconciler/write_outcome_test.go b/pkg/reconciler/write_outcome_test.go new file mode 100644 index 00000000000..d83bdffd84c --- /dev/null +++ b/pkg/reconciler/write_outcome_test.go @@ -0,0 +1,119 @@ +/* +Copyright 2026 The Tekton Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package reconciler + +import ( + "testing" + + v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" + tracesdk "go.opentelemetry.io/otel/sdk/trace" + "go.opentelemetry.io/otel/sdk/trace/tracetest" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestClassifyWriteOutcome(t *testing.T) { + tests := []struct { + name string + metadataChanged bool + statusChanged bool + want string + }{{ + name: "nothing changed is a no-op", + want: "no-op", + }, { + name: "only the status changed", + statusChanged: true, + want: "status-only", + }, { + name: "a labels/annotations change is a full object write", + metadataChanged: true, + want: "write", + }, { + name: "a metadata change outranks a status change", + metadataChanged: true, + statusChanged: true, + want: "write", + }} + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := ClassifyWriteOutcome(tc.metadataChanged, tc.statusChanged) + if got != tc.want { + t.Errorf("ClassifyWriteOutcome(%t, %t) = %q, want %q", tc.metadataChanged, tc.statusChanged, got, tc.want) + } + }) + } +} + +// TestRecordWriteOutcome runs the same comparison and span tagging the +// reconcilers do, on real PipelineRunStatus values, and reads the resulting +// attribute back off a recorded span. +func TestRecordWriteOutcome(t *testing.T) { + started := v1.PipelineRunStatus{} + started.StartTime = &metav1.Time{} + // initTracing persists span context into the status, which is a real write. + spanCtxOnly := v1.PipelineRunStatus{} + spanCtxOnly.SpanContext = map[string]string{"traceparent": "abc"} + labels := map[string]string{"tekton.dev/pipeline": "build"} + + tests := []struct { + name string + oldStatus, newStatus v1.PipelineRunStatus + oldLabels, newLabels map[string]string + want string + }{{ + name: "untouched run is a no-op", + want: "no-op", oldLabels: labels, newLabels: labels, + }, { + name: "status moved but labels did not", + newStatus: started, want: "status-only", oldLabels: labels, newLabels: labels, + }, { + name: "span context persisted into status is still a write", + newStatus: spanCtxOnly, want: "status-only", oldLabels: labels, newLabels: labels, + }, { + name: "a label changed", want: "write", + oldLabels: labels, newLabels: map[string]string{"tekton.dev/pipeline": "deploy"}, + }} + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + recorder := tracetest.NewSpanRecorder() + provider := tracesdk.NewTracerProvider(tracesdk.WithSpanProcessor(recorder)) + _, span := provider.Tracer("test").Start(t.Context(), "reconcile") + + RecordWriteOutcome(span, tc.oldStatus, tc.newStatus, tc.oldLabels, tc.newLabels, nil, nil) + span.End() + + ended := recorder.Ended() + if len(ended) != 1 { + t.Fatalf("recorded %d spans, want 1", len(ended)) + } + value, found := "", false + for _, attr := range ended[0].Attributes() { + if string(attr.Key) == "reconcile.write_outcome" { + value, found = attr.Value.AsString(), true + } + } + if !found { + t.Fatalf("span is missing the %q attribute, got %v", WriteOutcomeAttributeKey, ended[0].Attributes()) + } + if value != tc.want { + t.Errorf("write_outcome = %q, want %q", value, tc.want) + } + }) + } +}