Просмотр исходного кода

ref: fix typing in performance testutils (#69711)

mixins cannot be made typesafe unless they are strict subclasses so I
moved the mixin method to a free function

best viewed with
[?w=1](https://github.com/getsentry/sentry/pull/69711/files?w=1)

<!-- Describe your PR here. -->
anthony sottile 10 месяцев назад
Родитель
Сommit
2eac2ebce4

+ 0 - 2
pyproject.toml

@@ -471,8 +471,6 @@ module = [
     "sentry.testutils.helpers.notifications",
     "sentry.testutils.helpers.slack",
     "sentry.testutils.helpers.task_runner",
-    "sentry.testutils.performance_issues.span_builder",
-    "sentry.testutils.performance_issues.store_transaction",
     "sentry.tsdb.dummy",
     "sentry.tsdb.inmemory",
     "sentry.types.integrations",

+ 2 - 1
src/sentry/spans/grouping/strategy/base.py

@@ -1,7 +1,7 @@
 import re
 from collections.abc import Callable, Sequence
 from dataclasses import dataclass
-from typing import Any, Optional, TypedDict
+from typing import Any, NotRequired, Optional, TypedDict
 
 from sentry.spans.grouping.utils import Hash, parse_fingerprint_var
 from sentry.utils import urls
@@ -19,6 +19,7 @@ class Span(TypedDict):
     fingerprint: Sequence[str] | None
     tags: Any | None
     data: Any | None
+    hash: NotRequired[str]
 
 
 # A callable strategy is a callable that when given a span, it tries to

+ 8 - 2
src/sentry/testutils/factories.py

@@ -28,6 +28,7 @@ from django.utils.text import slugify
 from sentry.auth.access import RpcBackedAccess
 from sentry.constants import SentryAppInstallationStatus, SentryAppStatus
 from sentry.event_manager import EventManager
+from sentry.eventstore.models import Event
 from sentry.hybridcloud.models.webhookpayload import WebhookPayload
 from sentry.incidents.logic import (
     create_alert_rule,
@@ -896,7 +897,9 @@ class Factories:
 
     @staticmethod
     @assume_test_silo_mode(SiloMode.REGION)
-    def store_event(data, project_id, assert_no_errors=True, sent_at=None):
+    def store_event(
+        data, project_id: int, assert_no_errors: bool = True, sent_at: datetime | None = None
+    ) -> Event:
         # Like `create_event`, but closer to how events are actually
         # ingested. Prefer to use this method over `create_event`
         manager = EventManager(data, sent_at=sent_at)
@@ -1351,7 +1354,9 @@ class Factories:
 
     @staticmethod
     @assume_test_silo_mode(SiloMode.REGION)
-    def create_userreport(project, event_id=None, **kwargs):
+    def create_userreport(
+        project: Project, event_id: str | None = None, **kwargs: Any
+    ) -> UserReport:
         event = Factories.store_event(
             data={
                 "timestamp": datetime.now(UTC).isoformat(),
@@ -1360,6 +1365,7 @@ class Factories:
             },
             project_id=project.id,
         )
+        assert event.group is not None
 
         return UserReport.objects.create(
             group_id=event.group.id,

+ 6 - 6
src/sentry/testutils/performance_issues/span_builder.py

@@ -5,13 +5,13 @@ from sentry.spans.grouping.strategy.base import Span
 
 class SpanBuilder:
     def __init__(self) -> None:
-        self.trace_id: str = "a" * 32
-        self.parent_span_id: str | None = "a" * 16
-        self.span_id: str = "b" * 16
+        self.trace_id = "a" * 32
+        self.parent_span_id = "a" * 16
+        self.span_id = "b" * 16
         self.start_timestamp: float = 0
         self.timestamp: float = 1
-        self.same_process_as_parent: bool = True
-        self.op: str = "default"
+        self.same_process_as_parent = True
+        self.op = "default"
         self.description: str | None = None
         self.fingerprint: list[str] | None = None
         self.tags: Any | None = None
@@ -43,7 +43,7 @@ class SpanBuilder:
         return self
 
     def build(self) -> Span:
-        span = {
+        span: Span = {
             "trace_id": self.trace_id,
             "parent_span_id": self.parent_span_id,
             "span_id": self.span_id,

+ 60 - 58
src/sentry/testutils/performance_issues/store_transaction.py

@@ -4,71 +4,73 @@ from datetime import datetime, timedelta
 
 from django.utils import timezone
 
+from sentry.eventstore.models import Event
 from sentry.snuba.dataset import Dataset
 from sentry.testutils.cases import SnubaTestCase
+from sentry.utils import snuba
 
 
-class PerfIssueTransactionTestMixin:
-    def store_transaction(
-        self: SnubaTestCase,
-        project_id: int,
-        user_id: str,
-        fingerprint: Sequence[str],
-        environment: str | None = None,
-        timestamp: datetime | None = None,
-    ):
-        from sentry.utils import snuba
+def store_transaction(
+    test_case: SnubaTestCase,
+    project_id: int,
+    user_id: str,
+    fingerprint: Sequence[str],
+    environment: str | None = None,
+    timestamp: datetime | None = None,
+) -> Event:
+    # truncate microseconds since there's some loss in precision
+    insert_time = (timestamp if timestamp else timezone.now()).replace(microsecond=0)
 
-        # truncate microseconds since there's some loss in precision
-        insert_time = (timestamp if timestamp else timezone.now()).replace(microsecond=0)
+    user_id_val = f"id:{user_id}"
 
-        user_id_val = f"id:{user_id}"
+    extra = {}
+    tags = [("sentry:user", user_id_val)]
+    if environment is not None:
+        tags.append(("environment", environment))
+        extra["environment"] = environment
 
-        event_data = {
-            "type": "transaction",
-            "level": "info",
-            "message": "transaction message",
-            "tags": [("sentry:user", user_id_val)],
-            "contexts": {"trace": {"trace_id": "b" * 32, "span_id": "c" * 16, "op": ""}},
-            "timestamp": insert_time.timestamp(),
-            "start_timestamp": insert_time.timestamp(),
-            "received": insert_time.timestamp(),
-            # we need to randomize the value here to make sure ingestion doesn't dedupe these transactions
-            "transaction": "transaction: " + str(insert_time) + str(random.randint(0, 100000000)),
-            "fingerprint": fingerprint,
-        }
+    event_data = {
+        "type": "transaction",
+        "level": "info",
+        "message": "transaction message",
+        "tags": tags,
+        "contexts": {"trace": {"trace_id": "b" * 32, "span_id": "c" * 16, "op": ""}},
+        "timestamp": insert_time.timestamp(),
+        "start_timestamp": insert_time.timestamp(),
+        "received": insert_time.timestamp(),
+        # we need to randomize the value here to make sure ingestion doesn't dedupe these transactions
+        "transaction": "transaction: " + str(insert_time) + str(random.randint(0, 100000000)),
+        "fingerprint": fingerprint,
+        **extra,
+    }
 
-        if environment:
-            event_data["environment"] = environment
-            event_data["tags"].extend([("environment", environment)])
+    event = test_case.store_event(
+        data=event_data,
+        project_id=project_id,
+    )
 
-        event = self.store_event(
-            data=event_data,
-            project_id=project_id,
-        )
+    # read the transaction back and verify it was successfully written to snuba
+    result = snuba.raw_query(
+        dataset=Dataset.Transactions,
+        start=insert_time - timedelta(days=1),
+        end=insert_time + timedelta(days=1),
+        selected_columns=[
+            "event_id",
+            "project_id",
+            "environment",
+            "group_ids",
+            "tags[sentry:user]",
+            "timestamp",
+        ],
+        groupby=None,
+        filter_keys={"project_id": [project_id], "event_id": [event.event_id]},
+        referrer="_insert_transaction.verify_transaction",
+    )
+    assert len(result["data"]) == 1
+    assert result["data"][0]["project_id"] == project_id
+    assert result["data"][0]["group_ids"] == [g.id for g in event.groups]
+    assert result["data"][0]["tags[sentry:user]"] == user_id_val
+    assert result["data"][0]["environment"] == (environment)
+    assert result["data"][0]["timestamp"] == insert_time.isoformat()
 
-        # read the transaction back and verify it was successfully written to snuba
-        result = snuba.raw_query(
-            dataset=Dataset.Transactions,
-            start=insert_time - timedelta(days=1),
-            end=insert_time + timedelta(days=1),
-            selected_columns=[
-                "event_id",
-                "project_id",
-                "environment",
-                "group_ids",
-                "tags[sentry:user]",
-                "timestamp",
-            ],
-            groupby=None,
-            filter_keys={"project_id": [project_id], "event_id": [event.event_id]},
-            referrer="_insert_transaction.verify_transaction",
-        )
-        assert len(result["data"]) == 1
-        assert result["data"][0]["project_id"] == project_id
-        assert result["data"][0]["group_ids"] == [g.id for g in event.groups]
-        assert result["data"][0]["tags[sentry:user]"] == user_id_val
-        assert result["data"][0]["environment"] == (environment)
-        assert result["data"][0]["timestamp"] == insert_time.isoformat()
-
-        return event
+    return event

+ 0 - 2
tests/sentry/models/test_groupsnooze.py

@@ -11,7 +11,6 @@ from sentry.models.groupsnooze import GroupSnooze
 from sentry.testutils.cases import PerformanceIssueTestCase, SnubaTestCase, TestCase
 from sentry.testutils.helpers.datetime import before_now, freeze_time, iso_format
 from sentry.testutils.helpers.features import apply_feature_flag_on_cls
-from sentry.testutils.performance_issues.store_transaction import PerfIssueTransactionTestMixin
 from sentry.utils.samples import load_data
 from tests.sentry.issues.test_utils import SearchIssueTestMixin
 
@@ -19,7 +18,6 @@ from tests.sentry.issues.test_utils import SearchIssueTestMixin
 class GroupSnoozeTest(
     TestCase,
     SnubaTestCase,
-    PerfIssueTransactionTestMixin,
     SearchIssueTestMixin,
     PerformanceIssueTestCase,
 ):

+ 0 - 2
tests/sentry/rules/filters/test_issue_category.py

@@ -1,7 +1,6 @@
 from sentry.issues.grouptype import GroupCategory
 from sentry.rules.filters.issue_category import IssueCategoryFilter
 from sentry.testutils.cases import PerformanceIssueTestCase, RuleTestCase, SnubaTestCase
-from sentry.testutils.performance_issues.store_transaction import PerfIssueTransactionTestMixin
 from sentry.testutils.skips import requires_snuba
 
 pytestmark = [requires_snuba]
@@ -50,7 +49,6 @@ class IssueCategoryFilterErrorTest(RuleTestCase):
 class IssueCategoryFilterPerformanceTest(
     RuleTestCase,
     SnubaTestCase,
-    PerfIssueTransactionTestMixin,
     PerformanceIssueTestCase,
 ):
     rule_cls = IssueCategoryFilter

+ 3 - 4
tests/sentry/tasks/test_post_process.py

@@ -69,7 +69,7 @@ from sentry.testutils.helpers import with_feature
 from sentry.testutils.helpers.datetime import before_now, iso_format
 from sentry.testutils.helpers.eventprocessing import write_event_to_cache
 from sentry.testutils.helpers.options import override_options
-from sentry.testutils.performance_issues.store_transaction import PerfIssueTransactionTestMixin
+from sentry.testutils.performance_issues.store_transaction import store_transaction
 from sentry.testutils.silo import assume_test_silo_mode
 from sentry.testutils.skips import requires_snuba
 from sentry.types.activity import ActivityType
@@ -2356,7 +2356,6 @@ class PostProcessGroupErrorTest(
 class PostProcessGroupPerformanceTest(
     TestCase,
     SnubaTestCase,
-    PerfIssueTransactionTestMixin,
     CorePostProcessGroupTestMixin,
     InboxTestMixin,
     RuleProcessorTestMixin,
@@ -2399,7 +2398,8 @@ class PostProcessGroupPerformanceTest(
         generic_metrics_backend_mock,
     ):
         min_ago = before_now(minutes=1)
-        event = self.store_transaction(
+        event = store_transaction(
+            test_case=self,
             project_id=self.project.id,
             user_id=self.create_user(name="user1").name,
             fingerprint=[],
@@ -2475,7 +2475,6 @@ class PostProcessGroupPerformanceTest(
 class PostProcessGroupAggregateEventTest(
     TestCase,
     SnubaTestCase,
-    PerfIssueTransactionTestMixin,
     CorePostProcessGroupTestMixin,
     SnoozeTestSkipSnoozeMixin,
     PerformanceIssueTestCase,

+ 6 - 6
tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py

@@ -9,7 +9,7 @@ from sentry.eventstore.snuba.backend import SnubaEventStorage
 from sentry.issues.grouptype import PerformanceNPlusOneGroupType
 from sentry.testutils.cases import BaseTestCase, SnubaTestCase, TestCase
 from sentry.testutils.helpers.options import override_options
-from sentry.testutils.performance_issues.store_transaction import PerfIssueTransactionTestMixin
+from sentry.testutils.performance_issues.store_transaction import store_transaction
 from sentry.testutils.pytest.fixtures import django_db_all
 from sentry.utils.safe import get_path, set_path
 from sentry.utils.sdk_crashes.sdk_crash_detection import sdk_crash_detection
@@ -61,13 +61,12 @@ class BaseSDKCrashDetectionMixin(BaseTestCase, metaclass=abc.ABCMeta):
 
 
 @patch("sentry.utils.sdk_crashes.sdk_crash_detection.sdk_crash_detection.sdk_crash_reporter")
-class PerformanceEventTestMixin(
-    BaseSDKCrashDetectionMixin, SnubaTestCase, PerfIssueTransactionTestMixin
-):
+class PerformanceEventTestMixin(BaseSDKCrashDetectionMixin, SnubaTestCase):
     def test_performance_event_not_detected(self, mock_sdk_crash_reporter):
         fingerprint = "some_group"
         fingerprint = f"{PerformanceNPlusOneGroupType.type_id}-{fingerprint}"
-        event = self.store_transaction(
+        event = store_transaction(
+            test_case=self,
             project_id=self.project.id,
             user_id="hi",
             fingerprint=[fingerprint],
@@ -81,7 +80,8 @@ class PerformanceEventTestMixin(
     def test_performance_event_increments_counter(self, incr, mock_sdk_crash_reporter):
         fingerprint = "some_group"
         fingerprint = f"{PerformanceNPlusOneGroupType.type_id}-{fingerprint}"
-        event = self.store_transaction(
+        event = store_transaction(
+            test_case=self,
             project_id=self.project.id,
             user_id="hi",
             fingerprint=[fingerprint],

+ 0 - 2
tests/snuba/api/serializers/test_group.py

@@ -20,7 +20,6 @@ from sentry.silo.base import SiloMode
 from sentry.testutils.cases import APITestCase, PerformanceIssueTestCase, SnubaTestCase
 from sentry.testutils.helpers.datetime import before_now, iso_format
 from sentry.testutils.helpers.features import with_feature
-from sentry.testutils.performance_issues.store_transaction import PerfIssueTransactionTestMixin
 from sentry.testutils.silo import assume_test_silo_mode
 from sentry.types.group import PriorityLevel
 from sentry.utils.samples import load_data
@@ -479,7 +478,6 @@ class GroupSerializerSnubaTest(APITestCase, SnubaTestCase):
 class PerformanceGroupSerializerSnubaTest(
     APITestCase,
     SnubaTestCase,
-    PerfIssueTransactionTestMixin,
     PerformanceIssueTestCase,
 ):
     def test_perf_seen_stats(self):