Skip to content

feat: replace event destination_id with matched_destination_ids#801

Open
alexluong wants to merge 2 commits intomainfrom
feat/matched-destination-ids
Open

feat: replace event destination_id with matched_destination_ids#801
alexluong wants to merge 2 commits intomainfrom
feat/matched-destination-ids

Conversation

@alexluong
Copy link
Copy Markdown
Collaborator

Summary

Replace event.destination_id (publish input) with matched_destination_ids (actual routing results). Enables GET /events?destination_id=... filtering (#688). Drops destination_id from event persistence per team consensus (#799).

Resolves #688. Resolves #799.

  • Add matched_destination_ids to Event model, DB (pg + ch), all logstores, API, OpenAPI
  • Drop destination_id from events table (single migration per DB)
  • Update event metrics to use matched_destination_ids for filtering/dimensions
  • Fix flaky scheduler tests (unrelated, drive-by)

Discussion points

  1. No historical data — existing events will have empty matched_destination_ids. No backfill.
  2. Should we expose matched_destination_ids on the API? — currently returned on GET /events and GET /events/:id.
  3. Attempts gap — when events are embedded in attempt responses (include=event), matched_destination_ids is omitted because the attempts table doesn't have it. Options: accept the gap, denormalize into attempts, or join at query time.

Test plan

  • Unit tests (memlogstore, apirouter)
  • Integration tests (pglogstore, chlogstore, migrator)
  • Manual QA suite (10/10 pass against local outpost)
  • Scheduler flaky test fix verified (5 consecutive passes)

🤖 Generated with Claude Code

@vercel
Copy link
Copy Markdown

vercel bot commented Mar 31, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
outpost-docs Ready Ready Preview, Comment Mar 31, 2026 4:12pm
outpost-website Ready Ready Preview, Comment Mar 31, 2026 4:12pm

Request Review

@alexbouchardd
Copy link
Copy Markdown
Contributor

No historical data — existing events will have empty matched_destination_ids. No backfill.

Sounds good

Should we expose matched_destination_ids on the API? — currently returned on GET /events and GET /events/:id.

Yes

Attempts gap — when events are embedded in attempt responses (include=event), matched_destination_ids is omitted because the attempts table doesn't have it. Options: accept the gap, denormalize into attempts, or join at query time.

I'd omit

@alexluong
Copy link
Copy Markdown
Collaborator Author

@alexbouchardd perfect, that's what we already implemented so should be good to go, can you review/approve?

in: query
required: false
schema:
type: string
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.

Should support an array

alexluong and others added 2 commits March 31, 2026 23:10
…#688)

Events now track which destinations they were routed to via a new
matched_destination_ids field, replacing the confusing destination_id
which was just the publish input.

- Add MatchedDestinationIDs to Event model, stamped during Handle()
- Add DB migrations (Postgres text[], ClickHouse Array(String)) that
  add the new column and drop destination_id in one step
- Update all logstore implementations (pg, ch, mem) for persistence,
  queries, and metrics
- Enable GET /events?destination_id=... filtering via array overlap
- Update API responses and OpenAPI spec; omit field from
  attempt-embedded events where it's unavailable
- Un-skip all previously blocked destination filter tests

No backfill: existing events will have empty matched_destination_ids.
Only new events going forward will be populated.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…leanup

Monitor goroutines were outliving their tests — Shutdown() closed the
Redis client but the Monitor loop kept retrying, then panicked logging
to a completed test. Fix: cancel the context before shutdown so Monitor
exits cleanly.

Also use assert.Eventually for timing-sensitive assertions instead of
fixed sleep durations.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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.

Clarify Event.destination_id vs matched destinations Bug: GET /events with destination_id returns 500

2 participants