Skip to content

feat(audit): meter audit events that go missing before the store write (NAN-6472) - #6963

Open
pfreixes wants to merge 1 commit into
masterfrom
pau/nan-6472-audit-emit-dropped-metric
Open

feat(audit): meter audit events that go missing before the store write (NAN-6472)#6963
pfreixes wants to merge 1 commit into
masterfrom
pau/nan-6472-audit-emit-dropped-metric

Conversation

@pfreixes

@pfreixes pfreixes commented Jul 30, 2026

Copy link
Copy Markdown
Contributor
  • An audit event that vanishes upstream of the store is now countable. store.record() already meters every ClickHouse write attempt, so a failed write is alertable — but the two catch blocks that fire before it, when event construction or target resolution throws, were log-only. An event we intended to record disappearing was invisible to metrics.
  • Dropped and degraded are separate counters, because they are different failures. The finish listener is registered before target resolution runs, so throwing before registration loses the event outright, while throwing during resolution still emits it with no target. nango.audit.emit.dropped therefore stays the single number to alert on for events actually lost, and nango.audit.resolve.failed covers the recorded-but-incomplete case.

Both carry a source tag so the dedicated auth, sync-command and MFA middlewares report distinctly once they land, and so NAN-6471's shared emit tail can absorb both increments without changing the metric contract.

Deliberately not metered

  • DropAuditStore.record() returns Ok while discarding, so any deployment without CLICKHOUSE_URL drops every event silently. That is correct behaviour for self-hosting rather than a fault, and no counter here sees it.
  • emit()'s early return when there is no account — unreachable in practice, since the listener is only registered when one exists.

Test plan

  • ts-build, npm run lint (exit 0), prettier clean
  • 10/10 middleware unit tests, 5/5 audit middleware integration tests
  • One test per path: resolution failure (degraded — asserts the event is still recorded with its account and outcome), gate failure and emit failure (both dropped — assert nothing reaches the store). Each asserts the counter that should fire and the one that should not
  • Verified non-vacuous: removing both increments reds exactly the three new tests and leaves the other seven green
  • Verified the dropped/degraded split is load-bearing: flattening it to a single counter reds exactly the gate-failure test
  • After deploy: confirm both counters report, and set a threshold on nango.audit.emit.dropped — audit volume is low enough that any non-zero value is worth looking at

🤖 Generated with Claude Code

Review in cubic

…e (NAN-6472)

The store meters every ClickHouse write attempt, so a failed write is alertable.
The two catch blocks upstream of it were log-only, which left an event we meant
to record vanishing entirely invisible to metrics.

They are not the same failure though. Throwing before the finish listener is
registered loses the event; throwing during target resolution still emits it,
just without a target. So dropped and degraded get separate counters, keeping
one number to alert on for events actually lost.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@linear-code

linear-code Bot commented Jul 30, 2026

Copy link
Copy Markdown

NAN-6472

@cubic-dev-ai cubic-dev-ai Bot 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.

No issues found across 3 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Re-trigger cubic

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.

1 participant