From e165aec47de8a7afadcc57e98884c7ca5f1798f2 Mon Sep 17 00:00:00 2001 From: Fabian Schindler Date: Tue, 15 Oct 2024 11:18:46 +0200 Subject: [PATCH 1/6] feat(dynamic-sampling): adding org option for target sample rate Addresses https://github.com/getsentry/projects/issues/203 --- src/sentry/api/endpoints/organization_details.py | 2 ++ src/sentry/constants.py | 1 + 2 files changed, 3 insertions(+) diff --git a/src/sentry/api/endpoints/organization_details.py b/src/sentry/api/endpoints/organization_details.py index ac9cd0fa5dde6a..3bf277137a9127 100644 --- a/src/sentry/api/endpoints/organization_details.py +++ b/src/sentry/api/endpoints/organization_details.py @@ -62,6 +62,7 @@ SAFE_FIELDS_DEFAULT, SCRAPE_JAVASCRIPT_DEFAULT, SENSITIVE_FIELDS_DEFAULT, + TARGET_SAMPLE_RATE_DEFAULT, UPTIME_AUTODETECTION, ) from sentry.datascrubbing import validate_pii_config_update, validate_pii_selectors @@ -215,6 +216,7 @@ METRICS_ACTIVATE_LAST_FOR_GAUGES_DEFAULT, ), ("uptimeAutodetection", "sentry:uptime_autodetection", bool, UPTIME_AUTODETECTION), + ("targetSampleRate", "sentry:target_sample_rate", float, TARGET_SAMPLE_RATE_DEFAULT), ) DELETION_STATUSES = frozenset( diff --git a/src/sentry/constants.py b/src/sentry/constants.py index 3ce14a64301cb4..41bee5a4a92240 100644 --- a/src/sentry/constants.py +++ b/src/sentry/constants.py @@ -710,6 +710,7 @@ class InsightModules(Enum): METRICS_ACTIVATE_LAST_FOR_GAUGES_DEFAULT = False DATA_CONSENT_DEFAULT = False UPTIME_AUTODETECTION = True +TARGET_SAMPLE_RATE_DEFAULT = 1.0 # `sentry:events_member_admin` - controls whether the 'member' role gets the event:admin scope EVENTS_MEMBER_ADMIN_DEFAULT = True From 4b06a830c3afe1ad00cbf0f98453793a05c25e64 Mon Sep 17 00:00:00 2001 From: Fabian Schindler Date: Thu, 17 Oct 2024 10:11:49 +0200 Subject: [PATCH 2/6] fix(dynamic-sampling): add missing organization details serializer field --- src/sentry/api/endpoints/organization_details.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/sentry/api/endpoints/organization_details.py b/src/sentry/api/endpoints/organization_details.py index 3bf277137a9127..de8144e640971f 100644 --- a/src/sentry/api/endpoints/organization_details.py +++ b/src/sentry/api/endpoints/organization_details.py @@ -278,6 +278,7 @@ class OrganizationSerializer(BaseOrganizationSerializer): relayPiiConfig = serializers.CharField(required=False, allow_blank=True, allow_null=True) apdexThreshold = serializers.IntegerField(min_value=1, required=False) uptimeAutodetection = serializers.BooleanField(required=False) + targetSampleRate = serializers.FloatField(required=False) @cached_property def _has_legacy_rate_limits(self): From 57adfbeb86a4d15a9b3c0254ad01828d35c6a031 Mon Sep 17 00:00:00 2001 From: Fabian Schindler Date: Thu, 17 Oct 2024 10:12:39 +0200 Subject: [PATCH 3/6] feat(dynamic-sampling): adding test for organization option targetSampleRate --- tests/sentry/api/endpoints/test_organization_details.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/sentry/api/endpoints/test_organization_details.py b/tests/sentry/api/endpoints/test_organization_details.py index 03beb0566d2639..ad50c989c629da 100644 --- a/tests/sentry/api/endpoints/test_organization_details.py +++ b/tests/sentry/api/endpoints/test_organization_details.py @@ -455,6 +455,7 @@ def test_various_options(self, mock_get_repositories): "metricsActivatePercentiles": False, "metricsActivateLastForGauges": True, "uptimeAutodetection": False, + "targetSampleRate": 0.1, } # needed to set require2FA @@ -493,6 +494,7 @@ def test_various_options(self, mock_get_repositories): assert options.get("sentry:metrics_activate_percentiles") is False assert options.get("sentry:metrics_activate_last_for_gauges") is True assert options.get("sentry:uptime_autodetection") is False + assert options.get("sentry:target_sample_rate") == 0.1 # log created with assume_test_silo_mode_of(AuditLogEntry): From 96c81fc49eb8ed364d4404482d38bc82ef82442c Mon Sep 17 00:00:00 2001 From: Fabian Schindler Date: Thu, 17 Oct 2024 10:56:19 +0200 Subject: [PATCH 4/6] fix(dynamic-sampling): make sure that targetSampleRate is within [0:1] add tests for range validation --- .../api/endpoints/organization_details.py | 7 +++++++ .../endpoints/test_organization_details.py | 19 +++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/src/sentry/api/endpoints/organization_details.py b/src/sentry/api/endpoints/organization_details.py index de8144e640971f..892fedd73c7f01 100644 --- a/src/sentry/api/endpoints/organization_details.py +++ b/src/sentry/api/endpoints/organization_details.py @@ -368,6 +368,13 @@ def validate_projectRateLimit(self, value): ) return value + def validate_targetSampleRate(self, value): + if not 0.0 <= value <= 1.0: + raise serializers.ValidationError( + "The targetSampleRate option must be in the range [0:1]" + ) + return value + def validate(self, attrs): attrs = super().validate(attrs) if attrs.get("avatarType") == "upload": diff --git a/tests/sentry/api/endpoints/test_organization_details.py b/tests/sentry/api/endpoints/test_organization_details.py index ad50c989c629da..3ba5acdbb9f1f2 100644 --- a/tests/sentry/api/endpoints/test_organization_details.py +++ b/tests/sentry/api/endpoints/test_organization_details.py @@ -942,6 +942,25 @@ def test_org_mapping_already_taken(self): self.create_organization(slug="taken") self.get_error_response(self.organization.slug, slug="taken", status_code=400) + def test_target_sample_rate_range(self): + # low, within and high + data = {"targetSampleRate": 0.0} + self.get_success_response(self.organization.slug, **data) + + data = {"targetSampleRate": 0.1} + self.get_success_response(self.organization.slug, **data) + + data = {"targetSampleRate": 1.0} + self.get_success_response(self.organization.slug, **data) + + # below range + data = {"targetSampleRate": -0.1} + self.get_error_response(self.organization.slug, status_code=400, **data) + + # above range + data = {"targetSampleRate": 1.1} + self.get_error_response(self.organization.slug, status_code=400, **data) + class OrganizationDeleteTest(OrganizationDetailsTestBase): method = "delete" From 23ef8783a692b75f61b67a889f30e9b13d6370b0 Mon Sep 17 00:00:00 2001 From: Fabian Schindler Date: Thu, 17 Oct 2024 11:17:48 +0200 Subject: [PATCH 5/6] fix(dynamic-sampling): ensuring the feature flag is enabled add tests --- src/sentry/api/endpoints/organization_details.py | 12 ++++++++++++ .../api/endpoints/test_organization_details.py | 10 ++++++++++ 2 files changed, 22 insertions(+) diff --git a/src/sentry/api/endpoints/organization_details.py b/src/sentry/api/endpoints/organization_details.py index 892fedd73c7f01..258f3202a386f3 100644 --- a/src/sentry/api/endpoints/organization_details.py +++ b/src/sentry/api/endpoints/organization_details.py @@ -369,6 +369,18 @@ def validate_projectRateLimit(self, value): return value def validate_targetSampleRate(self, value): + from sentry import features + + organization = self.context["organization"] + request = self.context["request"] + has_dynamic_sampling_custom = features.has( + "organizations:dynamic-sampling-custom", organization, actor=request.user + ) + if not has_dynamic_sampling_custom: + raise serializers.ValidationError( + "Organization does not have the custom dynamic sample rate feature enabled." + ) + if not 0.0 <= value <= 1.0: raise serializers.ValidationError( "The targetSampleRate option must be in the range [0:1]" diff --git a/tests/sentry/api/endpoints/test_organization_details.py b/tests/sentry/api/endpoints/test_organization_details.py index 3ba5acdbb9f1f2..950ab8a8b18e41 100644 --- a/tests/sentry/api/endpoints/test_organization_details.py +++ b/tests/sentry/api/endpoints/test_organization_details.py @@ -942,6 +942,16 @@ def test_org_mapping_already_taken(self): self.create_organization(slug="taken") self.get_error_response(self.organization.slug, slug="taken", status_code=400) + def test_target_sample_rate_feature(self): + with self.feature("organizations:dynamic-sampling-custom"): + data = {"targetSampleRate": 0.1} + self.get_success_response(self.organization.slug, **data) + + with self.feature({"organizations:dynamic-sampling-custom": False}): + data = {"targetSampleRate": 0.1} + self.get_error_response(self.organization.slug, status_code=400, **data) + + @with_feature("organizations:dynamic-sampling-custom") def test_target_sample_rate_range(self): # low, within and high data = {"targetSampleRate": 0.0} From 841c3a6f2f2704b4ca7c68d580111f03e7dc9755 Mon Sep 17 00:00:00 2001 From: Fabian Schindler Date: Thu, 17 Oct 2024 11:25:24 +0200 Subject: [PATCH 6/6] fix(dynamic-sampling): fixing test case by enabling feature flag --- tests/sentry/api/endpoints/test_organization_details.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/sentry/api/endpoints/test_organization_details.py b/tests/sentry/api/endpoints/test_organization_details.py index 950ab8a8b18e41..309f82a8be5aad 100644 --- a/tests/sentry/api/endpoints/test_organization_details.py +++ b/tests/sentry/api/endpoints/test_organization_details.py @@ -410,7 +410,7 @@ def test_upload_avatar(self): "sentry.integrations.github.integration.GitHubApiClient.get_repositories", return_value=[{"name": "cool-repo", "full_name": "testgit/cool-repo"}], ) - @with_feature("organizations:codecov-integration") + @with_feature(["organizations:codecov-integration", "organizations:dynamic-sampling-custom"]) def test_various_options(self, mock_get_repositories): initial = self.organization.get_audit_log_data() with assume_test_silo_mode_of(AuditLogEntry):