diff --git a/docs/telemetry.md b/docs/telemetry.md index 0bd701928..d981c8ef4 100644 --- a/docs/telemetry.md +++ b/docs/telemetry.md @@ -10,6 +10,10 @@ Spans are sent over OTLP/HTTP (`http://` or `https://`, collector port 4318 by d Each build is one trace: a root `build` span plus a span per build phase and Dockerfile command. Command spans are named `Command` (low cardinality, so backends can aggregate on the name). The full instruction text is in the `kaniko.command` attribute. The build phases keep their descriptive names. +Set `KANIKO_TELEMETRY_OMIT_DOCKERFILE=true` to keep the Dockerfile source out of the trace. + +Attribute values are capped at 64 KiB. `OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT` and `OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT` override the cap, including an explicit `-1` for unlimited. + ## Build span | Attribute | Value | @@ -20,7 +24,7 @@ Each build is one trace: a root `build` span plus a span per build phase and Doc | `kaniko.target` | build target(s), comma-joined | | `kaniko.build_id` | sha256 of Dockerfile content + target, for grouping runs of the same build (falls back to the path when the Dockerfile is unreadable) | | `kaniko.ff.*` | explicitly-set `FF_KANIKO_*` feature flags (flags left at their defaults are not reported) | -| `service.name` | `kaniko` | +| `service.name` | `kaniko`, unless `OTEL_SERVICE_NAME` is set | ## Command spans diff --git a/pkg/executor/build.go b/pkg/executor/build.go index 36847a634..a5f8967d4 100644 --- a/pkg/executor/build.go +++ b/pkg/executor/build.go @@ -583,6 +583,7 @@ func (s *stageBuilder) build(compositeKey CompositeCache, opts *config.KanikoOpt cacheGroup := errgroup.Group{} var cmdTimer *timing.Timer + // stop on the way out too: an unended span is never exported defer func() { if cmdTimer != nil { timing.DefaultRun.Stop(cmdTimer) @@ -647,7 +648,7 @@ func (s *stageBuilder) build(compositeKey CompositeCache, opts *config.KanikoOpt } }() - if timing.Enabled() { + if timing.TracingEnabled() { phase := "kaniko" switch command.(type) { case *commands.RunCommand, *commands.RunMarkerCommand: diff --git a/pkg/timing/timing.go b/pkg/timing/timing.go index b54ef4351..c2027ba2b 100644 --- a/pkg/timing/timing.go +++ b/pkg/timing/timing.go @@ -33,16 +33,21 @@ var currentTimeFunc = time.Now var DefaultRun = NewTimedRun() var ( + tracerMu sync.Mutex tracer trace.Tracer parentCtx context.Context ) func SetTracer(ctx context.Context, t trace.Tracer) { + tracerMu.Lock() + defer tracerMu.Unlock() parentCtx = ctx tracer = t } -func Enabled() bool { +func TracingEnabled() bool { + tracerMu.Lock() + defer tracerMu.Unlock() return tracer != nil } @@ -95,8 +100,11 @@ func Start(category string) *Timer { category: category, startTime: currentTimeFunc(), } - if tracer != nil && !noSpanCategories[category] { - _, t.span = tracer.Start(parentCtx, category) + tracerMu.Lock() + tr, ctx := tracer, parentCtx + tracerMu.Unlock() + if tr != nil && !noSpanCategories[category] { + _, t.span = tr.Start(ctx, category) t.span.SetAttributes(attribute.String("kaniko.phase", phaseFor(category))) } return &t diff --git a/pkg/timing/timing_test.go b/pkg/timing/timing_test.go index ecf0b0d9a..c9155ec22 100644 --- a/pkg/timing/timing_test.go +++ b/pkg/timing/timing_test.go @@ -17,8 +17,11 @@ limitations under the License. package timing import ( + "context" "testing" "time" + + "go.opentelemetry.io/otel/trace/noop" ) func patchTime(timeFunc func() time.Time) func() { @@ -87,3 +90,21 @@ func TestTimedRun_StartStop(t *testing.T) { }) } } + +// Regression for the SetTracer/Start data race: cache-push goroutines call +// Start while the shutdown path unwires the tracer. Run under -race. +func TestSetTracerConcurrentWithStart(t *testing.T) { + done := make(chan struct{}) + go func() { + defer close(done) + for range 1000 { + DefaultRun.Stop(Start("race-probe")) + } + }() + tr := noop.NewTracerProvider().Tracer("test") + for range 1000 { + SetTracer(context.Background(), tr) + SetTracer(context.Background(), nil) + } + <-done +} diff --git a/pkg/tracing/tracing.go b/pkg/tracing/tracing.go index b5ac0d6e9..be8db470b 100644 --- a/pkg/tracing/tracing.go +++ b/pkg/tracing/tracing.go @@ -24,7 +24,9 @@ import ( "encoding/hex" "fmt" "os" + "strconv" "strings" + "sync" "time" "go.opentelemetry.io/otel/attribute" @@ -32,6 +34,7 @@ import ( "go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp" "go.opentelemetry.io/otel/sdk/resource" sdktrace "go.opentelemetry.io/otel/sdk/trace" + semconv "go.opentelemetry.io/otel/semconv/v1.37.0" "go.opentelemetry.io/otel/trace" "github.com/osscontainertools/kaniko/pkg/assert" @@ -42,18 +45,41 @@ import ( ) var ( + mu sync.Mutex provider *sdktrace.TracerProvider rootSpan trace.Span ) +// enables tracing when set to an OTLP-HTTP collector URL. const EndpointEnv = "KANIKO_TELEMETRY_ENDPOINT" + +// whether to keep the Dockerfile source out of the trace. +const OmitDockerfileEnv = "KANIKO_TELEMETRY_OMIT_DOCKERFILE" + const shutdownFlushTimeout = 5 * time.Second +// one oversized value gets the whole OTLP batch rejected, not just that attribute. +const attributeValueLengthLimit = 64 * 1024 + +// pass these raw, WithSpanLimits would clamp an explicit -1 (unlimited) to the default. +func spanLimits() sdktrace.SpanLimits { + limits := sdktrace.NewSpanLimits() + _, spanSet := os.LookupEnv("OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT") + _, generalSet := os.LookupEnv("OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT") + if !spanSet && !generalSet { + limits.AttributeValueLengthLimit = attributeValueLengthLimit + } + return limits +} + func Init(ctx context.Context, opts *config.KanikoOptions) { endpoint := os.Getenv(EndpointEnv) if endpoint == "" { return } + if strings.HasPrefix(endpoint, "http://") { + logrus.Warnf("%s uses plaintext http: spans (including Dockerfile content) are sent unencrypted", EndpointEnv) + } exp, err := otlptracehttp.New(ctx, otlptracehttp.WithEndpointURL(endpoint)) if err != nil { logrus.Debugf("tracing disabled: failed to create OTLP exporter: %v", err) @@ -65,23 +91,39 @@ func Init(ctx context.Context, opts *config.KanikoOptions) { logrus.Debugf("tracing: Dockerfile not readable, kaniko.dockerfile.content omitted: %v", cerr) } res, err := resource.New(ctx, - resource.WithFromEnv(), resource.WithAttributes(buildAttrs(opts, content)...), + resource.WithFromEnv(), ) if err != nil { logrus.Debugf("tracing: partial resource, continuing: %v", err) } - provider = sdktrace.NewTracerProvider( + + // Deliberately NOT otel.SetTracerProvider: kaniko takes its tracer from + // the provider directly, and the global would silently switch on client + // spans in the vendored GCS/GCR transports, polluting the trace. + tp := sdktrace.NewTracerProvider( sdktrace.WithBatcher(exp), sdktrace.WithResource(res), + sdktrace.WithRawSpanLimits(spanLimits()), ) - tracer := provider.Tracer("github.com/osscontainertools/kaniko") - var sctx context.Context - sctx, rootSpan = tracer.Start(ctx, "build") - if cerr == nil { - rootSpan.SetAttributes(attribute.String("kaniko.dockerfile.content", string(content))) + tracer := tp.Tracer("github.com/osscontainertools/kaniko") + sctx, span := tracer.Start(ctx, "build") + raw, set := os.LookupEnv(OmitDockerfileEnv) + if set { + _, perr := strconv.ParseBool(raw) + if perr != nil { + logrus.Warnf("%s=%q is not a valid boolean; Dockerfile content WILL be exported", OmitDockerfileEnv, raw) + } } + if cerr == nil && !config.EnvBool(OmitDockerfileEnv) { + span.SetAttributes(attribute.String("kaniko.dockerfile.content", string(content))) + } + + mu.Lock() + provider, rootSpan = tp, span + mu.Unlock() + timing.SetTracer(sctx, tracer) // hook, not import, so assert does not depend on tracing @@ -91,6 +133,7 @@ func Init(ctx context.Context, opts *config.KanikoOptions) { // onAssertion flushes before the panic from a violated assertion escapes. func onAssertion(name, msg string) { + mu.Lock() if rootSpan != nil { rootSpan.SetAttributes(attribute.Bool("kaniko.assertion_violated", true)) rootSpan.AddEvent("assertion violated", trace.WithAttributes( @@ -98,6 +141,7 @@ func onAssertion(name, msg string) { attribute.String("kaniko.assertion.message", msg), )) } + mu.Unlock() Shutdown(fmt.Errorf("assertion violated [%s]: %s", name, msg)) } @@ -106,7 +150,7 @@ func onAssertion(name, msg string) { func buildAttrs(opts *config.KanikoOptions, dockerfile []byte) []attribute.KeyValue { target := strings.Join(opts.Target, ",") attrs := []attribute.KeyValue{ - attribute.String("service.name", "kaniko"), + semconv.ServiceName("kaniko"), attribute.String("kaniko.version", version.Version()), attribute.String("kaniko.dockerfile", opts.DockerfilePath), attribute.String("kaniko.target", target), @@ -139,6 +183,8 @@ func buildID(path, target string, content []byte) string { // Shutdown ends the root span with the outcome and flushes. Idempotent. A // killed process leaves the root span unended, which the backend marks crashed. func Shutdown(err error) { + mu.Lock() + defer mu.Unlock() if provider == nil { return } @@ -151,6 +197,7 @@ func Shutdown(err error) { rootSpan.End() rootSpan = nil } + timing.SetTracer(context.Background(), nil) ctx, cancel := context.WithTimeout(context.Background(), shutdownFlushTimeout) defer cancel() if sderr := provider.Shutdown(ctx); sderr != nil { diff --git a/pkg/tracing/tracing_test.go b/pkg/tracing/tracing_test.go new file mode 100644 index 000000000..64ed9e955 --- /dev/null +++ b/pkg/tracing/tracing_test.go @@ -0,0 +1,72 @@ +/* +Copyright 2026 OSS Container Tools + +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 tracing + +import ( + "testing" + + "github.com/osscontainertools/kaniko/pkg/config" +) + +// Pins the attribute-name contract dashboards are built on. +func TestBuildAttrs(t *testing.T) { + t.Setenv("FF_KANIKO_TRACING_TEST_FLAG", "true") + opts := &config.KanikoOptions{DockerfilePath: "/workspace/Dockerfile"} + + got := map[string]string{} + for _, kv := range buildAttrs(opts, []byte("FROM scratch")) { + got[string(kv.Key)] = kv.Value.AsString() + } + + if got["service.name"] != "kaniko" { + t.Errorf("service.name = %q, want kaniko", got["service.name"]) + } + // FF keys must not double the prefix: kaniko.ff.TRACING_TEST_FLAG, + // not kaniko.ff.FF_KANIKO_TRACING_TEST_FLAG. + if got["kaniko.ff.TRACING_TEST_FLAG"] != "true" { + t.Errorf("kaniko.ff.TRACING_TEST_FLAG = %q, want true", got["kaniko.ff.TRACING_TEST_FLAG"]) + } + if _, dup := got["kaniko.ff.FF_KANIKO_TRACING_TEST_FLAG"]; dup { + t.Error("FF key kept its FF_KANIKO_ prefix") + } + if got["kaniko.dockerfile"] != "/workspace/Dockerfile" { + t.Errorf("kaniko.dockerfile = %q", got["kaniko.dockerfile"]) + } + if got["kaniko.build_id"] == "" { + t.Error("kaniko.build_id missing") + } +} + +func TestBuildID(t *testing.T) { + content := []byte("FROM scratch\nRUN true\n") + // Content-addressed: same content+target => same id, regardless of path. + if buildID("/a/Dockerfile", "", content) != buildID("/b/Dockerfile", "", content) { + t.Error("build_id must depend on content, not path, when content is available") + } + // Different content => different id. + if buildID("/a/Dockerfile", "", content) == buildID("/a/Dockerfile", "", []byte("FROM busybox\n")) { + t.Error("build_id must change with content") + } + // Fallback: no content => path-based, distinct from the content id. + if buildID("/a/Dockerfile", "", nil) == buildID("/a/Dockerfile", "", content) { + t.Error("path fallback must differ from the content-based id") + } + // A readable-but-empty Dockerfile is content-addressed, not path-based. + if buildID("/a/Dockerfile", "", []byte{}) != buildID("/b/Dockerfile", "", []byte{}) { + t.Error("empty readable Dockerfile must be content-addressed") + } +} diff --git a/scripts/golden.sh b/scripts/golden.sh index 436c0c9b0..ab34cab60 100755 --- a/scripts/golden.sh +++ b/scripts/golden.sh @@ -22,6 +22,7 @@ GREEN='\033[0;32m' RESET='\033[0m' FLAGS=( + "-race" "-cover" "-coverprofile=out/coverage.out" "-timeout=120s" diff --git a/scripts/test.sh b/scripts/test.sh index 43edaac80..8e24d4669 100755 --- a/scripts/test.sh +++ b/scripts/test.sh @@ -24,7 +24,7 @@ RESET='\033[0m' echo "Running go tests..." export KANIKO_DIR="/kaniko" -go test -cover -coverprofile=out/coverage.out -v -timeout 120s `go list ./... | grep -v vendor | grep -v golden | grep -v integration` | sed ''/PASS/s//$(printf "${GREEN}PASS${RESET}")/'' | sed ''/FAIL/s//$(printf "${RED}FAIL${RESET}")/'' +go test -race -cover -coverprofile=out/coverage.out -v -timeout 120s `go list ./... | grep -v vendor | grep -v golden | grep -v integration` | sed ''/PASS/s//$(printf "${GREEN}PASS${RESET}")/'' | sed ''/FAIL/s//$(printf "${RED}FAIL${RESET}")/'' GO_TEST_EXIT_CODE=${PIPESTATUS[0]} if [[ ${GO_TEST_EXIT_CODE} -ne 0 ]]; then exit "${GO_TEST_EXIT_CODE}"