feat: add browsers telemetry events read command (kernel-go-sdk v0.70.0)#190
feat: add browsers telemetry events read command (kernel-go-sdk v0.70.0)#190archandatta wants to merge 5 commits into
Conversation
Add `kernel browsers telemetry events <id>` to read the paginated telemetry archive, alongside the existing live `telemetry stream`. Supports --limit/--offset cursor paging, --since/--until window bounds, repeatable --category server-side filtering, --all to walk every page, and -o json for newline-delimited envelopes. Requires a kernel-go-sdk release exposing the telemetry Events method; the dependency bump lands separately after the API deploys. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Address review: remove the --offset flag (it advertised an X-Next-Offset cursor the command never emits; --all owns multi-page paging). Rename --category to --categories to match the telemetry stream sibling, and drop the now-inaccurate "ignored with --all"/"ignored when --offset" help clauses. Expand the param-mapping test to cover since/until and multi-event emit. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| telemetryEvents.Flags().String("until", "", "Window end (exclusive): RFC-3339 timestamp or duration like 5m") | ||
| telemetryEvents.Flags().StringSlice("categories", []string{}, "Filter by event category (console,network,page,interaction,control,connection,system,screenshot,captcha,monitor)") | ||
| telemetryEvents.Flags().Bool("all", false, "Fetch every page instead of just the first") | ||
| telemetryEvents.Flags().StringP("output", "o", "", "Output format: json for newline-delimited JSON envelopes") |
There was a problem hiding this comment.
Inline JSON output flag
Low Severity
The new telemetry events command registers -o/--output with a hand-rolled StringP and custom help instead of addJSONOutputFlag, which centralizes the flag and Output format: json for raw API response wording across CLI commands.
Triggered by learned rule: Use shared JSON output helpers in CLI commands
Reviewed by Cursor Bugbot for commit d18dca1. Configure here.
The telemetry events command needs the Events/EventsAutoPaging surface added in v0.70.0. Adapt the command's category param to the generated []string type and fix the stale validation-error assertion. The bump also changes APIKeyService.Get to take a params arg, so update the api_keys command and its fake to pass APIKeyGetParams (empty preserves prior behavior). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
archandatta
changed the title
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
rgarcia
left a comment
rgarcia
left a comment
There was a problem hiding this comment.
reviewed from correctness/qa and CLI surface consistency. make build and make test passed, but I think these need changes before merge.
QA / correctness
-
cmd/browsers_telemetry.go:273— multi-category filtering does not work in real QA. With a telemetry archive containingconsole,network, andpageevents,--categories consoleand--categories networkeach returned rows, but both--categories console,networkand repeated--categories console --categories networkreturned zero rows. This needs a live/SDK-level fix and a test that covers more than one category. -
cmd/browsers_telemetry.go:263—--limitaccepts invalid values even though the help says1-100. In QA,--limit -1exited 0 and returned the default page, and--limit 101also exited 0. Please validate the range client-side or loosen the help text. -
cmd/browsers_telemetry.go:267—--since 1sreturnedInternal_error: failed to read telemetry eventswith exit 1 on an otherwise valid session, while an empty--until 1hwindow returned zero rows cleanly. This may be API-side duration handling, but the CLI exposes it directly, so the command should either validate supported duration forms or the endpoint needs to stop returning a 500 for an empty/recent window.
CLI surface consistency
-
cmd/browsers.go:2757—telemetry eventsomits the--typesfilter that exists on siblingtelemetry stream. Since both commands expose telemetry event envelopes, users will expect the same category/type filtering shape unless there is a clear reason documented in help/docs. -
cmd/browsers.go:2754— the command uses list-like paging language (--limit, “per page”, “first page”) but exposes no--offset/cursor, and the NDJSON output only prints event items. That makes the default mode non-pageable, unlike the rest of the CLI list surfaces that expose--limit+--offset. Consider exposing a continuation mechanism or framing this as “latest N events” with--allfor archive dumps. -
README.md:316— the README telemetry section documentstelemetry streambut not the new recordedtelemetry eventscommand. Since this adds a user-facing CLI surface, please document the command, flags, output mode, and how it differs from live streaming.
- Multi-category filtering: send each category as a repeated query param via option.WithQueryAdd. The SDK serializes []string as one comma-joined value, which the endpoint reads as a single bogus category; repeating the param matches the endpoint's expected form. - Validate --limit client-side (1-100) instead of silently passing invalid values to the server. - Add a client-side --types filter for parity with telemetry stream. - Durable read: make the identifier-to-session lookup best-effort and fall back to the identifier when Get 404s, so an ended session's archive stays readable (verified live against staging). - Frame the default vs --all paging in help, and document the events command in the README.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 0e4cc08. Configure here.
Sayan-
left a comment
Sayan-
left a comment
There was a problem hiding this comment.
Confirming all feedback from \raf has been addressed except for
cmd/browsers_telemetry.go:267 — --since 1s returned Internal_error: failed to read telemetry events with exit 1 on an otherwise valid session, while an empty --until 1h window returned zero rows cleanly. This may be API-side duration handling, but the CLI exposes it directly, so the command should either validate supported duration forms or the endpoint needs to stop returning a 500 for an empty/recent window.
Which we should resolve server side
Once the flagged bugbot issue is addressed happy to approve
The archive endpoint filters only by category server-side, so --types is applied client-side after a page is fetched. In default (non --all) mode only the first page was read, silently dropping matching events on later pages. Route through the auto-pager whenever a type filter is active so results are complete.
Sayan-
left a comment
Sayan-
left a comment
There was a problem hiding this comment.
Thanks for iterating - confirming all marked issues have been addressed
3 participants
Summary
Adds
kernel browsers telemetry events <id>— reads the paginated telemetry archive for a browser session, alongside the existing livetelemetry stream.Flags:
--limit(1–100, default 20) — page size--all— fetch every page (walks the cursor via the SDK auto-pager); without it you get the first page--since/--until(RFC-3339 timestamp or a duration like5m)--categories(repeatable; server-side filter, validated against the event category set incl.monitor)-o json(newline-delimited JSON envelopes, matchingtelemetry stream)Resolves the identifier (name-or-id) to a session ID via
browsers.Get, same asstream. Text output istime \t seq \t [category] \t type.SDK bump (bundled)
This bumps
kernel-go-sdkv0.66.0 → v0.70.0 to pick up the telemetryEvents/EventsAutoPagingsurface (added when the API endpoint shipped). Bundled into this PR per the repo convention (e.g.2c06a2c, where a feature carries the SDK bump it needs).The bump also changed
APIKeyService.Getto take a params arg, socmd/api_keys.go(and its test fake) now passkernel.APIKeyGetParams{}— an empty value that preserves prior behavior. Unrelated to telemetry, but required for the bump to compile.Test coverage
Unit tests cover the single-page path: validation (nil telemetry, bad
-o, unknown category),browsers.Geterror surfacing, param mapping (limit/since/until/categories), multi-event emit, and JSON output.The
--allauto-paging path is not unit-tested: the SDK'sOffsetPaginationAutoPageradvances viaGetNextPage, which requires a live request config, so a hand-built pager can't be driven in a unit test (this is the first CLI consumer of an SDK auto-pager). The loop is a thinfor pager.Next()wrapper; the cursor/termination math lives in and is tested by the SDK, and end-to-end paging is exercised by the API-side staging smoke.Test plan
go build ./.../go vet ./.../gofmtcleango test ./...passing locally (incl. telemetry events + api_keys)make lint(golangci-lint) in CIevents <id>happy path + headers,--categories,--since/--until,--limit,--all,-o json, unknown id → 404Note
Low Risk
CLI-only additive feature plus a mechanical SDK signature fix; no auth or data-model changes beyond new read-only API calls.
Overview
Adds
kernel browsers telemetry events <id>to read the paginated telemetry archive (durable counterpart to livetelemetry stream). It supports windowing (--since/--until), server-side--categories(via repeated query params), client-side--types, paging (--limit,--allvia SDK auto-pager), and-o jsonNDJSON output.--typesforces a full window walk so filtering is not incomplete across pages.Resolves name-or-id to session ID when
browsers.Getsucceeds; if the session is gone but the archive remains, it falls back to the raw identifier.Bumps
kernel-go-sdkv0.66.0 → v0.70.0 forEvents/EventsAutoPaging.api-keys getnow passeskernel.APIKeyGetParams{}to match the updated SDKGetsignature (behavior unchanged).README documents the new command and clarifies live vs recorded telemetry.
Reviewed by Cursor Bugbot for commit 1fa9194. Bugbot is set up for automated code reviews on this repo. Configure here.