Skip to content

webhook: add pluggable Store interface to RouteLogger - #19

Open
dksogal wants to merge 2 commits into
masterfrom
dsogal/additive-store-interface
Open

webhook: add pluggable Store interface to RouteLogger#19
dksogal wants to merge 2 commits into
masterfrom
dsogal/additive-store-interface

Conversation

@dksogal

@dksogal dksogal commented Aug 18, 2026

Copy link
Copy Markdown

Summary

RouteLogger currently only knows about keys registered in its own process's in-memory map. During a rolling deploy of a consumer (e.g. mme-e2e), a webhook callback can arrive at a different process/pod than the one that registered the test, and is silently dropped by Record.

This adds an additive Store interface (Publish / Unpublish / Forward) that RouteLogger.Sent / Record / Delete consult, so a caller can plug in cross-process delivery (e.g. a shared cache keyed by pod) without Orbital taking on any new dependency.

  • Default behavior (localStore, used when no Store is configured) is identical to current behaviour.
  • NewRouteLogger's existing signature is unchanged — the new opts ...RouteLoggerOption param is backward compatible, so no existing caller needs to change.
  • Wait / WaitN are untouched — the reflect.Select multi-key wait stays exactly as-is.

Test plan

  • /review-toolkit:review has been run
  • go build ./... and go test ./webhook/... -race -v pass, including the two pre-existing tests unmodified.
  • New tests (store_test.go) use a bare bookkeeping registry double (no decision logic of its own) so the assertions exercise real RouteLogger wiring:
    • TestRouteLoggerStoreForwardsCrossPodDelivery — a callback that misses locally on one RouteLogger instance is delivered to a second instance via Store.Forward, unblocking its Wait.
    • TestRouteLoggerStoreNotCalledOnLocalHit — a local hit never calls Forward.
    • TestRouteLoggerDeleteUnpublishesFromStoreDelete releases ownership in the store, not just locally.
    • Verified these tests are load-bearing by mutating the implementation locally (removing the Forward-on-miss call, and making Record forward unconditionally) and confirming each mutation was caught.
  • Confirmed mme-e2e's two existing webhook.NewRouteLogger(...) call sites compile unmodified against this branch via a local replace directive.

🤖 Generated with Claude Code

@dksogal
dksogal marked this pull request as draft August 18, 2026 16:38
RouteLogger currently only knows about keys registered in its own
process's in-memory map, so a callback that arrives on a different
process (e.g. the old pod during a rolling deploy) is silently
dropped on Record.

Add an additive Store interface that RouteLogger.Sent/Record/Delete
consult (Publish/Forward/Unpublish) so a caller can plug in cross-process
delivery. The default (localStore) preserves today's behavior exactly:
single-process, in-memory only, drop on miss. NewRouteLogger's existing
signature is unchanged (new variadic RouteLoggerOption params are
backward compatible), so no caller needs to change.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@dksogal
dksogal force-pushed the dsogal/additive-store-interface branch from d30d796 to 7c6ac33 Compare August 18, 2026 19:31
@dksogal
dksogal marked this pull request as ready for review August 18, 2026 19:56
@dksogal
dksogal marked this pull request as draft August 18, 2026 20:24
@dksogal
dksogal marked this pull request as ready for review August 18, 2026 20:29
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