fix(customer-analytics): Optimize usage metrics query runner with interval grouping#54348
Merged
arthurdedeus merged 1 commit intomasterfrom Apr 22, 2026
Merged
Conversation
Contributor
Author
Contributor
|
Hey @arthurdedeus! 👋\nThis pull request seems to contain no description. Please add useful context, rationale, and/or any other information that will help make sense of this change now and in the distant Mars-based future. |
Contributor
Prompt To Fix All With AIThis is a comment left during a code review.
Path: products/customer_analytics/backend/hogql_queries/usage_metrics_query_runner.py
Line: 222-224
Comment:
**`datetime.now()` called independently from query builder**
`_process_group_results` recomputes `date_to`/`date_from`/`prev_date_from` via a fresh `datetime.now()`, but `_build_interval_group_query` already called `datetime.now()` at line 121 to embed timestamps in the SQL. If any time passes between these two calls (including query execution time), the date window used to interpret results won't match the one encoded in the query. At a day boundary this means the wrong days are included in `current_dates`/`previous_dates`, silently producing incorrect totals.
The fix is to compute the reference time once in `_calculate` and pass it to both methods:
```python
# In _calculate, before the for-loop:
date_to = datetime.now(tz=ZoneInfo("UTC"))
# Pass it down:
query = self._build_interval_group_query(interval, group, date_to=date_to)
results = self._process_group_results(response, interval, group, date_to=date_to)
```
Then update both method signatures to accept `date_to: datetime` and remove the internal `datetime.now()` calls.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix(customer-analytics): optimize usage ..." | Re-trigger Greptile |
9b7a48c to
3e24102
Compare
853c9b3 to
01e2feb
Compare
9b7a48c to
bbd12a4
Compare
Contributor
Query snapshots: Backend query snapshots updatedChanges: 1 snapshots (1 modified, 0 added, 0 deleted) What this means:
Next steps:
|
Contributor
Prompt To Fix All With AIThis is a comment left during a code review.
Path: products/customer_analytics/backend/hogql_queries/usage_metrics_query_runner.py
Line: 90-102
Comment:
**Repeated DB queries from dropping `@cached_property`**
`_get_usage_metrics()` was previously a `@cached_property` that executed the ORM query once per runner instance. It is now a plain method called independently from `get_cache_payload()`, `to_query()`, and `_calculate()` — each invocation issues a separate `SELECT` against the database. For callers that touch both `get_cache_payload()` (to check/populate the cache) and `_calculate()` (to run the query), the metrics table is read at least twice per request cycle.
Alternatively, keep the method but cache its result as `self._usage_metrics` on first call, or convert the three callers to share a single result computed once in `_calculate`/`to_query`.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: products/customer_analytics/backend/hogql_queries/usage_metrics_query_runner.py
Line: 236
Comment:
**Off-by-one in `previous_dates` upper bound**
The upper bound for the previous period is derived as `(date_from - timedelta(seconds=1)).date()`. The expression works, but it is expressing `date_from.date() - timedelta(days=1)` in a roundabout way that depends on `date_from` never being exactly midnight. The cleaner and more direct form is `(date_from - timedelta(days=1)).date()`, which is explicitly "one day before `date_from`'s date" and is always equivalent for daily bucketing.
```suggestion
previous_dates = self._date_range(prev_date_from.date(), (date_from - timedelta(days=1)).date())
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (2): Last reviewed commit: "test(backend): update query snapshots" | Re-trigger Greptile |
lricoy
approved these changes
Apr 22, 2026
f18bbea to
3cc2265
Compare
…erval grouping Generated-By: PostHog Code Task-Id: 59552aa7-0856-47b1-86c6-034bb0a4ac82
3cc2265 to
76f742d
Compare
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
The usage metrics query runner generated one
SELECTper metric, UNION ALL'd together, each independently scanning the events table. With multiple metrics configured, this meant redundant scans of the same data — all metrics for a given entity (person/group) share the same base filter.Changes
countIf/sumIfconditional aggregationtoStartOfDay+ gap-filling in Python, replacing per-metricparse_selecttemplate queriesHow did you test this code?
Existing test suite passes with updated snapshots confirming the new query structure.
hogli test products/customer_analytics/backend/hogql_queries/test/test_usage_metrics_query_runner.py— 25 tests pass.Publish to changelog?
No
Docs update
🤖 LLM context
Authored by PostHog Code (Claude Code).
Created with PostHog Code