Skip to content

Commit

Permalink
sampling fixes and do not attach shadow sample when not needed (#352)
Browse files Browse the repository at this point in the history
- check if we need to sample before calculating exposure key
- use exposure key for default sampling
- do not attach shadow log flag if its not sampled. slp only reads
sampling rate flag to get total samnpled
  • Loading branch information
kat-statsig authored Oct 15, 2024
1 parent a7b371c commit c388271
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 24 deletions.
41 changes: 22 additions & 19 deletions statsig/statsig_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -541,36 +541,39 @@ def __determine_sampling(self, type: EntityType, name: str, result: _ConfigEvalu
env = self._options.get_sdk_environment_tier()
sampling_mode = _SDK_Configs.get_config_str_value("sampling_mode")
special_case_sampling_rate = _SDK_Configs.get_config_int_value("special_case_sampling_rate")
special_case_rules = ["disabled", "default", ""]

if sampling_mode is None or sampling_mode == "none" or env != "production":
return True, None, "logged"
return True, None, None

if result.forward_all_exposures:
return True, None, "logged"
return True, None, None

samplingSetKey = f"{name}_{result.rule_id}"
if not self._sampling_key_set.contains(samplingSetKey):
self._sampling_key_set.add(samplingSetKey)
return True, None, "logged"
return True, None, None

should_sample = result.sample_rate is not None or result.rule_id in special_case_rules
if not should_sample:
return True, None, None

exposure_key = ""
if type == EntityType.GATE:
exposure_key = compute_dedupe_key_for_gate(name, result.rule_id, result.boolean_value,
user.user_id, user.custom_ids)
elif type == EntityType.CONFIG:
exposure_key = compute_dedupe_key_for_config(name, result.rule_id, user.user_id, user.custom_ids)
elif type == EntityType.LAYER:
exposure_key = compute_dedupe_key_for_layer(name, result.allocated_experiment, param_name,
result.rule_id,
user.user_id, user.custom_ids)

if result.sample_rate is not None:
exposure_key = ""
if type == EntityType.GATE:
exposure_key = compute_dedupe_key_for_gate(name, result.rule_id, result.boolean_value,
user.user_id, user.custom_ids)
elif type == EntityType.CONFIG:
exposure_key = compute_dedupe_key_for_config(name, result.rule_id, user.user_id, user.custom_ids)
elif type == EntityType.LAYER:
exposure_key = compute_dedupe_key_for_layer(name, result.allocated_experiment, param_name,
result.rule_id,
user.user_id, user.custom_ids)
shadow_should_log = is_hash_in_sampling_rate(exposure_key, result.sample_rate)
logged_sampling_rate = result.sample_rate

special_case_rules = ["disabled", "default", ""]

if result.rule_id in special_case_rules and special_case_sampling_rate is not None:
shadow_should_log = is_hash_in_sampling_rate(name, special_case_sampling_rate)
elif result.rule_id in special_case_rules and special_case_sampling_rate is not None:
shadow_should_log = is_hash_in_sampling_rate(exposure_key, special_case_sampling_rate)
logged_sampling_rate = special_case_sampling_rate

shadow_logged = None if result.sample_rate is None else "logged" if shadow_should_log else "dropped"
Expand All @@ -579,7 +582,7 @@ def __determine_sampling(self, type: EntityType, name: str, result: _ConfigEvalu
if sampling_mode == "shadow":
return True, logged_sampling_rate, shadow_logged

return True, None, "logged"
return True, None, None
except Exception as e:
self._errorBoundary.log_exception("__determine_sampling", e, log_mode="debug")
return True, None, None
Expand Down
10 changes: 5 additions & 5 deletions tests/test_sampling.py
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ def test_apply_shadow_sampling_in_production_dropped(self, mock_request):
self.assertEqual(sampled_event["statsigMetadata"]["samplingMode"], 'shadow')
not_sampled_event = self._events[2]
self.assertEqual(not_sampled_event["statsigMetadata"].get("sampleRate"), None)
self.assertEqual(not_sampled_event["statsigMetadata"].get("shadowLogged"), "logged")
self.assertEqual(not_sampled_event["statsigMetadata"].get("shadowLogged"), None)
self.assertEqual(not_sampled_event["statsigMetadata"]["samplingMode"], "shadow")

def test_apply_shadow_sampling_in_production_logged(self, mock_request):
Expand Down Expand Up @@ -207,12 +207,12 @@ def test_do_not_apply_shadow_sampling_in_development(self, mock_request):
self.assertEqual(2, len(self._events))
sampled_event = self._events[0]
self.assertEqual(sampled_event["statsigMetadata"].get("sampleRate"), None)
self.assertEqual(sampled_event["statsigMetadata"].get("shadowLogged"), "logged")
self.assertEqual(sampled_event["statsigMetadata"].get("shadowLogged"), None)
self.assertEqual(sampled_event["statsigMetadata"]["samplingMode"], "shadow")

not_sampled_event = self._events[1]
self.assertEqual(not_sampled_event["statsigMetadata"].get("sampleRate"), None)
self.assertEqual(not_sampled_event["statsigMetadata"].get("shadowLogged"), "logged")
self.assertEqual(not_sampled_event["statsigMetadata"].get("shadowLogged"), None)
self.assertEqual(not_sampled_event["statsigMetadata"]["samplingMode"], "shadow")

def test_do_not_apply_shadow_sampling_in_staging(self, mock_request):
Expand All @@ -230,12 +230,12 @@ def test_do_not_apply_shadow_sampling_in_staging(self, mock_request):
self.assertEqual(2, len(self._events))
sampled_event = self._events[0]
self.assertEqual(sampled_event["statsigMetadata"].get("sampleRate"), None)
self.assertEqual(sampled_event["statsigMetadata"].get("shadowLogged"), 'logged')
self.assertEqual(sampled_event["statsigMetadata"].get("shadowLogged"), None)
self.assertEqual(sampled_event["statsigMetadata"]["samplingMode"], "shadow")

not_sampled_event = self._events[1]
self.assertEqual(not_sampled_event["statsigMetadata"].get("sampleRate"), None)
self.assertEqual(not_sampled_event["statsigMetadata"].get("shadowLogged"), 'logged')
self.assertEqual(not_sampled_event["statsigMetadata"].get("shadowLogged"), None)
self.assertEqual(not_sampled_event["statsigMetadata"]["samplingMode"], "shadow")

def test_do_not_apply_sampling_to_all_exposure_forwarded_gate(self, mock_request):
Expand Down

0 comments on commit c388271

Please sign in to comment.