From ce2f532129f9bc57543332d4a0694e6f6e285aff Mon Sep 17 00:00:00 2001 From: Sayan Samanta Date: Mon, 8 Jun 2026 18:36:27 -0700 Subject: [PATCH] cli: opt-in telemetry categories + config echo Rework `browsers create/update --telemetry` for the opt-in model: - `--telemetry=all` captures the default set, `--telemetry=off` disables, and `--telemetry=console,network` captures exactly the listed categories (replace semantics). Drops the old name=on/off DSL. - settableCategories expands to the full 9 (control, connection, system, screenshot, captcha added); monitor is not settable (auto). - After create/update, echo the categories telemetry will capture. - `telemetry stream --categories` filter set covers all event categories including monitor; category now read from the SDK's typed field directly. NOTE: requires kernel-go-sdk >= v0.64 (new category fields). That bump is not included here because it also requires migrating cmd/proxies to the new SDK proxy API (unrelated). This change rebases on top of that SDK-bump PR; it does not compile against the current pinned SDK (v0.58). Co-authored-by: Cursor --- cmd/browsers.go | 12 +++- cmd/browsers_telemetry.go | 110 ++++++++++++++++++++------------- cmd/browsers_telemetry_test.go | 89 +++++++++++--------------- cmd/browsers_test.go | 10 +-- 4 files changed, 117 insertions(+), 104 deletions(-) diff --git a/cmd/browsers.go b/cmd/browsers.go index 9d587db..de8aa01 100644 --- a/cmd/browsers.go +++ b/cmd/browsers.go @@ -498,6 +498,9 @@ func (b BrowsersCmd) Create(ctx context.Context, in BrowsersCreateInput) error { } printBrowserSessionResult(browser.SessionID, browser.CdpWsURL, browser.BrowserLiveViewURL, browser.Profile, browser.StartURL, browser.Name, browser.Tags) + if in.Telemetry != "" { + printTelemetrySummary(browser.Telemetry) + } return nil } @@ -731,6 +734,9 @@ func (b BrowsersCmd) Update(ctx context.Context, in BrowsersUpdateInput) error { } pterm.Success.Printf("Updated browser %s\n", browser.SessionID) + if in.Telemetry != "" { + printTelemetrySummary(browser.Telemetry) + } return nil } @@ -2325,7 +2331,7 @@ func init() { browsersUpdateCmd.Flags().Bool("save-changes", false, "If set, save changes back to the profile when the session ends") 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") browsersUpdateCmd.Flags().Bool("force", false, "Force viewport resize even when a live view or recording/replay is active") - browsersUpdateCmd.Flags().String("telemetry", "", "Update telemetry: --telemetry=all to enable, --telemetry=off to disable, --telemetry=network=on,page=off for per-category") + 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)") browsersCmd.AddCommand(browsersListCmd) browsersCmd.AddCommand(browsersCreateCmd) @@ -2591,7 +2597,7 @@ func init() { browsersCreateCmd.Flags().Bool("viewport-interactive", false, "Interactively select viewport size from list") browsersCreateCmd.Flags().String("pool-id", "", "Browser pool ID to acquire from (mutually exclusive with --pool-name)") browsersCreateCmd.Flags().String("pool-name", "", "Browser pool name to acquire from (mutually exclusive with --pool-id)") - browsersCreateCmd.Flags().String("telemetry", "", "Configure telemetry: --telemetry=all to enable, --telemetry=off to disable, --telemetry=network=on,page=off for per-category") + browsersCreateCmd.Flags().String("telemetry", "", "Configure telemetry (opt-in): --telemetry=all (default set), --telemetry=off (disable), or --telemetry=console,network (capture exactly those categories)") browsersCreateCmd.Flags().String("name", "", "Optional unique name for the browser session (used to find it later; set at creation only)") browsersCreateCmd.Flags().StringArray("tag", nil, "Set a tag KEY=VALUE on the session (repeatable; up to 50 pairs)") @@ -2622,7 +2628,7 @@ followed automatically by Chromium.`, telemetryRoot := &cobra.Command{Use: "telemetry", Short: "Browser telemetry operations"} telemetryStream := &cobra.Command{Use: "stream ", Short: "Stream live telemetry events", Args: cobra.ExactArgs(1), RunE: runBrowsersTelemetryStream} - telemetryStream.Flags().StringSlice("categories", []string{}, "Filter by API event category (api,console,interaction,network,page,system); system covers monitor_* and cdp_* events") + telemetryStream.Flags().StringSlice("categories", []string{}, "Filter by event category (console,network,page,interaction,control,connection,system,screenshot,captcha,monitor)") telemetryStream.Flags().StringSlice("types", []string{}, "Filter by event type (e.g. network_response,console_error)") telemetryStream.Flags().Int64("seq", -1, "Resume after sequence number N (Last-Event-ID); replays events with seq > N. Default -1 streams from now") telemetryStream.Flags().StringP("output", "o", "", "Output format: json for newline-delimited JSON envelopes") diff --git a/cmd/browsers_telemetry.go b/cmd/browsers_telemetry.go index f3e3ad9..2621520 100644 --- a/cmd/browsers_telemetry.go +++ b/cmd/browsers_telemetry.go @@ -2,7 +2,6 @@ package cmd import ( "context" - "encoding/json" "errors" "fmt" "os" @@ -34,34 +33,38 @@ type BrowsersTelemetryStreamInput struct { Output string } -// parseTelemetryCategories parses a comma-separated "name=on|off" string into -// a BrowserTelemetryCategoriesConfigParam. Unmentioned categories are omitted. +// parseTelemetryCategories parses a comma-separated list of category names to +// enable into a BrowserTelemetryCategoriesConfigParam. Selection is opt-in: +// only the listed categories are captured; everything else is off. func parseTelemetryCategories(s string) (kernel.BrowserTelemetryCategoriesConfigParam, error) { p := kernel.BrowserTelemetryCategoriesConfigParam{} + on := func() kernel.BrowserTelemetryCategoryConfigParam { + return kernel.BrowserTelemetryCategoryConfigParam{Enabled: kernel.Opt(true)} + } for _, part := range strings.Split(s, ",") { - name, val, ok := strings.Cut(strings.TrimSpace(part), "=") - if !ok { - return p, fmt.Errorf("invalid category assignment %q: expected name=on or name=off", part) - } - name, val = strings.TrimSpace(name), strings.TrimSpace(val) - var enabled bool - switch val { - case "on": - enabled = true - case "off": - enabled = false - default: - return p, fmt.Errorf("invalid value %q for category %q: must be \"on\" or \"off\"", val, name) + name := strings.TrimSpace(part) + if name == "" { + continue } switch name { case "console": - p.Console = kernel.BrowserTelemetryCategoryConfigParam{Enabled: kernel.Opt(enabled)} - case "interaction": - p.Interaction = kernel.BrowserTelemetryCategoryConfigParam{Enabled: kernel.Opt(enabled)} + p.Console = on() case "network": - p.Network = kernel.BrowserTelemetryCategoryConfigParam{Enabled: kernel.Opt(enabled)} + p.Network = on() case "page": - p.Page = kernel.BrowserTelemetryCategoryConfigParam{Enabled: kernel.Opt(enabled)} + p.Page = on() + case "interaction": + p.Interaction = on() + case "control": + p.Control = on() + case "connection": + p.Connection = on() + case "system": + p.System = on() + case "screenshot": + p.Screenshot = on() + case "captcha": + p.Captcha = on() default: return p, fmt.Errorf("unknown category %q: must be one of %s", name, strings.Join(settableCategories, ", ")) } @@ -102,30 +105,53 @@ func buildUpdateTelemetryParam(s string) (kernel.BrowserUpdateParamsTelemetry, e } // settableCategories are the categories accepted by --telemetry=. -// "system" is always-on and cannot be toggled, but is valid as a --categories stream filter. -var settableCategories = []string{"console", "interaction", "network", "page"} +// The monitor category is not settable: it is collector-health metadata that +// flows automatically whenever a CDP category is captured. +var settableCategories = []string{ + "console", "network", "page", "interaction", + "control", "connection", "system", "screenshot", "captcha", +} // streamFilterCategories are the categories accepted by `telemetry stream --categories`. -var streamFilterCategories = []string{"api", "console", "interaction", "network", "page", "system"} +// This is the full set of categories an event may carry, including the auto-managed monitor. +var streamFilterCategories = append(append([]string{}, settableCategories...), "monitor") -// eventCategory returns the category field from the event JSON. -// Falls back to the type prefix if the field is absent (older API responses). -// TODO(sdk): kernel-go-sdk should surface Category directly on BrowserTelemetryEventUnion. -func eventCategory(ev kernel.BrowserTelemetryEventUnion) string { - var wire struct { - Category string `json:"category"` - } - if err := json.Unmarshal([]byte(ev.RawJSON()), &wire); err == nil && wire.Category != "" { - return wire.Category - } - prefix, _, ok := strings.Cut(ev.Type, "_") - if !ok { - return ev.Type - } - if prefix == "monitor" { - return "system" +// telemetryEnabledCategories returns the categories captured by a session's +// telemetry config, in display order. +func telemetryEnabledCategories(cfg kernel.BrowserTelemetryConfig) []string { + b := cfg.Browser + ordered := []struct { + name string + on bool + }{ + {"console", b.Console.Enabled}, + {"network", b.Network.Enabled}, + {"page", b.Page.Enabled}, + {"interaction", b.Interaction.Enabled}, + {"control", b.Control.Enabled}, + {"connection", b.Connection.Enabled}, + {"system", b.System.Enabled}, + {"screenshot", b.Screenshot.Enabled}, + {"captcha", b.Captcha.Enabled}, + } + on := make([]string, 0, len(ordered)) + for _, c := range ordered { + if c.on { + on = append(on, c.name) + } } - return prefix + return on +} + +// printTelemetrySummary echoes the categories telemetry will capture, so the +// effect of an opt-in selection is obvious after create/update. +func printTelemetrySummary(cfg kernel.BrowserTelemetryConfig) { + on := telemetryEnabledCategories(cfg) + if len(on) == 0 { + pterm.Info.Println("Telemetry: disabled") + return + } + pterm.Info.Printf("Telemetry capturing: %s\n", strings.Join(on, ", ")) } // shouldEmit applies client-side category/type filters to a telemetry event. @@ -168,7 +194,7 @@ func (b BrowsersCmd) TelemetryStream(ctx context.Context, in BrowsersTelemetrySt defer stream.Close() for stream.Next() { ev := stream.Current() - cat := eventCategory(ev.Event) + cat := ev.Event.Category if !shouldEmit(cat, ev.Event.Type, in.Categories, in.Types) { continue } diff --git a/cmd/browsers_telemetry_test.go b/cmd/browsers_telemetry_test.go index 7736879..f73a082 100644 --- a/cmd/browsers_telemetry_test.go +++ b/cmd/browsers_telemetry_test.go @@ -254,32 +254,6 @@ func makeEvent(t *testing.T, raw string) kernel.BrowserTelemetryEventUnion { return ev } -func TestEventCategory(t *testing.T) { - cases := []struct { - raw string - want string - }{ - // Wire category wins when present. - {`{"type":"network_response","category":"network","ts":0}`, "network"}, - {`{"type":"monitor_screenshot","category":"system","ts":0}`, "system"}, - // Wire category overrides what a naive type-prefix split would return, - // e.g. cdp_* events the server classifies as system. - {`{"type":"cdp_attached","category":"system","ts":0}`, "system"}, - // Fallback to prefix when wire category is absent. - {`{"type":"monitor_screenshot","ts":0}`, "system"}, - {`{"type":"monitor_disconnected","ts":0}`, "system"}, - {`{"type":"network_response","ts":0}`, "network"}, - {`{"type":"console_log","ts":0}`, "console"}, - {`{"type":"page_navigation","ts":0}`, "page"}, - {`{"type":"interaction_click","ts":0}`, "interaction"}, - {`{"type":"nounderscore","ts":0}`, "nounderscore"}, - } - for _, tc := range cases { - ev := makeEvent(t, tc.raw) - assert.Equal(t, tc.want, eventCategory(ev), "type=%s", ev.Type) - } -} - func TestShouldEmit(t *testing.T) { cases := []struct { name string @@ -291,7 +265,8 @@ func TestShouldEmit(t *testing.T) { {"no filters passes", `{"type":"network_response","category":"network","ts":0}`, nil, nil, true}, {"matching category passes", `{"type":"network_response","category":"network","ts":0}`, []string{"network"}, nil, true}, {"non-matching category drops", `{"type":"console_log","category":"console","ts":0}`, []string{"network"}, nil, false}, - {"system category matches monitor_screenshot", `{"type":"monitor_screenshot","category":"system","ts":0}`, []string{"system"}, nil, true}, + {"monitor category matches monitor_disconnected", `{"type":"monitor_disconnected","category":"monitor","ts":0}`, []string{"monitor"}, nil, true}, + {"connection category matches cdp_connect", `{"type":"cdp_connect","category":"connection","ts":0}`, []string{"connection"}, nil, true}, {"matching type passes", `{"type":"console_log","category":"console","ts":0}`, nil, []string{"console_log"}, true}, {"non-matching type drops", `{"type":"network_response","category":"network","ts":0}`, nil, []string{"console_log"}, false}, {"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) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { ev := makeEvent(t, tc.raw) - assert.Equal(t, tc.want, shouldEmit(eventCategory(ev), ev.Type, tc.categories, tc.types)) + assert.Equal(t, tc.want, shouldEmit(ev.Category, ev.Type, tc.categories, tc.types)) }) } } -func TestParseTelemetryCategories_PartialCategories(t *testing.T) { - p, err := parseTelemetryCategories("network=on,page=off") +func TestParseTelemetryCategories_OptInList(t *testing.T) { + p, err := parseTelemetryCategories("network,control,captcha") assert.NoError(t, err) - assert.True(t, p.Network.Enabled.Valid()) - assert.True(t, p.Network.Enabled.Value) - assert.True(t, p.Page.Enabled.Valid()) - assert.False(t, p.Page.Enabled.Value) - // Unspecified categories omitted — server retains their state + // Listed categories are enabled. + for _, c := range []kernel.BrowserTelemetryCategoryConfigParam{p.Network, p.Control, p.Captcha} { + assert.True(t, c.Enabled.Valid()) + assert.True(t, c.Enabled.Value) + } + // Unlisted categories are omitted (opt-in: the instance treats them as off). assert.False(t, p.Console.Enabled.Valid()) - assert.False(t, p.Interaction.Enabled.Valid()) + assert.False(t, p.Page.Enabled.Valid()) + assert.False(t, p.Screenshot.Enabled.Valid()) } func TestParseTelemetryCategories_InvalidCategory(t *testing.T) { - _, err := parseTelemetryCategories("foo=on") + _, err := parseTelemetryCategories("foo") assert.Error(t, err) assert.Contains(t, err.Error(), "unknown category") } -func TestParseTelemetryCategories_InvalidValue(t *testing.T) { - _, err := parseTelemetryCategories("network=yes") - - assert.Error(t, err) - assert.Contains(t, err.Error(), `must be "on" or "off"`) -} - func TestParseTelemetryCategories_WhitespaceTolerance(t *testing.T) { - p, err := parseTelemetryCategories(" network = on , page = off ") + p, err := parseTelemetryCategories(" network , page ") assert.NoError(t, err) assert.True(t, p.Network.Enabled.Valid()) assert.True(t, p.Network.Enabled.Value) assert.True(t, p.Page.Enabled.Valid()) - assert.False(t, p.Page.Enabled.Value) + assert.True(t, p.Page.Enabled.Value) } -// TestBuildTelemetryParam_WireEncoding locks in the three distinct wire shapes -// the API expects: enable-all sets Enabled=true without Browser, disable-all -// sets Enabled=false without Browser, and per-category sets only Browser so the -// API treats it as a merge rather than a replace. +// TestBuildTelemetryParam_WireEncoding locks in the three wire shapes the API +// expects: "all" sets Enabled=true without Browser (default set), "off" sets +// Enabled=false without Browser, and an opt-in list sets only Browser with the +// listed categories enabled (Enabled unset). func TestBuildTelemetryParam_WireEncoding(t *testing.T) { t.Run("all", func(t *testing.T) { p, err := buildNewTelemetryParam("all") @@ -361,11 +331,22 @@ func TestBuildTelemetryParam_WireEncoding(t *testing.T) { assert.False(t, p.Enabled.Value) assert.False(t, p.Browser.Network.Enabled.Valid()) }) - t.Run("per-category omits Enabled so API merges", func(t *testing.T) { - p, err := buildNewTelemetryParam("network=off") + t.Run("opt-in list sets only Browser", func(t *testing.T) { + p, err := buildNewTelemetryParam("network,control") assert.NoError(t, err) - assert.False(t, p.Enabled.Valid(), "Enabled must be unset so API takes the merge path") + assert.False(t, p.Enabled.Valid(), "Enabled must be unset for an opt-in selection") assert.True(t, p.Browser.Network.Enabled.Valid()) - assert.False(t, p.Browser.Network.Enabled.Value) + assert.True(t, p.Browser.Network.Enabled.Value) + assert.True(t, p.Browser.Control.Enabled.Valid()) + assert.True(t, p.Browser.Control.Enabled.Value) }) } + +func TestTelemetryEnabledCategories(t *testing.T) { + var cfg kernel.BrowserTelemetryConfig + raw := `{"browser":{"control":{"enabled":true},"system":{"enabled":true},"network":{"enabled":false}}}` + if err := json.Unmarshal([]byte(raw), &cfg); err != nil { + t.Fatalf("unmarshal: %v", err) + } + assert.Equal(t, []string{"control", "system"}, telemetryEnabledCategories(cfg)) +} diff --git a/cmd/browsers_test.go b/cmd/browsers_test.go index 8ab33e6..f864c67 100644 --- a/cmd/browsers_test.go +++ b/cmd/browsers_test.go @@ -1803,16 +1803,16 @@ func TestBrowsersCreate_WithTelemetryCategories(t *testing.T) { }} b := BrowsersCmd{browsers: fake} - err := b.Create(context.Background(), BrowsersCreateInput{Telemetry: "network=on,page=off"}) + err := b.Create(context.Background(), BrowsersCreateInput{Telemetry: "network,page"}) assert.NoError(t, err) - assert.False(t, captured.Telemetry.Enabled.Valid(), "per-category must omit Enabled so the API merges") + assert.False(t, captured.Telemetry.Enabled.Valid(), "an opt-in selection must omit Enabled so the API captures exactly the listed categories") assert.True(t, captured.Telemetry.Browser.Network.Enabled.Valid()) assert.True(t, captured.Telemetry.Browser.Network.Enabled.Value) assert.True(t, captured.Telemetry.Browser.Page.Enabled.Valid()) - assert.False(t, captured.Telemetry.Browser.Page.Enabled.Value) - assert.False(t, captured.Telemetry.Browser.Console.Enabled.Valid()) - assert.False(t, captured.Telemetry.Browser.Interaction.Enabled.Valid()) + assert.True(t, captured.Telemetry.Browser.Page.Enabled.Value) + assert.False(t, captured.Telemetry.Browser.Console.Enabled.Valid(), "unlisted categories stay omitted") + assert.False(t, captured.Telemetry.Browser.Interaction.Enabled.Valid(), "unlisted categories stay omitted") } func TestBrowsersCreate_WithTelemetryOff(t *testing.T) {