Record attachment counts in telemetry by BagToad · Pull Request #14327 · cli/cli · GitHub
Skip to content

Record attachment counts in telemetry - #14327

Merged
BagToad merged 1 commit into
trunkfrom
bagtoad/attach-flag-metrics
Sep 3, 2026
Merged

Record attachment counts in telemetry#14327
BagToad merged 1 commit into
trunkfrom
bagtoad/attach-flag-metrics

Conversation

@BagToad

@BagToad BagToad commented Sep 2, 2026

Copy link
Copy Markdown
Member

Description

Command telemetry records whether --attach was used, but not how many times it was provided. An invocation with several attachments is therefore indistinguishable from one with a single attachment.

This adds an attachment_invocation event to issue and pull request create, edit, and comment commands. The event includes the command path and an attach_count measure. Invocations using --attach are sampled at 100%, which also captures the paired command_invocation event at the same rate.

The event is recorded before attachment validation. It captures the supplied flag count when a conflict, missing file, or other validation error stops the command. It does not include file paths, alt text, body content, repository information, or asset data.

How did you test this change?

I set telemetry to log in an isolated configuration and ran all six supported commands with two missing image paths. Each command stopped before upload and printed exactly one attachment_invocation event with the corresponding command, attach_count set to 2, and sample_rate set to 100.

I then ran gh version without --attach and saw no attachment event.

Key points

  • The attachment flag owns its telemetry event. Generic command telemetry remains unaware of attachment behavior.
  • The recorder is passed explicitly from the root command through the issue and pull request command sets, following the existing skills telemetry pattern.
  • Sampling applies to the whole invocation, so an attachment invocation sends both the attachment event and standard command event at 100%.
  • The count describes flags provided, not files validated or uploaded.

Notes for reviewers

Start with the attachment flag telemetry method. The remaining production changes pass the recorder to each command. Assertions are integrated into the existing command constructor tables.

This follows the dependency injection and domain-specific event approach established by skills-specific telemetry.

Authorship and follow-up

Who wrote this:

  • A human wrote it.
  • An agent wrote it under close human direction.
  • An agent wrote it independently, and no human has guided the implementation beyond the initial prompt.

Who answers review comments:

  • @BagToad will read and reply directly. Name the account.
  • An agent will draft replies and @username will read them before they are posted.
  • Nobody has explicitly committed to replying.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings September 2, 2026 20:45
@BagToad
BagToad requested a review from a team as a code owner September 2, 2026 20:45
@BagToad
BagToad requested a review from tidy-dev September 2, 2026 20:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

PR and issue comment validation failures omit the paired command_invocation event.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Balanced
Findings: 2 Medium severity

New issues introduced by this change (2)
Severity Finding
Medium severity pkg/​cmd/​issue/​comment/​comment.go — 🛑 Requirement: Preserve the paired command_invocation event on validation failures This records…
Medium severity pkg/​cmd/​pr/​comment/​comment.go — 🛑 Requirement: Preserve the paired command_invocation event on validation failures This records…
What changed in this PR

Adds attachment-count telemetry to issue and pull request create, edit, and comment commands.

Changes:

  • Records attachment_invocation events with command path and attachment count.
  • Samples attachment invocations at 100%.
  • Propagates telemetry recorders through command constructors and adds tests.
File Description
pkg/​cmd/​root/​root.go Injects telemetry into issue and PR commands.
pkg/​cmd/​pr/​pr.go Propagates telemetry to PR subcommands.
pkg/​cmd/​pr/​edit/​edit.go Records PR edit attachment telemetry.
pkg/​cmd/​pr/​edit/​edit_test.go Tests PR edit telemetry.
pkg/​cmd/​pr/​create/​create.go Records PR create attachment telemetry.
pkg/​cmd/​pr/​create/​create_test.go Tests PR create telemetry.
pkg/​cmd/​pr/​comment/​comment.go Records PR comment attachment telemetry; paired command telemetry is lost on pre-run validation failures.
pkg/​cmd/​pr/​comment/​comment_test.go Tests PR comment telemetry.
pkg/​cmd/​issue/​issue.go Propagates telemetry to issue subcommands.
pkg/​cmd/​issue/​edit/​edit.go Records issue edit attachment telemetry.
pkg/​cmd/​issue/​edit/​edit_test.go Tests issue edit telemetry.
pkg/​cmd/​issue/​create/​create.go Records issue create attachment telemetry.
pkg/​cmd/​issue/​create/​create_test.go Tests issue create telemetry.
pkg/​cmd/​issue/​comment/​comment.go Records issue comment attachment telemetry; paired command telemetry is lost on pre-run validation failures.
pkg/​cmd/​issue/​comment/​comment_test.go Tests issue comment telemetry.
internal/​attachments/​flags.go Implements attachment telemetry recording.
internal/​attachments/​flags_test.go Tests event contents, counts, and sampling.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread pkg/cmd/issue/comment/comment.go
Comment thread pkg/cmd/pr/comment/comment.go
@babakks
babakks self-requested a review September 3, 2026 09:09

@babakks babakks left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Confirmed with running with GH_TELEMETRY=log.

Also, the failing CI is due to a dependency that :dependabot: hasn't yet opened a PR for (due to cool down config).

Update: I opened this to bump the dependency: #14331

@BagToad
BagToad merged commit 6e82bc5 into trunk Sep 3, 2026
25 of 26 checks passed
@BagToad
BagToad deleted the bagtoad/attach-flag-metrics branch September 3, 2026 13:18
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