[matheus-vb]

Problem

On call never gets paged for a real cohort recalculation SLO breach, because the gauges the alerts read vanish whenever the Django worker pods autoscale.
The gauges are process local multiprocess gauges, set only inside the per minute calculate_cohort coordinator and scraped from pods that scale up and down.
When a pod holding fresh samples rolls, max() over the series goes empty and the alerting for timer resets.
The coordinator also runs with no lock, so two overlapping runs can pick the same candidates.

Related to https://github.com/PostHog/charts/pull/14590.

Changes

• A new record_cohort_metrics task emits the SLO gauges every 2 minutes through the pushgateway, as stable region level series that survive worker scale events, so the alerts stay continuous.
• update_cohort_metrics now writes to a passed in registry; metric names, help strings, and the hours label stay identical, so the alerts and dashboard keep keying on them.
• calculate_cohort takes a singleton cache lock (300s TTL, released in finally) so overlapping runs cannot double pick candidates, and posthog_calculate_cohort_lock_contentions_total counts skips.
• The per minute coordinator sheds several full table counts, now run by the metrics task instead.
• The day coordinator expires drops to match the every minute cadence.
• Mechanical: removed the five module level gauge objects and the boot time backlog init in celery.py; cohort_staleness_hours stays worker side.

How did you test this code?

• Reworked the cohort metric tests and added lock and wiring tests in posthog/tasks/test/test_calculate_cohort.py, and trimmed posthog/test/test_celery.py.
• The metric tests catch queryset or label drift; the lock tests catch a coordinator that double runs or never releases; the wiring test catches gauges emitted to the default registry instead of the pushed one.
• Ran those two test files against a local Postgres, plus repo wide mypy and ruff. Did not run the full CI suite.

Docs update

No.

[trunk-io]

Merging to master in this repository is managed by Trunk.

• To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here

[posthog]

🦔 ReviewHog reviewed this pull request

Found 1 must fix, 0 should fix, 1 consider.

Published 2 findings (view the review).

Resolved comments: 1 fixed, 1 declined

[github-actions]

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

✅ Playwright — all passed

All tests passed.

View test results →

⚠️ Backend coverage — 98.0% of changed backend lines covered — 1 uncovered

🧪 Backend test coverage

Patch coverage — changed backend lines (products + core): ████████████████████ 98.0% (98 / 99)

File	Patch	Uncovered changed lines
posthog/tasks/tasks.py	95.0%	954

🤖 Agents: add a test covering the lines above, or note why under "How did you test this code?". Machine-readable gap list: the patch-coverage artifact on this run (gh run download 32662917246 -n patch-coverage), or the coverage-data block at the end of this comment.

Per-product line coverage (touched products)

Product	Coverage	Lines
platform_features	██░░░░░░░░░░░░░░░░░░ 12.1%	7 / 58
warehouse_sources_queue	██████░░░░░░░░░░░░░░ 29.4%	91 / 309
batch_exports	████████░░░░░░░░░░░░ 37.9%	8,916 / 23,531
demo	███████████░░░░░░░░░ 56.9%	1,516 / 2,666
data_tools	█████████████░░░░░░░ 66.2%	90 / 136
tasks	██████████████░░░░░░ 70.5%	43,201 / 61,319
ai_gateway	███████████████░░░░░ 75.0%	9 / 12
apm	█████████████████░░░ 83.3%	1,234 / 1,481
signals	█████████████████░░░ 83.4%	29,902 / 35,873
actions	█████████████████░░░ 86.6%	717 / 828
wizard	█████████████████░░░ 86.7%	1,090 / 1,257
data_modeling	█████████████████░░░ 86.9%	9,644 / 11,092
notebooks	█████████████████░░░ 87.2%	8,553 / 9,812
cdp	█████████████████░░░ 87.4%	4,522 / 5,174
cohorts	█████████████████░░░ 87.5%	6,550 / 7,486
product_tours	██████████████████░░ 87.9%	1,303 / 1,482
business_knowledge	██████████████████░░ 89.1%	4,431 / 4,975
data_warehouse	██████████████████░░ 89.1%	12,631 / 14,177
dashboards	██████████████████░░ 89.1%	6,386 / 7,165
exports	██████████████████░░ 89.3%	7,701 / 8,626
engineering_analytics	██████████████████░░ 89.5%	6,828 / 7,632
visual_review	██████████████████░░ 89.8%	6,057 / 6,745
alerts	██████████████████░░ 90.4%	4,707 / 5,209
mcp_analytics	██████████████████░░ 90.5%	4,001 / 4,421
links	██████████████████░░ 90.6%	183 / 202
streamlit_apps	██████████████████░░ 90.7%	2,630 / 2,901
managed_warehouse	██████████████████░░ 90.7%	8,555 / 9,427
conversations	██████████████████░░ 90.8%	20,468 / 22,531
error_tracking	██████████████████░░ 91.3%	11,940 / 13,083
stamphog	██████████████████░░ 91.6%	5,399 / 5,892
canvas	██████████████████░░ 91.9%	4,408 / 4,798
early_access_features	███████████████████░ 92.5%	1,323 / 1,430
slack_app	███████████████████░ 92.5%	12,113 / 13,090
managed_migrations	███████████████████░ 92.6%	1,556 / 1,681
web_analytics	███████████████████░ 92.8%	17,033 / 18,360
notifications	███████████████████░ 92.8%	1,074 / 1,157
mcp_store	███████████████████░ 93.2%	7,425 / 7,971
surveys	███████████████████░ 93.3%	5,943 / 6,373
marketing_analytics	███████████████████░ 93.4%	16,881 / 18,081
reminders	███████████████████░ 93.4%	468 / 501
posthog_ai	███████████████████░ 93.5%	1,371 / 1,467
approvals	███████████████████░ 93.5%	3,491 / 3,734
ai_observability	███████████████████░ 93.5%	17,411 / 18,618
product_analytics	███████████████████░ 93.7%	7,331 / 7,827
billing_alerts	███████████████████░ 93.9%	2,047 / 2,181
legal_documents	███████████████████░ 94.0%	1,914 / 2,037
workflows	███████████████████░ 94.1%	9,503 / 10,097
endpoints	███████████████████░ 94.3%	8,814 / 9,349
review_hog	███████████████████░ 94.3%	9,969 / 10,570
tracing	███████████████████░ 94.4%	2,678 / 2,837
annotations	███████████████████░ 95.1%	734 / 772
skills	███████████████████░ 95.2%	4,698 / 4,935
customer_analytics	███████████████████░ 95.4%	15,213 / 15,940
experiments	███████████████████░ 95.5%	29,169 / 30,539
messaging	███████████████████░ 95.7%	3,545 / 3,704
logs	███████████████████░ 95.7%	13,002 / 13,581
replay_vision	███████████████████░ 95.8%	22,750 / 23,750
feature_flags	███████████████████░ 96.1%	19,113 / 19,895
revenue_analytics	███████████████████░ 96.3%	1,841 / 1,911
growth	███████████████████░ 96.5%	6,808 / 7,057
user_interviews	███████████████████░ 96.5%	2,639 / 2,735
data_quality	███████████████████░ 96.5%	2,692 / 2,789
access_control	███████████████████░ 96.7%	873 / 903
warehouse_sources	████████████████████ 97.6%	425,719 / 436,109
analytics_platform	████████████████████ 98.1%	2,286 / 2,330
data_catalog	████████████████████ 98.1%	3,180 / 3,240
metrics	████████████████████ 98.2%	3,086 / 3,142
pulse	████████████████████ 98.4%	2,017 / 2,049
live_debugger	████████████████████ 99.2%	613 / 618
field_notes	████████████████████ 99.4%	158 / 159

Report-only. Patch coverage = changed backend lines covered vs origin/master. Sorted lowest first.
Known gaps: lines covered only by Temporal tests show as uncovered; core line numbers may drift if master changed the same file.

[trunk-io]

   

Failed Test	Failure Summary	Logs
test_annotation_events_do_not_defer_settling	The process failed because no trace activity was visible when expected, indicating the trace was not settled.	Logs ↗︎
test_settled_when_quiet_beyond_margin	The test failed because no session activity was detected when it was expected to be settled.	Logs ↗︎
test_evaluation_events_never_defer_settling	The process failed because no session activity was detected when expected.	Logs ↗︎
test_events_carrying_another_session_id_do_not_count_as_activity	The test failed because no session activity was detected, indicating the session was not settled.	Logs ↗︎

... and 1 more

View Full Report ↗︎ ⋅ Docs

[posthog]

ReviewHog Alpha 🦔 If you find any issues helpful - please reply "valid", "invalid", etc., for evaluation purposes 🙏

[posthog]

ReviewHog Report

Found 1 must fix, 1 consider.

[JamesPatrickGill]

nit (charts follow-up, nothing to change here): nothing alerts when these pushes stop.

The pushgateway serves the last pushed value forever. If record_cohort_metrics dies (stats queue stuck, a future regression), the SLO alerts read that frozen value indefinitely. The new last_over_time(...[30m]) expressions never see a gap, because the gateway answers every scrape with the stale sample. A real breach after that point pages nobody. That is the failure this PR fixes, back through a different path.

The pushed success gauge cannot catch this. When the task body raises, pushed_metrics_registry skips the push (posthog/metrics.py:76-79). So posthog_celery_record_cohort_metrics_success only ever reaches the gateway as 1.

The charts repo already has the pattern: CidrGroupSyncerGroupMetricsMissing in alerts/specs/infra.yaml reads freshness from the gateway's own push_time_seconds. A rule like this closes the gap:

- alert: CohortSLOMetricsStale
  expr: time() - push_time_seconds{job="celery_record_cohort_metrics"} > 900
  for: 5m
  labels:
    severity: warning
    rotation: feature-flags
  annotations:
    summary: Cohort SLO metrics are stale.
    description: "record_cohort_metrics pushes every 2m. No push for 15m means the cohort SLO alerts read stale data."

The 900s threshold allows 7 missed pushes, so a slow stats queue does not make it flap.

[scheduled-actions-posthog]

This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, please remove the stale label – otherwise this will be closed in another week. If you want to permanently keep it open, use the waiting label.

[haacked]

One blocking issue and a few suggestions inline.

[haacked]

blocking: The coordinator's fixed 300-second cache lease can expire while the task is still running. If a coordinator runs for more than 300 seconds, a later Beat task can acquire the expired lock key before the first coordinator exits.

The task has no hard time limit and does not renew CALCULATE_COHORT_LOCK_TTL_SECONDS, so it can outlive the lease. After the second coordinator acquires the key, the first coordinator's unconditional cache.delete() deletes that key because the lock value has no ownership token, allowing another coordinator to enter. Coordinators can then select the same cohort while it has is_calculating=False; the candidate refetch filters only by primary key and does not recheck that condition. The preparation path has no atomic guard, so each coordinator can increment pending_version, set is_calculating=True, and dispatch another calculation. Multiple coordinators can therefore overlap, increment versions, consume coordinator capacity, and dispatch the same cohort calculation twice.

Enforce a task runtime below the lease, or replace the fixed lease with an ownership-aware renewable Redis lock whose release deletes only the caller's token. Cover a lease expiring, a second coordinator acquiring the key, and the original coordinator finishing without deleting the second coordinator's lock.

[haacked]

suggestion: The two-minute metrics task issues seven serial global COUNT queries, which is 5,040 queries per day. The predicates have no matching index, and several scans differ only by filters over the same cohort table.

Please compute the gauges from one filtered aggregate() query. This reduces the scheduled workload to 720 queries per day while keeping the same metric values and labels.

[haacked]

suggestion: The new record_cohort_metrics registration is not asserted by test_scheduled_tasks; that test only verifies setup does not raise. Deleting the registration or changing its */2 cadence would still pass while the pushed gauges stop refreshing.

Please mock add_periodic_task_with_expiry and assert the exact registration call.

mock_add_periodic_task_with_expiry.assert_any_call(
    app,
    crontab(minute="*/2"),
    record_cohort_metrics.s(),
    name="cohort SLO metrics",
)

[haacked]

suggestion: The successful branch of test_calculate_cohort_releases_lock mocks reset_stuck_cohorts but never asserts it runs. Removing that coordinator step would leave these lock tests green while stuck cohorts remain ineligible for recalculation.

Please bind the existing mock and assert mock_reset.assert_called_once_with() on the success case.

-            patch("posthog.tasks.calculate_cohort.reset_stuck_cohorts"),
+            patch("posthog.tasks.calculate_cohort.reset_stuck_cohorts") as mock_reset,
...
+        if enqueue_side_effect is None:
+            mock_reset.assert_called_once_with()

[scheduled-actions-posthog]

This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, please remove the stale label – otherwise this will be closed in another week. If you want to permanently keep it open, use the waiting label.

[scheduled-actions-posthog]

This PR was closed due to lack of activity. Feel free to reopen if it's still relevant.