Repository navigation
feat(workflows): report blocked and failed workflow sends - #127135
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…send-funnel-events
|
😎 Merged successfully - details. |
🤖 CI report✅ Trunk lane — non-backend laneThis PR is assigned to the non-backend lane. It does not run backend Python tests and may merge in parallel with PRs in other lanes. ✅ Duplication (Python) — cleanNew Python code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.
|
| First copy | Second copy | Lines | Tokens |
|---|---|---|---|
nodejs/src/cdp/consumers/cdp-data-warehouse-events.consumer.ts:55 |
nodejs/src/cdp/consumers/cdp-internal-event.consumer.ts:60 |
34 | 201 |
nodejs/src/cdp/consumers/cdp-cohort-membership-trigger.consumer.ts:97 |
nodejs/src/cdp/consumers/cdp-data-warehouse-events.consumer.ts:68 |
23 | 128 |
nodejs/src/cdp/consumers/cdp-cohort-membership-trigger.consumer.ts:105 |
nodejs/src/cdp/consumers/cdp-person-updates-consumer.ts:41 |
15 | 85 |
nodejs/src/cdp/consumers/cdp-data-warehouse-events.consumer.ts:67 |
nodejs/src/cdp/consumers/cdp-events.consumer.ts:65 |
17 | 77 |
nodejs/src/cdp/consumers/cdp-data-warehouse-events.consumer.ts:67 |
nodejs/src/cdp/consumers/cdp-dlq-replay.consumer.ts:145 |
13 | 73 |
✅ Hogbox preview — ready, open the preview
▶ Open the preview
| 🔑 Login | test@posthog.com / 12345678 (demo data) |
| 🧩 Running | this PR's backend on the PostHog :master base (frontend unchanged by this PR) |
| 🔗 Link | stable across rebuilds: a re-push swaps the box underneath, the URL stays |
| 🔒 Access | tailnet only (PostHog VPN) |
| 🛠️ Admin | inspect & debug state in hogland |
| 💤 Idle | sleeps after ~30 min idle (snapshot to S3, zero node cost) and wakes on your next visit in ~30s, behind a brief "waking up" screen |
commit 7985d45 · box box-80692bcea250 · ready in 1968s (push → usable) · build log · rebuilds on every push, torn down on close
HostHog preview —
|
A run blocked by the destination quota drops its email steps too, and it reported nothing. Report `workflows send blocked` with `quota_limited` for every quota-blocked run that has an email step, whichever quota stopped it, and drop the `limitedBy` discriminator that only served the old rule. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…send-funnel-events
|
Risk: No findings The delta since the last review is a master merge bringing in three production changes alongside this PR's analytics reporting: the events consumer can route team lookup off a Kafka Sentinel reviewed |
|
@PostHog review |
|
@Silthus PostHog Review is reviewing this pull request. |
There was a problem hiding this comment.
Approved.
The risky paths have current-head review assurance and owning-team authorship. No unresolved substantive concerns or other refusal grounds were found.
- PostHog Review (Standard) reviewed the current head: no findings.
- posthog[bot] reviewed the current head.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 104L, 11F substantive, 369L/19F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1d-complex (369L, 19F, single-area, feat) |
| stamphog 2.7.0 | .stamphog/policy.yml @ 30f2376 · reviewed head 30f2376 |
…send-funnel-events
|
/trunk merge |
EmailService now takes a required WorkflowsActivationReporter (#127135). The local SES suite built the service without it and stopped compiling after the master merge. It passes a stub, as the email service suite does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Deploy status
This table shows the |
…fake (#127175) * chore(workflows): cover the email send outcome matrix Send test email through a local SES fake, so the send outcome matrix runs against real SDK requests without mocking SES. * test(workflows): tighten the email outcome matrix after review - Use the production Redis pool and drop the TestRedisV2 copy. - Cap and pacing cases assert their own log reason and a drained bucket, so a limiter outage cannot pass for a denial. - Rows carry their own setup and expected values; tests with two flows are split. - Titles name the behavior only, without matrix numbers or fork references. - Drop the harness self-test and the duplicate SES throttle row. - Keep the handbook to how to run the suite. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(workflows): name the start command when the local SES fake is down The SES suite runs in the default Jest set, but the fake sits behind the compose `ses` profile. A run without it now fails with the command that starts it, instead of an error that reads like a regression. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(workflows): share one Valkey pool across the SES suite RedisV2 has no close, so a pool per test left one connection per test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * chore(workflows): start the SES fake in test:full and services:start Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * chore(workflows): close the SES suite's redis client so jest exits Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * chore(cdp): pass the activation reporter in the local SES email tests EmailService now takes a required WorkflowsActivationReporter (#127135). The local SES suite built the service without it and stopped compiling after the master merge. It passes a stub, as the email service suite does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(workflows): move the uncovered email budget cases onto the in-process SES fake Master's email service suite now sends through the real SESv2 client to an in-process SES fake (#132730) and already covers most of this outcome matrix. Add the cases it lacks there: shadow mode charges the team budget and sends past an empty one, every to, cc and bcc copy is charged and a send with more copies than the budget takes all of it and still sends, and a send that a team suspension, a workflow pause or the suppression list skips spends neither the workflow pacing nor the team budget. Drop the Docker SES fake, its CI step, compose service, package scripts and handbook line, which only served the separate suite. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(workflows): pin each skip gate's own metric and the full starting budget Assert that each skipped send records its own gate's metric with no error, and that the shadow test starts from a full budget, so a send that stops early for another reason cannot pass as a gate skip. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Problem
Impact
Pays into Grow Workflows through activation: the share of teams whose live workflow reaches its first billed action. The "went live → first billed action" step of the funnel has no reasons today. These events name them per team, so the funnel's drop-off splits by cause in PostHog's internal project. Expected move: small, up, through the fixes the drop-off reasons point to (quota upgrades, domain verification, recipient filters).
Merge Danger
Warning
🟡 Merge risk: medium · two-way door (merge-risk rubric)
EmailServiceand every CDP consumer gain a required reporter dependency, which the typecheck guards. Worst case is silent harm on that path, though no send decision changes.try/catch, so a failed team lookup or capture cannot fail or delay a send. Reporter, quota, email service, pipeline and consumer suites cover the paths, and a live run on the hog flow pipeline shows the quota reports.git revertthe merge commit. Events already captured stay in the internal project and need no cleanup.Changes
The CDP service reports two team events to PostHog's own analytics:
workflows send blockedworkflow_id,channel: email,reason: quota_limitedormissing_recipientworkflows send failedworkflow_id,channel: email,reason: unverified_domainWorkflowsActivationReporterreports each team, event and reason at most once per hour per service instance. The funnel needs only the first occurrence.EmailService(#120078), the single place every email send checks for an emptyto.Recreates the send-funnel part of Silthus/posthog#357, originally #111267 (closed). Moved here from the fork PR #115352 so CI can run.
How did you test this code?
Test rationale:
email.service.test.tsfailed with no report beforeEmailServicereported the skip, then passed. It and the unverified-domain case cover a workflow send, an editor test send and a hog function send.Test seams: the reporter's
reportagainstcaptureTeamEvent,EmailService.executeSendEmail, and the hog flow quota check and pipeline.Lane gate, green locally on
7985d45(master merged in):pnpm install --frozen-lockfile,hogli ci:preflight --strict, Nodetsc, ESLint, oxlint and Prettier on the changed files, the ingestion boundary guard, the Jest suites of every touched module (nodejs/src/cdp/consumers,templates, the email service, quota limiting, pipeline, hog flow executor, hog function handler and hog executor suites, the reporter; 90 suites, 1571 passed, 1 skipped), andcdp-api.serial.test.ts(141/141).Proofs on
7985d45, the head after the latest master merge, with both live runs repeated there. That merge brought five master commits that touch this PR's files, none of them its report call sites: #126304 labels email queue waits that the team sending cap delayed (email.service.ts), #127164 skips parsing events a consumer has no work for (cdp-events.consumer.ts), #124635 sends hog flow logs to the logs product (cdp-services.ts), and #127579 and #126479 add tests tohog_function.test.ts. The PR's own patch against master is unchanged (same patch-id as on30f2376and64a9c11). Each report still fires once where the send stops: the unverified-domain failure and the missing-recipient skip both come before the team-cap reschedule, a send held by #127579's frequency-cap hold stays in the hog function step and never reaches the email service while held, and the quota check runs only when a run is created.7985d45, and earlier on30f2376and64a9c11, hog flow pipeline (cdp-processed-eventsconsumer at HEAD, local stack, workflows created and activated through thehog_flowsAPI, analytics captures recorded by a local listener). Two events per workflow:{ "event": "workflows send blocked", "reason": "quota_limited", "channel": "email", "workflow_id": "<the workflow>", "groups": { "project": "<team uuid>", "organization": "<org uuid>" } }workflows send blockedwithquota_limited.7985d45, and earlier on30f2376and64a9c11, email steps (events consumer, cyclotron hogflow worker and cyclotron email worker at HEAD, local PersonHog, people in the persons database). A team with an unverified and a verified email sender, two events per workflow:workflows send failedwithunverified_domain.workflows send blockedwithmissing_recipient.64a9c11run).30f2376and7985d45reruns show the same two events, one per workflow, with both email jobs per workflow completed.64a9c11, carried across the two clean master merges, read-only production data: most live email workflows have a non-transactional email step and some also have a destination step, so both quota branches meet real workflows; a few live email workflows send through an unverified domain today. No team sits at a workflow quota limit in the stored usage right now, so quota reports start as teams reach a limit. Missing recipients depend on person data at send time and are left to the post-deploy watch. Every stored field the change reads has the shape the code and fixtures assume.7985d45; see this PR's checks.Release status
Docs update
None. No existing doc covers Workflows analytics events.
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Agent: T3 Code with Claude Code, Claude Opus 5.5 (
claude-opus-5-5). An earlier Codex worker (GPT-6.1 Sol) recreated the change before its session dropped.workflows message delivered: billing usage reports already give the first send, and delivery depended on SES returning the signed tracking header. Replaced the earlier delivery, block and failure scope.quota_limited?EmailService's missing-recipient skip that master added, asworkflows send blocked. It replaces the earlier report inRecipientPreferencesServiceasworkflows send failed, since master now treats a missing address as a skip.unverified_domain?verifiedflag records, and matching SES error text is fragile.Involvement: no grilling rounds, no prototypes. Reviews on the fork PR: qa-swarm round 1 (GPT-6.1 Sol: Correctness, Risk, Design, Systems, the author's Team lens; Opus 5.5: Fresh eyes), round 2, a final read, and a full-scope GPT-6.1 Sol review after the trim. On this PR: a full-scope panel (GPT-6.1 Sol: Correctness, Risk, Systems; Opus 5.5: Fresh eyes), a GPT-6.1 Sol final read of the fix, and GPT-6.1 Sol final reads of both master merges (their four findings were all in master files the PR does not touch). Proof captured: live runs of all three reasons and prod shape. CodeRabbit is retired on this repo.
tdd,modernize-pr,qa-swarm,merge-risk,pr-description,writing-voice.nodejs/src/cdp), and fixed the review finding above. A second review suggestion,lru-cache'sttlin place of the explicit hour check, was tried and dropped: the cache reads its clock at import, so Jest's fake timers cannot pin the hour boundary.🤖 Generated with Claude Code