-
Notifications
You must be signed in to change notification settings - Fork 562
feat: Keep host on aggregated API usage buckets #8471
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| # Generated by Django 5.2.17 on 2026-09-05 02:32 | ||
|
|
||
| from django.db import migrations, models | ||
|
|
||
|
|
||
| class Migration(migrations.Migration): | ||
|
|
||
| dependencies = [ | ||
| ("app_analytics", "0008_labels_jsonb"), | ||
| ] | ||
|
|
||
| operations = [ | ||
| migrations.AddField( | ||
| model_name="apiusagebucket", | ||
| name="host", | ||
| field=models.CharField(default="", max_length=255), | ||
| ), | ||
| ] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,10 +26,12 @@ | |
| pytestmark = pytest.mark.use_analytics_db | ||
|
|
||
|
|
||
| def _create_api_usage_event(environment_id: int, when: datetime) -> APIUsageRaw: | ||
| def _create_api_usage_event( | ||
| environment_id: int, when: datetime, host: str = "host1" | ||
| ) -> APIUsageRaw: | ||
| event = APIUsageRaw.objects.create( | ||
| environment_id=environment_id, | ||
| host="host1", | ||
| host=host, | ||
| resource=Resource.FLAGS, | ||
| ) | ||
| # update created_at | ||
|
|
@@ -534,6 +536,28 @@ def test_populate_api_usage_bucket__source_bucket_size__aggregates_correctly( | |
| assert APIUsageBucket.objects.filter(bucket_size=15, total_count=300).count() == 1 | ||
|
|
||
|
|
||
| def test_populate_api_usage_bucket__multiple_hosts__preserves_host( | ||
| freezer: FrozenDateTimeFactory, | ||
| ) -> None: | ||
| # Given events from two hosts in the same bucket window | ||
| environment_id = 1 | ||
| when = timezone.now() - timedelta(minutes=90) | ||
| for _ in range(3): | ||
| _create_api_usage_event(environment_id, when, host="edge-proxy") | ||
| _create_api_usage_event(environment_id, when) | ||
|
|
||
| # When | ||
| freezer.move_to(timezone.now() - timedelta(hours=1)) | ||
| populate_api_usage_bucket(bucket_size=15, run_every=60) | ||
|
|
||
| # Then the buckets are split by host, each keeping its host | ||
| buckets = APIUsageBucket.objects.filter(environment_id=environment_id) | ||
| assert {(bucket.host, bucket.total_count) for bucket in buckets} == { | ||
| ("edge-proxy", 3), | ||
| ("host1", 1), | ||
| } | ||
|
Comment on lines
+539
to
+558
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win Add coverage for bucket-to-bucket host grouping. This test omits |
||
|
|
||
|
|
||
| def _create_feature_evaluation_event( | ||
| environment_id: int, | ||
| feature_name: str, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Prevent duplicate counts when legacy buckets are reprocessed.
When a time window containing a pre-migration bucket is reprocessed from raw data, the migrated row has
host="", while the raw aggregate has a real host such as"host1". Line 194 therefore uses a different lookup key and creates a second bucket. Line 101 does not treat the legacy empty-host row as overlapping. Because read paths still sum across hosts, the same usage can be counted twice.api/app_analytics/tasks.py#L194-L194: handle or avoid legacy empty-host rows before using the host-specific lookup.api/app_analytics/models.py#L101-L101: keep overlap detection consistent with the legacy-row transition.api/app_analytics/migrations/0009_apiusagebucket_host.py#L16-L16: add a backfill/rebuild strategy or prevent reprocessing of windows containing legacy rows;default=""alone is insufficient.📍 Affects 3 files
api/app_analytics/tasks.py#L194-L194(this comment)api/app_analytics/models.py#L101-L101api/app_analytics/migrations/0009_apiusagebucket_host.py#L16-L16