From f091120eb90545df2439bfd7d2f4cf5c3a29b264 Mon Sep 17 00:00:00 2001 From: Armen Zambrano G <44410+armenzg@users.noreply.github.com> Date: Thu, 12 Sep 2024 11:29:19 -0400 Subject: [PATCH 01/15] Initial typing --- pyproject.toml | 1 - src/sentry/eventstore/models.py | 5 +---- src/sentry/grouping/api.py | 6 +++--- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 6753a72c85a35..4c94a01a92a1c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -214,7 +214,6 @@ module = [ "sentry.db.postgres.base", "sentry.db.router", "sentry.discover.endpoints.discover_key_transactions", - "sentry.eventstore.models", "sentry.features.handler", "sentry.features.manager", "sentry.grouping.strategies.legacy", diff --git a/src/sentry/eventstore/models.py b/src/sentry/eventstore/models.py index 56715c060198e..4f8e9414c8b5d 100644 --- a/src/sentry/eventstore/models.py +++ b/src/sentry/eventstore/models.py @@ -295,10 +295,7 @@ def project(self) -> Project: @project.setter def project(self, project: Project) -> None: - if project is None: - self.project_id = None - else: - self.project_id = project.id + self.project_id = project.id self._project_cache = project @cached_property diff --git a/src/sentry/grouping/api.py b/src/sentry/grouping/api.py index 357d7c9cc3476..9269451847bc3 100644 --- a/src/sentry/grouping/api.py +++ b/src/sentry/grouping/api.py @@ -33,7 +33,7 @@ from sentry.models.grouphash import GroupHash if TYPE_CHECKING: - from sentry.eventstore.models import Event + from sentry.eventstore.models import BaseEvent from sentry.grouping.fingerprinting import FingerprintingRules from sentry.grouping.strategies.base import StrategyConfiguration from sentry.models.project import Project @@ -259,7 +259,7 @@ def apply_server_fingerprinting(event, config, allow_custom_title=True): def _get_calculated_grouping_variants_for_event( - event: Event, context: GroupingContext + event: BaseEvent, context: GroupingContext ) -> dict[str, GroupingComponent]: winning_strategy: str | None = None precedence_hint: str | None = None @@ -297,7 +297,7 @@ def _get_calculated_grouping_variants_for_event( def get_grouping_variants_for_event( - event: Event, config: StrategyConfiguration | None = None + event: BaseEvent, config: StrategyConfiguration | None = None ) -> dict[str, BaseVariant]: """Returns a dict of all grouping variants for this event.""" # If a checksum is set the only variant that comes back from this From 15d024d2bec4a0c0c4101fef9f735a7451f9d648 Mon Sep 17 00:00:00 2001 From: Armen Zambrano G <44410+armenzg@users.noreply.github.com> Date: Thu, 12 Sep 2024 11:29:41 -0400 Subject: [PATCH 02/15] Commit that will need to be reverted --- src/sentry/eventstore/models.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/sentry/eventstore/models.py b/src/sentry/eventstore/models.py index 4f8e9414c8b5d..83efb596eb3a0 100644 --- a/src/sentry/eventstore/models.py +++ b/src/sentry/eventstore/models.py @@ -303,12 +303,10 @@ def interfaces(self) -> Mapping[str, Interface]: return get_interfaces(self.data) @overload - def get_interface(self, name: Literal["user"]) -> User: - ... + def get_interface(self, name: Literal["user"]) -> User: ... @overload - def get_interface(self, name: str) -> Interface | None: - ... + def get_interface(self, name: str) -> Interface | None: ... def get_interface(self, name: str) -> Interface | None: return self.interfaces.get(name) From 419654c9bfee34be0e2b9c9c115bb19fbecb0400 Mon Sep 17 00:00:00 2001 From: Armen Zambrano G <44410+armenzg@users.noreply.github.com> Date: Thu, 12 Sep 2024 11:37:29 -0400 Subject: [PATCH 03/15] Move get_email_subject to GroupEvent --- src/sentry/eventstore/models.py | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/src/sentry/eventstore/models.py b/src/sentry/eventstore/models.py index 83efb596eb3a0..3ca7300f94da5 100644 --- a/src/sentry/eventstore/models.py +++ b/src/sentry/eventstore/models.py @@ -61,7 +61,7 @@ def __init__( project_id: int, event_id: str, snuba_data: Mapping[str, Any] | None = None, - ): + ) -> None: self.project_id = project_id self.event_id = event_id self._snuba_data = snuba_data or {} @@ -303,10 +303,12 @@ def interfaces(self) -> Mapping[str, Interface]: return get_interfaces(self.data) @overload - def get_interface(self, name: Literal["user"]) -> User: ... + def get_interface(self, name: Literal["user"]) -> User: + ... @overload - def get_interface(self, name: str) -> Interface | None: ... + def get_interface(self, name: str) -> Interface | None: + ... def get_interface(self, name: str) -> Interface | None: return self.interfaces.get(name) @@ -494,18 +496,6 @@ def get_raw_data(self, for_stream: bool = False) -> Mapping[str, Any]: def size(self) -> int: return len(orjson.dumps(dict(self.data)).decode()) - def get_email_subject(self) -> str: - template = self.project.get_option("mail:subject_template") - if template: - template = EventSubjectTemplate(template) - elif self.group.issue_category == GroupCategory.PERFORMANCE: - template = EventSubjectTemplate("$shortID - $issueType") - else: - template = DEFAULT_SUBJECT_TEMPLATE - return cast( - str, truncatechars(template.safe_substitute(EventSubjectTemplateData(self)), 128) - ) - def as_dict(self) -> dict[str, Any]: """Returns the data in normalized form for external consumers.""" data: dict[str, Any] = {} @@ -680,6 +670,18 @@ def groups(self, values: Sequence[Group] | None): def for_group(self, group: Group) -> GroupEvent: return GroupEvent.from_event(self, group) + def get_email_subject(self) -> str: + template = self.project.get_option("mail:subject_template") + if template: + template = EventSubjectTemplate(template) + elif self.group.issue_category == GroupCategory.PERFORMANCE: + template = EventSubjectTemplate("$shortID - $issueType") + else: + template = DEFAULT_SUBJECT_TEMPLATE + return cast( + str, truncatechars(template.safe_substitute(EventSubjectTemplateData(self)), 128) + ) + class GroupEvent(BaseEvent): def __init__( From 00919d3d840452124e5dbf113bceec263b36194d Mon Sep 17 00:00:00 2001 From: Armen Zambrano G <44410+armenzg@users.noreply.github.com> Date: Thu, 12 Sep 2024 11:39:00 -0400 Subject: [PATCH 04/15] Move as_dict --- src/sentry/eventstore/models.py | 58 ++++++++++++++++----------------- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/src/sentry/eventstore/models.py b/src/sentry/eventstore/models.py index 3ca7300f94da5..34498f5bcdec0 100644 --- a/src/sentry/eventstore/models.py +++ b/src/sentry/eventstore/models.py @@ -496,35 +496,6 @@ def get_raw_data(self, for_stream: bool = False) -> Mapping[str, Any]: def size(self) -> int: return len(orjson.dumps(dict(self.data)).decode()) - def as_dict(self) -> dict[str, Any]: - """Returns the data in normalized form for external consumers.""" - data: dict[str, Any] = {} - data["event_id"] = self.event_id - data["project"] = self.project_id - data["release"] = self.release - data["dist"] = self.dist - data["platform"] = self.platform - data["message"] = self.message - data["datetime"] = self.datetime - data["tags"] = [(k.split("sentry:", 1)[-1], v) for (k, v) in self.tags] - for k, v in sorted(self.data.items()): - if k in data: - continue - if k == "sdk": - v = {v_k: v_v for v_k, v_v in v.items() if v_k != "client_ip"} - data[k] = v - - # for a long time culprit was not persisted. In those cases put - # the culprit in from the group. - if data.get("culprit") is None and self.group_id and self.group: - data["culprit"] = self.group.culprit - - # Override title and location with dynamically generated data - data["title"] = self.title - data["location"] = self.location - - return data - @cached_property def search_message(self) -> str: """ @@ -771,6 +742,35 @@ def search_message(self) -> str: return message + def as_dict(self) -> dict[str, Any]: + """Returns the data in normalized form for external consumers.""" + data: dict[str, Any] = {} + data["event_id"] = self.event_id + data["project"] = self.project_id + data["release"] = self.release + data["dist"] = self.dist + data["platform"] = self.platform + data["message"] = self.message + data["datetime"] = self.datetime + data["tags"] = [(k.split("sentry:", 1)[-1], v) for (k, v) in self.tags] + for k, v in sorted(self.data.items()): + if k in data: + continue + if k == "sdk": + v = {v_k: v_v for v_k, v_v in v.items() if v_k != "client_ip"} + data[k] = v + + # for a long time culprit was not persisted. In those cases put + # the culprit in from the group. + if data.get("culprit") is None and self.group_id and self.group: + data["culprit"] = self.group.culprit + + # Override title and location with dynamically generated data + data["title"] = self.title + data["location"] = self.location + + return data + def augment_message_with_occurrence(message: str, occurrence: IssueOccurrence) -> str: for attr in ("issue_title", "subtitle", "culprit"): From 13dfd482e56b96efd59634915e99e05211c1bb76 Mon Sep 17 00:00:00 2001 From: Armen Zambrano G <44410+armenzg@users.noreply.github.com> Date: Thu, 12 Sep 2024 11:42:11 -0400 Subject: [PATCH 05/15] Change get_event_metadata return type --- src/sentry/eventstore/models.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/sentry/eventstore/models.py b/src/sentry/eventstore/models.py index 34498f5bcdec0..c4ce497d99e43 100644 --- a/src/sentry/eventstore/models.py +++ b/src/sentry/eventstore/models.py @@ -313,16 +313,16 @@ def get_interface(self, name: str) -> Interface | None: def get_interface(self, name: str) -> Interface | None: return self.interfaces.get(name) - def get_event_metadata(self) -> Mapping[str, Any]: + def get_event_metadata(self) -> Mapping[str, Any] | None: """ Return the metadata of this event. See ``sentry.eventtypes``. """ # For some inexplicable reason we have some cases where the data - # is completely empty. In that case we want to hobble along + # is completely empty. In that case we want to hobble along # further. - return self.data.get("metadata") or {} + return self.data.get("metadata") def get_grouping_config(self) -> GroupingConfig: """Returns the event grouping config.""" From a608c72d172251850e1c6b2d7c661ff51234f21a Mon Sep 17 00:00:00 2001 From: Armen Zambrano G <44410+armenzg@users.noreply.github.com> Date: Thu, 12 Sep 2024 11:42:29 -0400 Subject: [PATCH 06/15] Fix groups and data error --- src/sentry/eventstore/models.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sentry/eventstore/models.py b/src/sentry/eventstore/models.py index c4ce497d99e43..8f3fe2b4ed382 100644 --- a/src/sentry/eventstore/models.py +++ b/src/sentry/eventstore/models.py @@ -547,8 +547,8 @@ def __init__( ): super().__init__(project_id, event_id, snuba_data=snuba_data) self.group_id = group_id - self.groups = groups - self.data = data + self.groups = groups or [] + self.data = data or {} def __getstate__(self) -> Mapping[str, Any]: state = super().__getstate__() From bcf92cf48b6e80e79b4f001f816ca6a9d18f6fa0 Mon Sep 17 00:00:00 2001 From: Armen Zambrano G <44410+armenzg@users.noreply.github.com> Date: Thu, 12 Sep 2024 11:48:44 -0400 Subject: [PATCH 07/15] Fix __getstate__ type --- src/sentry/eventstore/models.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sentry/eventstore/models.py b/src/sentry/eventstore/models.py index 8f3fe2b4ed382..af8c9c61ef908 100644 --- a/src/sentry/eventstore/models.py +++ b/src/sentry/eventstore/models.py @@ -66,7 +66,7 @@ def __init__( self.event_id = event_id self._snuba_data = snuba_data or {} - def __getstate__(self) -> Mapping[str, Any]: + def __getstate__(self) -> dict[str, Any]: state = self.__dict__.copy() # do not pickle cached info. We want to fetch this on demand # again. @@ -550,7 +550,7 @@ def __init__( self.groups = groups or [] self.data = data or {} - def __getstate__(self) -> Mapping[str, Any]: + def __getstate__(self) -> dict[str, Any]: state = super().__getstate__() state.pop("_group_cache", None) state.pop("_groups_cache", None) From af793a1f6393fd98f9ea4411ca9d34e0cb36dab9 Mon Sep 17 00:00:00 2001 From: Armen Zambrano G <44410+armenzg@users.noreply.github.com> Date: Thu, 12 Sep 2024 11:55:59 -0400 Subject: [PATCH 08/15] Fix issue occurrence complaints --- src/sentry/eventstore/models.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sentry/eventstore/models.py b/src/sentry/eventstore/models.py index af8c9c61ef908..0c0d9f348e59b 100644 --- a/src/sentry/eventstore/models.py +++ b/src/sentry/eventstore/models.py @@ -788,7 +788,7 @@ class EventSubjectTemplate(string.Template): class EventSubjectTemplateData: tag_aliases = {"release": "sentry:release", "dist": "sentry:dist", "user": "sentry:user"} - def __init__(self, event: Event): + def __init__(self, event: GroupEvent): self.event = event def __getitem__(self, name: str) -> str: @@ -810,7 +810,7 @@ def __getitem__(self, name: str) -> str: elif name == "orgID": return self.event.organization.slug elif name == "title": - if getattr(self.event, "occurrence", None): + if getattr(self.event, "occurrence", None) and self.event.occurrence is not None: return self.event.occurrence.issue_title else: return self.event.title From 91d7182f218557ed6ea1d24e59b1e4d2c6f6ed63 Mon Sep 17 00:00:00 2001 From: Armen Zambrano G <44410+armenzg@users.noreply.github.com> Date: Thu, 12 Sep 2024 11:57:21 -0400 Subject: [PATCH 09/15] Fix _occurrence --- src/sentry/eventstore/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sentry/eventstore/models.py b/src/sentry/eventstore/models.py index 0c0d9f348e59b..64941adf06368 100644 --- a/src/sentry/eventstore/models.py +++ b/src/sentry/eventstore/models.py @@ -725,7 +725,7 @@ def occurrence(self, value: IssueOccurrence) -> None: @property def occurrence_id(self) -> str | None: if self._occurrence: - return self.occurrence.id + return self._occurrence.id column = self._get_column_name(Columns.OCCURRENCE_ID) if column in self._snuba_data: From 6c2737bf85161ef16a7c615eb368af8bd2306d40 Mon Sep 17 00:00:00 2001 From: Armen Zambrano G <44410+armenzg@users.noreply.github.com> Date: Thu, 12 Sep 2024 11:58:24 -0400 Subject: [PATCH 10/15] Fix self.group complaint --- src/sentry/eventstore/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sentry/eventstore/models.py b/src/sentry/eventstore/models.py index 64941adf06368..7ddb97dd00cdf 100644 --- a/src/sentry/eventstore/models.py +++ b/src/sentry/eventstore/models.py @@ -645,7 +645,7 @@ def get_email_subject(self) -> str: template = self.project.get_option("mail:subject_template") if template: template = EventSubjectTemplate(template) - elif self.group.issue_category == GroupCategory.PERFORMANCE: + elif self.group and self.group.issue_category == GroupCategory.PERFORMANCE: template = EventSubjectTemplate("$shortID - $issueType") else: template = DEFAULT_SUBJECT_TEMPLATE From cbed92a66ecac629c749c9860d1a54cccba5055d Mon Sep 17 00:00:00 2001 From: Armen Zambrano G <44410+armenzg@users.noreply.github.com> Date: Thu, 12 Sep 2024 12:13:55 -0400 Subject: [PATCH 11/15] Move get_email_subject --- src/sentry/eventstore/models.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/sentry/eventstore/models.py b/src/sentry/eventstore/models.py index 7ddb97dd00cdf..4e5eb588ea7ce 100644 --- a/src/sentry/eventstore/models.py +++ b/src/sentry/eventstore/models.py @@ -641,18 +641,6 @@ def groups(self, values: Sequence[Group] | None): def for_group(self, group: Group) -> GroupEvent: return GroupEvent.from_event(self, group) - def get_email_subject(self) -> str: - template = self.project.get_option("mail:subject_template") - if template: - template = EventSubjectTemplate(template) - elif self.group and self.group.issue_category == GroupCategory.PERFORMANCE: - template = EventSubjectTemplate("$shortID - $issueType") - else: - template = DEFAULT_SUBJECT_TEMPLATE - return cast( - str, truncatechars(template.safe_substitute(EventSubjectTemplateData(self)), 128) - ) - class GroupEvent(BaseEvent): def __init__( @@ -771,6 +759,18 @@ def as_dict(self) -> dict[str, Any]: return data + def get_email_subject(self) -> str: + template = self.project.get_option("mail:subject_template") + if template: + template = EventSubjectTemplate(template) + elif self.group and self.group.issue_category == GroupCategory.PERFORMANCE: + template = EventSubjectTemplate("$shortID - $issueType") + else: + template = DEFAULT_SUBJECT_TEMPLATE + return cast( + str, truncatechars(template.safe_substitute(EventSubjectTemplateData(self)), 128) + ) + def augment_message_with_occurrence(message: str, occurrence: IssueOccurrence) -> str: for attr in ("issue_title", "subtitle", "culprit"): From f89e1ad44fc58ad431fc68035fa4095065eed4f0 Mon Sep 17 00:00:00 2001 From: Armen Zambrano G <44410+armenzg@users.noreply.github.com> Date: Thu, 12 Sep 2024 12:19:16 -0400 Subject: [PATCH 12/15] Remove some casting --- src/sentry/eventstore/models.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/sentry/eventstore/models.py b/src/sentry/eventstore/models.py index 4e5eb588ea7ce..c10d6d6f37632 100644 --- a/src/sentry/eventstore/models.py +++ b/src/sentry/eventstore/models.py @@ -99,7 +99,7 @@ def platform(self) -> str | None: column = self._get_column_name(Columns.PLATFORM) if column in self._snuba_data: return cast(str, self._snuba_data[column]) - return cast(str, self.data.get("platform", None)) + return self.data.get("platform") @property def message(self) -> str: @@ -226,7 +226,7 @@ def get_event_type(self) -> str: column = self._get_column_name(Columns.TYPE) if column in self._snuba_data: return cast(str, self._snuba_data[column]) - return cast(str, self.data.get("type", "default")) + return self.data.get("type", "default") @property def ip_address(self) -> str | None: @@ -480,7 +480,7 @@ def organization(self) -> Organization: @property def version(self) -> str: - return cast(str, self.data.get("version", "5")) + return self.data.get("version", "5") def get_raw_data(self, for_stream: bool = False) -> Mapping[str, Any]: """Returns the internal raw event data dict.""" @@ -767,9 +767,9 @@ def get_email_subject(self) -> str: template = EventSubjectTemplate("$shortID - $issueType") else: template = DEFAULT_SUBJECT_TEMPLATE - return cast( - str, truncatechars(template.safe_substitute(EventSubjectTemplateData(self)), 128) - ) + + template_data = EventSubjectTemplateData(self) + return truncatechars(template.safe_substitute(template_data), 128) def augment_message_with_occurrence(message: str, occurrence: IssueOccurrence) -> str: From 95888db3a26dac977987898f7d6f46c2df741723 Mon Sep 17 00:00:00 2001 From: Armen Zambrano G <44410+armenzg@users.noreply.github.com> Date: Thu, 12 Sep 2024 12:20:36 -0400 Subject: [PATCH 13/15] Fix signature --- src/sentry/eventstore/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sentry/eventstore/models.py b/src/sentry/eventstore/models.py index c10d6d6f37632..4e69b4dd55119 100644 --- a/src/sentry/eventstore/models.py +++ b/src/sentry/eventstore/models.py @@ -634,7 +634,7 @@ def groups(self) -> Sequence[Group]: return groups @groups.setter - def groups(self, values: Sequence[Group] | None): + def groups(self, values: Sequence[Group]) -> None: self._groups_cache = values self._group_ids = [group.id for group in values] if values else None From 86985de016f2340e86772e57f847fc4577a73c2c Mon Sep 17 00:00:00 2001 From: Armen Zambrano G <44410+armenzg@users.noreply.github.com> Date: Thu, 12 Sep 2024 12:30:26 -0400 Subject: [PATCH 14/15] Change data to be NodeData --- src/sentry/eventstore/models.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sentry/eventstore/models.py b/src/sentry/eventstore/models.py index 4e69b4dd55119..50efb14acba32 100644 --- a/src/sentry/eventstore/models.py +++ b/src/sentry/eventstore/models.py @@ -541,14 +541,14 @@ def __init__( project_id: int, event_id: str, group_id: int | None = None, - data: Mapping[str, Any] | None = None, + data: NodeData | None = None, snuba_data: Mapping[str, Any] | None = None, groups: Sequence[Group] | None = None, ): super().__init__(project_id, event_id, snuba_data=snuba_data) self.group_id = group_id self.groups = groups or [] - self.data = data or {} + self.data = data def __getstate__(self) -> dict[str, Any]: state = super().__getstate__() From 85ac1113b8cf1ed3fac8f581cd016e8108a845e9 Mon Sep 17 00:00:00 2001 From: Armen Zambrano G <44410+armenzg@users.noreply.github.com> Date: Thu, 12 Sep 2024 12:47:10 -0400 Subject: [PATCH 15/15] Revert to original type --- src/sentry/eventstore/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sentry/eventstore/models.py b/src/sentry/eventstore/models.py index 50efb14acba32..23938f8f72fa1 100644 --- a/src/sentry/eventstore/models.py +++ b/src/sentry/eventstore/models.py @@ -541,7 +541,7 @@ def __init__( project_id: int, event_id: str, group_id: int | None = None, - data: NodeData | None = None, + data: Mapping[str, Any] | None = None, snuba_data: Mapping[str, Any] | None = None, groups: Sequence[Group] | None = None, ):