Browse Source

ref: fix typing in sentry.utils.performance_issues (#69206)

<!-- Describe your PR here. -->
anthony sottile 10 months ago
parent
commit
12398b1180

+ 0 - 7
pyproject.toml

@@ -490,13 +490,6 @@ module = [
     "sentry.utils.distutils.commands.build_assets",
     "sentry.utils.email.signer",
     "sentry.utils.locking.backends.migration",
-    "sentry.utils.performance_issues.detectors.io_main_thread_detector",
-    "sentry.utils.performance_issues.detectors.mn_plus_one_db_span_detector",
-    "sentry.utils.performance_issues.detectors.n_plus_one_api_calls_detector",
-    "sentry.utils.performance_issues.detectors.n_plus_one_db_span_detector",
-    "sentry.utils.performance_issues.detectors.render_blocking_asset_span_detector",
-    "sentry.utils.performance_issues.detectors.slow_db_query_detector",
-    "sentry.utils.performance_issues.performance_detection",
     "sentry.utils.sentry_apps.webhooks",
     "sentry.utils.services",
     "sentry.utils.snowflake",

+ 10 - 9
src/sentry/utils/performance_issues/base.py

@@ -6,11 +6,12 @@ import re
 from abc import ABC, abstractmethod
 from datetime import timedelta
 from enum import Enum
-from typing import Any, ClassVar, cast
+from typing import Any, ClassVar
 from urllib.parse import parse_qs, urlparse
 
 from sentry import options
 from sentry.issues.grouptype import (
+    GroupType,
     PerformanceConsecutiveDBQueriesGroupType,
     PerformanceConsecutiveHTTPQueriesGroupType,
     PerformanceDBMainThreadGroupType,
@@ -46,7 +47,7 @@ class DetectorType(Enum):
     HTTP_OVERHEAD = "http_overhead"
 
 
-DETECTOR_TYPE_TO_GROUP_TYPE = {
+DETECTOR_TYPE_TO_GROUP_TYPE: dict[DetectorType, type[GroupType]] = {
     DetectorType.SLOW_DB_QUERY: PerformanceSlowDBQueryGroupType,
     DetectorType.RENDER_BLOCKING_ASSET_SPAN: PerformanceRenderBlockingAssetSpanGroupType,
     DetectorType.N_PLUS_ONE_DB_QUERIES: PerformanceNPlusOneGroupType,
@@ -382,13 +383,13 @@ def get_span_evidence_value(
     value = "no value"
     if not span:
         return value
-    if not span.get("op") and span.get("description"):
-        value = cast(str, span["description"])
-    if span.get("op") and not span.get("description"):
-        value = cast(str, span["op"])
-    if span.get("op") and span.get("description"):
-        op = cast(str, span["op"])
-        desc = cast(str, span["description"])
+    op = span.get("op")
+    desc = span.get("description")
+    if not op and desc and isinstance(desc, str):
+        value = desc
+    elif not desc and op and isinstance(op, str):
+        value = op
+    elif op and isinstance(op, str) and desc and isinstance(desc, str):
         value = f"{op} - {desc}"
         if not include_op:
             value = desc

+ 20 - 15
src/sentry/utils/performance_issues/detectors/io_main_thread_detector.py

@@ -6,9 +6,11 @@ from collections import defaultdict
 from typing import Any
 
 import sentry_sdk
+from symbolic.proguard import ProguardMapper
 
 from sentry import features, options
 from sentry.issues.grouptype import (
+    GroupType,
     PerformanceDBMainThreadGroupType,
     PerformanceFileIOMainThreadGroupType,
 )
@@ -30,23 +32,29 @@ from ..types import Span
 
 
 class BaseIOMainThreadDetector(PerformanceDetector):
-    __slots__ = ("spans_involved", "stored_problems")
+    __slots__ = ("stored_problems",)
+
+    SPAN_PREFIX: str  # abstract
+    group_type: type[GroupType]  # abstract
+
+    def _is_io_on_main_thread(self, span: Span) -> bool:
+        raise NotImplementedError
+
+    def _fingerprint(self, span_list: list[Span]) -> str:
+        raise NotImplementedError
 
     def __init__(self, settings: dict[DetectorType, Any], event: dict[str, Any]) -> None:
         super().__init__(settings, event)
 
-        self.spans_involved = {}
-        self.most_recent_start_time = {}
-        self.most_recent_hash = {}
         self.stored_problems = {}
-        self.mapper = None
-        self.parent_to_blocked_span = defaultdict(list)
+        self.mapper: ProguardMapper | None = None
+        self.parent_to_blocked_span: dict[str, list[Span]] = defaultdict(list)
 
     def visit_span(self, span: Span):
         if self._is_io_on_main_thread(span) and span.get("op", "").lower().startswith(
             self.SPAN_PREFIX
         ):
-            parent_span_id = span.get("parent_span_id")
+            parent_span_id = span["parent_span_id"]
             self.parent_to_blocked_span[parent_span_id].append(span)
 
     def on_complete(self):
@@ -65,7 +73,7 @@ class BaseIOMainThreadDetector(PerformanceDetector):
                 offender_spans = [span for span in span_list if "span_id" in span]
                 self.stored_problems[fingerprint] = PerformanceProblem(
                     fingerprint=fingerprint,
-                    op=span_list[0].get("op"),
+                    op=span_list[0].get("op", ""),
                     desc=span_list[0].get("description", ""),
                     parent_span_ids=[parent_span_id],
                     type=self.group_type,
@@ -107,7 +115,7 @@ class FileIOMainThreadDetector(BaseIOMainThreadDetector):
     Checks for a file io span on the main thread
     """
 
-    __slots__ = ("spans_involved", "stored_problems")
+    __slots__ = ("stored_problems",)
 
     IGNORED_EXTENSIONS = {".nib", ".plist"}
     SPAN_PREFIX = "file"
@@ -148,7 +156,7 @@ class FileIOMainThreadDetector(BaseIOMainThreadDetector):
                     self.mapper = mapper
                     return
 
-    def _deobfuscate_module(self, module: str) -> str:
+    def _deobfuscate_module(self, module: str) -> str | None:
         if self.mapper is not None:
             return self.mapper.remap_class(module)
         else:
@@ -163,7 +171,7 @@ class FileIOMainThreadDetector(BaseIOMainThreadDetector):
         else:
             return frame.get("function", "")
 
-    def _fingerprint(self, span_list) -> str:
+    def _fingerprint(self, span_list: list[Span]) -> str:
         call_stack_strings = []
         overall_stack = []
         # only prepare deobfuscation once we need to fingerprint cause its expensive
@@ -202,7 +210,7 @@ class DBMainThreadDetector(BaseIOMainThreadDetector):
     Checks for a DB span on the main thread
     """
 
-    __slots__ = ("spans_involved", "stored_problems")
+    __slots__ = ("stored_problems",)
 
     SPAN_PREFIX = "db"
     type = DetectorType.DB_MAIN_THREAD
@@ -212,9 +220,6 @@ class DBMainThreadDetector(BaseIOMainThreadDetector):
     def __init__(self, settings: dict[DetectorType, Any], event: dict[str, Any]) -> None:
         super().__init__(settings, event)
 
-        self.spans_involved = {}
-        self.most_recent_start_time = {}
-        self.most_recent_hash = {}
         self.stored_problems = {}
         self.mapper = None
         self.parent_to_blocked_span = defaultdict(list)

+ 11 - 7
src/sentry/utils/performance_issues/detectors/mn_plus_one_db_span_detector.py

@@ -6,7 +6,6 @@ from collections import deque
 from collections.abc import Sequence
 from typing import Any
 
-from sentry.eventstore.models import Event
 from sentry.issues.grouptype import (
     PerformanceMNPlusOneDBQueriesGroupType,
     PerformanceNPlusOneGroupType,
@@ -62,7 +61,10 @@ class SearchingForMNPlusOne(MNPlusOneState):
     __slots__ = ("settings", "event", "recent_spans")
 
     def __init__(
-        self, settings: dict[str, Any], event: Event, initial_spans: Sequence[Span] | None = None
+        self,
+        settings: dict[str, Any],
+        event: dict[str, Any],
+        initial_spans: Sequence[Span] | None = None,
     ) -> None:
         self.settings = settings
         self.event = event
@@ -98,7 +100,7 @@ class SearchingForMNPlusOne(MNPlusOneState):
         for span in pattern:
             op = span.get("op") or ""
             description = span.get("description") or ""
-            found_db_op = found_db_op or (
+            found_db_op = found_db_op or bool(
                 op.startswith("db")
                 and not op.startswith("db.redis")
                 and description
@@ -124,18 +126,18 @@ class ContinuingMNPlusOne(MNPlusOneState):
     __slots__ = ("settings", "event", "pattern", "spans", "pattern_index")
 
     def __init__(
-        self, settings: dict[str, Any], event: Event, pattern: Sequence[Span], first_span: Span
+        self, settings: dict[str, Any], event: dict[str, Any], pattern: list[Span], first_span: Span
     ) -> None:
         self.settings = settings
         self.event = event
         self.pattern = pattern
 
         # The full list of spans involved in the MN pattern.
-        self.spans: Sequence[Span] = pattern.copy()
+        self.spans = pattern.copy()
         self.spans.append(first_span)
         self.pattern_index = 1
 
-    def next(self, span: Span) -> MNPlusOneState:
+    def next(self, span: Span) -> tuple[MNPlusOneState, PerformanceProblem | None]:
         # If the MN pattern is continuing, carry on in this state.
         pattern_span = self.pattern[self.pattern_index]
         if self._equivalent(pattern_span, span):
@@ -179,6 +181,8 @@ class ContinuingMNPlusOne(MNPlusOneState):
             return None
 
         db_span = self._first_db_span()
+        if not db_span:
+            return None
         return PerformanceProblem(
             fingerprint=self._fingerprint(db_span["hash"], parent_span),
             op="db",
@@ -263,7 +267,7 @@ class MNPlusOneDBSpanDetector(PerformanceDetector):
         super().__init__(settings, event)
 
         self.stored_problems = {}
-        self.state = SearchingForMNPlusOne(self.settings, self.event())
+        self.state: MNPlusOneState = SearchingForMNPlusOne(self.settings, self.event())
 
     def is_creation_allowed_for_organization(self, organization: Organization | None) -> bool:
         return True

+ 1 - 1
src/sentry/utils/performance_issues/detectors/n_plus_one_api_calls_detector.py

@@ -54,7 +54,7 @@ class NPlusOneAPICallsDetector(PerformanceDetector):
         # TODO: Only store the span IDs and timestamps instead of entire span objects
         self.stored_problems: PerformanceProblemsMap = {}
         self.spans: list[Span] = []
-        self.span_hashes = {}
+        self.span_hashes: dict[str, str | None] = {}
 
     def visit_span(self, span: Span) -> None:
         if not NPlusOneAPICallsDetector.is_span_eligible(span):

+ 7 - 4
src/sentry/utils/performance_issues/detectors/n_plus_one_db_span_detector.py

@@ -64,9 +64,9 @@ class NPlusOneDBSpanDetector(PerformanceDetector):
 
         self.stored_problems = {}
         self.potential_parents = {}
-        self.n_hash = None
-        self.n_spans = []
-        self.source_span = None
+        self.n_hash: str | None = None
+        self.n_spans: list[Span] = []
+        self.source_span: Span | None = None
         root_span = get_path(self._event, "contexts", "trace")
         if root_span:
             self.potential_parents[root_span.get("span_id")] = root_span
@@ -128,7 +128,10 @@ class NPlusOneDBSpanDetector(PerformanceDetector):
 
         self.source_span = span
 
-    def _continues_n_plus_1(self, span: Span):
+    def _continues_n_plus_1(self, span: Span) -> bool:
+        if self.source_span is None:
+            return False
+
         expected_parent_id = self.source_span.get("parent_span_id", None)
         parent_id = span.get("parent_span_id", None)
         if not parent_id or parent_id != expected_parent_id:

+ 4 - 2
src/sentry/utils/performance_issues/detectors/render_blocking_asset_span_detector.py

@@ -124,7 +124,9 @@ class RenderBlockingAssetSpanDetector(PerformanceDetector):
 
         return (end - start) * 1000
 
-    def _is_blocking_render(self, span):
+    def _is_blocking_render(self, span: Span) -> bool:
+        assert self.fcp is not None
+
         data = span.get("data", None)
         render_blocking_status = data and data.get("resource.render_blocking_status")
         if render_blocking_status == "non-blocking":
@@ -150,6 +152,6 @@ class RenderBlockingAssetSpanDetector(PerformanceDetector):
         fcp_ratio_threshold = self.settings.get("fcp_ratio_threshold")
         return span_duration / self.fcp > fcp_ratio_threshold
 
-    def _fingerprint(self, span: Span):
+    def _fingerprint(self, span: Span) -> str:
         resource_url_hash = fingerprint_resource_span(span)
         return f"1-{PerformanceRenderBlockingAssetSpanGroupType.type_id}-{resource_url_hash}"

+ 5 - 5
src/sentry/utils/performance_issues/performance_detection.py

@@ -91,10 +91,10 @@ class EventPerformanceProblem:
         return evidence_hashes
 
     def save(self):
-        nodestore.set(self.identifier, self.problem.to_dict())
+        nodestore.backend.set(self.identifier, self.problem.to_dict())
 
     @classmethod
-    def fetch(cls, event: Event, problem_hash: str) -> EventPerformanceProblem:
+    def fetch(cls, event: Event, problem_hash: str) -> EventPerformanceProblem | None:
         return cls.fetch_multi([(event, problem_hash)])[0]
 
     @classmethod
@@ -102,7 +102,7 @@ class EventPerformanceProblem:
         cls, items: Sequence[tuple[Event, str]]
     ) -> Sequence[EventPerformanceProblem | None]:
         ids = [cls.build_identifier(event.event_id, problem_hash) for event, problem_hash in items]
-        results = nodestore.get_multi(ids)
+        results = nodestore.backend.get_multi(ids)
         return [
             cls(event, PerformanceProblem.from_dict(results[_id])) if results.get(_id) else None
             for _id, (event, _) in zip(ids, items)
@@ -403,7 +403,7 @@ def run_detector_on_data(detector, data):
 
 # Reports metrics and creates spans for detection
 def report_metrics_for_detectors(
-    event: Event,
+    event: dict[str, Any],
     event_id: str | None,
     detectors: Sequence[PerformanceDetector],
     sdk_span: Any,
@@ -457,7 +457,7 @@ def report_metrics_for_detectors(
 
     detected_tags = {
         "sdk_name": sdk_name,
-        "is_early_adopter": organization.flags.early_adopter.is_set,
+        "is_early_adopter": bool(organization.flags.early_adopter),
         "is_standalone_spans": is_standalone_spans,
     }
 

+ 4 - 4
src/sentry/utils/performance_issues/performance_problem.py

@@ -1,6 +1,6 @@
 from collections.abc import Mapping, Sequence
 from dataclasses import dataclass
-from typing import Any
+from typing import Any, Self
 
 from sentry.issues.grouptype import GroupType, get_group_type_by_type_id
 from sentry.issues.issue_occurrence import IssueEvidence
@@ -46,7 +46,7 @@ class PerformanceProblem:
         return self.type.description
 
     @classmethod
-    def from_dict(cls, data: dict):
+    def from_dict(cls, data: dict[str, Any]) -> Self:
         return cls(
             data["fingerprint"],
             data["op"],
@@ -62,7 +62,7 @@ class PerformanceProblem:
             ],
         )
 
-    def __eq__(self, other):
+    def __eq__(self, other: object) -> bool:
         if not isinstance(other, PerformanceProblem):
             return NotImplemented
         return (
@@ -71,7 +71,7 @@ class PerformanceProblem:
             and self.type == other.type
         )
 
-    def __hash__(self):
+    def __hash__(self) -> int:
         # This will de-duplicate on fingerprint and type and only for offending span ids.
         # Fingerprint should incorporate the 'uniqueness' enough that parent and span checks etc. are not required.
         return hash((self.fingerprint, frozenset(self.offender_span_ids), self.type))

+ 3 - 1
tests/sentry/utils/performance_issues/test_performance_detection.py

@@ -519,7 +519,9 @@ class EventPerformanceProblemTest(TestCase):
         )
 
         EventPerformanceProblem(event, problem).save()
-        assert EventPerformanceProblem.fetch(event, problem.fingerprint).problem == problem
+        found = EventPerformanceProblem.fetch(event, problem.fingerprint)
+        assert found is not None
+        assert found.problem == problem
 
     def test_fetch_multi(self):
         event_1 = Event(self.project.id, "something")