Skip to content

fix(cdp): answer transactional email sends without waiting on kafka - #125444

Merged
trunk-io[bot] merged 3 commits into
masterfrom
posthog/transactional-email-respond-before-flush
Oct 29, 2026
Merged

trunk-io[bot] merged 3 commits into
masterfrom
posthog/transactional-email-respond-before-flush

Conversation

@mayteio

@mayteio mayteio commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

Problem

  • Transactional email API callers wait ~900ms (p50) for {queued: true}, ~800ms of it after the job is queued.
  • cdp-api awaits two WarpStream acks for telemetry first; a slow Kafka response slows every send.

Changes

  • The 202 returns after the job insert and idempotency mark. Telemetry rows go to the producer, which flushes on shutdown; acks finish in the background.
  • The email worker counts triggered once, from a triggerMetricPending marker the API sets. Unmarked jobs from old pods aren't counted, so a rollout can dip the count but never double it.
flowchart LR
  subgraph Before
    A1[Insert job] --> B1[Await results + status acks] --> C1[Await app metric ack] --> D1[202]
  end
  subgraph After
    A2[Insert job] --> D2[202]
    A2 -.-> E2[Rows to producer, acks in background]
  end
  classDef phYellow fill:#f9bd2b,stroke:#f9bd2b,color:#000;
  classDef phRed fill:#f54e00,stroke:#f54e00,color:#fff;
  classDef phGray fill:#e5e7eb,stroke:#c7ccd1,color:#000;
  class D1,D2 phYellow;
  class B1,C1 phRed;
  class A1,A2,E2 phGray;
Loading

How did you test this code?

Test rationale: a hanging telemetry flush still gets a 202 (times out with the old await). A marked job counts once across a reschedule; unmarked jobs don't count.

Not checked: latency in prod.

Release status

  • No feature flag controls this change

Docs update

  • None.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Agent: Claude Code, Claude Opus 5.5 (claude-opus-5-5)

Skills: /writing-tests, /writing-code-comments, /writing-pr-descriptions

More context for AI agents
  • Decisions: the route awaits handOffQueuedRows() (gzip only, no network), not the acks.
  • Follow-ups: an alert on messaging_transactional_email_stage_duration_seconds{stage="total"} p95 once this ships.
  • Read closely: cdp-api.ts around the metrics_flush stage, and countPendingTrigger in the email worker.

Created with PostHog Desktop

🤖 Generated with Claude Code

…acks

The send route waited on two WarpStream produce acks for best-effort telemetry after the job was already queued, which put about 800ms on every 202. It now hands the result and status rows to the producer and replies; the queue insert and the idempotency mark stay awaited. The triggered app metric moves to the email worker, which counts a run once from a marker the API sets.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Generated-By: PostHog Desktop
Task-Id: 59a60ccd-2dab-47ac-8695-8d3e7a9e8eef
@mayteio mayteio self-assigned this Oct 29, 2026
@trunk-io

trunk-io Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

😎 Merged successfully - details.

@github-actions

github-actions Bot commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

🤖 CI report

✅ Trunk lane — non-backend lane

This 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) — clean

New 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.

✅ Duplication (TypeScript) — clean

New TypeScript 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.

🚨 Comment density — 9% of added code lines are comments (16 of 175)

This section warns when comments are more than 3% of the code lines a PR adds, and alerts above 6%. Before agent-assisted PRs, the typical share was about 2%. Only full-line comments count. Docstrings, generated files, snapshots, migrations, and workflow files are left out.

Comments that restate the code, record how the change came about, or narrate the next line add noise for the next reader. Keep the comments that explain a reason the code cannot show, and remove the rest. See .agents/skills/writing-code-comments/SKILL.md for the house rules.

Files with the most added comment lines:

File Comment lines Added lines
nodejs/src/cdp/services/monitoring/hog-invocation-results.service.ts 5 37
nodejs/src/cdp/cdp-api.ts 3 12
nodejs/src/cdp/consumers/cdp-cyclotron-worker-email.consumer.ts 3 25
nodejs/src/cdp/consumers/cdp-cyclotron-worker-email.consumer.test.ts 2 68
nodejs/src/cdp/types.ts 2 3
nodejs/src/cdp/cdp-api.serial.test.ts 1 29

This check does not block merging. It updates on every push and clears when the share drops.

@mayteio
mayteio marked this pull request as ready for review October 29, 2026 12:42
@parameterai

parameterai Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

Risk: No findings

The only change since the last review is a no-op refactor in hog-invocation-results.service.ts: replacing rows.map returning undefined entries with rows.flatMap returning empty arrays inside handOffQueuedRows. Behavior, index alignment, and error handling are unchanged, and no security-relevant surface is touched.

Sentinel reviewed 6c1260c · Review settings

@posthog posthog Bot left a comment

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.

PostHog Review · Standard · reviewed 326775b: no findings.

This Standard review started from the posthog-review label. Comment @posthog review --deep for a Deep review, or @posthog help to see all commands.

@posthog posthog Bot removed the posthog-review label Oct 29, 2026
@pr-assigner-resolver-posthog
pr-assigner-resolver-posthog Bot requested a review from a team October 29, 2026 12:43
@trunk-io

trunk-io Bot commented Oct 29, 2026

Copy link
Copy Markdown

Static Badge   Static Badge   Static Badge

View Full Report ↗︎ ⋅ Docs

@mayteio mayteio added the stamphog Request AI approval (no full review) label Oct 29, 2026
stamphog[bot]

This comment was marked as outdated.

Rows that failed to encode returned undefined inside Promise.all, which oxlint's await-thenable rule rejects.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Generated-By: PostHog Desktop
Task-Id: 59a60ccd-2dab-47ac-8695-8d3e7a9e8eef
@stamphog
stamphog Bot dismissed their stale review October 29, 2026 15:04

A new stamphog review started for this PR — the fresh verdict replaces this approval.

@stamphog stamphog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved.

The risky write-path and persisted-state changes have owning-team authorship as assurance, with targeted regression tests and no outstanding substantive concerns. The supplied bot review is on an older commit and does not count as a current-head review.

  • Author wrote 69% of the modified lines and has 37 merged PRs in these paths (familiarity MODERATE).
Gate mechanics and policy version
Gate Result
prerequisites ✓ all clear
deny-list ✓ no deny categories matched
size ✓ 118L, 5F substantive, 226L/7F incl. docs/generated/snapshots — within ceiling
tier ✓ T1-agent / T1c-medium (226L, 7F, single-area, fix)
stamphog 2.6.1 .stamphog/policy.yml @ 6c1260c · reviewed head 6c1260c

@hosthog

hosthog Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

HostHog preview — posthog-desktop-web

The previews for this PR have been torn down and no longer serve.

mayteio commented Oct 29, 2026

Copy link
Copy Markdown
Contributor Author

/depot retry

1 similar comment

mayteio commented Oct 29, 2026

Copy link
Copy Markdown
Contributor Author

/depot retry

@trunk-io
trunk-io Bot merged commit b30cac6 into master Oct 29, 2026
204 checks passed
@trunk-io
trunk-io Bot deleted the posthog/transactional-email-respond-before-flush branch October 29, 2026 17:06
@deployment-status-posthog

deployment-status-posthog Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

Deploy status

Environment Status Deployed At Workflow
dev ✅ Deployed 2026-10-29 17:41 UTC Run
prod-us ✅ Deployed 2026-10-29 18:04 UTC Run
prod-eu ✅ Deployed 2026-10-29 18:00 UTC Run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stamphog Request AI approval (no full review)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant