Skip to content

chore(ci): add trigger context to rate-limit monitor#61007

Merged
rnegron merged 3 commits into
masterfrom
worktree-rate-limit-monitor-trigger-context
Jun 1, 2026
Merged

chore(ci): add trigger context to rate-limit monitor#61007
rnegron merged 3 commits into
masterfrom
worktree-rate-limit-monitor-trigger-context

Conversation

@rnegron
Copy link
Copy Markdown
Member

@rnegron rnegron commented Jun 1, 2026

Problem

The Monitor GitHub Rate Limit workflow records the repo's /rate_limit snapshot to PostHog on every PR open/synchronize and master push, but each sample only carries the aggregate per-resource numbers (used, remaining, limit). Nothing records what triggered the sample, so the time series can't be broken down by event type or PR characteristics — which makes it hard to see which kinds of CI activity drive core (GITHUB_TOKEN) consumption.

Changes

Adds a buildTrigger(context) helper that tags every emitted sample with the triggering event context:

  • trigger_event / trigger_action — e.g. pull_request / synchronize, or push
  • head_ref, pr_number, pr_author
  • pr_changed_files, pr_additions, pr_deletions

These merge into each github_rate_limit_observed event's properties. Non-PR events (push, schedule) fall back to ref and leave the PR fields null. No change to triggers, cadence, permissions, or the measured bucket — purely additive instrumentation.

Downstream in PostHog this enables breakdowns the aggregate series alone can't answer — burn rate by event type, and whether larger PRs correlate with higher per-event consumption.

How did you test this code?

I'm an agent (Claude Code) — automated tests only, no manual/live testing:

  • Extended monitor-github-rate-limit.test.js with two cases: one asserting the PR trigger fields are captured on each sample, one asserting buildTrigger falls back to ref and nulls PR fields for non-PR events.
  • npx jest .github/scripts/monitor-github-rate-limit.test.js → 5/5 passing.

Automatic notifications

  • Publish to changelog?
  • Alert Sales and Marketing teams?

Docs update

Not needed — internal CI/observability tooling.

🤖 Agent context

Authored with Claude Code while investigating core GITHUB_TOKEN rate-limit pressure. The monitor measures the aggregate per-repo bucket but can't attribute consumption to a workflow or event type; this is the first, lowest-effort ("event-level") attribution layer, reusing the existing high-frequency sample stream. A complementary per-workflow approach (snapshot used delta regressed against job elapsed time to cancel the shared-bucket background) was considered for finer attribution and deferred as a heavier follow-up. The PR-size fields are included deliberately to test whether large diffs correlate with higher per-event burn. Field set kept minimal and additive.

@rnegron rnegron added the skip-inkeep-docs Use this label to skip an Inkeep docs PR in posthog.com label Jun 1, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Jun 1, 2026

Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
.github/scripts/monitor-github-rate-limit.js:44
`pr_number` is the only numeric PR field that bypasses the `num()` guard used for `changed_files`, `additions`, and `deletions`. While GitHub always returns a numeric PR number, the inconsistency means a non-numeric value (e.g. a stringified number from a mock or a future payload change) would be forwarded as-is rather than normalised to `null`.

```suggestion
        pr_number: pr ? num(pr.number) : null,
```

### Issue 2 of 2
.github/scripts/monitor-github-rate-limit.test.js:128-137
The project prefers parameterised tests. The non-PR fallback case only exercises `push`; wrapping this in a `test.each` across the other trigger types the workflow can see (`schedule`, `workflow_dispatch`, `push` with no `ref`, etc.) would confirm the nulling logic holds for all of them with no additional boilerplate.

Reviews (1): Last reviewed commit: "chore(ci): add trigger context to rate-l..." | Re-trigger Greptile

Comment thread .github/scripts/monitor-github-rate-limit.js Outdated
Comment thread .github/scripts/monitor-github-rate-limit.test.js Outdated
@rnegron rnegron added the stamphog Request AI review from stamphog label Jun 1, 2026
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

Additive analytics enrichment to a GitHub Actions monitoring script — no production code, no data model changes, no security risk. Both bot review concerns were addressed in the current diff.

@rnegron rnegron merged commit d9109d3 into master Jun 1, 2026
189 checks passed
@rnegron rnegron deleted the worktree-rate-limit-monitor-trigger-context branch June 1, 2026 19:57
@deployment-status-posthog
Copy link
Copy Markdown

deployment-status-posthog Bot commented Jun 1, 2026

Deploy status

Environment Status Deployed At Workflow
dev ✅ Deployed 2026-06-01 20:24 UTC Run
prod-us ✅ Deployed 2026-06-01 20:54 UTC Run
prod-eu ✅ Deployed 2026-06-01 21:00 UTC Run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-inkeep-docs Use this label to skip an Inkeep docs PR in posthog.com stamphog Request AI review from stamphog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant