CXP-383 Correct audit-log pagination cursor and sample skip-org warnings

[JavierCarnelli-ConductorOne]

• nextAuditLogPage now checks resp.After first (the cursor GHEC and GHES actually return for the org audit-log endpoint's rel="next" Link), falling back to NextPageToken/NextPage for any page-style Link header GHES may still emit. The prior fix (CXP-383 Support audit log pagination for both GHEC and GHES #192) only checked NextPageToken/NextPage, so it silently truncated every GHEC org - and most GHES orgs - to a single page.
• Request side now sends the cursor via After or Page depending on which shape was received (usageEventPageToken.AuditLogCursorIsPage), instead of always sending Page.
• Orgs that permanently lack audit-log access are now warned via a sampled logger (1st, 10th, 100th, then every 1000th occurrence, with total_occurrences) instead of on every poll pass forever. The counter is intentionally shared across all orgs on the feed, not keyed per org.

[linear-code]

CXP-383

[github-actions]

Connector PR Review: CXP-383 Correct audit-log pagination cursor and sample skip-org warnings

Blocking Issues: 0 | Suggestions: 2 | Threads Resolved: 0
Criteria: Criteria status: loaded .claude/skills/ci-review.md from trusted base 39aa762fbb07.
Review mode: incremental since 23310bb
View review run

Review Summary

The new commit swaps the shared sampledWarn for a per-org perKeySampledWarn (sync.Map + atomic.Uint64), which addresses the prior finding that a shared counter could hide a newly-failing org's first failure; the two new tests correctly cover per-key budgets and the noisy-key-starves-new-key case. The full PR diff was scanned for security and correctness: the After-first cursor logic matches go-github v69's populatePageValues (an after= rel="next" link lands in Response.After with NextPageToken/NextPage left empty), the request side correctly round-trips AuditLogCursorIsPage, and the vendored go.uber.org/zap/zaptest/observer addition is consistent with modules.txt and needs no go.mod change since zap is already a direct dependency — no blocking issues. Three earlier suggestions are still open and are not re-listed below: the len(entries) == 0 termination guard (usage_event_feed.go:218-223), legacy tokens decoding AuditLogCursorIsPage as false on a mid-pass upgrade (usage_event_feed.go:161-165), and the absent end-to-end test that derives after from a response Link header.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

• pkg/connector/logging.go:28-31 — per-org keying makes skip-org warn volume O(#orgs) per pass; a token missing the audit_log scope now emits one line per org on the first pass instead of one, so consider also gating on the aggregate counter.
• pkg/connector/usage_event_feed.go:35-37 — the PR description still says "The counter is intentionally shared across all orgs on the feed, not keyed per org," which this commit reverses; update it so the rationale of record matches the code.

Prompt for AI agents

Verify each finding against the current code and only fix it if needed.

## Suggestions

In `pkg/connector/logging.go`:
- Around line 28-31: `perKeySampledWarn.log` gives every key its own sampling
  budget, which fixes first-occurrence starvation but makes total warn volume
  scale with the number of keys. When the GitHub token lacks the `audit_log`
  scope, every org hits the skip path, so the first feed pass emits one warn
  line per org (thousands for a large enterprise) instead of one. Add a second
  gate: keep an aggregate `atomic.Uint64` on `perKeySampledWarn` alongside the
  per-key counters and only call through to the underlying `sampledWarn.log`
  when the per-key occurrence is sampled AND the aggregate count is below a
  bound (or the aggregate occurrence is itself sampled). Keep the existing
  `total_occurrences` field and consider adding a `total_occurrences_all_keys`
  field so the bounded case is still diagnosable. Extend `logging_test.go` with
  a case asserting that N distinct keys each failing once produces a bounded
  number of lines.

In `pkg/connector/usage_event_feed.go`:
- Around line 35-37: The PR description asserts "The counter is intentionally
  shared across all orgs on the feed, not keyed per org," which the latest
  commit reverses by switching `skippedOrgs` to `perKeySampledWarn`. Update the
  PR description to state that the counter is now keyed per org and why, so the
  recorded rationale matches the shipped behavior.

Note: this run's sandbox blocked every path for emitting the machine-readable review-state marker (shell JSON literals, pipes, and file writes were all denied), so the marker is absent from this comment. Reviewed head SHA was dc219ce236761395a1ba3978d1cb54bba81e1e0e against base 39aa762fbb0704e1cff00b58cdd24d7aaad8d87c; the next review will fall back to full mode.

[github-actions]

No blocking issues found.

[github-actions]

No blocking issues found.