feat(customer-analytics): add sum aggregation to usage metrics#54346
Merged
arthurdedeus merged 8 commits intomasterfrom Apr 22, 2026
Merged
feat(customer-analytics): add sum aggregation to usage metrics#54346arthurdedeus merged 8 commits intomasterfrom
arthurdedeus merged 8 commits intomasterfrom
Conversation
Contributor
Author
Contributor
|
Size Change: 0 B Total Size: 130 MB ℹ️ View Unchanged
|
Contributor
Migration SQL ChangesHey 👋, we've detected some migrations on this PR. Here's the SQL output for each migration, make sure they make sense:
|
Contributor
🔍 Migration Risk AnalysisWe've analyzed your migrations for potential risks. Summary: 1 Safe | 0 Needs Review | 0 Blocked ✅ SafeBrief or no lock, backwards compatible 📚 How to Deploy These Changes SafelyAddField: This operation acquires a brief lock but doesn't rewrite the table. Deployment uses lock timeouts with automatic retries, so lock contention will cause retries rather than connection pile-up. Last updated: 2026-04-22 17:30 UTC (9eef96d) |
This was referenced Apr 13, 2026
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: 164-165
Comment:
**`math_property` can be `None` in the SUM branch**
The model field `math_property` is declared with `null=True, blank=True` but the query runner assumes it is always set when `math == SUM`. If a SUM metric reaches this code with `math_property=None` (e.g., created via Django admin, management command, or a direct DB write bypassing the serializer), `ast.Field(chain=["properties", None])` is constructed and the HogQL query will raise an error at execution time.
A guard that returns early (consistent with the `filter_expr == True` bail-out a few lines above) would prevent a silent 500:
```suggestion
if metric.math == GroupUsageMetric.Math.SUM:
if not metric.math_property:
return None
prop_as_float = ast.Call(name="toFloat", args=[ast.Field(chain=["properties", metric.math_property])])
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "chore: update OpenAPI generated types" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
|
🎭 Playwright report · View test results →
These issues are not necessarily caused by your changes. |
arthurdedeus
force-pushed
the
posthog-code/usage-metric-sum-aggregation-backend
branch
2 times, most recently
from
9cfe93e to
d8dfc0d
Compare
Contributor
Prompt To Fix All With AIThis is a comment left during a code review.
Path: products/customer_analytics/backend/hogql_queries/test/test_usage_metrics_query_runner.py
Line: 275-330
Comment:
**Non-parameterized tests repeat the same setup**
`test_sum_math_with_missing_property_returns_zero` and `test_sum_math_with_null_math_property_returns_zero` both create the same `GroupUsageMetric` (same name, format, interval, display, filters) but differ only in event properties or `math_property`. Per the team convention of preferring parameterized tests and the "OnceAndOnlyOnce" rule, these (and potentially `test_sum_math_aggregation`) could be collapsed using `@parameterized.expand`. For example:
```python
@parameterized.expand([
("basic_sum", "amount", {"amount": 100}, 100.0),
("missing_property", "amount", {}, 0.0),
])
@freeze_time("2025-10-09T12:11:00")
def test_sum_math_value(self, _name, math_property, event_props, expected_value):
GroupUsageMetric.objects.create(
id=self.test_metric_id, team=self.team, group_type_index=0,
name="Revenue", format=GroupUsageMetric.Format.CURRENCY, interval=7,
display=GroupUsageMetric.Display.NUMBER,
filters={"events": [{"id": "purchase", "type": "events", "order": 0}]},
math=GroupUsageMetric.Math.SUM, math_property=math_property,
)
_create_event(event="purchase", team=self.team, person_id=str(self.person.uuid),
distinct_id=self.person_distinct_id, properties=event_props)
flush_persons_and_events()
results = self._calculate(person_id=str(self.person.uuid))["results"]
self.assertEqual(results[0]["value"], expected_value)
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (2): Last reviewed commit: "Merge branch 'master' into posthog-code/..." | Re-trigger Greptile |
arthurdedeus
force-pushed
the
posthog-code/usage-metric-sum-aggregation-backend
branch
2 times, most recently
from
60e47f4 to
d8dfc0d
Compare
Contributor
|
⏭️ Skipped snapshot commit because branch advanced to The new commit will trigger its own snapshot update workflow. If you expected this workflow to succeed: This can happen due to concurrent commits. To get a fresh workflow run, either:
|
arthurdedeus
force-pushed
the
posthog-code/usage-metric-sum-aggregation-backend
branch
2 times, most recently
from
e0673f4 to
659b6d3
Compare
lricoy
approved these changes
Apr 22, 2026
Member
lricoy
left a comment
lricoy
left a comment
There was a problem hiding this comment.
🚀
nit: next time, could you post some screenshots of the feature? I know it is a bit of a hassle now, but it might help us with onboarding on the product features incrementally.
Generated-By: PostHog Code Task-Id: 887010a6-d3c0-4a87-b260-bdbc5cb48df1
Generated-By: PostHog Code Task-Id: 53331ac7-595f-446a-9633-ab0b8a18017d
Generated-By: PostHog Code Task-Id: 887010a6-d3c0-4a87-b260-bdbc5cb48df1
…egation Generated-By: PostHog Code Task-Id: 887010a6-d3c0-4a87-b260-bdbc5cb48df1
…rializer Generated-By: PostHog Code Task-Id: e5d85715-a2aa-4bbe-911b-a4c7ec098d4e
… merge Generated-By: PostHog Code Task-Id: e5d85715-a2aa-4bbe-911b-a4c7ec098d4e
arthurdedeus
force-pushed
the
posthog-code/usage-metric-sum-aggregation-backend
branch
from
659b6d3 to
9eef96d
Compare
arthurdedeus
deleted the
posthog-code/usage-metric-sum-aggregation-backend
branch
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
2 participants
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
Usage metrics in Customer analytics only support counting events. Users need the ability to aggregate by property values (e.g., sum of revenue or tokens) to track non-count-based metrics.
Changes
Added
mathfield toGroupUsageMetricmodel and update CRUD. Also enabled openapi schema generation for usage metrics endpointCalculation options in usage metrics form with properties to sum showing conditionally
How did you test this code?
New parameterized tests in
test_usage_metrics_query_runner.pycover both count and sum aggregation paths, including the generated SQL snapshots.Manual tests in the UI
Publish to changelog?
Yes
Docs update
Usage metrics needs its own
conceptsection in Customer analytics docs, covering everything the feature does, not only this PR additions.🤖 LLM context
Agent-authored PR. Backend half of a stacked PR — frontend changes follow in a dependent PR.
Created with PostHog Code