Add error-category telemetry to SshTunnelEvent

[anton-107]

Changes

Add error_category to SshTunnelEvent (libs/telemetry/protos/ssh_tunnel.go) and set it at each failure site in the ssh connect flow (experimental/ssh/internal/client/client.go). Categories name the distinct early-return sites — IDE_COMMAND_NOT_ON_PATH, CLUSTER_ACCESS_DENIED, SERVER_START_TIMEOUT, USER_ABORTED, etc. — so no raw error text, cluster name, or path is logged.

Two details worth a reviewer's attention:

• The telemetry defer now registers before the IDE precondition checks. It sat after them, so --ide failures on a missing code/cursor command returned early and emitted no event at all. They were not merely uncategorized, they were absent from the failure counts entirely.
• The field is declared without omitempty, so a success sends TYPE_UNSPECIFIED explicitly rather than collapsing to a null that cannot be told apart from a CLI too old to report the field.

The failure outcome is collected in a small connectOutcome struct and Run uses a named return, so the deferred logger observes the error the caller sees. A cancelled context maps to USER_ABORTED and takes precedence over the category recorded at the failure site, since Ctrl-C surfaces as a cancellation from whichever call happens to observe it first.

Why

IDE-mode connections have a 40–55% failure rate, but telemetry only records that a connection failed, so the cause is invisible. The leading hypothesis was that CheckIDECommand rejects users whose IDE shell command is not on PATH — a permanent per-machine condition, which matches the observed stickiness (a retry after a failed first attempt succeeds only 17–21% of the time).

That hypothesis was untestable for a second reason beyond the missing field: those checks ran before the telemetry defer, so they produced no event. Adding the field alone would not have confirmed or refuted it. This also means the sub-5s failure bucket in the original analysis could not have contained the PATH failures.

Deliberately left uncategorized: the malformed --metadata paths fall through to UNKNOWN. --metadata is a hidden flag whose value the CLI generates itself in ToProxyCommand, so a parse failure is a CLI bug, not a user-environment blocker. Mapping it to SERVER_START_TIMEOUT would pollute the bucket that tracks unreachable servers.

Scope note: is_success still carries omitempty, so failures remain NULL rather than false. That is tracked separately and not touched here. Until it changes, count failures via error_category (NOT IN ('TYPE_UNSPECIFIED'), plus an IS NOT NULL guard for rows from CLIs predating this field) rather than is_success = false.

The matching backend schema change has landed, so these values are queryable once this rolls out. Every enum spelling matches the constants added here exactly: the CLI serializes the enum name as a string, so a drift would silently decode to TYPE_UNSPECIFIED rather than fail loudly.

Tests

Unit tests in client_internal_test.go cover the category mapping: success reports TYPE_UNSPECIFIED, an attributed failure keeps its category, an unattributed one falls back to UNKNOWN, a cancellation reports USER_ABORTED and wins over the site category, and a non-zero exit after the tunnel is up is not counted as a connection failure.

Verified locally: ./task test-exp-ssh (278 unit + 4 acceptance) and full ./task lint (0 issues, all three modules).

I also drove Run with PATH emptied — the exact condition of the hypothesis above, since CheckIDECommand resolves the IDE command with exec.LookPath. The emitted payload is:

{"compute_type":"DEDICATED","ide_type":"vscode","client_mode":"IDE",
 "server_start_time_ms":0,"error_category":"IDE_COMMAND_NOT_ON_PATH"}

That confirms an event is emitted at all on this path, and that every category serializes to one of the declared enum names.

Still not verified end to end against real compute — no event has been observed landing in the telemetry table. Worth doing before this is relied on for dashboards.

No changelog fragment: the feature is under experimental/ and this is internal telemetry, matching #4881 and #6058.

This PR was written by Claude Code.

[eng-dev-ecosystem-bot]

Integration test report

Commit: 112dbc6

Run: 32477924023

	Env	💚​RECOVERED	🙈​SKIP	✅​pass	🙈​skip	Time
💚​	aws linux	1	4	274	1162	5:17
💚​	aws windows	1	4	276	1160	7:26
💚​	azure linux	1	4	273	1162	5:23
💚​	azure windows	1	4	275	1160	2:56
💚​	gcp linux	1	4	274	1162	6:10
💚​	gcp windows	1	4	276	1160	7:24

	Test Name	aws linux	aws windows	azure linux	azure windows	gcp linux	gcp windows
💚​	TestAccept	💚​R	💚​R	💚​R	💚​R	💚​R	💚​R
🙈​	TestAccept/bundle/invariant/no_drift	🙈​S	🙈​S	🙈​S	🙈​S	🙈​S	🙈​S
🙈​	TestAccept/bundle/resources/vector_search_endpoints/drift/recreated_same_name	🙈​S	🙈​S	🙈​S	🙈​S	🙈​S	🙈​S
🙈​	TestAccept/bundle/resources/vector_search_indexes/recreate/embedding_dimension	🙈​S	🙈​S	🙈​S	🙈​S	🙈​S	🙈​S
🙈​	TestAccept/ssh/connection	🙈​S	🙈​S	🙈​S	🙈​S	🙈​S	🙈​S

Top 5 slowest tests (at least 2 minutes):

duration	env	testname
7:22	aws windows	TestAccept
7:20	gcp windows	TestAccept
3:56	azure linux	TestAccept
3:53	aws linux	TestAccept
3:53	gcp linux	TestAccept

[renaudhartert-db]

[optional] Wondering if we could make Run a thin life-cycle wrapper. The current code is a little hard to follow with outcome in different place to ultimately being used upstream.

Something like this:

  func Run(ctx context.Context, client *databricks.WorkspaceClient, opts ClientOptions) error {
        ctx, cancel := context.WithCancel(ctx)
        defer cancel()

        outcome, err := connect(ctx, client, opts)
        logSshTunnelEvent(ctx, opts, outcome, err)
        return err
  }

[anton-107]

Taking a look, this isn't quite the two-line move it appears to be, so I'd rather not fold it into this PR.

Threading outcome out through a connect return means touching all 24 return statements in the current Run body, not just the 13 that set a category. It also changes the shape of the telemetry helpers: connectOutcome.err folds into a new logSshTunnelEvent parameter, which in turn touches category() (it reads o.err for both the context.Canceled precedence and the UNKNOWN fallback) and the buildSshTunnelEvent unit tests that assert on it.

One detail on the sketch: the signal handler between cancel() and the defer would need to stay in Run, since USER_ABORTED depends on the context.Canceled it produces.

I don't disagree with the shape you're describing -- splitting the lifecycle from the flow does read better, and you're right that outcome being written in ~13 places and read in one is the awkward part. It's just a wide enough mechanical change that I'd rather keep it out of the commit that adds the field, per the repo's guidance on not mixing refactors with content changes. Happy to do it as a follow-up if you'd like.