Skip to content

Commit fd2a3c9

Browse files
Sayan-cursoragent
andauthored
cli: opt-in telemetry categories + config echo (#178)
### What changes - `kernel browsers create/update --telemetry`: - `--telemetry=all` -> default category set - `--telemetry=off` -> disable - `--telemetry=console,network` -> capture exactly those categories (opt-in, **replace** semantics). The old `name=on/off` DSL is removed. - `settableCategories` now covers all 9 (`console, network, page, interaction, control, connection, system, screenshot, captcha`). `monitor` is not settable (auto-managed, flows when a CDP category is captured). - After create/update, the CLI echoes the categories telemetry will capture, so the effect of an opt-in selection is obvious. - `telemetry stream --categories` filter set now covers every event category incl. `monitor`; event category is read from the SDK's typed `Category` field (removed the JSON/prefix fallback and the stale `monitor->system` mapping). ## Test plan - [x] CI - [x] manual: `--telemetry=all` / `=off` / `=console,network`; confirm the echoed config; `telemetry stream --categories monitor` <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes user-facing `--telemetry` semantics (breaking for scripts using `name=on/off`) and how stream filtering maps categories; behavior is localized to CLI with solid test coverage. > > **Overview** > Reworks browser **`--telemetry`** on create/update to **opt-in category selection** with **replace** semantics: **`all`** (default set), **`off`**, or a comma list like **`console,network`** that captures **only** those categories. The old **`name=on/off`** DSL is removed. > > **Settable categories** expand to nine (`control`, `connection`, `system`, `screenshot`, `captcha`, etc.); **`monitor`** stays non-settable but is included in **`telemetry stream --categories`**. After create/update, the CLI **prints which categories** telemetry will capture when **`--telemetry`** was used. > > Streaming uses the SDK’s typed **`Category`** field (drops JSON/prefix fallback and **`monitor`→`system`** remapping). Tests and flag help text are updated for the new behavior. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit ce2f532. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 06fbbaa commit fd2a3c9

4 files changed

Lines changed: 117 additions & 104 deletions

File tree

cmd/browsers.go

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -498,6 +498,9 @@ func (b BrowsersCmd) Create(ctx context.Context, in BrowsersCreateInput) error {
498498
}
499499

500500
printBrowserSessionResult(browser.SessionID, browser.CdpWsURL, browser.BrowserLiveViewURL, browser.Profile, browser.StartURL, browser.Name, browser.Tags)
501+
if in.Telemetry != "" {
502+
printTelemetrySummary(browser.Telemetry)
503+
}
501504
return nil
502505
}
503506

@@ -731,6 +734,9 @@ func (b BrowsersCmd) Update(ctx context.Context, in BrowsersUpdateInput) error {
731734
}
732735

733736
pterm.Success.Printf("Updated browser %s\n", browser.SessionID)
737+
if in.Telemetry != "" {
738+
printTelemetrySummary(browser.Telemetry)
739+
}
734740
return nil
735741
}
736742

@@ -2325,7 +2331,7 @@ func init() {
23252331
browsersUpdateCmd.Flags().Bool("save-changes", false, "If set, save changes back to the profile when the session ends")
23262332
browsersUpdateCmd.Flags().String("viewport", "", "Browser viewport size (e.g., 1920x1080@25). Supported: 2560x1440@10, 1920x1080@25, 1920x1200@25, 1440x900@25, 1024x768@60, 1200x800@60, 1280x800@60")
23272333
browsersUpdateCmd.Flags().Bool("force", false, "Force viewport resize even when a live view or recording/replay is active")
2328-
browsersUpdateCmd.Flags().String("telemetry", "", "Update telemetry: --telemetry=all to enable, --telemetry=off to disable, --telemetry=network=on,page=off for per-category")
2334+
browsersUpdateCmd.Flags().String("telemetry", "", "Update telemetry (opt-in, replaces current selection): --telemetry=all (default set), --telemetry=off (disable), or --telemetry=console,network (capture exactly those categories)")
23292335

23302336
browsersCmd.AddCommand(browsersListCmd)
23312337
browsersCmd.AddCommand(browsersCreateCmd)
@@ -2591,7 +2597,7 @@ func init() {
25912597
browsersCreateCmd.Flags().Bool("viewport-interactive", false, "Interactively select viewport size from list")
25922598
browsersCreateCmd.Flags().String("pool-id", "", "Browser pool ID to acquire from (mutually exclusive with --pool-name)")
25932599
browsersCreateCmd.Flags().String("pool-name", "", "Browser pool name to acquire from (mutually exclusive with --pool-id)")
2594-
browsersCreateCmd.Flags().String("telemetry", "", "Configure telemetry: --telemetry=all to enable, --telemetry=off to disable, --telemetry=network=on,page=off for per-category")
2600+
browsersCreateCmd.Flags().String("telemetry", "", "Configure telemetry (opt-in): --telemetry=all (default set), --telemetry=off (disable), or --telemetry=console,network (capture exactly those categories)")
25952601
browsersCreateCmd.Flags().String("name", "", "Optional unique name for the browser session (used to find it later; set at creation only)")
25962602
browsersCreateCmd.Flags().StringArray("tag", nil, "Set a tag KEY=VALUE on the session (repeatable; up to 50 pairs)")
25972603

@@ -2622,7 +2628,7 @@ followed automatically by Chromium.`,
26222628

26232629
telemetryRoot := &cobra.Command{Use: "telemetry", Short: "Browser telemetry operations"}
26242630
telemetryStream := &cobra.Command{Use: "stream <id>", Short: "Stream live telemetry events", Args: cobra.ExactArgs(1), RunE: runBrowsersTelemetryStream}
2625-
telemetryStream.Flags().StringSlice("categories", []string{}, "Filter by API event category (api,console,interaction,network,page,system); system covers monitor_* and cdp_* events")
2631+
telemetryStream.Flags().StringSlice("categories", []string{}, "Filter by event category (console,network,page,interaction,control,connection,system,screenshot,captcha,monitor)")
26262632
telemetryStream.Flags().StringSlice("types", []string{}, "Filter by event type (e.g. network_response,console_error)")
26272633
telemetryStream.Flags().Int64("seq", -1, "Resume after sequence number N (Last-Event-ID); replays events with seq > N. Default -1 streams from now")
26282634
telemetryStream.Flags().StringP("output", "o", "", "Output format: json for newline-delimited JSON envelopes")

cmd/browsers_telemetry.go

Lines changed: 68 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package cmd
22

33
import (
44
"context"
5-
"encoding/json"
65
"errors"
76
"fmt"
87
"os"
@@ -34,34 +33,38 @@ type BrowsersTelemetryStreamInput struct {
3433
Output string
3534
}
3635

37-
// parseTelemetryCategories parses a comma-separated "name=on|off" string into
38-
// a BrowserTelemetryCategoriesConfigParam. Unmentioned categories are omitted.
36+
// parseTelemetryCategories parses a comma-separated list of category names to
37+
// enable into a BrowserTelemetryCategoriesConfigParam. Selection is opt-in:
38+
// only the listed categories are captured; everything else is off.
3939
func parseTelemetryCategories(s string) (kernel.BrowserTelemetryCategoriesConfigParam, error) {
4040
p := kernel.BrowserTelemetryCategoriesConfigParam{}
41+
on := func() kernel.BrowserTelemetryCategoryConfigParam {
42+
return kernel.BrowserTelemetryCategoryConfigParam{Enabled: kernel.Opt(true)}
43+
}
4144
for _, part := range strings.Split(s, ",") {
42-
name, val, ok := strings.Cut(strings.TrimSpace(part), "=")
43-
if !ok {
44-
return p, fmt.Errorf("invalid category assignment %q: expected name=on or name=off", part)
45-
}
46-
name, val = strings.TrimSpace(name), strings.TrimSpace(val)
47-
var enabled bool
48-
switch val {
49-
case "on":
50-
enabled = true
51-
case "off":
52-
enabled = false
53-
default:
54-
return p, fmt.Errorf("invalid value %q for category %q: must be \"on\" or \"off\"", val, name)
45+
name := strings.TrimSpace(part)
46+
if name == "" {
47+
continue
5548
}
5649
switch name {
5750
case "console":
58-
p.Console = kernel.BrowserTelemetryCategoryConfigParam{Enabled: kernel.Opt(enabled)}
59-
case "interaction":
60-
p.Interaction = kernel.BrowserTelemetryCategoryConfigParam{Enabled: kernel.Opt(enabled)}
51+
p.Console = on()
6152
case "network":
62-
p.Network = kernel.BrowserTelemetryCategoryConfigParam{Enabled: kernel.Opt(enabled)}
53+
p.Network = on()
6354
case "page":
64-
p.Page = kernel.BrowserTelemetryCategoryConfigParam{Enabled: kernel.Opt(enabled)}
55+
p.Page = on()
56+
case "interaction":
57+
p.Interaction = on()
58+
case "control":
59+
p.Control = on()
60+
case "connection":
61+
p.Connection = on()
62+
case "system":
63+
p.System = on()
64+
case "screenshot":
65+
p.Screenshot = on()
66+
case "captcha":
67+
p.Captcha = on()
6568
default:
6669
return p, fmt.Errorf("unknown category %q: must be one of %s", name, strings.Join(settableCategories, ", "))
6770
}
@@ -102,30 +105,53 @@ func buildUpdateTelemetryParam(s string) (kernel.BrowserUpdateParamsTelemetry, e
102105
}
103106

104107
// settableCategories are the categories accepted by --telemetry=<categories>.
105-
// "system" is always-on and cannot be toggled, but is valid as a --categories stream filter.
106-
var settableCategories = []string{"console", "interaction", "network", "page"}
108+
// The monitor category is not settable: it is collector-health metadata that
109+
// flows automatically whenever a CDP category is captured.
110+
var settableCategories = []string{
111+
"console", "network", "page", "interaction",
112+
"control", "connection", "system", "screenshot", "captcha",
113+
}
107114

108115
// streamFilterCategories are the categories accepted by `telemetry stream --categories`.
109-
var streamFilterCategories = []string{"api", "console", "interaction", "network", "page", "system"}
116+
// This is the full set of categories an event may carry, including the auto-managed monitor.
117+
var streamFilterCategories = append(append([]string{}, settableCategories...), "monitor")
110118

111-
// eventCategory returns the category field from the event JSON.
112-
// Falls back to the type prefix if the field is absent (older API responses).
113-
// TODO(sdk): kernel-go-sdk should surface Category directly on BrowserTelemetryEventUnion.
114-
func eventCategory(ev kernel.BrowserTelemetryEventUnion) string {
115-
var wire struct {
116-
Category string `json:"category"`
117-
}
118-
if err := json.Unmarshal([]byte(ev.RawJSON()), &wire); err == nil && wire.Category != "" {
119-
return wire.Category
120-
}
121-
prefix, _, ok := strings.Cut(ev.Type, "_")
122-
if !ok {
123-
return ev.Type
124-
}
125-
if prefix == "monitor" {
126-
return "system"
119+
// telemetryEnabledCategories returns the categories captured by a session's
120+
// telemetry config, in display order.
121+
func telemetryEnabledCategories(cfg kernel.BrowserTelemetryConfig) []string {
122+
b := cfg.Browser
123+
ordered := []struct {
124+
name string
125+
on bool
126+
}{
127+
{"console", b.Console.Enabled},
128+
{"network", b.Network.Enabled},
129+
{"page", b.Page.Enabled},
130+
{"interaction", b.Interaction.Enabled},
131+
{"control", b.Control.Enabled},
132+
{"connection", b.Connection.Enabled},
133+
{"system", b.System.Enabled},
134+
{"screenshot", b.Screenshot.Enabled},
135+
{"captcha", b.Captcha.Enabled},
136+
}
137+
on := make([]string, 0, len(ordered))
138+
for _, c := range ordered {
139+
if c.on {
140+
on = append(on, c.name)
141+
}
127142
}
128-
return prefix
143+
return on
144+
}
145+
146+
// printTelemetrySummary echoes the categories telemetry will capture, so the
147+
// effect of an opt-in selection is obvious after create/update.
148+
func printTelemetrySummary(cfg kernel.BrowserTelemetryConfig) {
149+
on := telemetryEnabledCategories(cfg)
150+
if len(on) == 0 {
151+
pterm.Info.Println("Telemetry: disabled")
152+
return
153+
}
154+
pterm.Info.Printf("Telemetry capturing: %s\n", strings.Join(on, ", "))
129155
}
130156

131157
// shouldEmit applies client-side category/type filters to a telemetry event.
@@ -168,7 +194,7 @@ func (b BrowsersCmd) TelemetryStream(ctx context.Context, in BrowsersTelemetrySt
168194
defer stream.Close()
169195
for stream.Next() {
170196
ev := stream.Current()
171-
cat := eventCategory(ev.Event)
197+
cat := ev.Event.Category
172198
if !shouldEmit(cat, ev.Event.Type, in.Categories, in.Types) {
173199
continue
174200
}

cmd/browsers_telemetry_test.go

Lines changed: 35 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -254,32 +254,6 @@ func makeEvent(t *testing.T, raw string) kernel.BrowserTelemetryEventUnion {
254254
return ev
255255
}
256256

257-
func TestEventCategory(t *testing.T) {
258-
cases := []struct {
259-
raw string
260-
want string
261-
}{
262-
// Wire category wins when present.
263-
{`{"type":"network_response","category":"network","ts":0}`, "network"},
264-
{`{"type":"monitor_screenshot","category":"system","ts":0}`, "system"},
265-
// Wire category overrides what a naive type-prefix split would return,
266-
// e.g. cdp_* events the server classifies as system.
267-
{`{"type":"cdp_attached","category":"system","ts":0}`, "system"},
268-
// Fallback to prefix when wire category is absent.
269-
{`{"type":"monitor_screenshot","ts":0}`, "system"},
270-
{`{"type":"monitor_disconnected","ts":0}`, "system"},
271-
{`{"type":"network_response","ts":0}`, "network"},
272-
{`{"type":"console_log","ts":0}`, "console"},
273-
{`{"type":"page_navigation","ts":0}`, "page"},
274-
{`{"type":"interaction_click","ts":0}`, "interaction"},
275-
{`{"type":"nounderscore","ts":0}`, "nounderscore"},
276-
}
277-
for _, tc := range cases {
278-
ev := makeEvent(t, tc.raw)
279-
assert.Equal(t, tc.want, eventCategory(ev), "type=%s", ev.Type)
280-
}
281-
}
282-
283257
func TestShouldEmit(t *testing.T) {
284258
cases := []struct {
285259
name string
@@ -291,7 +265,8 @@ func TestShouldEmit(t *testing.T) {
291265
{"no filters passes", `{"type":"network_response","category":"network","ts":0}`, nil, nil, true},
292266
{"matching category passes", `{"type":"network_response","category":"network","ts":0}`, []string{"network"}, nil, true},
293267
{"non-matching category drops", `{"type":"console_log","category":"console","ts":0}`, []string{"network"}, nil, false},
294-
{"system category matches monitor_screenshot", `{"type":"monitor_screenshot","category":"system","ts":0}`, []string{"system"}, nil, true},
268+
{"monitor category matches monitor_disconnected", `{"type":"monitor_disconnected","category":"monitor","ts":0}`, []string{"monitor"}, nil, true},
269+
{"connection category matches cdp_connect", `{"type":"cdp_connect","category":"connection","ts":0}`, []string{"connection"}, nil, true},
295270
{"matching type passes", `{"type":"console_log","category":"console","ts":0}`, nil, []string{"console_log"}, true},
296271
{"non-matching type drops", `{"type":"network_response","category":"network","ts":0}`, nil, []string{"console_log"}, false},
297272
{"both filters pass when both match", `{"type":"network_response","category":"network","ts":0}`, []string{"network"}, []string{"network_response"}, true},
@@ -300,52 +275,47 @@ func TestShouldEmit(t *testing.T) {
300275
for _, tc := range cases {
301276
t.Run(tc.name, func(t *testing.T) {
302277
ev := makeEvent(t, tc.raw)
303-
assert.Equal(t, tc.want, shouldEmit(eventCategory(ev), ev.Type, tc.categories, tc.types))
278+
assert.Equal(t, tc.want, shouldEmit(ev.Category, ev.Type, tc.categories, tc.types))
304279
})
305280
}
306281
}
307282

308-
func TestParseTelemetryCategories_PartialCategories(t *testing.T) {
309-
p, err := parseTelemetryCategories("network=on,page=off")
283+
func TestParseTelemetryCategories_OptInList(t *testing.T) {
284+
p, err := parseTelemetryCategories("network,control,captcha")
310285

311286
assert.NoError(t, err)
312-
assert.True(t, p.Network.Enabled.Valid())
313-
assert.True(t, p.Network.Enabled.Value)
314-
assert.True(t, p.Page.Enabled.Valid())
315-
assert.False(t, p.Page.Enabled.Value)
316-
// Unspecified categories omitted — server retains their state
287+
// Listed categories are enabled.
288+
for _, c := range []kernel.BrowserTelemetryCategoryConfigParam{p.Network, p.Control, p.Captcha} {
289+
assert.True(t, c.Enabled.Valid())
290+
assert.True(t, c.Enabled.Value)
291+
}
292+
// Unlisted categories are omitted (opt-in: the instance treats them as off).
317293
assert.False(t, p.Console.Enabled.Valid())
318-
assert.False(t, p.Interaction.Enabled.Valid())
294+
assert.False(t, p.Page.Enabled.Valid())
295+
assert.False(t, p.Screenshot.Enabled.Valid())
319296
}
320297

321298
func TestParseTelemetryCategories_InvalidCategory(t *testing.T) {
322-
_, err := parseTelemetryCategories("foo=on")
299+
_, err := parseTelemetryCategories("foo")
323300

324301
assert.Error(t, err)
325302
assert.Contains(t, err.Error(), "unknown category")
326303
}
327304

328-
func TestParseTelemetryCategories_InvalidValue(t *testing.T) {
329-
_, err := parseTelemetryCategories("network=yes")
330-
331-
assert.Error(t, err)
332-
assert.Contains(t, err.Error(), `must be "on" or "off"`)
333-
}
334-
335305
func TestParseTelemetryCategories_WhitespaceTolerance(t *testing.T) {
336-
p, err := parseTelemetryCategories(" network = on , page = off ")
306+
p, err := parseTelemetryCategories(" network , page ")
337307

338308
assert.NoError(t, err)
339309
assert.True(t, p.Network.Enabled.Valid())
340310
assert.True(t, p.Network.Enabled.Value)
341311
assert.True(t, p.Page.Enabled.Valid())
342-
assert.False(t, p.Page.Enabled.Value)
312+
assert.True(t, p.Page.Enabled.Value)
343313
}
344314

345-
// TestBuildTelemetryParam_WireEncoding locks in the three distinct wire shapes
346-
// the API expects: enable-all sets Enabled=true without Browser, disable-all
347-
// sets Enabled=false without Browser, and per-category sets only Browser so the
348-
// API treats it as a merge rather than a replace.
315+
// TestBuildTelemetryParam_WireEncoding locks in the three wire shapes the API
316+
// expects: "all" sets Enabled=true without Browser (default set), "off" sets
317+
// Enabled=false without Browser, and an opt-in list sets only Browser with the
318+
// listed categories enabled (Enabled unset).
349319
func TestBuildTelemetryParam_WireEncoding(t *testing.T) {
350320
t.Run("all", func(t *testing.T) {
351321
p, err := buildNewTelemetryParam("all")
@@ -361,11 +331,22 @@ func TestBuildTelemetryParam_WireEncoding(t *testing.T) {
361331
assert.False(t, p.Enabled.Value)
362332
assert.False(t, p.Browser.Network.Enabled.Valid())
363333
})
364-
t.Run("per-category omits Enabled so API merges", func(t *testing.T) {
365-
p, err := buildNewTelemetryParam("network=off")
334+
t.Run("opt-in list sets only Browser", func(t *testing.T) {
335+
p, err := buildNewTelemetryParam("network,control")
366336
assert.NoError(t, err)
367-
assert.False(t, p.Enabled.Valid(), "Enabled must be unset so API takes the merge path")
337+
assert.False(t, p.Enabled.Valid(), "Enabled must be unset for an opt-in selection")
368338
assert.True(t, p.Browser.Network.Enabled.Valid())
369-
assert.False(t, p.Browser.Network.Enabled.Value)
339+
assert.True(t, p.Browser.Network.Enabled.Value)
340+
assert.True(t, p.Browser.Control.Enabled.Valid())
341+
assert.True(t, p.Browser.Control.Enabled.Value)
370342
})
371343
}
344+
345+
func TestTelemetryEnabledCategories(t *testing.T) {
346+
var cfg kernel.BrowserTelemetryConfig
347+
raw := `{"browser":{"control":{"enabled":true},"system":{"enabled":true},"network":{"enabled":false}}}`
348+
if err := json.Unmarshal([]byte(raw), &cfg); err != nil {
349+
t.Fatalf("unmarshal: %v", err)
350+
}
351+
assert.Equal(t, []string{"control", "system"}, telemetryEnabledCategories(cfg))
352+
}

cmd/browsers_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1803,16 +1803,16 @@ func TestBrowsersCreate_WithTelemetryCategories(t *testing.T) {
18031803
}}
18041804
b := BrowsersCmd{browsers: fake}
18051805

1806-
err := b.Create(context.Background(), BrowsersCreateInput{Telemetry: "network=on,page=off"})
1806+
err := b.Create(context.Background(), BrowsersCreateInput{Telemetry: "network,page"})
18071807

18081808
assert.NoError(t, err)
1809-
assert.False(t, captured.Telemetry.Enabled.Valid(), "per-category must omit Enabled so the API merges")
1809+
assert.False(t, captured.Telemetry.Enabled.Valid(), "an opt-in selection must omit Enabled so the API captures exactly the listed categories")
18101810
assert.True(t, captured.Telemetry.Browser.Network.Enabled.Valid())
18111811
assert.True(t, captured.Telemetry.Browser.Network.Enabled.Value)
18121812
assert.True(t, captured.Telemetry.Browser.Page.Enabled.Valid())
1813-
assert.False(t, captured.Telemetry.Browser.Page.Enabled.Value)
1814-
assert.False(t, captured.Telemetry.Browser.Console.Enabled.Valid())
1815-
assert.False(t, captured.Telemetry.Browser.Interaction.Enabled.Valid())
1813+
assert.True(t, captured.Telemetry.Browser.Page.Enabled.Value)
1814+
assert.False(t, captured.Telemetry.Browser.Console.Enabled.Valid(), "unlisted categories stay omitted")
1815+
assert.False(t, captured.Telemetry.Browser.Interaction.Enabled.Valid(), "unlisted categories stay omitted")
18161816
}
18171817

18181818
func TestBrowsersCreate_WithTelemetryOff(t *testing.T) {

0 commit comments

Comments
 (0)