Trace reconciles: root span wrapper plus state-machine sub-spans
Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -6,7 +6,7 @@ Plan: `docs/plans/2026-08-24-1025-otel-tracing.md`
|
|||||||
- [x] Step 2 — New package `internal/tracing`
|
- [x] Step 2 — New package `internal/tracing`
|
||||||
- [x] Step 3 — `provider.WithTracing` decorator
|
- [x] Step 3 — `provider.WithTracing` decorator
|
||||||
- [x] Step 4 — `cmd/main.go` wiring
|
- [x] Step 4 — `cmd/main.go` wiring
|
||||||
- [ ] Step 5 — Reconciler spans
|
- [x] Step 5 — Reconciler spans
|
||||||
- [ ] Step 6 — Discovery server
|
- [ ] Step 6 — Discovery server
|
||||||
- [ ] Step 7 — GC + health
|
- [ ] Step 7 — GC + health
|
||||||
- [ ] Step 8 — GCP wire-log enrichment
|
- [ ] Step 8 — GCP wire-log enrichment
|
||||||
@@ -100,3 +100,21 @@ gcp constructor already used for wire-log options.
|
|||||||
Deviation: the plan put `--trace-health-probes` here, but the flag needs the
|
Deviation: the plan put `--trace-health-probes` here, but the flag needs the
|
||||||
`health.Engine.TraceProbes` field that Step 7 introduces — moved there to
|
`health.Engine.TraceProbes` field that Step 7 introduces — moved there to
|
||||||
keep every commit compiling.
|
keep every commit compiling.
|
||||||
|
|
||||||
|
## Step 5 — Reconciler spans
|
||||||
|
|
||||||
|
`SetupWithManager` completes with `tracing.NewReconciler("Proxy", r)`;
|
||||||
|
sub-spans `reconcile.managed` / `reconcile.replaceInstance` /
|
||||||
|
`reconcile.delete` open at the top of each state machine, and `status.patch`
|
||||||
|
opens inside the deferred flush closure so it stays within the root span
|
||||||
|
while its error still folds into the recorded result. `reconcileExternal`
|
||||||
|
left unspanned as planned (no I/O).
|
||||||
|
|
||||||
|
Two small judgment calls: the sub-spans carry no extra attributes — the
|
||||||
|
provider decorator already records `provider.id`, and the root span carries
|
||||||
|
the object identity, so duplicating them was noise; and `status.patch` is
|
||||||
|
emitted every reconcile even when nothing changed (the no-op compare is the
|
||||||
|
span's content — a real PATCH shows up as its k8s HTTP child). Tests that
|
||||||
|
call `r.Reconcile` directly bypass the wrapper; with no global tracer set
|
||||||
|
they see no-op spans, so the existing fake-client and envtest suites run
|
||||||
|
unchanged.
|
||||||
|
|||||||
@@ -41,6 +41,7 @@ import (
|
|||||||
crawlv1alpha1 "gitea.home.hrajfrisbee.cz/kacerr/egress-proxies-operator/api/v1alpha1"
|
crawlv1alpha1 "gitea.home.hrajfrisbee.cz/kacerr/egress-proxies-operator/api/v1alpha1"
|
||||||
"gitea.home.hrajfrisbee.cz/kacerr/egress-proxies-operator/internal/health"
|
"gitea.home.hrajfrisbee.cz/kacerr/egress-proxies-operator/internal/health"
|
||||||
"gitea.home.hrajfrisbee.cz/kacerr/egress-proxies-operator/internal/provider"
|
"gitea.home.hrajfrisbee.cz/kacerr/egress-proxies-operator/internal/provider"
|
||||||
|
"gitea.home.hrajfrisbee.cz/kacerr/egress-proxies-operator/internal/tracing"
|
||||||
)
|
)
|
||||||
|
|
||||||
// HealthSnapshotter provides the current probe verdict for a proxy. The
|
// HealthSnapshotter provides the current probe verdict for a proxy. The
|
||||||
@@ -96,9 +97,14 @@ func (r *ProxyReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res
|
|||||||
}
|
}
|
||||||
base := p.DeepCopy()
|
base := p.DeepCopy()
|
||||||
defer func() {
|
defer func() {
|
||||||
|
// Runs inside the root reconcile span (this defer fires before the
|
||||||
|
// tracing.NewReconciler wrapper sees the return), and its error is
|
||||||
|
// folded into err, which that wrapper records.
|
||||||
|
pctx, span := tracing.Start(ctx, "status.patch")
|
||||||
|
defer span.End()
|
||||||
// NotFound is expected when this reconcile just removed the last
|
// NotFound is expected when this reconcile just removed the last
|
||||||
// finalizer and the object is already gone.
|
// finalizer and the object is already gone.
|
||||||
if perr := r.patchStatusIfChanged(ctx, base, &p); perr != nil && !apierrors.IsNotFound(perr) {
|
if perr := r.patchStatusIfChanged(pctx, base, &p); perr != nil && !apierrors.IsNotFound(perr) {
|
||||||
err = errors.Join(err, perr)
|
err = errors.Join(err, perr)
|
||||||
}
|
}
|
||||||
}()
|
}()
|
||||||
@@ -114,6 +120,8 @@ func (r *ProxyReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res
|
|||||||
}
|
}
|
||||||
|
|
||||||
func (r *ProxyReconciler) reconcileManaged(ctx context.Context, p *crawlv1alpha1.Proxy) (ctrl.Result, error) {
|
func (r *ProxyReconciler) reconcileManaged(ctx context.Context, p *crawlv1alpha1.Proxy) (ctrl.Result, error) {
|
||||||
|
ctx, span := tracing.Start(ctx, "reconcile.managed")
|
||||||
|
defer span.End()
|
||||||
log := logf.FromContext(ctx)
|
log := logf.FromContext(ctx)
|
||||||
|
|
||||||
if controllerutil.AddFinalizer(p, crawlv1alpha1.FinalizerName) {
|
if controllerutil.AddFinalizer(p, crawlv1alpha1.FinalizerName) {
|
||||||
@@ -224,6 +232,8 @@ func (r *ProxyReconciler) reconcileManaged(ctx context.Context, p *crawlv1alpha1
|
|||||||
// exists" — hence: delete, poll to NotFound, only then advance the hash and
|
// exists" — hence: delete, poll to NotFound, only then advance the hash and
|
||||||
// let the create branch run.
|
// let the create branch run.
|
||||||
func (r *ProxyReconciler) replaceInstance(ctx context.Context, p *crawlv1alpha1.Proxy, prov provider.Provider, hash string) (ctrl.Result, error) {
|
func (r *ProxyReconciler) replaceInstance(ctx context.Context, p *crawlv1alpha1.Proxy, prov provider.Provider, hash string) (ctrl.Result, error) {
|
||||||
|
ctx, span := tracing.Start(ctx, "reconcile.replaceInstance")
|
||||||
|
defer span.End()
|
||||||
_, err := prov.Get(ctx, p.Status.ProviderID)
|
_, err := prov.Get(ctx, p.Status.ProviderID)
|
||||||
if provider.Class(err) == provider.ErrNotFound {
|
if provider.Class(err) == provider.ErrNotFound {
|
||||||
// Old instance is gone. The Update inside setSpecHash refreshes p
|
// Old instance is gone. The Update inside setSpecHash refreshes p
|
||||||
@@ -252,6 +262,8 @@ func (r *ProxyReconciler) replaceInstance(ctx context.Context, p *crawlv1alpha1.
|
|||||||
}
|
}
|
||||||
|
|
||||||
func (r *ProxyReconciler) reconcileDelete(ctx context.Context, p *crawlv1alpha1.Proxy) (ctrl.Result, error) {
|
func (r *ProxyReconciler) reconcileDelete(ctx context.Context, p *crawlv1alpha1.Proxy) (ctrl.Result, error) {
|
||||||
|
ctx, span := tracing.Start(ctx, "reconcile.delete")
|
||||||
|
defer span.End()
|
||||||
if !controllerutil.ContainsFinalizer(p, crawlv1alpha1.FinalizerName) {
|
if !controllerutil.ContainsFinalizer(p, crawlv1alpha1.FinalizerName) {
|
||||||
return ctrl.Result{}, nil
|
return ctrl.Result{}, nil
|
||||||
}
|
}
|
||||||
@@ -409,7 +421,10 @@ func (r *ProxyReconciler) SetupWithManager(mgr ctrl.Manager) error {
|
|||||||
if r.HealthEvents != nil {
|
if r.HealthEvents != nil {
|
||||||
b = b.WatchesRawSource(source.Channel(r.HealthEvents, &handler.EnqueueRequestForObject{}))
|
b = b.WatchesRawSource(source.Channel(r.HealthEvents, &handler.EnqueueRequestForObject{}))
|
||||||
}
|
}
|
||||||
return b.Complete(r)
|
// Root span per reconcile; sub-reconcilers and the status flush hang
|
||||||
|
// their spans off it. Tests calling r.Reconcile directly bypass the
|
||||||
|
// wrapper and see no-op spans — the global tracer is never set there.
|
||||||
|
return b.Complete(tracing.NewReconciler("Proxy", r))
|
||||||
}
|
}
|
||||||
|
|
||||||
func (r *ProxyReconciler) applyDefaults() {
|
func (r *ProxyReconciler) applyDefaults() {
|
||||||
|
|||||||
Reference in New Issue
Block a user