fix(tracing): harden span export and telemetry semantics

[babs]

Follow-up to #902, addressing the review findings on the opt-in OpenTelemetry tracing. Stacked on mz-otel-instrumentation.

Span export correctness

• Timers now stop on every error path (command loop, FS unpacking, initial snapshots, Total Build Time incl. --dryrun), so a failed build exports the span of the command that failed — previously exactly that span was dropped (unended spans never reach the processor). Pinned by a regression test that goes red without the fix.
• logrus.Fatal deaths (e.g. snapshot timeout) now flush via an exit handler; the exit-time flush is bounded to 5s so a dead collector can't stall teardown, and errors print to stderr before the flush.
• Removed otel.SetTracerProvider: kaniko never consumed the global, but the vendored GCS/GCR transports do — it silently enabled orphan client spans.
• Tracing globals are mutex-guarded (the assertion hook can fire from any goroutine); Shutdown unwires the timing tracer so nothing mints spans into a dead provider.

Payload safety

• Attribute values capped at 64 KiB; OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT / OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT override, explicit -1 honored.
• New KANIKO_TELEMETRY_OMIT_DOCKERFILE opt-out for the Dockerfile-content attribute; warns when set to an unparseable boolean (fail-open would be silent otherwise) and when the endpoint is plaintext http://.
• README now states exactly what leaves the machine (Dockerfile source, instruction text incl. --mount=type=secret ids/env names — never values, FF values, cache keys), the HTTP-only protocol limit, and the full attribute reference.

Attribute semantics

• kaniko.build_id is content-addressed (sha256 of Dockerfile content + target) instead of hashing the mount path, which made it constant across a fleet.
• kaniko.ff.* keys lose the redundant FF_KANIKO_ prefix; README documents that only explicitly-set flags are reported.
• kaniko.cache.hit is only emitted when caching is on (absence ≠ miss); phase is derived from the command type so a cached RUN is no longer labeled build; kaniko.stage is an int; span names are a stable "Command" (full text stays in kaniko.command; BENCHMARK_FILE keys unchanged).
• OTEL_SERVICE_NAME now overrides the service.name default (resource env applied last, via semconv).

Deliberately not addressed here: trace start point (context fetch happens before Init), inferred cross-stage cache.key reporting, SIGTERM handling, category-map refactor — each noted in the README or code comments where relevant.

Summary by CodeRabbit

• New Features

  • Added an option to exclude Dockerfile content from telemetry.
  • Added warnings for plaintext telemetry endpoints.
  • Added safeguards to limit telemetry attribute sizes.

• Bug Fixes

  • Improved tracing reliability during concurrent operations and shutdown.
  • Ensured telemetry configuration overrides are applied consistently.

• Documentation

  • Documented Dockerfile telemetry exclusion, attribute size limits, and service name configuration.

• Tests

  • Expanded tracing and concurrency coverage.
  • Enabled race detection in standard and golden test runs.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 3f85d699-7ee6-4e02-8d56-b7112b50a370

📥 Commits

Reviewing files that changed from the base of the PR and between f5e51ab and c032023.

📒 Files selected for processing (2)

• docs/telemetry.md
• pkg/tracing/tracing.go

🚧 Files skipped from review as they are similar to previous changes (2)

• docs/telemetry.md
• pkg/tracing/tracing.go

---

📝 Walkthrough

Walkthrough

Changes

Telemetry and build observability

Layer / File(s)	Summary
Tracing lifecycle and metadata pkg/tracing/tracing.go, pkg/tracing/tracing_test.go, docs/telemetry.md	Tracing state is synchronized. Dockerfile export can be disabled. Span attributes are bounded. Resource attributes and service naming are updated. Shutdown uses a five-second flush timeout. Tests cover tracing attributes and build IDs.
Timing span synchronization and race validation pkg/timing/timing.go, pkg/timing/timing_test.go, scripts/test.sh, scripts/golden.sh	Enabled is renamed to TracingEnabled. Tracer and parent context access is synchronized. Tests and scripts run with the race detector.
Build command tracing integration pkg/executor/build.go	Command tracing uses TracingEnabled, and deferred timer behavior is documented.

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

Possibly related PRs

• osscontainertools/kaniko#902: Updates the same timing and build tracing integration, including the TracingEnabled rename.

Suggested labels: enhancement, tests

Suggested reviewers: mzihlmann

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 73.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the PR's main changes to harden tracing span export and telemetry behavior.
Description check	✅ Passed	The description clearly explains the tracing, payload, semantic, testing, and documentation changes, but omits the template checklist and release-notes section.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/otel-review-findings

---

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.}

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@pkg/tracing/tracing_test.go`:
- Around line 37-45: Update the test loop around spanLimits() to unset both
OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT and OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT
with cleanup before applying tc.spanEnv and tc.genEnv. Preserve the existing
conditional setup so empty table values remain truly unset and each case is
isolated from the test runner environment.

In `@pkg/tracing/tracing.go`:
- Around line 216-223: Prevent spans from being created during provider shutdown
and synchronize timing state: in pkg/tracing/tracing.go lines 216-223, call
timing.SetTracer with nil before provider.Shutdown; in pkg/timing/timing.go
lines 43-51, guard the paired parentCtx/tracer update and status read with a
mutex; and in pkg/timing/timing.go lines 98-111, copy both values under the same
read lock before starting a span.
- Around line 189-195: Update buildID to distinguish an empty readable
Dockerfile from an unreadable file by checking whether content is nil rather
than whether its length is nonzero. When content is non-nil, hash its contents
even when empty; retain the path fallback only for unreadable or absent content.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: c6de3485-63be-45ad-92e5-20bb65a96363

📥 Commits

Reviewing files that changed from the base of the PR and between 2b7031c and 5d3c0da.

📒 Files selected for processing (7)

• README.md
• cmd/executor/cmd/root.go
• pkg/executor/build.go
• pkg/executor/build_timer_test.go
• pkg/timing/timing.go
• pkg/tracing/tracing.go
• pkg/tracing/tracing_test.go

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)

docs/telemetry.md (2)

11-11: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Do not promise a span for every phase.

pkg/timing/timing.go suppresses spans for several timing categories, so change this to “each traced phase” or document the exclusions. Otherwise missing phase spans may be mistaken for exporter loss.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/telemetry.md` at line 11, Update the telemetry documentation’s
build-span description to avoid promising a span for every build phase: say
“each traced phase” or explicitly document the phase exclusions enforced by
pkg/timing/timing.go. Preserve the existing descriptions of the root build span,
Dockerfile command spans, span naming, and attributes.

---

9-9: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

Document the plaintext endpoint warning and opt-in contract.

pkg/tracing/tracing.go warns when KANIKO_TELEMETRY_ENDPOINT uses http://, and the standard OTEL endpoint alone does not enable tracing. This page presents HTTP without either warning, so add the same security and configuration guidance here.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/telemetry.md` at line 9, Update the telemetry documentation describing
the endpoint to warn that http:// sends data without transport encryption, and
state that tracing requires explicit opt-in via KANIKO_TELEMETRY_ENABLED;
setting only the standard OTEL endpoint does not enable it. Keep the existing
OTLP/HTTP and authentication guidance intact.

pkg/executor/build.go (1)

655-680: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Attach core command attributes before fallible cache work.

The command span is started at Line 601, but kaniko.command, phase, index, line, and stage are not attached until after FilesUsedFromContext and cache-key generation. If either operation fails, the deferred stop preserves an otherwise anonymous Command span, undermining the failing-command trace objective. Attach the non-cache attributes immediately after starting the timer; add cache attributes once the key is available.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/executor/build.go` around lines 655 - 680, Move construction and
attachment of the non-cache command span attributes from the current tracing
block to immediately after the command timer starts, including kaniko.command,
kaniko.command.hash, kaniko.phase, kaniko.instruction.index,
kaniko.instruction.line, and kaniko.stage. Keep cache-specific attributes in the
existing opts.Cache flow and append them only after cache-key generation
succeeds, ensuring failures in FilesUsedFromContext or compositeKey.Hash still
leave the command span identified.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@pkg/timing/timing.go`:
- Around line 109-115: Update the span creation flow around tracerMu and
tr.Start to keep tracerMu held through tr.Start, preventing Shutdown from
draining the provider between tracer snapshot and span creation. Preserve the
existing category check, span assignment, and attribute behavior while ensuring
tracer shutdown cannot overlap an in-flight start.

---

Outside diff comments:
In `@docs/telemetry.md`:
- Line 11: Update the telemetry documentation’s build-span description to avoid
promising a span for every build phase: say “each traced phase” or explicitly
document the phase exclusions enforced by pkg/timing/timing.go. Preserve the
existing descriptions of the root build span, Dockerfile command spans, span
naming, and attributes.
- Line 9: Update the telemetry documentation describing the endpoint to warn
that http:// sends data without transport encryption, and state that tracing
requires explicit opt-in via KANIKO_TELEMETRY_ENABLED; setting only the standard
OTEL endpoint does not enable it. Keep the existing OTLP/HTTP and authentication
guidance intact.

In `@pkg/executor/build.go`:
- Around line 655-680: Move construction and attachment of the non-cache command
span attributes from the current tracing block to immediately after the command
timer starts, including kaniko.command, kaniko.command.hash, kaniko.phase,
kaniko.instruction.index, kaniko.instruction.line, and kaniko.stage. Keep
cache-specific attributes in the existing opts.Cache flow and append them only
after cache-key generation succeeds, ensuring failures in FilesUsedFromContext
or compositeKey.Hash still leave the command span identified.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: c7ed2775-b5c6-425d-88fd-1942e2b31ed3

📥 Commits

Reviewing files that changed from the base of the PR and between 5d3c0da and 3b4e2ed.

📒 Files selected for processing (9)

• README.md
• cmd/executor/cmd/root.go
• docs/telemetry.md
• pkg/executor/build.go
• pkg/executor/build_timer_test.go
• pkg/timing/timing.go
• pkg/timing/timing_test.go
• pkg/tracing/tracing.go
• pkg/tracing/tracing_test.go

🚧 Files skipped from review as they are similar to previous changes (3)

• pkg/executor/build_timer_test.go
• pkg/tracing/tracing_test.go
• pkg/tracing/tracing.go

[codecov]

Codecov Report

❌ Patch coverage is 80.48780% with 8 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
pkg/tracing/tracing.go	73.33%	6 Missing and 2 partials ⚠️

📢 Thoughts on this report? Let us know!