From ecf984af48695de6eba36d645f1be3df6c893e6f Mon Sep 17 00:00:00 2001 From: Bianca Yang Date: Fri, 26 Jan 2024 15:31:02 -0800 Subject: [PATCH 01/21] use feature flag to control whether to quota limit or not --- ee/billing/quota_limiting.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/ee/billing/quota_limiting.py b/ee/billing/quota_limiting.py index 0809266b1db64..feb1dfe0c966c 100644 --- a/ee/billing/quota_limiting.py +++ b/ee/billing/quota_limiting.py @@ -4,6 +4,7 @@ from typing import Dict, List, Mapping, Optional, Sequence, TypedDict, cast import dateutil.parser +import posthoganalytics from django.db.models import Q from django.utils import timezone from sentry_sdk import capture_exception @@ -23,6 +24,8 @@ QUOTA_LIMITER_CACHE_KEY = "@posthog/quota-limits/" +QUOTA_LIMIT_DATA_RETENTION_FLAG = "retain-data-past-quota-limit" + class QuotaResource(Enum): EVENTS = "events" @@ -89,6 +92,14 @@ def org_quota_limited_until(organization: Organization, resource: QuotaResource) if is_quota_limited and organization.never_drop_data: return None + if posthoganalytics.feature_enabled(QUOTA_LIMIT_DATA_RETENTION_FLAG): + # Don't drop data for this user but record that they __would have__ been + # limited. + report_organization_action( + organization.id, "quota_limiting_suspended", properties={"current_usage": todays_usage} + ) + return None + if is_quota_limited and billing_period_end: return billing_period_end From 17b1144e292e2e767def37753269e8d00c995cbd Mon Sep 17 00:00:00 2001 From: Bianca Yang Date: Fri, 26 Jan 2024 15:55:21 -0800 Subject: [PATCH 02/21] add simple test --- ee/billing/quota_limiting.py | 6 ++--- ee/billing/test/test_quota_limiting.py | 37 ++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/ee/billing/quota_limiting.py b/ee/billing/quota_limiting.py index feb1dfe0c966c..16dd23bfd0a74 100644 --- a/ee/billing/quota_limiting.py +++ b/ee/billing/quota_limiting.py @@ -93,11 +93,9 @@ def org_quota_limited_until(organization: Organization, resource: QuotaResource) return None if posthoganalytics.feature_enabled(QUOTA_LIMIT_DATA_RETENTION_FLAG): - # Don't drop data for this user but record that they __would have__ been + # Don't drop data for this org but record that they __would have__ been # limited. - report_organization_action( - organization.id, "quota_limiting_suspended", properties={"current_usage": todays_usage} - ) + report_organization_action(organization, "quota_limiting_suspended", properties={"current_usage": todays_usage}) return None if is_quota_limited and billing_period_end: diff --git a/ee/billing/test/test_quota_limiting.py b/ee/billing/test/test_quota_limiting.py index b8e68c235b2c5..60b5c50328b08 100644 --- a/ee/billing/test/test_quota_limiting.py +++ b/ee/billing/test/test_quota_limiting.py @@ -1,4 +1,5 @@ import time +from unittest.mock import patch from uuid import uuid4 from dateutil.relativedelta import relativedelta @@ -7,6 +8,7 @@ from freezegun import freeze_time from ee.billing.quota_limiting import ( + QUOTA_LIMIT_DATA_RETENTION_FLAG, QUOTA_LIMITER_CACHE_KEY, QuotaResource, list_limited_team_attributes, @@ -26,6 +28,41 @@ def setUp(self) -> None: super().setUp() self.redis_client = get_client() + @patch("posthoganalytics.feature_enabled", return_value=True) + @patch("posthoganalytics.capture") + def test_dont_quota_limit_feature_flag_enabled(self, patch_feature_enabled, patch_capture) -> None: + with self.settings(USE_TZ=False): + self.organization.usage = { + "events": {"usage": 99, "limit": 100}, + "recordings": {"usage": 1, "limit": 100}, + "rows_synced": {"usage": 5, "limit": 100}, + "period": ["2021-01-01T00:00:00Z", "2021-01-31T23:59:59Z"], + } + self.organization.save() + + distinct_id = str(uuid4()) + + # add a bunch of events so that the organization is over the limit + # Because the feature flag is enabled + for _ in range(0, 10): + _create_event( + distinct_id=distinct_id, + event="$event1", + properties={"$lib": "$web"}, + timestamp=now() - relativedelta(hours=1), + team=self.team, + ) + time.sleep(1) + result = update_all_org_billing_quotas() + assert patch_feature_enabled.called_once_with(QUOTA_LIMIT_DATA_RETENTION_FLAG) + assert result["events"] == {} + assert result["recordings"] == {} + assert result["rows_synced"] == {} + + assert self.redis_client.zrange(f"{QUOTA_LIMITER_CACHE_KEY}events", 0, -1) == [] + assert self.redis_client.zrange(f"{QUOTA_LIMITER_CACHE_KEY}recordings", 0, -1) == [] + assert self.redis_client.zrange(f"{QUOTA_LIMITER_CACHE_KEY}rows_synced", 0, -1) == [] + def test_billing_rate_limit_not_set_if_missing_org_usage(self) -> None: with self.settings(USE_TZ=False): self.organization.usage = {} From 57eff15c3ccb92f2a7595d75ed86876baf4a2268 Mon Sep 17 00:00:00 2001 From: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Date: Sat, 27 Jan 2024 00:01:03 +0000 Subject: [PATCH 03/21] Update query snapshots --- .../test_clickhouse_experiment_secondary_results.ambr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/clickhouse/views/test/__snapshots__/test_clickhouse_experiment_secondary_results.ambr b/ee/clickhouse/views/test/__snapshots__/test_clickhouse_experiment_secondary_results.ambr index 613bdc0de3a2a..4785fd9b16690 100644 --- a/ee/clickhouse/views/test/__snapshots__/test_clickhouse_experiment_secondary_results.ambr +++ b/ee/clickhouse/views/test/__snapshots__/test_clickhouse_experiment_secondary_results.ambr @@ -1,7 +1,7 @@ # serializer version: 1 # name: ClickhouseTestExperimentSecondaryResults.test_basic_secondary_metric_results ''' - /* user_id:132 celery:posthog.tasks.tasks.sync_insight_caching_state */ + /* user_id:133 celery:posthog.tasks.tasks.sync_insight_caching_state */ SELECT team_id, date_diff('second', max(timestamp), now()) AS age FROM events From 84e9f07e3f884e146cc964d4bcd88a85b6451794 Mon Sep 17 00:00:00 2001 From: Bianca Yang Date: Mon, 29 Jan 2024 11:13:41 -0800 Subject: [PATCH 04/21] fix up the feature flag activation logic --- ee/billing/quota_limiting.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/billing/quota_limiting.py b/ee/billing/quota_limiting.py index 16dd23bfd0a74..88fa0f80f8152 100644 --- a/ee/billing/quota_limiting.py +++ b/ee/billing/quota_limiting.py @@ -92,7 +92,7 @@ def org_quota_limited_until(organization: Organization, resource: QuotaResource) if is_quota_limited and organization.never_drop_data: return None - if posthoganalytics.feature_enabled(QUOTA_LIMIT_DATA_RETENTION_FLAG): + if is_quota_limited and posthoganalytics.feature_enabled(QUOTA_LIMIT_DATA_RETENTION_FLAG): # Don't drop data for this org but record that they __would have__ been # limited. report_organization_action(organization, "quota_limiting_suspended", properties={"current_usage": todays_usage}) From d9f0f870a5d12b2992e19708fb2d3ff0f18f81e8 Mon Sep 17 00:00:00 2001 From: Bianca Yang Date: Mon, 29 Jan 2024 11:26:19 -0800 Subject: [PATCH 05/21] confirm that the fixed feature flag logic works --- ee/billing/test/test_quota_limiting.py | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/ee/billing/test/test_quota_limiting.py b/ee/billing/test/test_quota_limiting.py index 60b5c50328b08..3913ee1b3ad12 100644 --- a/ee/billing/test/test_quota_limiting.py +++ b/ee/billing/test/test_quota_limiting.py @@ -63,6 +63,31 @@ def test_dont_quota_limit_feature_flag_enabled(self, patch_feature_enabled, patc assert self.redis_client.zrange(f"{QUOTA_LIMITER_CACHE_KEY}recordings", 0, -1) == [] assert self.redis_client.zrange(f"{QUOTA_LIMITER_CACHE_KEY}rows_synced", 0, -1) == [] + @patch("posthoganalytics.feature_enabled", return_value=True) + @patch("posthoganalytics.capture") + def test_quota_limit_feature_flag_not_on(self, patch_feature_enabled, patch_capture) -> None: + # Confirm that we don't send an event if they weren't going to be limited. + with self.settings(USE_TZ=False): + self.organization.usage = { + "events": {"usage": 99, "limit": 100}, + "recordings": {"usage": 1, "limit": 100}, + "rows_synced": {"usage": 5, "limit": 100}, + "period": ["2021-01-01T00:00:00Z", "2021-01-31T23:59:59Z"], + } + self.organization.save() + + time.sleep(1) + result = update_all_org_billing_quotas() + assert patch_feature_enabled.called_once_with(QUOTA_LIMIT_DATA_RETENTION_FLAG) + assert not patch_capture.called + assert result["events"] == {} + assert result["recordings"] == {} + assert result["rows_synced"] == {} + + assert self.redis_client.zrange(f"{QUOTA_LIMITER_CACHE_KEY}events", 0, -1) == [] + assert self.redis_client.zrange(f"{QUOTA_LIMITER_CACHE_KEY}recordings", 0, -1) == [] + assert self.redis_client.zrange(f"{QUOTA_LIMITER_CACHE_KEY}rows_synced", 0, -1) == [] + def test_billing_rate_limit_not_set_if_missing_org_usage(self) -> None: with self.settings(USE_TZ=False): self.organization.usage = {} From 20f1ef67c50d3409930bbfec044182175e23d86e Mon Sep 17 00:00:00 2001 From: Bianca Yang Date: Mon, 29 Jan 2024 11:27:50 -0800 Subject: [PATCH 06/21] remove unnecessary change --- .../test_clickhouse_experiment_secondary_results.ambr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/clickhouse/views/test/__snapshots__/test_clickhouse_experiment_secondary_results.ambr b/ee/clickhouse/views/test/__snapshots__/test_clickhouse_experiment_secondary_results.ambr index 4785fd9b16690..613bdc0de3a2a 100644 --- a/ee/clickhouse/views/test/__snapshots__/test_clickhouse_experiment_secondary_results.ambr +++ b/ee/clickhouse/views/test/__snapshots__/test_clickhouse_experiment_secondary_results.ambr @@ -1,7 +1,7 @@ # serializer version: 1 # name: ClickhouseTestExperimentSecondaryResults.test_basic_secondary_metric_results ''' - /* user_id:133 celery:posthog.tasks.tasks.sync_insight_caching_state */ + /* user_id:132 celery:posthog.tasks.tasks.sync_insight_caching_state */ SELECT team_id, date_diff('second', max(timestamp), now()) AS age FROM events From 39a7147a9e0ec9b216eee5aee77a57cf680da26f Mon Sep 17 00:00:00 2001 From: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 29 Jan 2024 19:32:26 +0000 Subject: [PATCH 07/21] Update query snapshots --- ee/clickhouse/models/test/__snapshots__/test_property.ambr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/clickhouse/models/test/__snapshots__/test_property.ambr b/ee/clickhouse/models/test/__snapshots__/test_property.ambr index 084d684e685af..38956c679251e 100644 --- a/ee/clickhouse/models/test/__snapshots__/test_property.ambr +++ b/ee/clickhouse/models/test/__snapshots__/test_property.ambr @@ -147,7 +147,7 @@ )) ''', dict({ - 'global_cohort_id_0': 47, + 'global_cohort_id_0': 1, 'global_version_0': None, }), ) From 76a207731c9abe4c7d3802e3e7df5c0865515597 Mon Sep 17 00:00:00 2001 From: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 29 Jan 2024 19:47:31 +0000 Subject: [PATCH 08/21] Update query snapshots --- .../test_clickhouse_experiment_secondary_results.ambr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/clickhouse/views/test/__snapshots__/test_clickhouse_experiment_secondary_results.ambr b/ee/clickhouse/views/test/__snapshots__/test_clickhouse_experiment_secondary_results.ambr index 613bdc0de3a2a..3559e5f05ae24 100644 --- a/ee/clickhouse/views/test/__snapshots__/test_clickhouse_experiment_secondary_results.ambr +++ b/ee/clickhouse/views/test/__snapshots__/test_clickhouse_experiment_secondary_results.ambr @@ -1,7 +1,7 @@ # serializer version: 1 # name: ClickhouseTestExperimentSecondaryResults.test_basic_secondary_metric_results ''' - /* user_id:132 celery:posthog.tasks.tasks.sync_insight_caching_state */ + /* user_id:134 celery:posthog.tasks.tasks.sync_insight_caching_state */ SELECT team_id, date_diff('second', max(timestamp), now()) AS age FROM events From 6dc8ddc4c0a479e0b12118b075075090a61bffc4 Mon Sep 17 00:00:00 2001 From: Bianca Yang Date: Mon, 29 Jan 2024 14:06:35 -0800 Subject: [PATCH 09/21] fix up tests --- ee/billing/test/test_quota_limiting.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/ee/billing/test/test_quota_limiting.py b/ee/billing/test/test_quota_limiting.py index 3913ee1b3ad12..91cc20906e4fd 100644 --- a/ee/billing/test/test_quota_limiting.py +++ b/ee/billing/test/test_quota_limiting.py @@ -28,8 +28,8 @@ def setUp(self) -> None: super().setUp() self.redis_client = get_client() - @patch("posthoganalytics.feature_enabled", return_value=True) @patch("posthoganalytics.capture") + @patch("posthoganalytics.feature_enabled", return_value=True) def test_dont_quota_limit_feature_flag_enabled(self, patch_feature_enabled, patch_capture) -> None: with self.settings(USE_TZ=False): self.organization.usage = { @@ -54,7 +54,8 @@ def test_dont_quota_limit_feature_flag_enabled(self, patch_feature_enabled, patc ) time.sleep(1) result = update_all_org_billing_quotas() - assert patch_feature_enabled.called_once_with(QUOTA_LIMIT_DATA_RETENTION_FLAG) + patch_feature_enabled.assert_called_with(QUOTA_LIMIT_DATA_RETENTION_FLAG) + patch_capture.called_once_with(self.organization.id, "quota_limiting_suspended", {"current_usage": 109}) assert result["events"] == {} assert result["recordings"] == {} assert result["rows_synced"] == {} @@ -63,8 +64,8 @@ def test_dont_quota_limit_feature_flag_enabled(self, patch_feature_enabled, patc assert self.redis_client.zrange(f"{QUOTA_LIMITER_CACHE_KEY}recordings", 0, -1) == [] assert self.redis_client.zrange(f"{QUOTA_LIMITER_CACHE_KEY}rows_synced", 0, -1) == [] - @patch("posthoganalytics.feature_enabled", return_value=True) @patch("posthoganalytics.capture") + @patch("posthoganalytics.feature_enabled", return_value=True) def test_quota_limit_feature_flag_not_on(self, patch_feature_enabled, patch_capture) -> None: # Confirm that we don't send an event if they weren't going to be limited. with self.settings(USE_TZ=False): @@ -78,8 +79,9 @@ def test_quota_limit_feature_flag_not_on(self, patch_feature_enabled, patch_capt time.sleep(1) result = update_all_org_billing_quotas() - assert patch_feature_enabled.called_once_with(QUOTA_LIMIT_DATA_RETENTION_FLAG) - assert not patch_capture.called + # Shouldn't be called due to lazy evaluation of the conditional + patch_feature_enabled.assert_not_called() + patch_capture.assert_not_called() assert result["events"] == {} assert result["recordings"] == {} assert result["rows_synced"] == {} From 95b737a107058b8c216d1fb7b83554eda01547f4 Mon Sep 17 00:00:00 2001 From: Bianca Yang Date: Mon, 29 Jan 2024 14:36:00 -0800 Subject: [PATCH 10/21] another fix for tests --- ee/billing/quota_limiting.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/billing/quota_limiting.py b/ee/billing/quota_limiting.py index 88fa0f80f8152..654eaec6f7fa7 100644 --- a/ee/billing/quota_limiting.py +++ b/ee/billing/quota_limiting.py @@ -92,7 +92,7 @@ def org_quota_limited_until(organization: Organization, resource: QuotaResource) if is_quota_limited and organization.never_drop_data: return None - if is_quota_limited and posthoganalytics.feature_enabled(QUOTA_LIMIT_DATA_RETENTION_FLAG): + if is_quota_limited and posthoganalytics.feature_enabled(QUOTA_LIMIT_DATA_RETENTION_FLAG, organization.id): # Don't drop data for this org but record that they __would have__ been # limited. report_organization_action(organization, "quota_limiting_suspended", properties={"current_usage": todays_usage}) From c2608c52788bce21df82e426674621362852804c Mon Sep 17 00:00:00 2001 From: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 29 Jan 2024 22:50:08 +0000 Subject: [PATCH 11/21] Update query snapshots --- .../test/__snapshots__/test_session_recordings.ambr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/posthog/session_recordings/test/__snapshots__/test_session_recordings.ambr b/posthog/session_recordings/test/__snapshots__/test_session_recordings.ambr index affde7090e435..b035f09e38c79 100644 --- a/posthog/session_recordings/test/__snapshots__/test_session_recordings.ambr +++ b/posthog/session_recordings/test/__snapshots__/test_session_recordings.ambr @@ -643,7 +643,7 @@ FROM "posthog_persondistinctid" INNER JOIN "posthog_person" ON ("posthog_persondistinctid"."person_id" = "posthog_person"."id") WHERE ("posthog_persondistinctid"."distinct_id" IN ('user2', - 'user_one_2') + 'user_one_0') AND "posthog_persondistinctid"."team_id" = 2) /*controller='project_session_recordings-list',route='api/projects/%28%3FP%3Cparent_lookup_team_id%3E%5B%5E/.%5D%2B%29/session_recordings/%3F%24'*/ ''' # --- From 7bd59625f65aa1aa615d01d80d30de0a5327d4c4 Mon Sep 17 00:00:00 2001 From: Bianca Yang Date: Mon, 29 Jan 2024 14:57:03 -0800 Subject: [PATCH 12/21] still fixing tests --- ee/billing/test/test_quota_limiting.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ee/billing/test/test_quota_limiting.py b/ee/billing/test/test_quota_limiting.py index 91cc20906e4fd..aea6c80839f7c 100644 --- a/ee/billing/test/test_quota_limiting.py +++ b/ee/billing/test/test_quota_limiting.py @@ -54,7 +54,7 @@ def test_dont_quota_limit_feature_flag_enabled(self, patch_feature_enabled, patc ) time.sleep(1) result = update_all_org_billing_quotas() - patch_feature_enabled.assert_called_with(QUOTA_LIMIT_DATA_RETENTION_FLAG) + patch_feature_enabled.assert_called_with(QUOTA_LIMIT_DATA_RETENTION_FLAG, self.organization.id) patch_capture.called_once_with(self.organization.id, "quota_limiting_suspended", {"current_usage": 109}) assert result["events"] == {} assert result["recordings"] == {} @@ -70,7 +70,7 @@ def test_quota_limit_feature_flag_not_on(self, patch_feature_enabled, patch_capt # Confirm that we don't send an event if they weren't going to be limited. with self.settings(USE_TZ=False): self.organization.usage = { - "events": {"usage": 99, "limit": 100}, + "events": {"usage": 60, "limit": 100}, "recordings": {"usage": 1, "limit": 100}, "rows_synced": {"usage": 5, "limit": 100}, "period": ["2021-01-01T00:00:00Z", "2021-01-31T23:59:59Z"], From 27a9e7d00e188a6252a3985ca3df3d870b268798 Mon Sep 17 00:00:00 2001 From: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 29 Jan 2024 23:04:01 +0000 Subject: [PATCH 13/21] Update query snapshots --- .../test/__snapshots__/test_session_recordings.ambr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/posthog/session_recordings/test/__snapshots__/test_session_recordings.ambr b/posthog/session_recordings/test/__snapshots__/test_session_recordings.ambr index b035f09e38c79..affde7090e435 100644 --- a/posthog/session_recordings/test/__snapshots__/test_session_recordings.ambr +++ b/posthog/session_recordings/test/__snapshots__/test_session_recordings.ambr @@ -643,7 +643,7 @@ FROM "posthog_persondistinctid" INNER JOIN "posthog_person" ON ("posthog_persondistinctid"."person_id" = "posthog_person"."id") WHERE ("posthog_persondistinctid"."distinct_id" IN ('user2', - 'user_one_0') + 'user_one_2') AND "posthog_persondistinctid"."team_id" = 2) /*controller='project_session_recordings-list',route='api/projects/%28%3FP%3Cparent_lookup_team_id%3E%5B%5E/.%5D%2B%29/session_recordings/%3F%24'*/ ''' # --- From 04a83ad2b4610537e6d6adff5f048d4dfe8a7f94 Mon Sep 17 00:00:00 2001 From: Bianca Yang Date: Tue, 30 Jan 2024 01:20:29 -0800 Subject: [PATCH 14/21] pr feedback, more tests --- ee/billing/quota_limiting.py | 4 +++- ee/billing/test/test_quota_limiting.py | 24 +++++++++++++++--------- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/ee/billing/quota_limiting.py b/ee/billing/quota_limiting.py index 654eaec6f7fa7..4e67c7f819dc5 100644 --- a/ee/billing/quota_limiting.py +++ b/ee/billing/quota_limiting.py @@ -95,7 +95,9 @@ def org_quota_limited_until(organization: Organization, resource: QuotaResource) if is_quota_limited and posthoganalytics.feature_enabled(QUOTA_LIMIT_DATA_RETENTION_FLAG, organization.id): # Don't drop data for this org but record that they __would have__ been # limited. - report_organization_action(organization, "quota_limiting_suspended", properties={"current_usage": todays_usage}) + report_organization_action( + organization, "quota limiting suspended", properties={"current_usage": usage + todays_usage} + ) return None if is_quota_limited and billing_period_end: diff --git a/ee/billing/test/test_quota_limiting.py b/ee/billing/test/test_quota_limiting.py index aea6c80839f7c..0287c23ce865a 100644 --- a/ee/billing/test/test_quota_limiting.py +++ b/ee/billing/test/test_quota_limiting.py @@ -24,6 +24,8 @@ class TestQuotaLimiting(BaseTest): + CLASS_DATA_LEVEL_SETUP = False + def setUp(self) -> None: super().setUp() self.redis_client = get_client() @@ -55,7 +57,12 @@ def test_dont_quota_limit_feature_flag_enabled(self, patch_feature_enabled, patc time.sleep(1) result = update_all_org_billing_quotas() patch_feature_enabled.assert_called_with(QUOTA_LIMIT_DATA_RETENTION_FLAG, self.organization.id) - patch_capture.called_once_with(self.organization.id, "quota_limiting_suspended", {"current_usage": 109}) + patch_capture.assert_called_once_with( + str(self.organization.id), + "quota limiting suspended", + properties={"current_usage": 109}, + groups={"instance": "http://localhost:8000", "organization": str(self.organization.id)}, + ) assert result["events"] == {} assert result["recordings"] == {} assert result["rows_synced"] == {} @@ -68,14 +75,13 @@ def test_dont_quota_limit_feature_flag_enabled(self, patch_feature_enabled, patc @patch("posthoganalytics.feature_enabled", return_value=True) def test_quota_limit_feature_flag_not_on(self, patch_feature_enabled, patch_capture) -> None: # Confirm that we don't send an event if they weren't going to be limited. - with self.settings(USE_TZ=False): - self.organization.usage = { - "events": {"usage": 60, "limit": 100}, - "recordings": {"usage": 1, "limit": 100}, - "rows_synced": {"usage": 5, "limit": 100}, - "period": ["2021-01-01T00:00:00Z", "2021-01-31T23:59:59Z"], - } - self.organization.save() + self.organization.usage = { + "events": {"usage": 99, "limit": 100}, + "recordings": {"usage": 1, "limit": 100}, + "rows_synced": {"usage": 5, "limit": 100}, + "period": ["2021-01-01T00:00:00Z", "2021-01-31T23:59:59Z"], + } + self.organization.save() time.sleep(1) result = update_all_org_billing_quotas() From ff31a7161ecf2f27d73392c4718815bb8ad77446 Mon Sep 17 00:00:00 2001 From: Bianca Yang Date: Tue, 30 Jan 2024 11:00:23 -0800 Subject: [PATCH 15/21] tune up ff enabled call, add capture check to test for quota limit changed --- ee/billing/quota_limiting.py | 9 ++++++++- ee/billing/test/test_quota_limiting.py | 14 +++++++++++++- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/ee/billing/quota_limiting.py b/ee/billing/quota_limiting.py index 4e67c7f819dc5..0476ce5d8bb2a 100644 --- a/ee/billing/quota_limiting.py +++ b/ee/billing/quota_limiting.py @@ -92,7 +92,14 @@ def org_quota_limited_until(organization: Organization, resource: QuotaResource) if is_quota_limited and organization.never_drop_data: return None - if is_quota_limited and posthoganalytics.feature_enabled(QUOTA_LIMIT_DATA_RETENTION_FLAG, organization.id): + if is_quota_limited and posthoganalytics.feature_enabled( + QUOTA_LIMIT_DATA_RETENTION_FLAG, + organization.id, + groups={"organization": str(organization.id)}, + group_properties={ + "organization": str(organization.id), + }, + ): # Don't drop data for this org but record that they __would have__ been # limited. report_organization_action( diff --git a/ee/billing/test/test_quota_limiting.py b/ee/billing/test/test_quota_limiting.py index 0287c23ce865a..75f626da75c41 100644 --- a/ee/billing/test/test_quota_limiting.py +++ b/ee/billing/test/test_quota_limiting.py @@ -123,7 +123,8 @@ def test_billing_rate_limit_not_set_if_missing_org_usage(self) -> None: assert self.redis_client.zrange(f"{QUOTA_LIMITER_CACHE_KEY}recordings", 0, -1) == [] assert self.redis_client.zrange(f"{QUOTA_LIMITER_CACHE_KEY}rows_synced", 0, -1) == [] - def test_billing_rate_limit(self) -> None: + @patch("posthoganalytics.capture") + def test_billing_rate_limit(self, patch_capture) -> None: with self.settings(USE_TZ=False): self.organization.usage = { "events": {"usage": 99, "limit": 100}, @@ -152,6 +153,17 @@ def test_billing_rate_limit(self) -> None: assert result["recordings"] == {} assert result["rows_synced"] == {} + patch_capture.assert_called_once_with( + org_id, + "organization quota limits changed", + properties={ + "quota_limited_events": 1612137599, + "quota_limited_recordings": 1612137599, + "quota_limited_rows_synced": None, + }, + groups={"instance": "http://localhost:8000", "organization": org_id}, + ) + assert self.redis_client.zrange(f"{QUOTA_LIMITER_CACHE_KEY}events", 0, -1) == [ self.team.api_token.encode("UTF-8") ] From 8a6f5d17874190e8d3044d701e51264ace943f5a Mon Sep 17 00:00:00 2001 From: Bianca Yang Date: Tue, 30 Jan 2024 12:30:10 -0800 Subject: [PATCH 16/21] i really need to run tests locally --- ee/billing/test/test_quota_limiting.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/ee/billing/test/test_quota_limiting.py b/ee/billing/test/test_quota_limiting.py index 75f626da75c41..bffed0f9f3578 100644 --- a/ee/billing/test/test_quota_limiting.py +++ b/ee/billing/test/test_quota_limiting.py @@ -56,7 +56,12 @@ def test_dont_quota_limit_feature_flag_enabled(self, patch_feature_enabled, patc ) time.sleep(1) result = update_all_org_billing_quotas() - patch_feature_enabled.assert_called_with(QUOTA_LIMIT_DATA_RETENTION_FLAG, self.organization.id) + patch_feature_enabled.assert_called_with( + QUOTA_LIMIT_DATA_RETENTION_FLAG, + self.organization.id, + groups={"organization": str(self.organization.id)}, + group_properties={"organization": str(self.organization.id)}, + ) patch_capture.assert_called_once_with( str(self.organization.id), "quota limiting suspended", From b6cafac13654a6debfce148cb82211195c7a8111 Mon Sep 17 00:00:00 2001 From: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 31 Jan 2024 20:42:19 +0000 Subject: [PATCH 17/21] Update query snapshots --- ee/clickhouse/models/test/__snapshots__/test_property.ambr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/clickhouse/models/test/__snapshots__/test_property.ambr b/ee/clickhouse/models/test/__snapshots__/test_property.ambr index 9395f19a9652a..d804c6ca9363e 100644 --- a/ee/clickhouse/models/test/__snapshots__/test_property.ambr +++ b/ee/clickhouse/models/test/__snapshots__/test_property.ambr @@ -148,7 +148,7 @@ )) ''', dict({ - 'global_cohort_id_0': 1, + 'global_cohort_id_0': 47, 'global_version_0': None, }), ) From af3203d5b15586fc4137ecfa44712ab577bd0738 Mon Sep 17 00:00:00 2001 From: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 31 Jan 2024 20:44:05 +0000 Subject: [PATCH 18/21] Update query snapshots --- .../test_clickhouse_experiment_secondary_results.ambr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/clickhouse/views/test/__snapshots__/test_clickhouse_experiment_secondary_results.ambr b/ee/clickhouse/views/test/__snapshots__/test_clickhouse_experiment_secondary_results.ambr index e5c434f501653..9012dcd46ecd9 100644 --- a/ee/clickhouse/views/test/__snapshots__/test_clickhouse_experiment_secondary_results.ambr +++ b/ee/clickhouse/views/test/__snapshots__/test_clickhouse_experiment_secondary_results.ambr @@ -1,7 +1,7 @@ # serializer version: 1 # name: ClickhouseTestExperimentSecondaryResults.test_basic_secondary_metric_results ''' - /* user_id:129 celery:posthog.tasks.tasks.sync_insight_caching_state */ + /* user_id:131 celery:posthog.tasks.tasks.sync_insight_caching_state */ SELECT team_id, date_diff('second', max(timestamp), now()) AS age FROM events From 4a5cd231069d02a427ffd36e98c3364f2762d961 Mon Sep 17 00:00:00 2001 From: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 31 Jan 2024 20:44:18 +0000 Subject: [PATCH 19/21] Update query snapshots --- ee/clickhouse/models/test/__snapshots__/test_property.ambr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/clickhouse/models/test/__snapshots__/test_property.ambr b/ee/clickhouse/models/test/__snapshots__/test_property.ambr index d804c6ca9363e..0f4e20f1888ab 100644 --- a/ee/clickhouse/models/test/__snapshots__/test_property.ambr +++ b/ee/clickhouse/models/test/__snapshots__/test_property.ambr @@ -58,7 +58,7 @@ # --- # name: test_parse_groups_persons_edge_case_with_single_filter tuple( - 'AND ( has(%(vglobalperson_0)s, "pmat_email"))', + 'AND ( has(%(vglobalperson_0)s, replaceRegexpAll(JSONExtractRaw(person_props, %(kglobalperson_0)s), \'^"|"$\', \'\')))', dict({ 'kglobalperson_0': 'email', 'vglobalperson_0': list([ From 5443db1c3aad125e7613eb234f5a5241097b9186 Mon Sep 17 00:00:00 2001 From: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 31 Jan 2024 21:07:30 +0000 Subject: [PATCH 20/21] Update query snapshots --- .../test/__snapshots__/test_session_recordings.ambr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/posthog/session_recordings/test/__snapshots__/test_session_recordings.ambr b/posthog/session_recordings/test/__snapshots__/test_session_recordings.ambr index affde7090e435..6ce9f29c0945d 100644 --- a/posthog/session_recordings/test/__snapshots__/test_session_recordings.ambr +++ b/posthog/session_recordings/test/__snapshots__/test_session_recordings.ambr @@ -643,7 +643,7 @@ FROM "posthog_persondistinctid" INNER JOIN "posthog_person" ON ("posthog_persondistinctid"."person_id" = "posthog_person"."id") WHERE ("posthog_persondistinctid"."distinct_id" IN ('user2', - 'user_one_2') + 'user_one_1') AND "posthog_persondistinctid"."team_id" = 2) /*controller='project_session_recordings-list',route='api/projects/%28%3FP%3Cparent_lookup_team_id%3E%5B%5E/.%5D%2B%29/session_recordings/%3F%24'*/ ''' # --- From 0c2f7b0a7f7730a9e8c36ad3e0126d49cdf4e2ab Mon Sep 17 00:00:00 2001 From: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 31 Jan 2024 21:22:19 +0000 Subject: [PATCH 21/21] Update query snapshots --- .../test/__snapshots__/test_session_recordings.ambr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/posthog/session_recordings/test/__snapshots__/test_session_recordings.ambr b/posthog/session_recordings/test/__snapshots__/test_session_recordings.ambr index 6ce9f29c0945d..affde7090e435 100644 --- a/posthog/session_recordings/test/__snapshots__/test_session_recordings.ambr +++ b/posthog/session_recordings/test/__snapshots__/test_session_recordings.ambr @@ -643,7 +643,7 @@ FROM "posthog_persondistinctid" INNER JOIN "posthog_person" ON ("posthog_persondistinctid"."person_id" = "posthog_person"."id") WHERE ("posthog_persondistinctid"."distinct_id" IN ('user2', - 'user_one_1') + 'user_one_2') AND "posthog_persondistinctid"."team_id" = 2) /*controller='project_session_recordings-list',route='api/projects/%28%3FP%3Cparent_lookup_team_id%3E%5B%5E/.%5D%2B%29/session_recordings/%3F%24'*/ ''' # ---