From b124588598b4f5a549d09bc82d8a110a8127723f Mon Sep 17 00:00:00 2001 From: Neil Kakkar Date: Thu, 19 Oct 2023 09:37:08 +0100 Subject: [PATCH] fix(decide): Don't create exceptions for survey targeting flags (#18062) --- posthog/api/decide.py | 12 ++++-------- posthog/api/test/test_decide.py | 6 +++--- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/posthog/api/decide.py b/posthog/api/decide.py index 5bd187b819cb7..efb205ee41e35 100644 --- a/posthog/api/decide.py +++ b/posthog/api/decide.py @@ -2,7 +2,6 @@ import re from typing import Any, Dict, List, Optional, Union from urllib.parse import urlparse -from posthog.api.survey import SURVEY_TARGETING_FLAG_PREFIX from posthog.database_healthcheck import DATABASE_FOR_FLAG_MATCHING from posthog.metrics import LABEL_TEAM_ID from posthog.models.feature_flag.flag_analytics import increment_request_count @@ -245,13 +244,10 @@ def get_decide(request: HttpRequest): if feature_flags: # Billing analytics for decide requests with feature flags - # Don't count if all requests are for survey targeting flags only. - if not all(flag.startswith(SURVEY_TARGETING_FLAG_PREFIX) for flag in feature_flags.keys()): - - # Sample no. of decide requests with feature flags - if settings.DECIDE_BILLING_SAMPLING_RATE and random() < settings.DECIDE_BILLING_SAMPLING_RATE: - count = int(1 / settings.DECIDE_BILLING_SAMPLING_RATE) - increment_request_count(team.pk, count) + # Sample no. of decide requests with feature flags + if settings.DECIDE_BILLING_SAMPLING_RATE and random() < settings.DECIDE_BILLING_SAMPLING_RATE: + count = int(1 / settings.DECIDE_BILLING_SAMPLING_RATE) + increment_request_count(team.pk, count) else: # no auth provided diff --git a/posthog/api/test/test_decide.py b/posthog/api/test/test_decide.py index 9ca6b3a28f409..e85fca7c7fba6 100644 --- a/posthog/api/test/test_decide.py +++ b/posthog/api/test/test_decide.py @@ -2090,7 +2090,7 @@ def test_decide_analytics_samples_dont_break_with_zero_sampling(self, *args): self.assertEqual(client.hgetall(f"posthog:decide_requests:{self.team.pk}"), {}) @patch("posthog.models.feature_flag.flag_analytics.CACHE_BUCKET_SIZE", 10) - def test_decide_analytics_only_fires_with_non_survey_targeting_flags(self, *args): + def test_decide_analytics_fires_with_survey_linked_and_targeting_flags(self, *args): ff = FeatureFlag.objects.create( team=self.team, rollout_percentage=50, name="Beta feature", key="beta-feature", created_by=self.user ) @@ -2135,7 +2135,7 @@ def test_decide_analytics_only_fires_with_non_survey_targeting_flags(self, *args self.assertEqual(client.hgetall(f"posthog:decide_requests:{self.team.pk}"), {b"165192618": b"1"}) @patch("posthog.models.feature_flag.flag_analytics.CACHE_BUCKET_SIZE", 10) - def test_decide_analytics_does_not_fire_for_survey_targeting_flags(self, *args): + def test_decide_analytics_fire_for_survey_targeting_flags(self, *args): FeatureFlag.objects.create( team=self.team, rollout_percentage=50, @@ -2180,7 +2180,7 @@ def test_decide_analytics_does_not_fire_for_survey_targeting_flags(self, *args): client = redis.get_client() # check that single increment made it to redis - self.assertEqual(client.hgetall(f"posthog:decide_requests:{self.team.pk}"), {}) + self.assertEqual(client.hgetall(f"posthog:decide_requests:{self.team.pk}"), {b"165192618": b"1"}) class TestDatabaseCheckForDecide(BaseTest, QueryMatchingTest):