feat(analytics): ingest backend item events over gRPC - #165
Conversation
The ADR-0002 client loop: dial backend:50051, ListItems snapshot
reconcile (upsert current_items, tombstone rows missing from the
snapshot), then consume WatchItemEvents; on any failure, equal-jitter
exponential backoff (1s base, 30s cap) and a fresh dial + snapshot.
Snapshot recovers state, not missed events -- aggregate dips stay
visible by design, and reconcile never synthesizes bucket entries.
Each stream event runs one transaction: raw insert with ON CONFLICT
DO NOTHING on (item_id, event_type, event_time), an hourly bucket
upsert keyed on date_trunc('hour', event_time, 'UTC') -- event time,
never arrival time, timezone pinned explicitly -- that increments only
when the raw insert actually landed (replays cannot double-count), and
an event-time-gated current_items upsert (a late out-of-order event
cannot un-tombstone or rename a row the future already wrote).
analytics_last_event_time_seconds is a monotonic watermark (CAS
max-advance) so the Phase 5 seeder's deliberately late events will not
regress staleness panels.
Read API for the canary and reports: GET /api/v1/items/{id} (200 with
tombstoned flag when seen, 404 never-seen) and GET /api/v1/stats.
AnalyticsStreamDown alert fires only while the process itself is up --
stream loss and service loss stay distinct signals -- with runbook.
Ships the analytics ingest dashboard and the break-the-event-stream
exercise. Analytics tests now run go test -p 1: store and ingest
suites share one throwaway Postgres and raced each other's schema
teardown under cross-package parallelism.
|
Warning Review limit reached
Next review available in: 47 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (10)
WalkthroughAnalytics now ingests backend snapshots and events over reconnecting gRPC, persists reconciled item state and event aggregates, exposes read APIs, and adds deployment configuration, metrics, alerts, dashboards, runbooks, exercises, and CI code-generation updates. ChangesAnalytics ingest
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Analytics as Analytics ingest client
participant Backend as Backend ItemService
participant Store as Analytics store
participant Metrics as Prometheus metrics
Analytics->>Backend: ListItems
Backend-->>Analytics: Snapshot
Analytics->>Store: ReconcileSnapshot
Analytics->>Backend: WatchItemEvents
Backend-->>Analytics: ItemEvent stream
Analytics->>Store: IngestEvent
Analytics->>Metrics: Update stream and ingest metrics
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Test job now runs go test -p 1: the store and ingest test binaries share one Postgres service container and raced each other's schema teardown -- the same fix the local Makefile already carries; the workflow bypassed it by invoking go test directly. The govulncheck job broke the moment ingest started importing the generated internal/pb stubs: generate-in-build means they exist only after codegen, which the action's own checkout never runs. The job now checks out, generates, and passes repo-checkout: false so the scan sees the same tree.
|
@coderabbitai review |
✅ Action performedReview finished.
|
protoc-gen-go and protoc-gen-go-grpc move from ad-hoc go install pins (workflow env vars + Dockerfile args, invisible to Renovate) into go.mod tool directives: buf.gen.yaml invokes them through go tool, so their versions live in go.mod/go.sum where Renovate manages them, and protoc-gen-go is MVS-locked to the protobuf runtime the service compiles against -- generator/runtime drift becomes structurally impossible. A new make -C services/analytics generate target is the single codegen entry point for the root Makefile, CI, and the Dockerfile docs; CI installs buf via bufbuild/buf-action setup_only, the same pattern proto.yml already uses.
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/analytics.yml:
- Around line 119-151: Add job-level permissions to audit-analytics granting
only contents: read for checkout and code generation. Do not add security-events
or any broader permission scope.
In `@docs/architecture.md`:
- Around line 72-79: Update the Backend component description in
docs/architecture.md to remove the stale statement that no consumer is
connected. Keep the documented analytics consumer and WatchItemEvents topology
consistent with the gRPC ingest client description.
In `@services/analytics/internal/ingest/client.go`:
- Around line 139-143: Update the event-processing loop around handleEvent so
persistence failures from Store.IngestEvent are retried for the same event
before calling Recv again, preserving the stream-connected state and preventing
data loss. Distinguish these transient write failures from validated malformed
payloads, which should remain intentionally logged and discarded; use the
existing handleEvent and persistence error path rather than changing unrelated
stream handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 613fb8a8-eeed-4013-bdfd-bb9c8ec93da8
📒 Files selected for processing (23)
.github/workflows/analytics.ymlREADME.mddeploy/compose/docker-compose.ymldocs/architecture.mddocs/exercises/03-break-the-event-stream.mddocs/runbooks/analytics-stream-down.mdobservability/grafana/dashboards/analytics.jsonobservability/prometheus_alerts.ymlservices/analytics/Makefileservices/analytics/README.mdservices/analytics/cmd/analytics/main.goservices/analytics/go.modservices/analytics/internal/httpserver/server.goservices/analytics/internal/httpserver/server_test.goservices/analytics/internal/ingest/client.goservices/analytics/internal/ingest/client_test.goservices/analytics/internal/ingest/config.goservices/analytics/internal/ingest/metrics.goservices/analytics/internal/ingest/trace.goservices/analytics/internal/store/current_items.goservices/analytics/internal/store/current_items_test.goservices/analytics/internal/store/migrate_test.goservices/analytics/internal/store/migrations/0002_current_items.sql
Review findings: a DB blip during stream consumption silently dropped the event while the stream stayed healthy -- the loop now retries the same event (1s pace) before the next Recv, and only validated-malformed payloads (unknown enum, marked with a sentinel error) are logged and discarded. A sustained outage self-resolves through the backend's bounded per-subscriber queue: it disconnects this client and the normal reconnect+snapshot path takes over. Regression test drives a flaky store (two failures, then success) plus a malformed event through a live stream. Also: audit-analytics job gets least-privilege contents: read, and docs/architecture.md drops the stale 'no consumer connected yet' backend note now that the ingest client exists.
|
The latest Buf updates on your PR. Results from workflow Proto / breaking-proto (pull_request).
|
|
The latest Buf updates on your PR. Results from workflow Proto / lint-proto (pull_request).
|
1 participant
The ADR-0002 client loop: dial backend:50051, ListItems snapshot reconcile (upsert current_items, tombstone rows missing from the snapshot), then consume WatchItemEvents; on any failure, equal-jitter exponential backoff (1s base, 30s cap) and a fresh dial + snapshot. Snapshot recovers state, not missed events -- aggregate dips stay visible by design, and reconcile never synthesizes bucket entries.
Each stream event runs one transaction: raw insert with ON CONFLICT DO NOTHING on (item_id, event_type, event_time), an hourly bucket upsert keyed on date_trunc('hour', event_time, 'UTC') -- event time, never arrival time, timezone pinned explicitly -- that increments only when the raw insert actually landed (replays cannot double-count), and an event-time-gated current_items upsert (a late out-of-order event cannot un-tombstone or rename a row the future already wrote). analytics_last_event_time_seconds is a monotonic watermark (CAS max-advance) so the Phase 5 seeder's deliberately late events will not regress staleness panels.
Read API for the canary and reports: GET /api/v1/items/{id} (200 with tombstoned flag when seen, 404 never-seen) and GET /api/v1/stats. AnalyticsStreamDown alert fires only while the process itself is up -- stream loss and service loss stay distinct signals -- with runbook. Ships the analytics ingest dashboard and the break-the-event-stream exercise. Analytics tests now run go test -p 1: store and ingest suites share one throwaway Postgres and raced each other's schema teardown under cross-package parallelism.
Summary by CodeRabbit