Skip to content

Commit

Permalink
fix(decide): Don't create exceptions for survey targeting flags (#18062)
Browse files Browse the repository at this point in the history
  • Loading branch information
neilkakkar authored Oct 19, 2023
1 parent 665bc2d commit b124588
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 11 deletions.
12 changes: 4 additions & 8 deletions posthog/api/decide.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions posthog/api/test/test_decide.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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):
Expand Down

0 comments on commit b124588

Please sign in to comment.