Distinguish proxied tool exits from lstk errors in command telemetry

CLAUDE.md

@@ -182,6 +182,11 @@ When lstk's stdout and stderr are both terminals, `lstk aws` runs the child via
 
 `lstk az` has the identical wrapper pattern and gets the identical treatment (DEVX-1028, and the DEVX-1049 pager-input fix arrives through the shared `proc.RunInPTY`): `azurecli.Exec` takes the same `usePTY` parameter under the same both-streams-are-terminals condition, and `azurecli`'s own `execEnv` injects `PYTHONUNBUFFERED=1` (leaving a user-set value alone). Unlike the frozen aws v2 binary, every `az` distribution runs a real Python interpreter, so that env var is effective there — the PTY additionally makes az see a terminal, which is what its own terminal-gated output depends on. `azurecli.Run` (short captured-output execs behind `setup azure`/interception) always passes `usePTY: false`.
 
+# Attributing Wrapped-Tool Exits
+
+Separately from how a tool is *run*, each proxy exec site declares *whose* failure a non-zero exit is by passing the result through `proc.MarkUserToolExit`: `lstk aws`, `terraform`, `cdk`, `sam`, the `lstk az` passthrough (`azurecli.Exec` — **not** `azurecli.Run`, which is lstk's own orchestration), and extensions. Telemetry reads it back via `proc.IsUserToolExit` for `result.proxy_error` on `lstk_command` events, which keeps users' own CLI usage errors out of the lstk error ranking (DEVX-1004).
+
+Do not infer this from the runner. `proc.Run` promises signal handling only, and lstk shells out for its own purposes too (`brew upgrade` behind `lstk update`, the `aws` calls behind terraform backend provisioning). A new proxy that forgets the mark loses a data point; marking one of lstk's own execs hides an lstk bug behind the user's name — so leave it off when unsure.
 
 # Snapshots
 

cmd/extension.go

@@ -15,6 +15,7 @@ import (
 	"github.com/localstack/lstk/internal/extension"
 	"github.com/localstack/lstk/internal/log"
 	"github.com/localstack/lstk/internal/output"
+	"github.com/localstack/lstk/internal/proc"
 	"github.com/localstack/lstk/internal/runtime"
 	"github.com/localstack/lstk/internal/telemetry"
 	"github.com/spf13/cobra"
@@ -82,7 +83,7 @@ func dispatchExtension(ctx context.Context, cfg *env.Env, tel *telemetry.Client,
 	if runErr != nil {
 		errorMsg = runErr.Error()
 	}
-	tel.EmitCommand(ctx, "ext:"+name, "", nil, time.Since(start).Milliseconds(), exitCode, errorMsg)
+	tel.EmitCommand(ctx, "ext:"+name, "", nil, time.Since(start).Milliseconds(), exitCode, errorMsg, proc.IsUserToolExit(runErr))
 
 	return runErr
 }

cmd/root.go

@@ -23,6 +23,7 @@ import (
 	"github.com/localstack/lstk/internal/env"
 	"github.com/localstack/lstk/internal/log"
 	"github.com/localstack/lstk/internal/output"
+	"github.com/localstack/lstk/internal/proc"
 	"github.com/localstack/lstk/internal/runtime"
 	"github.com/localstack/lstk/internal/telemetry"
 	"github.com/localstack/lstk/internal/tracing"
@@ -581,7 +582,7 @@ func instrumentCommands(cmd *cobra.Command, tel *telemetry.Client) {
 				errorMsg = runErr.Error()
 			}
 
-			tel.EmitCommand(c.Context(), commandDisplayName(c), subcommand, flags, time.Since(startTime).Milliseconds(), exitCode, errorMsg)
+			tel.EmitCommand(c.Context(), commandDisplayName(c), subcommand, flags, time.Since(startTime).Milliseconds(), exitCode, errorMsg, proc.IsUserToolExit(runErr))
 
 			return runErr
 		}

internal/awscli/exec.go

@@ -108,7 +108,7 @@ func Exec(ctx context.Context, opts ExecOptions, stdout, stderr io.Writer, args
 		runErr = proc.Run(cmd)
 	}
 
-	if err := runErr; err != nil {
+	if err := proc.MarkUserToolExit(runErr); err != nil {
 		var exitErr *exec.ExitError
 		if errors.As(err, &exitErr) {
 			span.SetAttributes(attribute.Int("aws.exit_code", exitErr.ExitCode()))

internal/azurecli/exec.go

@@ -31,14 +31,21 @@ func CheckInstalled() error {
 	return nil
 }
 
-// Exec runs `az <args...>`. extraEnv is appended to the inherited process environment
-// (later entries win), letting callers inject AZURE_CONFIG_DIR, proxy, and CA settings
-// without mutating the user's global Azure CLI configuration.
+// Exec runs `az <args...>` on the user's behalf — the `lstk az` passthrough.
+// lstk's own az calls go through Run instead; only Exec attributes a non-zero
+// exit to the user. extraEnv is appended to the inherited process environment
+// (later entries win), letting callers inject AZURE_CONFIG_DIR, proxy, and CA
+// settings without mutating the user's global Azure CLI configuration.
 //
 // When usePTY is true (lstk's stdout and stderr are both terminals), the child's
 // output goes through a pseudo-terminal merged into stdout — see proc.RunInPTY
 // for why; otherwise stdout/stderr are wired as given.
 func Exec(ctx context.Context, extraEnv []string, usePTY bool, stdin io.Reader, stdout, stderr io.Writer, args ...string) error {
+	return proc.MarkUserToolExit(execAz(ctx, extraEnv, usePTY, stdin, stdout, stderr, args...))
+}
+
+// execAz is the shared body of Exec and Run, and claims neither's ownership.
+func execAz(ctx context.Context, extraEnv []string, usePTY bool, stdin io.Reader, stdout, stderr io.Writer, args ...string) error {
 	ctx, span := otel.Tracer("github.com/localstack/lstk/internal/azurecli").Start(ctx, "az cli")
 	defer span.End()
 
@@ -111,11 +118,13 @@ func setIfAbsent(env *[]string, key, value string) {
 	*env = append(*env, prefix+value)
 }
 
-// Run executes `az <args...>` with extraEnv and returns the captured stdout, stderr,
-// and any error. On non-zero exit, the error wraps stderr to aid debugging.
+// Run executes an `az <args...>` call lstk composed itself — the short
+// captured-output execs behind `setup azure` and interception — and returns the
+// captured stdout, stderr, and any error. On non-zero exit, the error wraps
+// stderr to aid debugging. A failure here is lstk's own, not the user's.
 func Run(ctx context.Context, extraEnv []string, args ...string) (stdout, stderr string, err error) {
 	var outBuf, errBuf bytes.Buffer
-	runErr := Exec(ctx, extraEnv, false, nil, &outBuf, &errBuf, args...)
+	runErr := execAz(ctx, extraEnv, false, nil, &outBuf, &errBuf, args...)
 	stdout = outBuf.String()
 	stderr = errBuf.String()
 	if runErr != nil {

internal/extension/exec.go

@@ -52,7 +52,7 @@ func Invoke(ctx context.Context, ext *Extension, args []string, runCtx Context)
 	cmd.Stdout = os.Stdout
 	cmd.Stderr = os.Stderr
 
-	if err := proc.Run(cmd); err != nil {
+	if err := proc.MarkUserToolExit(proc.Run(cmd)); err != nil {
 		var exitErr *exec.ExitError
 		if errors.As(err, &exitErr) {
 			span.SetAttributes(attribute.Int("extension.exit_code", exitErr.ExitCode()))

internal/iac/cdk/cli/exec.go

@@ -88,7 +88,7 @@ func Run(ctx context.Context, endpointURL, region string, sink output.Sink, logg
 	cmd.Stderr = os.Stderr
 	cmd.Env = BuildEnv(os.Environ(), effectiveEndpoint, s3Endpoint, region, forcePathStyle)
 
-	if err := proc.Run(cmd); err != nil {
+	if err := proc.MarkUserToolExit(proc.Run(cmd)); err != nil {
 		var exitErr *exec.ExitError
 		if errors.As(err, &exitErr) {
 			span.SetAttributes(attribute.Int("cdk.exit_code", exitErr.ExitCode()))

internal/iac/sam/cli/exec.go

@@ -77,7 +77,7 @@ func Run(ctx context.Context, endpointURL, account, region string, regionSelecte
 	cmd.Stderr = os.Stderr
 	cmd.Env = BuildEnv(os.Environ(), effectiveEndpoint, account, region)
 
-	if err := proc.Run(cmd); err != nil {
+	if err := proc.MarkUserToolExit(proc.Run(cmd)); err != nil {
 		var exitErr *exec.ExitError
 		if errors.As(err, &exitErr) {
 			span.SetAttributes(attribute.Int("sam.exit_code", exitErr.ExitCode()))

internal/iac/terraform/cli/exec.go

@@ -229,7 +229,7 @@ func runTerraform(ctx context.Context, span trace.Span, tfBin string, args []str
 	cmd.Stdout = os.Stdout
 	cmd.Stderr = os.Stderr
 
-	if err := proc.Run(cmd); err != nil {
+	if err := proc.MarkUserToolExit(proc.Run(cmd)); err != nil {
 		var exitErr *exec.ExitError
 		if errors.As(err, &exitErr) {
 			span.SetAttributes(attribute.Int("terraform.exit_code", exitErr.ExitCode()))

internal/proc/exit.go

@@ -0,0 +1,34 @@
+package proc
+
+import (
+	"errors"
+	"os/exec"
+)
+
+// userToolExitError is transparent — Error and Unwrap delegate — so callers
+// keep seeing the *exec.ExitError they always did.
+type userToolExitError struct{ err error }
+
+func (e *userToolExitError) Error() string { return e.err.Error() }
+func (e *userToolExitError) Unwrap() error { return e.err }
+
+// MarkUserToolExit marks a wrapped tool the user asked for exiting non-zero, so
+// telemetry attributes the failure to them rather than to lstk.
+//
+// Call it only where the invocation is the user's; running through Run is not
+// that claim. lstk shells out for its own purposes too, and marking one of
+// those hides an lstk bug behind the user's name.
+func MarkUserToolExit(err error) error {
+	var exitErr *exec.ExitError
+	if !errors.As(err, &exitErr) {
+		return err
+	}
+	return &userToolExitError{err: err}
+}
+
+// IsUserToolExit backs result.proxy_error on lstk_command telemetry events
+// (DEVX-1004).
+func IsUserToolExit(err error) bool {
+	var userTool *userToolExitError
+	return errors.As(err, &userTool)
+}

internal/proc/exit_test.go

@@ -0,0 +1,76 @@
+package proc
+
+import (
+	"errors"
+	"fmt"
+	"os/exec"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// The half that is easy to lose: an *exec.ExitError alone is not the user's
+// tool exiting, since lstk's own helper execs produce one too.
+func TestIsUserToolExitOnlyForMarkedToolExits(t *testing.T) {
+	t.Parallel()
+
+	t.Run("nil is not a user tool exit", func(t *testing.T) {
+		t.Parallel()
+		assert.False(t, IsUserToolExit(nil))
+		assert.NoError(t, MarkUserToolExit(nil))
+	})
+
+	t.Run("plain error is not a user tool exit", func(t *testing.T) {
+		t.Parallel()
+
+		err := errors.New("runtime not healthy")
+		assert.False(t, IsUserToolExit(err))
+		assert.False(t, IsUserToolExit(MarkUserToolExit(err)), "only a tool exit is the tool's fault")
+	})
+
+	t.Run("unmarked ExitError is not a user tool exit", func(t *testing.T) {
+		t.Parallel()
+		requireUnixShell(t)
+
+		err := exec.Command("sh", "-c", "exit 7").Run()
+		require.Error(t, err)
+		var exitErr *exec.ExitError
+		require.ErrorAs(t, err, &exitErr)
+		assert.False(t, IsUserToolExit(err), "lstk's own helper execs exit non-zero too")
+		assert.False(t, IsUserToolExit(fmt.Errorf("update failed: %w", err)))
+	})
+
+	t.Run("marked tool exit survives the caller's wrapping", func(t *testing.T) {
+		t.Parallel()
+		requireUnixShell(t)
+
+		err := MarkUserToolExit(Run(exec.Command("sh", "-c", "exit 252")))
+		require.Error(t, err)
+		assert.True(t, IsUserToolExit(err))
+		assert.True(t, IsUserToolExit(fmt.Errorf("terraform: %w", err)))
+	})
+
+	t.Run("missing binary is not a user tool exit", func(t *testing.T) {
+		t.Parallel()
+
+		err := MarkUserToolExit(Run(exec.Command("lstk-no-such-binary-devx1004")))
+		require.Error(t, err)
+		assert.False(t, IsUserToolExit(err), "the tool never ran, so its exit cannot be the cause")
+	})
+}
+
+// The marker must stay transparent: the proxy exec paths' errors.As checks,
+// cmd.ExitCode, and the telemetry error_msg all read through it.
+func TestMarkUserToolExitPreservesExitError(t *testing.T) {
+	t.Parallel()
+	requireUnixShell(t)
+
+	err := MarkUserToolExit(Run(exec.Command("sh", "-c", "exit 252")))
+	require.Error(t, err)
+
+	var exitErr *exec.ExitError
+	require.ErrorAs(t, err, &exitErr)
+	assert.Equal(t, 252, exitErr.ExitCode())
+	assert.Equal(t, "exit status 252", err.Error())
+}

internal/proc/run.go

@@ -1,6 +1,7 @@
 // Package proc runs wrapped external tools (aws, terraform, cdk, sam, az, and
 // extensions) so that termination signals reach the child gracefully instead of
-// hard-killing it.
+// hard-killing it, and lets a caller record whose failure a non-zero exit was
+// (MarkUserToolExit) — a separate claim from having run through Run.
 package proc
 
 import (

internal/telemetry/events.go

@@ -47,11 +47,18 @@ type CommandParameters struct {
 	Flags      []string `json:"flags"`
 }
 
-// CommandResult holds the outcome of a command invocation.
+// CommandResult holds the outcome of a command invocation. ProxyError marks a
+// failure caused by the user's own wrapped-tool invocation (see
+// proc.MarkUserToolExit), which analytics ranks apart from lstk's own; it is
+// false for every other outcome, success included — the field describes the
+// error's origin, not whether a tool was proxied. Not omitempty, so absence
+// means only "emitted before this field existed" and the consumer can date the
+// cutover.
 type CommandResult struct {
 	DurationMS int64  `json:"duration_ms"`
 	ExitCode   int    `json:"exit_code"`
 	ErrorMsg   string `json:"error_msg,omitempty"`
+	ProxyError bool   `json:"proxy_error"`
 }
 
 // LifecycleEvent is the payload for an lstk_lifecycle telemetry event.
@@ -108,14 +115,15 @@ func (c *Client) GetEnvironment(ctx context.Context) Environment {
 
 // EmitCommand emits an lstk_command telemetry event. The Environment block is
 // populated automatically from the client state.
-func (c *Client) EmitCommand(ctx context.Context, command, subcommand string, flags []string, durationMS int64, exitCode int, errorMsg string) {
+func (c *Client) EmitCommand(ctx context.Context, command, subcommand string, flags []string, durationMS int64, exitCode int, errorMsg string, proxyError bool) {
 	c.Emit(ctx, "lstk_command", ToMap(CommandEvent{
 		Environment: c.GetEnvironment(ctx),
 		Parameters:  CommandParameters{Command: command, Subcommand: subcommand, Flags: flags},
 		Result: CommandResult{
 			DurationMS: durationMS,
 			ExitCode:   exitCode,
 			ErrorMsg:   errorMsg,
+			ProxyError: proxyError,
 		},
 	}))
 }

internal/telemetry/events_test.go

@@ -68,7 +68,7 @@ func TestEmitCommand_SendsCorrectEventNameAndStructure(t *testing.T) {
 	tel, ch := captureEvents(t)
 
 	tel.SetAuthToken("ls-token")
-	tel.EmitCommand(context.Background(), "start", "", []string{"--non-interactive"}, 1200, 0, "")
+	tel.EmitCommand(context.Background(), "start", "", []string{"--non-interactive"}, 1200, 0, "", false)
 
 	got := drainEvent(t, tel, ch)
 
@@ -102,7 +102,7 @@ func TestEmitCommand_SendsCorrectEventNameAndStructure(t *testing.T) {
 func TestEmitCommand_IncludesErrorMsgOnFailure(t *testing.T) {
 	tel, ch := captureEvents(t)
 
-	tel.EmitCommand(context.Background(), "start", "", nil, 50, 1, "port 4566 already in use")
+	tel.EmitCommand(context.Background(), "start", "", nil, 50, 1, "port 4566 already in use", false)
 
 	got := drainEvent(t, tel, ch)
 	payload := got["payload"].(map[string]any)
@@ -114,7 +114,7 @@ func TestEmitCommand_IncludesErrorMsgOnFailure(t *testing.T) {
 func TestEmitCommand_RecordsSubcommandAndRealExitCode(t *testing.T) {
 	tel, ch := captureEvents(t)
 
-	tel.EmitCommand(context.Background(), "aws", "s3 ls", nil, 80, 252, "exit status 252")
+	tel.EmitCommand(context.Background(), "aws", "s3 ls", nil, 80, 252, "exit status 252", true)
 
 	got := drainEvent(t, tel, ch)
 	payload := got["payload"].(map[string]any)
@@ -123,12 +123,27 @@ func TestEmitCommand_RecordsSubcommandAndRealExitCode(t *testing.T) {
 	assert.Equal(t, "s3 ls", params["subcommand"])
 	result := payload["result"].(map[string]any)
 	assert.InDelta(t, 252, result["exit_code"], 0)
+	assert.Equal(t, true, result["proxy_error"])
+}
+
+// An lstk failure sends proxy_error: false rather than omitting the field, so
+// it is distinguishable from an event emitted before the field existed.
+func TestEmitCommand_SendsProxyErrorFalseForLstkFailures(t *testing.T) {
+	tel, ch := captureEvents(t)
+
+	tel.EmitCommand(context.Background(), "aws", "s3 ls", nil, 80, 1, "runtime not healthy", false)
+
+	got := drainEvent(t, tel, ch)
+	payload := got["payload"].(map[string]any)
+	result := payload["result"].(map[string]any)
+	require.Contains(t, result, "proxy_error")
+	assert.Equal(t, false, result["proxy_error"])
 }
 
 func TestEmitCommand_OmitsSubcommandWhenEmpty(t *testing.T) {
 	tel, ch := captureEvents(t)
 
-	tel.EmitCommand(context.Background(), "start", "", nil, 80, 0, "")
+	tel.EmitCommand(context.Background(), "start", "", nil, 80, 0, "", false)
 
 	got := drainEvent(t, tel, ch)
 	payload := got["payload"].(map[string]any)
@@ -145,7 +160,7 @@ func TestEmitCommand_IsNoOpWhenDisabled(t *testing.T) {
 	defer srv.Close()
 
 	tel := New(srv.URL, true) // disabled
-	tel.EmitCommand(context.Background(), "start", "", nil, 0, 0, "")
+	tel.EmitCommand(context.Background(), "start", "", nil, 0, 0, "", false)
 	tel.Close()
 
 	select {

test/integration/env/env.go

@@ -88,6 +88,10 @@ var ambientAWSKeys = []Key{
 func base() Environ {
 	return Environ(os.Environ()).
 		Without(ambientAWSKeys...).
+		// Commonly exported by LocalStack developers; inherited, it disables the
+		// telemetry client and every telemetry assertion times out. Tests
+		// covering the disabled path set it explicitly via With.
+		Without(DisableEvents).
 		With(AnalyticsEndpoint, UnreachableAnalyticsEndpoint).
 		With(AzureCollectTelemetry, "false").
 		With(SamCliTelemetry, "0")

test/integration/env/env_test.go

@@ -99,3 +99,18 @@ func TestExplicitAnalyticsEndpointOverridesDefault(t *testing.T) {
 		t.Fatalf("analytics endpoint = %q, want %q (explicit override must win over default)", got, mock)
 	}
 }
+
+// An ambient LOCALSTACK_DISABLE_EVENTS=1 must not reach the binary under test:
+// it disables telemetry, and every telemetry assertion then times out.
+func TestBaseEnvDropsAmbientDisableEvents(t *testing.T) {
+	t.Setenv(string(DisableEvents), "1")
+
+	if _, found := resolve(With("SOME_VAR", "value"), DisableEvents); found {
+		t.Fatalf("%s must be stripped from the base test environment", DisableEvents)
+	}
+
+	got, found := resolve(With(DisableEvents, "1"), DisableEvents)
+	if !found || got != "1" {
+		t.Fatalf("an explicit With(%s) must still win, got %q (found=%v)", DisableEvents, got, found)
+	}
+}