fix(telemetry): skip git exec when disabled, shorten flush, clarify recorded data

- Guard getHeadSHA behind span.IsRecording() so it never execs when telemetry is off (was running git rev-parse on every repo every sync regardless)
- Shorten flush timeout 3s -> 1s so an unreachable endpoint adds at most 1s on exit
- Document that CLI args and action command lines are recorded; advise keeping secrets in env (never exported)
- Drop redundant gitte.repo double-set; drop always-nil error return from Init

Author: LauJosefsen

README.md

@@ -174,11 +174,16 @@ See [docs/config.md](./docs/config.md) for the full configuration reference.
 ## Telemetry
 
 Gitte can export OpenTelemetry traces to an OTLP/HTTP endpoint (e.g. Elastic
-APM) to help debug failures. Traces capture the command run, per-repo git
-context (branch, commit SHA, dirty state), and per-task outcomes with errors.
-To identify which developer and machine hit a failure, the OS username
-(`user.name`) and hostname (`host.name`) are attached to every trace. Full
-remote URLs and command environment values are never collected.
+APM) to help debug failures. Traces capture per-repo git context (branch,
+commit SHA, dirty state) and per-task outcomes with errors. To identify which
+developer and machine hit a failure, the OS username (`user.name`) and hostname
+(`host.name`) are attached to every trace.
+
+**What is recorded:** the gitte CLI arguments and each action's command line are
+exported as span attributes (this is intentional — knowing what ran is the
+point). Keep secrets out of action command definitions and CLI arguments; pass
+them through environment variables, which are **not** exported. Full remote
+URLs are never collected either (repos are identified by name only).
 
 Enable it via the shared config:
 

cmd/root.go

@@ -66,8 +66,7 @@ with dependency resolution.`,
 
 		// Telemetry: best-effort, never blocks. Stores root span context in globalCtx
 		// so it propagates through the executor into gitops/actions leaf spans.
-		shutdown, _ := telemetry.Init(globalCtx, globalCfg, cmd.Root().Version)
-		globalTelemetryShutdown = shutdown
+		globalTelemetryShutdown = telemetry.Init(globalCtx, globalCfg, cmd.Root().Version)
 		globalCtx, globalRootSpan = telemetry.StartCommandSpan(globalCtx, cmd.CommandPath(), args)
 		return nil
 	},

gitops/gitops.go

@@ -260,7 +260,11 @@ func syncProject(
 	if err != nil {
 		return err
 	}
-	setGitContextAttrs(span, name, currentBranch, getHeadSHA(ctx, projectPath), dirty)
+	// Guard on IsRecording so getHeadSHA (a git exec) never runs when telemetry
+	// is disabled — the span is non-recording and would discard the attributes.
+	if span.IsRecording() {
+		setGitContextAttrs(span, currentBranch, getHeadSHA(ctx, projectPath), dirty)
+	}
 	if dirty {
 		setDetail("skipped")
 		if currentBranch != defaultBranch {
@@ -423,11 +427,11 @@ func staleDays(ctx context.Context, dir, defaultBranch string) int {
 	return 0
 }
 
-// setGitContextAttrs records git context on a span. repo is the repo name/path
-// (never the full remote URL, which can embed credentials).
-func setGitContextAttrs(span trace.Span, repo, branch, sha string, dirty bool) {
+// setGitContextAttrs records git context on a span. The caller sets gitte.repo
+// separately (the repo name/path, never the full remote URL which can embed
+// credentials) so it is present on every span, including early-return paths.
+func setGitContextAttrs(span trace.Span, branch, sha string, dirty bool) {
 	span.SetAttributes(
-		attribute.String("gitte.repo", repo),
 		attribute.String("git.branch", branch),
 		attribute.String("git.sha", sha),
 		attribute.Bool("git.dirty", dirty),

gitops/telemetry_test.go

@@ -16,7 +16,7 @@ func TestSetGitContextAttrs(t *testing.T) {
 	t.Cleanup(func() { _ = tp.Shutdown(context.Background()) })
 
 	_, span := tp.Tracer("test").Start(context.Background(), "gitops.sync")
-	setGitContextAttrs(span, "group/repo", "main", "abc123", true)
+	setGitContextAttrs(span, "main", "abc123", true)
 	span.End()
 
 	spans := exp.GetSpans()
@@ -27,8 +27,6 @@ func TestSetGitContextAttrs(t *testing.T) {
 	dirty := false
 	for _, kv := range spans[0].Attributes {
 		switch kv.Key {
-		case "gitte.repo":
-			attrs["repo"] = kv.Value.AsString()
 		case "git.branch":
 			attrs["branch"] = kv.Value.AsString()
 		case "git.sha":
@@ -37,7 +35,7 @@ func TestSetGitContextAttrs(t *testing.T) {
 			dirty = kv.Value.AsBool()
 		}
 	}
-	if attrs["repo"] != "group/repo" || attrs["branch"] != "main" || attrs["sha"] != "abc123" || !dirty {
+	if attrs["branch"] != "main" || attrs["sha"] != "abc123" || !dirty {
 		t.Fatalf("attrs = %+v dirty=%v", attrs, dirty)
 	}
 }

telemetry/telemetry.go

@@ -24,7 +24,10 @@ import (
 
 const tracerName = "github.com/cego/gitte"
 
-const flushTimeout = 3 * time.Second
+// flushTimeout bounds how long exit can block flushing spans. Kept short so an
+// enabled-but-unreachable endpoint (e.g. laptop with the VPN off) adds at most
+// this delay to every command.
+const flushTimeout = 1 * time.Second
 
 // Resolved is the outcome of resolving telemetry settings from config + env.
 type Resolved struct {
@@ -92,15 +95,16 @@ func resourceAttributes(version, username, hostname string) []attribute.KeyValue
 	return attrs
 }
 
-// Init configures the global tracer provider. The returned shutdown function is
-// always non-nil and safe to call; it flushes pending spans with a bounded
-// timeout. Setup failures degrade to a no-op rather than returning an error.
-func Init(ctx context.Context, cfg *config.GitteConfig, version string) (func(), error) {
+// Init configures the global tracer provider and returns a shutdown function
+// that flushes pending spans with a bounded timeout. The returned function is
+// always non-nil and safe to call; setup failures and disabled telemetry both
+// degrade to a no-op shutdown.
+func Init(ctx context.Context, cfg *config.GitteConfig, version string) func() {
 	otel.SetErrorHandler(noopErrorHandler{})
 
 	r := Resolve(cfg)
 	if !r.Enabled {
-		return func() {}, nil
+		return func() {}
 	}
 
 	var opts []otlptracehttp.Option
@@ -114,7 +118,7 @@ func Init(ctx context.Context, cfg *config.GitteConfig, version string) (func(),
 	exporter, err := otlptracehttp.New(ctx, opts...)
 	if err != nil {
 		// Never block gitte: disable telemetry on exporter setup failure.
-		return func() {}, nil
+		return func() {}
 	}
 
 	username := ""
@@ -135,7 +139,7 @@ func Init(ctx context.Context, cfg *config.GitteConfig, version string) (func(),
 		shutdownCtx, cancel := context.WithTimeout(context.Background(), flushTimeout)
 		defer cancel()
 		_ = tp.Shutdown(shutdownCtx)
-	}, nil
+	}
 }
 
 // Tracer returns gitte's tracer from the global provider (a no-op tracer when

telemetry/telemetry_test.go

@@ -104,10 +104,7 @@ func TestResolve_Precedence(t *testing.T) {
 
 func TestInit_DisabledReturnsNoopShutdown(t *testing.T) {
 	t.Setenv("GITTE_TELEMETRY", "off")
-	shutdown, err := Init(context.Background(), &config.GitteConfig{}, "test")
-	if err != nil {
-		t.Fatalf("unexpected error: %v", err)
-	}
+	shutdown := Init(context.Background(), &config.GitteConfig{}, "test")
 	if shutdown == nil {
 		t.Fatal("shutdown must never be nil")
 	}
@@ -116,7 +113,7 @@ func TestInit_DisabledReturnsNoopShutdown(t *testing.T) {
 
 func TestInit_EnabledReturnsCallableShutdown(t *testing.T) {
 	// Verify that Init with a valid endpoint returns a non-nil shutdown function
-	// that can be called without panicking or hanging (bounded 3s flush).
+	// that can be called without panicking or hanging (bounded flush timeout).
 	// Note: otlptracehttp.New is lazy — it accepts any URL including unreachable
 	// endpoints without error, so Init succeeds and returns a real shutdown.
 	// The exporter-error branch (where New returns an error and Init falls back to
@@ -126,14 +123,11 @@ func TestInit_EnabledReturnsCallableShutdown(t *testing.T) {
 	prev := otel.GetTracerProvider()
 	t.Cleanup(func() { otel.SetTracerProvider(prev) })
 	cfg := &config.GitteConfig{Telemetry: config.TelemetryConfig{Endpoint: "http://localhost:4318"}}
-	shutdown, err := Init(context.Background(), cfg, "test")
-	if err != nil {
-		t.Fatalf("unexpected error: %v", err)
-	}
+	shutdown := Init(context.Background(), cfg, "test")
 	if shutdown == nil {
 		t.Fatal("shutdown must never be nil on the enabled path")
 	}
-	shutdown() // must not panic or hang beyond the 3s flush timeout
+	shutdown() // must not panic or hang beyond the flush timeout
 }
 
 func TestStartCommandSpan_NoProviderDoesNotPanic(t *testing.T) {