Include categorized host in telemetry by williammartin · Pull Request #13250 · cli/cli

williammartin · 2026-04-21T10:19:14Z

Description

Categorizes the host that is most likely being targeted as github.com or tenant. It does categorize GHES, but we turn off telemetry for GHES so we would never expect that to be included.

babakks

LGTM, with a few suggestions. I'm okay with doing them in a follow-up though, so I'm approving.

Copilot

Pull request overview

This PR enhances CLI telemetry by adding a categorized “target host type” dimension to command_invocation events, using a best-effort host inference based on flags/args/env/git/config. It also centralizes telemetry recording in ghcmd.Main and introduces new internal error/exit-code helpers that feed additional telemetry dimensions.

Changes:

Add host inference (GuessTargetHost) and record guessed_host_type in command invocation telemetry.
Move command invocation telemetry recording from per-command wrapping to a single deferred recorder in internal/ghcmd.Main.
Introduce internal/gherrs error types/exit codes and add tests for error-type telemetry dimensions and host categorization.

Show a summary per file

File	Description
pkg/cmdutil/telemetry.go	Removes per-command telemetry wrapping; keeps/expands telemetry-disable helpers and exports `IsTelemetryDisabled`.
pkg/cmdutil/telemetry_test.go	Updates tests to cover telemetry-disable behavior instead of command invocation recording.
pkg/cmd/root/root.go	Removes recursive subcommand telemetry instrumentation call from root command setup.
pkg/cmd/auth/auth.go	Switches to disabling telemetry for `auth` commands recursively.
internal/telemetry/host.go	Adds host-guessing logic to infer the command’s likely target host.
internal/telemetry/host_test.go	Adds tests covering host inference priority order and fallbacks.
internal/ghinstance/host.go	Ensures empty host categorizes to `uncategorized`.
internal/ghinstance/host_test.go	Adds test for empty-host categorization.
internal/gherrs/errors.go	Introduces internal error/exit-code abstractions used by `ghcmd.Main`.
internal/ghcmd/cmd.go	Centralizes telemetry recording (including host type + error dims) and refactors error handling/exit code mapping.
internal/ghcmd/cmd_test.go	Adds tests for error-type collection, err-dimension logic, and end-to-end telemetry capture.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comments suppressed due to low confidence (2)

internal/ghcmd/cmd_test.go:474

This test asserts the wrapper type name "fmt.wrapError" in errTypes. Wrapper type names are not a stable API and can change between Go versions/toolchains, making this assertion brittle; consider asserting only that the terminal domain error type (e.g. "gherrs.silentError") is present.

			name: "single fmt wrap",
			err:  fmt.Errorf("context: %w", gherrs.SilentError),
			want: "fmt.wrapError,gherrs.silentError",
		},

internal/ghcmd/cmd_test.go:479

This case also hard-codes "fmt.wrapError" in the expected errTypes output. To reduce brittleness across Go/toolchain changes, consider loosening the assertion to focus on your own error type(s) and general shape rather than exact stdlib wrapper names.

			name: "double fmt wrap",
			err:  fmt.Errorf("outer: %w", fmt.Errorf("inner: %w", gherrs.SilentError)),
			want: "fmt.wrapError,fmt.wrapError,gherrs.silentError",
		},

Files reviewed: 11/11 changed files
Comments generated: 3

Copilot

Copilot's findings

Files reviewed: 5/5 changed files
Comments generated: 2

- Normalize --hostname flag values via ghauth.NormalizeHostname - Gate baseRepo() call behind --repo flag check to avoid git discovery for non-repo-scoped commands like 'gh version' - Convert TestGuessTargetHost to table-driven test format Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Copilot's findings

Files reviewed: 9/9 changed files
Comments generated: 2

- Extract recordCommandTelemetry into a testable function - Add PersistentPostRun hook on root to capture the real executed command when aliases re-dispatch via root.Execute(), so the expanded command gets telemetry recorded instead of silently skipping it - Add tests for command_invocation recording: normal commands, visited flags, missing command, and telemetry-disabled commands Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

williammartin · 2026-05-14T05:28:58Z

Actually @babakks I'm thinking maybe we should close this and approach it a different way, where instead of trying to capture this in command_invocation, we just create another event closer to the API request itself when we know where API requests are going. It'll still be connectable to the command via common invocation_id, and will involve a lot less of this super janky guessing.

Not closing right now cause I'd prefer your opinion on it.

babakks · 2026-05-14T07:41:40Z

So, you're thinking about an explicit new event type (like in skill install)?

williammartin · 2026-05-14T11:51:29Z

Yes I'm thinking that we will also eventually want a way to tie commands to their REQUEST_IDs as well to join across server side data, so it might naturally fit there.

williammartin force-pushed the wm/host-categorization-telemetry branch from ab709aa to b0929b5 Compare April 21, 2026 15:02

babakks approved these changes Apr 21, 2026

View reviewed changes

Comment thread internal/ghcmd/cmd.go Outdated

Comment thread internal/ghcmd/cmd.go Outdated

Comment thread internal/ghcmd/cmd_test.go Outdated

Comment thread internal/ghcmd/cmd_test.go Outdated

Comment thread internal/gherrs/errors.go Outdated

Comment thread internal/telemetry/host_test.go

williammartin changed the title ~~Include guessed host in telemetry~~ Include categorized host in telemetry Apr 22, 2026

williammartin requested a review from Copilot May 13, 2026 15:11

Copilot started reviewing on behalf of williammartin May 13, 2026 15:11 View session

Copilot AI reviewed May 13, 2026

View reviewed changes

Comment thread internal/ghcmd/cmd_test.go Outdated

Comment thread internal/ghcmd/cmd_test.go Outdated

Comment thread internal/gherrs/errors.go Outdated

williammartin force-pushed the wm/host-categorization-telemetry branch 3 times, most recently from daac0e1 to d878b17 Compare May 13, 2026 15:49

williammartin changed the base branch from trunk to wm/record-error-telemetry May 13, 2026 15:50

williammartin requested a review from Copilot May 13, 2026 15:53

Copilot started reviewing on behalf of williammartin May 13, 2026 15:54 View session

Copilot AI reviewed May 13, 2026

View reviewed changes

Comment thread internal/telemetry/host.go

Comment thread internal/telemetry/host.go

williammartin force-pushed the wm/host-categorization-telemetry branch from a6b8b97 to df7e15b Compare May 13, 2026 16:25

williammartin and others added 2 commits May 13, 2026 19:27

Include guessed host in telemetry

07fbed4

williammartin force-pushed the wm/host-categorization-telemetry branch from df7e15b to 393fe08 Compare May 13, 2026 17:28

williammartin changed the base branch from wm/record-error-telemetry to trunk May 13, 2026 17:28

williammartin requested a review from Copilot May 13, 2026 17:38

Copilot started reviewing on behalf of williammartin May 13, 2026 17:39 View session

Copilot AI reviewed May 13, 2026

View reviewed changes

Comment thread internal/ghcmd/cmd.go Outdated

Comment thread internal/ghcmd/cmd.go Outdated

williammartin force-pushed the wm/host-categorization-telemetry branch from b7e0758 to b77f5f2 Compare May 13, 2026 18:05

williammartin force-pushed the wm/host-categorization-telemetry branch from b77f5f2 to d725e0a Compare May 13, 2026 18:08

williammartin marked this pull request as ready for review May 13, 2026 18:11

williammartin requested a review from a team as a code owner May 13, 2026 18:11

williammartin requested review from BagToad and removed request for BagToad May 13, 2026 18:11

Conversation

williammartin commented Apr 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

babakks left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Copilot's findings

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot's findings

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot's findings

Uh oh!

Uh oh!

Uh oh!

williammartin commented May 14, 2026

Uh oh!

babakks commented May 14, 2026

Uh oh!

williammartin commented May 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

williammartin commented Apr 21, 2026 •

edited

Loading