Skip to content

fix(email): stop blocking inline email sends on an analytics flush - #124631

Merged
trunk-io[bot] merged 3 commits into
masterfrom
posthog/email-capture-no-blocking-flush
Oct 28, 2026
Merged

trunk-io[bot] merged 3 commits into
masterfrom
posthog/email-capture-no-blocking-flush

Conversation

@mayteio

@mayteio mayteio commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Problem

  • Signup and login requests that send an email inline wait for an analytics event upload before they respond.
  • #122019 moved email events onto ph_scoped_capture, which builds a new client per send and calls a blocking shutdown().
  • The upload goes to US ingestion, so EU requests wait on a cross-region call.
  • Raised in this Slack thread.

Changes

  • Emails sent inside a request no longer wait on the analytics upload. Users see the signup and login responses sooner.
  • A send outside a Celery worker task uses the global posthoganalytics client. That client uploads in the background and already sets the region super property.
  • A send that a Celery worker runs keeps ph_scoped_capture, so a recycled worker child still flushes its events.
  • Both paths keep the is_cloud() guard, so a self-hosted instance sends no recipient addresses to the PostHog analytics project.
  • Event names and properties do not change.
  • Gap: an eager send nested inside another worker task uses the global client. This matches the behavior before feat(email): send transactional emails through workflows behind a flag #122019.

How did you test this code?

Test rationale: test_send_via_http_flushes_events_only_in_a_worker covers which client a send uses. It fails if a request send blocks on the scoped flush, if a worker send skips it, or if a self-hosted send captures events. The existing posthog/test/test_email.py cases still assert every captured event and its properties.

  • Ran hogli test posthog/test/test_email.py locally.
  • Not checked: the latency effect in production. Watch the signup and login POST latency after deploy.

Release status

  • No feature flag controls this change

Docs update

None.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

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

  • Skills invoked: /writing-tests, /writing-pr-descriptions.
  • PostHog Review findings led to the worker and request split and to keeping the cloud guard. The first version used the global client for every send.
  • This PR removes only the analytics flush. The inline Workflows send that signup and login codes now make is a separate cause of the slowdown, and it is out of scope here.
  • The search for open PRs that fix this found none.

Created with PostHog Desktop

🤖 Generated with Claude Code

HTTP email sends captured their events through ph_scoped_capture, which builds a client per send and waits for it to upload before returning. Requests that send an email inline, such as signup and login codes, waited on that upload.

Capture through the global posthoganalytics client again, which already sets the region super property and uploads in the background.

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

Generated-By: PostHog Desktop
Task-Id: 43b649a0-cb22-4866-8c03-fb38148fd5ac
@mayteio mayteio self-assigned this Oct 28, 2026
@trunk-io

trunk-io Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

😎 Merged successfully - details.

@mayteio
mayteio marked this pull request as ready for review October 28, 2026 08:58
@mayteio mayteio added the posthog-review Run a Standard PostHog Review label Oct 28, 2026
@parameterai

parameterai Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

Risk: No findings

This change routes inline (web-request) email analytics through the global posthoganalytics client instead of the per-send flushing client, so signup/login responses no longer wait on a cross-region analytics upload; Celery worker sends keep the dedicated flushing client. Both transports keep the cloud-only guard and the region super property, so I found no security regression — only the acknowledged, documented risk that an event enqueued just before a process exit can be lost.

Sentinel reviewed 5374e8c · Review settings

@github-actions

github-actions Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

🤖 CI report

⚠️ Trunk lane — backend Python lane

This PR is assigned to the backend Python lane. It runs 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 — 3% of added code lines are comments (2 of 58)

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
posthog/email.py 2 5

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

✅ Playwright — all passed

All tests passed.

View test results →

@pr-assigner-resolver-posthog
pr-assigner-resolver-posthog Bot requested a review from a team October 28, 2026 09:00

@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 bba4c6b: 2 Should fix.

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

See this review and its findings in PostHog Review.

Comment thread posthog/email.py Resolved
Comment thread posthog/email.py Resolved
@posthog posthog Bot removed the posthog-review Run a Standard PostHog Review label Oct 28, 2026
@trunk-io

trunk-io Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

Static Badge   Static Badge   Static Badge

View Full Report ↗︎ ⋅ Docs

@yasen-posthog yasen-posthog 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.

Approved to unblock, just make sure to address the bot comments, and thank you 🙏

…uard

A send that a Celery worker runs uses ph_scoped_capture again, so a recycled worker child does not drop the email events. Only sends outside a worker task, such as eager sends inside a request, use the global client.

Both paths go through ScopedCapture, so a self-hosted instance does not send recipient addresses to the PostHog analytics project.

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

Generated-By: PostHog Desktop
Task-Id: 43b649a0-cb22-4866-8c03-fb38148fd5ac
Generated-By: PostHog Desktop
Task-Id: 43b649a0-cb22-4866-8c03-fb38148fd5ac
@trunk-io
trunk-io Bot merged commit bc2e932 into master Oct 28, 2026
255 checks passed
@trunk-io
trunk-io Bot deleted the posthog/email-capture-no-blocking-flush branch October 28, 2026 12:00
@deployment-status-posthog

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

Copy link
Copy Markdown

Deploy status

Environment Status Deployed At Workflow
dev ✅ Deployed 2026-10-28 12:20 UTC Run
prod-us ✅ Deployed 2026-10-28 12:40 UTC Run
prod-eu ✅ Deployed 2026-10-28 12:38 UTC Run

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.

2 participants