tracing: stop evaluating span attributes when tracing is off

[ultrotter]

tracing.NewTracer already short-circuits span creation behind an atomic flag, so creating a span costs nothing when no tracer provider is installed. The arguments of every tracer.Start call were still evaluated at the call site, though: formatting fingerprints with Sprintf, calling Name() and GroupKey(), building attribute.KeyValue structs and the variadic option slice. On the per-alert ingestion path (provider Put, routeAlert, groupAlert, aggregation group insert, inhibitor processAlert, both Mutes methods) that work ran for every alert with tracing disabled.

Measured with tracing disabled on a single Start call carrying an alert name and a formatted fingerprint:

attributes passed to Start: 314 ns/op, 5 allocs/op
bare Start, SetAttributes behind
span.IsRecording(): 5 ns/op, 0 allocs/op

The guarded number needed one more fix in tracing itself: Start returned the package-level noopSpan variable, and boxing a copy of that 80-byte noop.Span struct into the trace.Span interface allocated on every call. The variable is now of interface type, so it is boxed once at package init.

Move the attributes out of every tracer.Start call and set them with span.SetAttributes behind span.IsRecording(). On the hot paths, guard span.AddEvent calls that build attributes from computed values the same way. Constant span kinds stay in Start.

In the in-memory provider, stop allocating the propagation header map and running the propagator for every alert and listener when tracing is disabled; leave provider.Alert.Header nil, which both consumers (the dispatcher and the inhibitor) already treat as "nothing to extract". tracing.Enabled is exported for that check.

Add the convention to AGENTS.md and a go/ast guard test in tracing/ that fails when any tracer.Start call receives trace.WithAttributes, so the pattern does not creep back in with new spans.

Pull Request Checklist

Please check all the applicable boxes.

• Is this a bugfix?
  • I have added tests that can reproduce the bug which pass with this bugfix applied
• Does this change affect performance?
  • I have provided benchmarks comparison that shows performance is improved or is not degraded
• Is this a breaking change?
  • My changes do not break the existing cluster messages
  • My changes do not break the existing api
• I have signed-off my commits
• I will follow best practices for contributing to this project

Which user-facing changes does this PR introduce?

[PERF] Improved performance when tracing is disabled, avoiding memory allocations calculating values that are then discarded.

[coderabbitai]

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

⚙️ Run configuration

• Configuration used: Repository: prometheus/alertmanager/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: 61d381c8-ea81-4196-a1ba-21be57067156

📥 Commits

Reviewing files that changed from the base of the PR and between 72cea22 and edc5402.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: prometheus/alertmanager/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: 38e8fcca-6a8b-4e34-939c-6040b7ab8599

📥 Commits

Reviewing files that changed from the base of the PR and between ac5b996 and 72cea22.

📒 Files selected for processing: 1

• silence/silence.go

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review.

📝 Walkthrough

Tracing instrumentation now sets selected attributes and records selected events and errors only when spans are recording. The in-memory provider injects propagation metadata only when tracing is enabled. The tracing package exposes its enabled state, and tests check these conventions and disabled-tracing allocations.

Changes: Tracing changes

Layer / File(s)	Summary
Alert processing span guards dispatch/dispatch.go, inhibit/inhibit.go, inhibit/tracing_test.go, silence/silence.go	Dispatch, inhibition, and silence paths conditionally add span attributes, events, and error details when spans are recording. Inhibition tests cover mute behavior and allocations with tracing disabled.
Notification stage span guards notify/dedup_stage.go, notify/mute.go, notify/notify.go, notify/retry_stage.go, notify/set_notifies_stage.go, notify/tracing_test.go	Notification stages conditionally set attributes and record errors based on span recording state. Tests cover Integration.Notify verdicts and allocations with tracing disabled.
Memory provider propagation provider/mem/mem.go, provider/mem/tracing_test.go, tracing/tracing.go	The memory provider injects propagation metadata only when tracing is enabled. tracing.Enabled() reports the current tracing state. Tests cover empty-batch Put allocations with tracing disabled.
Tracing convention and enforcement AGENTS.md, tracing/attrs_guard_test.go, tracing/attrs_guard_cases_test.go	The guidance describes span attribute conventions. AST tests check recording guards, event and error options, and span-kind option placement.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Merge Risk: ⚪ Minimal · up to 72cea

The reviewed silence-path changes preserve mute behavior and the effective span kind. No actionable merge risk is identified in the supplied context.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 23.81% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 21 functions across 15 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly and concisely describes the main performance change: avoiding span-attribute evaluation when tracing is disabled.
Description check		The description explains the motivation, implementation, performance measurements, tests, and user-facing impact. It also completes the applicable bugfix, performance, compatibility, sign-off, and con…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Description check

Explanation

The description explains the motivation, implementation, performance measurements, tests, and user-facing impact. It also completes the applicable bugfix, performance, compatibility, sign-off, and contribution checklist items.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[SoloJacobs]

Can we cleanup the tracing callsite instead by using predefined functions? tracing should of course stay agnostic types in alertmanager, but we can do something like this:

// tracing/spans.go
package tracing

import (
        "context"

        "go.opentelemetry.io/otel/attribute"
        "go.opentelemetry.io/otel/trace"
        "go.opentelemetry.io/otel/trace/noop"
)

var (
        producer = []trace.SpanStartOption{trace.WithSpanKind(trace.SpanKindProducer)}

        // Boxed once, since converting noop.Span{} per call allocates.
        noopSpan trace.Span = noop.Span{}
)

// Tracer starts Alertmanager's spans. The zero value starts none.
type Tracer struct {
        t trace.Tracer
}

// StartSilencerMutes starts the span for silence.Silencer.Mutes.
func (tr Tracer) StartSilencerMutes(ctx context.Context, fingerprint func() string) (context.Context, trace.Span) {
        if tr.t == nil {
                return ctx, noopSpan
        }
        ctx, span := tr.t.Start(ctx, "silence.Silencer.Mutes")
        span.SetAttributes(attribute.String("alerting.alert.fingerprint", fingerprint()))
        return ctx, span
}

// StartInhibitorMutes starts the span for inhibit.Inhibitor.Mutes.
func (tr Tracer) StartInhibitorMutes(ctx context.Context, fingerprint func() st.Span) {
        if tr.t == nil {
                return ctx, noopSpan
        }
        ctx, span := tr.t.Start(ctx, "inhibit.Inhibitor.Mutes")
        span.SetAttributes(attribute.String("alerting.alert.fingerprint", fingerprint()))
        return ctx, span
}

// StartAlertFilter starts the span for the API alert filter.
func (tr Tracer) StartAlertFilter(ctx context.Context) (context.Context, trace.
        if tr.t == nil {
                return ctx, noopSpan
        }
        return tr.t.Start(ctx, "alertFilter")
}

// StartGroupAlert starts the span for dispatch.Dispatcher.groupAlert.
func (tr Tracer) StartGroupAlert(ctx context.Context, alertName string, fingerpstring) (context.Context, trace.Span) {
        if tr.t == nil {
                return ctx, noopSpan
        }
        ctx, span := tr.t.Start(ctx, "dispatch.Dispatcher.groupAlert")
        span.SetAttributes(
                attribute.String("alerting.alert.name", alertName),
                attribute.String("alerting.alert.fingerprint", fingerprint()),
                attribute.String("alerting.route.receiver.name", receiver),
        )
        return ctx, span
}

// StartAlertsPut starts the span for provider.mem.Put.
func (tr Tracer) StartAlertsPut(ctx context.Context, count int) (context.Contex
        if tr.t == nil {
                return ctx, noopSpan
        }
        ctx, span := tr.t.Start(ctx, "provider.mem.Put", producer...)
        span.SetAttributes(attribute.Int("alerting.alerts.count", count))
        return ctx, span
}

// tracing/tracing.go, in Manager

// 1. silence/silence.go
ctx, span := s.tracer.StartSilencerMutes(ctx, fp.String)
defer span.End()

// 2. inhibit/inhibit.go
_, span := ih.tracer.StartInhibitorMutes(ctx, fp.String)
defer span.End()

// 3. api/v2/api.go
ctx, span := api.tracer.StartAlertFilter(parent)
defer span.End()

// 4. dispatch/dispatch.go: a closure, since the method value
// alrt.Fingerprint().String would hash eagerly to bind its receiver.
_, span := d.tracer.StartGroupAlert(ctx, alrt.Name(),
        func() string { return alrt.Fingerprint().String() },
        route.RouteOpts.Receiver,
)
defer span.End()

// 5. provider/mem/mem.go
ctx, span := a.tracer.StartAlertsPut(ctx, len(alerts))
defer span.End()