Skip to content

Report ssh_tunnel_event booleans explicitly in telemetry - #6338

Open
anton-107 wants to merge 1 commit into
mainfrom
deco-28097-telemetry-omitempty-start-working-on-8
Open

Report ssh_tunnel_event booleans explicitly in telemetry#6338
anton-107 wants to merge 1 commit into
mainfrom
deco-28097-telemetry-omitempty-start-working-on-8

Conversation

@anton-107

Copy link
Copy Markdown
Contributor

Changes

Drop omitempty from all five bool fields on SshTunnelEvent
(libs/telemetry/protos/ssh_tunnel.go): IsSuccess, IsReconnect,
AutoStartCluster, HasBaseEnvironment, HasUsagePolicy.

Why

omitempty drops the zero value, so a failed ssh connect sent no is_success
field at all and landed in the telemetry table as NULL rather than false.
Filtering on is_success = false returned zero rows across all history — any
straightforward "are users hitting errors?" query reported a clean bill of health
while the real IDE-mode failure rate was 40-55%.

All five bools are populated unconditionally in buildSshTunnelEvent, so each
has a meaningful false. The other four were equally unrepresentable and matter
for funnel analysis, so they are fixed in the same pass rather than left to be
rediscovered. ServerStartTimeMs in the same struct already omitted the tag,
which is what confirmed this was a field-level mistake rather than an intended
encoding.

Historical rows cannot be backfilled, so the struct doc comment records the
cutover: queries spanning it must count NULL as failure for pre-fix data.

No changelog fragment — experimental/ changes don't get one until the feature
graduates, and the payload isn't user-visible.

Tests

  • New libs/telemetry/protos/ssh_tunnel_test.go: pins the zero-value wire
    payload, plus a reflection guard that fails if a future bool on this struct
    arrives with omitempty.
  • New TestBuildSshTunnelEventReportsFailure covers the failure path in
    experimental/ssh/internal/client; the existing table test only ever passed
    true.
  • Both new tests were confirmed to fail against the pre-fix tag, with
    actual: <nil> — the exact NULL this fixes.
  • Verified end-to-end on the full FrontendLog envelope as uploaded to
    /telemetry-ext: "is_success": false is now transmitted.
  • task lint-go (0 issues, all 3 modules), task test-exp-ssh, telemetry and
    ssh acceptance tests pass. One pre-existing failure in
    bundle/templates/lakeflow-integrations, unrelated: it fails identically on
    unmodified origin/main (blocked PyPI fetch in my sandbox).

This PR was written by Claude Code.

The bool fields on SshTunnelEvent carried `omitempty`, which drops the zero
value. A failed connection therefore sent no `is_success` field at all and
landed in the telemetry table as NULL rather than false, so filtering on
`is_success = false` returned zero rows across all history while the real
IDE-mode failure rate was 40-55%.

Drop `omitempty` from all five bools. Every one is populated unconditionally in
buildSshTunnelEvent, so each has a meaningful false: IsReconnect,
AutoStartCluster, HasBaseEnvironment and HasUsagePolicy were equally
unrepresentable and matter for funnel analysis. ServerStartTimeMs already
omitted the tag, which is what confirmed this was a field-level mistake.

Historical rows cannot be backfilled, so the struct comment records the
cutover: queries spanning it must count NULL as false.

Co-authored-by: Isaac
@github-actions

Copy link
Copy Markdown
Contributor

Approval status: pending

/libs/telemetry/ - needs approval

Files: libs/telemetry/protos/ssh_tunnel.go, libs/telemetry/protos/ssh_tunnel_test.go
Suggested: @parthban-db
Also eligible: @simonfaltum, @renaudhartert-db, @hectorcast-db, @tanmay-db, @Divyansh-db, @tejaskochar-db, @mihaimitrea-db, @chrisst, @rauchy

General files (require maintainer)

Files: experimental/ssh/internal/client/client_internal_test.go
Based on git history:

  • @rclarey -- recent work in libs/telemetry/protos/, experimental/ssh/internal/client/

Any maintainer (@andrewnester, @denik, @pietern, @shreyas-goenka, @simonfaltum, @renaudhartert-db, @janniklasrose, @lennartkats-db, @rugpanov, @rclarey) can approve all areas.
See OWNERS for ownership rules.

@eng-dev-ecosystem-bot

Copy link
Copy Markdown
Collaborator

Integration test report

Commit: 2dc338b

Run: 32479759911

Env 💚​RECOVERED 🙈​SKIP ✅​pass 🙈​skip Time
💚​ aws linux 1 4 274 1162 5:24
💚​ aws windows 1 4 276 1160 3:17
💚​ azure linux 1 4 273 1162 5:14
💚​ azure windows 1 4 275 1160 3:40
💚​ gcp linux 1 4 274 1162 5:45
💚​ gcp windows 1 4 276 1160 3:20
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 6 slowest tests (at least 2 minutes):
duration env testname
3:57 azure linux TestAccept
3:51 gcp linux TestAccept
3:49 aws linux TestAccept
3:35 azure windows TestAccept
3:13 aws windows TestAccept
3:02 gcp windows TestAccept

@anton-107
anton-107 requested a review from rugpanov August 21, 2026 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants