Skip to content

Record proxied, proxy_exit_code and cancelled on command telemetry - #499

Open
joe4dev wants to merge 6 commits into
mainfrom
devx-1004-distinguish-proxied-tool-exits-from-lstk-errors-in-command
Open

joe4dev wants to merge 6 commits into
mainfrom
devx-1004-distinguish-proxied-tool-exits-from-lstk-errors-in-command

Conversation

@joe4dev

@joe4dev joe4dev commented Sep 9, 2026 •

Copy link
Copy Markdown
Member

Motivation

lstk aws s3 lss (the user's typo, AWS CLI exit 252) and lstk aws s3 ls failing because Docker is down produce identical lstk_command events, so the analytics error ranking counts users' CLI mistakes as lstk errors (DEVX-1003). The failure rate is skewed the same way: a successful lstk aws s3 ls cannot be told from a successful lstk start. And a wrapped tool that traps Ctrl-C exits 1 or 130, which reads as a failure.

Solution

Three raw fields on lstk_command. Whose failure it was is derived in the fct_lstk_command pipe, which has to derive it for historical rows anyway.

Field Emitted Meaning
parameters.proxied always The invocation asked for a wrapped third-party tool (aws, az, cdk, sam, terraform). Extensions are lstk's own code and are not proxied
result.proxy_exit_code only when the tool ran to completion The tool's own exit code, 0 included. Absent means no third-party tool ran
result.cancelled always lstk's signal context was cancelled, or the interactive PTY pump forwarded a Ctrl-C to the tool. Never derived from the child's exit code
proxied proxy_exit_code Reading
false absent lstk's own command
true absent lstk failed before the tool ran (preflight, tool not installed)
true 0 tool succeeded
true 252 tool failed with its real code

Origin is declared at the five proxy exec sites via proc.MarkUserToolExit, not inferred from an *exec.ExitError, because lstk shells out for its own purposes too (brew, az cloud list, aws s3api create-bucket) and those exits read exit status N exactly like the user's. azurecli.Exec (the user's lstk az) and azurecli.Run (lstk's own) are split so only Exec marks. A proxyCommandAnnotation on the five proxies replaces DisableFlagParsing as the proxied signal and also drives subcommand. proc.RunInPTY records a forwarded Ctrl-C (proc.WasInterrupted) because on the interactive PTY path only the child receives SIGINT.

Also: make telemetry-sink prints events locally, and the integration env no longer inherits an ambient LOCALSTACK_DISABLE_EVENTS=1 that silently disabled every telemetry assertion.

Telemetry Data

lstk aws s3 lss against a running emulator, captured with make telemetry-sink (environment block omitted, it is unchanged).

Before (main):

{
  "parameters": {"command": "aws", "flags": null, "subcommand": "s3 lss"},
  "result": {"duration_ms": 302, "error_msg": "exit status 252", "exit_code": 252}
}

After:

{
  "parameters": {"command": "aws", "flags": null, "proxied": true, "subcommand": "s3 lss"},
  "result": {"cancelled": false, "duration_ms": 214, "error_msg": "exit status 252", "exit_code": 252, "proxy_exit_code": 252}
}

The same command with Docker down records "proxied": true and no proxy_exit_code; lstk start records "proxied": false; a Ctrl-C'd lstk aws whose CLI exits 130 records "cancelled": true, "proxy_exit_code": 130.

PTY exit-code capturing

The wrapped-tool PTY design (#476, full virtualization) routes an interactive Ctrl-C as a 0x03 byte into the tool's own PTY, so only the tool receives SIGINT and lstk's signal context never fires. cancelled therefore watches the pumped bytes: interruptWatcher forwards every byte unchanged and flags an ETX, and RunInPTY marks the returned error (proc.WasInterrupted). Verified side by side on a main build and this PR with fake tools in a PTY: a SIGINT-counting tool receives exactly one interrupt (exit 41) on both; a raw-mode tool still gets 0x03 as a key; Ctrl-Z stays a no-op; piped stdin containing 0x03 passes through byte-exact and is not counted; the terminal is restored after every run. The existing pager and streaming PTY tests pass. Two known asymmetries, documented in design.md: a raw-mode tool that consumes Ctrl-C as input and later exits non-zero reads as cancelled, and a Ctrl-C after which the tool exits 0 does not (the pipe reads exit_code = 0 first either way).

Analytics side

Design, pipe SQL, retroactive rule and panel changes: openspec/changes/distinguish-proxied-command-errors/design.md. Read proxy_exit_code with JSONHas first (JSONExtractInt returns 0 for a missing key); JSONHas(parameters, 'proxied') marks a post-cutover row, so no version gating is needed. This supersedes the proxy_error boolean discussed on DEVX-1004, per the DevX Weekly of 2026-09-21 ("the raw data rather than the interpretation"). The why axis (error_code/error_category) is the stacked #517.

Docs

Nothing user-facing: the lstk_command payload is internal and undocumented. make telemetry-sink is developer tooling, documented in CLAUDE.md.

Review

Human review advised: it changes a telemetry contract a pipe and two Grafana panels depend on, and the field shape departs from what the Linear thread agreed. An independent adversarial review at 42b8df6 found no blockers; its hygiene findings are in b67ab27.

Main questions for reviewers:

  • Is it better to track proxied=true|false explicitly or query it in Tinybird using command IN ('aws','az','cdk','sam','terraform') (asked in #support-data here)? -> This PR suggests explicit tracking/marking as the responsibility of lstk
  • Why do we track exit_code and proxy_exit_code separately when they are mostly the same? -> proxy_exit_code primarily carries presence information and could be replaced with proxy_error=true|false. Capturing the raw data makes no assumption (e.g., potential edge cases) and allows for direct interpretation.
  • Do we want to track cancelled=true|false explicitly or guess it from exit_code (typically 1 or 130) in Tinybird? -> This PR suggests explicit tracking because exit code rules would need to be tool-specific and could be polluted
  • @peter-smith-phd Why is this not a 10-file change only? -> Explicit marking of proxied tools is required and already a 12-file change excluding tests. The spec captures design decisions adds another 4 meta files. I removed everything related to error code telemetry as discussed (see Record the shown error code and category on command telemetry #517)
  • @dominikschubert How does cancelled tracking affect wrapped PTY behavior (Feed keyboard input into the wrapped-tool PTY #476)? -> Discussed in the section "PTY exit-code capturing"

Todo

  • Pipe change in localstack-dwh (fct_lstk_command.pipe) and the two Grafana panels once this ships

🤖 Generated with Claude Code

Closes DEVX-1004

@joe4dev joe4dev added semver: patch docs: skip Pull request does not require documentation changes labels Sep 9, 2026
@joe4dev
joe4dev force-pushed the devx-1004-distinguish-proxied-tool-exits-from-lstk-errors-in-command branch 3 times, most recently from e2f5cec to 8a19bc0 Compare September 10, 2026 13:00
@joe4dev joe4dev changed the title Distinguish proxied tool exits from lstk errors in command telemetry Record proxied, proxy_exit_code and cancelled on command telemetry Sep 24, 2026
joe4dev and others added 4 commits September 24, 2026 10:28
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
@joe4dev
joe4dev force-pushed the devx-1004-distinguish-proxied-tool-exits-from-lstk-errors-in-command branch from 3d88d9e to b802329 Compare September 24, 2026 08:32
Co-Authored-By: Claude <noreply@anthropic.com>
@joe4dev joe4dev added docs: needed Pull request requires documentation updates and removed docs: skip Pull request does not require documentation changes labels Sep 24, 2026
@joe4dev
joe4dev force-pushed the devx-1004-distinguish-proxied-tool-exits-from-lstk-errors-in-command branch from 97e23b2 to 42b8df6 Compare September 25, 2026 07:11
@joe4dev joe4dev added docs: skip Pull request does not require documentation changes and removed docs: needed Pull request requires documentation updates labels Sep 25, 2026
…de and sink

Co-Authored-By: Claude <noreply@anthropic.com>
@joe4dev

joe4dev commented Sep 25, 2026 •

Copy link
Copy Markdown
Member Author

Scenario test results (b67ab27)

40 scenarios, all matching the design. Setup: make build; make telemetry-sink in one terminal; every run with LSTK_ANALYTICS_ENDPOINT=http://127.0.0.1:8089, LSTK_KEYRING=file, an isolated HOME holding a one-block config.toml, and LOCALSTACK_DISABLE_EVENTS unset. "Real" rows used the running localstack-aws (or Azure) emulator and the installed aws/terraform/sam/cdk/az CLIs. "Fake X" is a script named X first on PATH that exits with the given code; the trap variant is trap 'exit 130' INT TERM; echo READY; sleep 30 & wait $!. Ctrl-C rows run lstk in a PTY that answers the terminal's colour/cursor queries, then write \x03. Columns are the event's proxied / proxy_exit_code / cancelled / exit_code.

# Setup Command proxied proxy_exit_code cancelled exit_code
1 real lstk aws s3 lss true 252 false 252
2 real lstk aws s3 ls true 0 false 0
3 real lstk aws s3 ls s3://missing true 254 false 254
4 real lstk tf version → command: terraform true 0 false 0
5 real, invalid HCL lstk terraform validate true 1 false 1
6 Azure emulator via lstk start --type azure, lstk setup azure lstk az group list / az group lst / az account show true 0 / 2 / 0 false 0 / 2 / 0
7 real, empty dir lstk sam validate, lstk cdk ls true 1 false 1
8 fake aws (trap 130), PTY Ctrl-C during lstk aws s3 ls true 130 true 130
9 fake aws (trap 130), stdout piped kill -TERM <lstk>, kill -INT <lstk> true 130 true 130
10 fake aws (sleep) kill -9 <aws child> true -1 false -1 (process exits 255)
12 DOCKER_HOST=tcp://localhost:1 lstk aws s3 ls true absent false 1
13 emulator stopped (lstk stop) lstk aws s3 ls true absent false 1
14 PATH = empty dir, --endpoint-url to a stub /_localstack/health lstk aws s3 ls true absent false 1
15 as 14 lstk aws --account 123 s3 ls true absent false 1
16 --endpoint-url http://127.0.0.1:1 lstk aws s3 ls true absent false 1
17 --endpoint-url to an Azure-shaped stub lstk aws s3 ls true absent false 1
18 real; backend "s3" project; fake aws exit 5 first on PATH lstk terraform init true absent false 5
19 real; initialised project LSTK_TF_DRY_RUN=1 lstk terraform plan true 0 (documented phantom) false 0
21 emulator stopped, keychain token, PTY lstk start + Ctrl-C at 0.3s, at 3s, and q false absent true 1
22 real, PTY lstk logs -f + q, + Ctrl-C false absent true 1
23 fresh HOME, Docker down lstk start --json --non-interactive false absent false 1
25 fake az exit 1, Azure config lstk setup azure --non-interactive → setup azure false absent false 1
26 as 25 lstk az stop-interception → az stop-interception false absent false 1
27, 28 isolated HOME lstk completion bash, lstk config path false absent false 0
29 reference extension lstk-ref on PATH lstk ref exit 7 → ext:ref false absent false 7
30 as 29, stdout piped lstk ref signal-wait + kill -TERM <lstk> false absent true 41
31 lstk-ref as bin/bundled-extensions + lstk-extensions.toml lstk ref exit 7 → ext:ref false absent false 7
32–35, 37 unknown command; --config /nonexistent; unknown flag; malformed config; LOCALSTACK_DISABLE_EVENTS=1 various no event (pre-existing PreRunE holes)

Not run: 11 (tool exits 0 then lstk fails; not triggerable), 24 (lstk update with brew failing; needs a Homebrew install). Environment restored afterwards (AWS emulator running, config type aws).

Human testing example

Screenshot 2026-09-25 at 10 45 22

Comment thread scripts/telemetry-sink.py
@@ -0,0 +1,44 @@
#!/usr/bin/env python3

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That addition makes it easy to smoke-test telemetry locally

// 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).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoids breaking tests when LOCALSTACK_DISABLE_EVENTS=1 is set (which I do by default)

Comment thread cmd/extension.go
tel.EmitCommand(ctx, "ext:"+name, "", nil, time.Since(start).Milliseconds(), exitCode, errorMsg)
// An extension is lstk's own code shipped separately, not a wrapped
// third-party tool, so its exit is lstk's and the event is not proxied.
result := commandResult(ctx, runErr, false)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new helper commandResult refactors several duplicates usages

Comment thread internal/awscli/exec.go
}

if err := runErr; err != nil {
if err := proc.MarkUserToolExit(runErr); err != nil {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Explicitly mark each proxy (i.e., user tool) because lstk uses external tools internally

Comment thread internal/proc/pty_unix.go

func (p *interruptWatcher) Write(b []byte) (int, error) {
if bytes.IndexByte(b, 0x03) >= 0 {
p.seen.Store(true)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's the place where we capture the cancelled flag by detecting 0x03, everything gets forwarded directly

@joe4dev
joe4dev marked this pull request as ready for review September 25, 2026 11:05
@joe4dev
joe4dev requested review from a team and peter-smith-phd as code owners September 25, 2026 11:05
@joe4dev
joe4dev requested a review from gtsiolis September 25, 2026 11:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs: skip Pull request does not require documentation changes semver: patch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant