feat(rust): upgrade workspace to posthog-rs 0.18.0 and on_error metric#67600
Merged
Conversation
Contributor
|
Reviews (1): Last reviewed commit: "feat(observability): surface posthog-rs ..." | Re-trigger Greptile |
…error Register posthog-rs's on_error hook in common_posthog::init so terminal SDK delivery failures (exhausted retries / permanent rejects) across capture, flags, and local-evaluation become a metric instead of only a log line. - common_posthog::init wires on_error -> posthog_sdk_delivery_failures_total, labeled service/surface/reason; the service label keeps the shared metric faceted per service regardless of scrape topology - registering the hook also silences the SDK's default per-reject WARN, so this metric now owns that signal - document why batch-import-worker stays pinned at posthog-rs 0.13.3 (0.14.0 made capture fire-and-forget, dropping the synchronous delivery Result its at-least-once offset gating relies on)
Registering an on_error hook suppresses posthog-rs's own default per-drop warn!, so a metric-only hook went dark on logs (and cymbal's old per-capture error! was already gone with the fire-and-forget migration. Re-emit a single warn! alongside the metric so the hook is a superset of the SDK default rather than a regression. warn (not error) matches the SDK's severity for dropped telemetry and fires only on terminal failure, so it stays bounded. EOF )
posthog-rs 0.18.0 restores an awaited, Result-returning batch capture (capture_batch_immediate), so the worker no longer needs its 0.13.3 pin. - workspace posthog-rs 0.17.3 -> 0.18.0; batch-import-worker drops its separate 0.13.3 override and inherits the workspace pin, so the whole Rust workspace now resolves a single posthog-rs version - emit/capture.rs: capture_batch -> capture_batch_immediate. In 0.18.0 capture_batch is fire-and-forget (non-async, returns nothing), so this is compile-forced; capture_batch_immediate reuses the identical v1 Ok/Err response classification, so offset gating and failure metrics are unchanged (all 21 emit::capture tests pass verbatim)
3e9ac67 to
6c8750c
Compare
Contributor
|
Reviews (2): Last reviewed commit: "feat(rust): migrate batch-import-worker ..." | Re-trigger Greptile |
This was referenced Jul 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When a posthog-rs capture,
/flags, or local-evaluation request fails terminally (retries exhausted or a permanent reject), our Rust services only emit a log line. There is no metric, so we cannot alert on the internal telemetry pipeline silently dropping events during an outage.The workspace was also stuck on a split posthog-rs: everything ran 0.13.3, and
batch-import-workercould not move forward because 0.14.0 made batch capture fire-and-forget, dropping the synchronous deliveryResultthe import commit path relies on to gate Kafka offset rollback (its at-least-once guarantee).posthog-rs 0.18.0 unblocks both at once. 0.17.0 added the
on_errorobservability hook, and 0.18.0 (PostHog/posthog-rs#175) restores an awaited,Result-returning batch capture (capture_batch_immediate). So we can unify the whole workspace on one version and get the failure signal.Changes
posthog-rsto0.18.0and dropbatch-import-worker's separate0.13.3pin, so the entire Rust workspace now resolves a single posthog-rs version (verified:Cargo.lockcollapses two entries to one).on_errorincommon_posthog::init, emittingposthog_sdk_delivery_failures_totallabeledservice/surface(capture, flags, local_evaluation) /reason(bounded failure class). Thereasonsplit mirrors the worker's existingfailure_reasonsoquota(expected billing enforcement) stays separable from actionable transport/server failures. Theservicelabel keeps the shared metric faceted per service.cymbal: capture has been non-blocking since 0.14, so the oldspawning_capture(spawn a task, await, log on error) collapses to a directfire_capture. Theon_errorhook now owns the failure signal it used to log.batch-import-worker:capture_batch->capture_batch_immediate. This is compile-forced, not cosmetic: in 0.18.0capture_batchis fire-and-forget (non-async, returns nothing).capture_batch_immediatereuses the identical v1 Ok/Err response classification, so the worker's offset gating and success/failure metrics are unchanged.Companion Grafana dashboard PR: PostHog/grafana-dashboards#231.
How did you test this code?
Automated tests I (actually Claude, in Cursor) ran under flox, all passing:
cargo build+cargo clippy --all-targets+cargo fmt --checkforbatch-import-worker,common-posthog,cymbal— clean (only a pre-existingcelesfuture-incompat note, unrelated).cargo test -p batch-import-worker— the 21emit::capturetests are the parity gate and pass verbatim againstcapture_batch_immediate:test_commit_write_fails_immediately_on_400,_fails_on_402_billing_limit,_retries_on_500_then_succeeds,_exhausts_retries_on_persistent_500,quota_failure_is_separable_from_actionable_failures,bad_request_failure_labels_events_and_requests,success_counts_events_total_and_requests_per_subbatch. These assert the exact offset-rollback and metric outcomes per HTTP status, so a green run proves behavior is identical to the 0.13.3 pin.cargo test -p common-posthog—sdk_error_reason_maps_every_variant(the alerting contract:quotastays separable) andon_error_hook::init_hook_emits_delivery_failure_metric_on_terminal_reject(drives a real client throughinitagainst a mock server returning 400, asserts the metric fired exactly once with the right labels).Automatic notifications
Docs update
No user-facing docs affected (internal service telemetry only).
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Directed by @eli-r-ph; implemented by Claude (Cursor). Key decisions:
.crateexposes bothcapture_batch_immediateand theon_errorhook before touching the workspace.CaptureSummary,capture_immediate,capture_batch_immediate, a newsecret_keybuilder), nothing this PR relies on was removed or changed.capture_batchand 0.18.0capture_batch_immediatedrive the identical v1after_responseclassification (is_retryable_statusandError::from_http_responseare byte-identical across the two tags), so Err (roll back the offset) and Ok (advance) fire on exactly the same responses. The worker ignored the oldOk(())payload and ignores the newOk(CaptureSummary)the same way.Resultand deliberately did NOT route it throughon_error: the hook fires post-return on a background thread and cannot gate a Kafka offset.on_erroris for the fire-and-forget crates (cymbal, embedding-worker) only.