Skip to content

Commit

Permalink
feat(eventuser): Migrate IssuesByTagProcessor away from EventUser
Browse files Browse the repository at this point in the history
  • Loading branch information
NisanthanNanthakumar committed Nov 13, 2023
1 parent 02571b5 commit e59edf3
Show file tree
Hide file tree
Showing 4 changed files with 122 additions and 32 deletions.
4 changes: 2 additions & 2 deletions src/sentry/data_export/processors/issues_by_tag.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
from __future__ import annotations

from sentry import tagstore
from sentry.models.eventuser import EventUser
from sentry.models.group import Group, get_group_with_redirect
from sentry.models.project import Project
from sentry.utils.eventuser import EventUser

from ..base import ExportError

Expand Down Expand Up @@ -97,7 +97,7 @@ def serialize_row(item, key):
}
if key == "user":
euser = item._eventuser
result["id"] = euser.ident if euser else ""
result["id"] = euser.user_ident if euser else ""
result["email"] = euser.email if euser else ""
result["username"] = euser.username if euser else ""
result["ip_address"] = euser.ip_address if euser else ""
Expand Down
75 changes: 63 additions & 12 deletions src/sentry/utils/eventuser.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
Direction,
Entity,
Function,
Limit,
Op,
OrderBy,
Query,
Expand Down Expand Up @@ -72,7 +71,10 @@ def from_event(event: Event) -> EventUser:
project_id=event.project_id if event else None,
email=event.data.get("user", {}).get("email") if event else None,
username=event.data.get("user", {}).get("username") if event else None,
name=event.data.get("user", {}).get("name") if event else None,
name=event.data.get("user", {}).get("name")
or event.data.get("user", {}).get("username")
if event
else None,
ip_address=event.data.get("user", {}).get("ip_address") if event else None,
user_ident=event.data.get("user", {}).get("id") if event else None,
)
Expand All @@ -82,7 +84,11 @@ def get_display_name(self):

@classmethod
def for_projects(
self, projects: List[Project], keyword_filters: Mapping[str, Any]
self,
projects: List[Project],
keyword_filters: Mapping[str, List[Any]],
filter_boolean="AND",
return_all=False,
) -> List[EventUser]:
"""
Fetch the EventUser with a Snuba query that exists within a list of projects
Expand All @@ -96,29 +102,43 @@ def for_projects(
Condition(Column("timestamp"), Op.GTE, oldest_project.date_added),
]

keyword_where_conditions = []
for keyword, value in keyword_filters.items():
if not isinstance(value, list):
raise Exception(f"{keyword} filter must be a list of values")

snuba_column = SNUBA_KEYWORD_MAP.get_key(keyword)
if isinstance(snuba_column, tuple):
where_conditions.append(
keyword_where_conditions.append(
BooleanCondition(
BooleanOp.OR,
[
Condition(
Column(column),
Op.EQ,
Op.IN,
value
if SNUBA_COLUMN_COALASCE.get(column, None) is None
else Function(
SNUBA_COLUMN_COALASCE.get(column), parameters=[value]
),
else Function(SNUBA_COLUMN_COALASCE.get(column), parameters=value),
)
for column in snuba_column
],
)
)

else:
where_conditions.append(Condition(Column(snuba_column), Op.EQ, value))
keyword_where_conditions.append(Condition(Column(snuba_column), Op.IN, value))

if len(keyword_where_conditions) > 1:
where_conditions.append(
BooleanCondition(
BooleanOp.AND if filter_boolean == "AND" else BooleanOp.OR,
keyword_where_conditions,
)
)

if len(keyword_where_conditions) == 1:
where_conditions.extend(
keyword_where_conditions,
)

query = Query(
match=Entity(EntityKey.Events.value),
Expand All @@ -134,10 +154,12 @@ def for_projects(
Column("user_email"),
],
where=where_conditions,
limit=Limit(1),
orderby=[OrderBy(Column("timestamp"), Direction.DESC)],
)

if return_all:
query.set_limit(1)
query.set_orderby([OrderBy(Column("timestamp"), Direction.DESC)])

request = Request(
dataset=Dataset.Events.value,
app_id=REFERRER,
Expand All @@ -164,6 +186,35 @@ def from_snuba(result: Mapping[str, Any]) -> EventUser:
user_ident=result.get("user_id"),
)

@classmethod
def for_tags(cls, project_id: int, values):
"""
Finds matching EventUser objects from a list of tag values.
Return a dictionary of {tag_value: event_user}.
"""
projects = Project.objects.filter(id=project_id)
result = {}
keyword_filters = {}
for value in values:
key, value = value.split(":", 1)[0], value.split(":", 1)[-1]
if keyword_filters.get(key):
keyword_filters[key].append(value)
else:
keyword_filters[key] = [value]

eventusers = EventUser.for_projects(projects, keyword_filters, return_all=True)
for keyword, values in keyword_filters.items():
column = KEYWORD_MAP.get_key(keyword)
for value in values:
matching_euser = next(
(euser for euser in eventusers if getattr(euser, column, None) == value), None
)
if matching_euser:
result[f"{keyword}:{value}"] = matching_euser

return result

@property
def tag_value(self):
"""
Expand Down
17 changes: 15 additions & 2 deletions tests/sentry/data_export/processors/test_issues_by_tag.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
from datetime import timedelta

import pytest
from django.utils import timezone

from sentry.data_export.base import ExportError
from sentry.data_export.processors.issues_by_tag import IssuesByTagProcessor
from sentry.models.eventuser import EventUser
from sentry.models.group import Group
from sentry.models.project import Project
from sentry.testutils.cases import SnubaTestCase, TestCase
from sentry.testutils.helpers.datetime import before_now, iso_format
from sentry.utils.eventuser import EventUser


class IssuesByTagProcessorTest(TestCase, SnubaTestCase):
Expand All @@ -27,6 +30,8 @@ def setUp(self):
self.user = self.create_user()
self.org = self.create_organization(owner=self.user)
self.project = self.create_project(organization=self.org)
self.project.date_added = timezone.now() - timedelta(minutes=10)
self.project.save()
self.event = self.store_event(
data={
"fingerprint": ["group-1"],
Expand All @@ -36,7 +41,15 @@ def setUp(self):
project_id=self.project.id,
)
self.group = self.event.group
self.euser = EventUser.objects.get(email=self.user.email, project_id=self.project.id)
self.euser = EventUser(
project_id=self.project.id,
email=self.user.email,
username=None,
name=None,
ip_address=None,
user_ident=None,
id=None,
)

def test_get_project(self):
project = IssuesByTagProcessor.get_project(project_id=self.project.id)
Expand Down
58 changes: 42 additions & 16 deletions tests/sentry/utils/test_eventuser.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ def setUp(self):

self.project = self.create_project(date_added=(timezone.now() - timedelta(hours=2)))

self.store_event(
self.event_1 = self.store_event(
data={
"user": {
"id": 1,
Expand All @@ -30,7 +30,7 @@ def setUp(self):
project_id=self.project.id,
)

self.store_event(
self.event_2 = self.store_event(
data={
"user": {
"id": 2,
Expand All @@ -43,10 +43,10 @@ def setUp(self):
project_id=self.project.id,
)

self.store_event(
self.event_3 = self.store_event(
data={
"user": {
"id": 3,
"id": "myminion",
"email": "[email protected]",
"username": "minion",
"ip_address": "8.8.8.8",
Expand All @@ -57,31 +57,48 @@ def setUp(self):
)

def test_for_projects_query_filter_id(self):
euser = EventUser.for_projects([self.project], {"id": "2"})
euser = EventUser.for_projects([self.project], {"id": ["2"]})
assert len(euser) == 1
assert euser[0].user_ident == "2"
assert euser[0].email == "[email protected]"

def test_for_projects_query_filter_username(self):
euser = EventUser.for_projects([self.project], {"username": "minion"})
euser = EventUser.for_projects([self.project], {"username": ["minion"]})
assert len(euser) == 1
assert euser[0].user_ident == "3"
assert euser[0].user_ident == "myminion"
assert euser[0].email == "[email protected]"

def test_for_projects_query_filter_email(self):
euser = EventUser.for_projects([self.project], {"email": "[email protected]"})
euser = EventUser.for_projects([self.project], {"email": ["[email protected]"]})
assert len(euser) == 1
assert euser[0].user_ident == "1"
assert euser[0].email == "[email protected]"

def test_for_projects_query_filter_ip(self):
euser = EventUser.for_projects([self.project], {"ip": "8.8.8.8"})
euser = EventUser.for_projects([self.project], {"ip": ["8.8.8.8"]})
assert len(euser) == 1
assert euser[0].user_ident == "3"
assert euser[0].user_ident == "myminion"
assert euser[0].email == "[email protected]"

def test_for_projects_query_multiple_OR_filters(self):
eusers = EventUser.for_projects(
[self.project],
{"username": ["minion"], "email": ["[email protected]"]},
filter_boolean="OR",
return_all=True,
)
assert len(eusers) == 2

def test_for_projects_query_multiple_AND_filters(self):
eusers = EventUser.for_projects(
[self.project],
{"username": ["minion"], "email": ["[email protected]"], "ip": ["8.8.8.8"]},
return_all=True,
)
assert len(eusers) == 1

def test_tag_value_primary_is_user_ident(self):
euser = EventUser.for_projects([self.project], {"id": "2"})
euser = EventUser.for_projects([self.project], {"id": ["2"]})
assert len(euser) == 1
assert euser[0].user_ident == "2"
assert euser[0].tag_value == "id:2"
Expand All @@ -99,7 +116,7 @@ def test_tag_value_primary_is_username(self):
},
project_id=self.project.id,
)
euser = EventUser.for_projects([self.project], {"username": "cocoa"})
euser = EventUser.for_projects([self.project], {"username": ["cocoa"]})
assert len(euser) == 1
assert euser[0].user_ident is None
assert euser[0].tag_value == "username:cocoa"
Expand All @@ -117,7 +134,7 @@ def test_tag_value_primary_is_email(self):
},
project_id=self.project.id,
)
euser = EventUser.for_projects([self.project], {"email": "[email protected]"})
euser = EventUser.for_projects([self.project], {"email": ["[email protected]"]})
assert len(euser) == 1
assert euser[0].user_ident is None
assert euser[0].username is None
Expand All @@ -130,15 +147,24 @@ def test_tag_value_primary_is_ip(self):
"id": None,
"email": None,
"username": None,
"ip_address": "8.8.8.8",
"ip_address": "8.8.8.1",
},
"timestamp": iso_format(before_now(seconds=30)),
},
project_id=self.project.id,
)
euser = EventUser.for_projects([self.project], {"ip": "8.8.8.8"})
euser = EventUser.for_projects([self.project], {"ip": ["8.8.8.1"]})
assert len(euser) == 1
assert euser[0].user_ident is None
assert euser[0].username is None
assert euser[0].email is None
assert euser[0].tag_value == "ip:8.8.8.8"
assert euser[0].tag_value == "ip:8.8.8.1"

def test_for_tags(self):
assert EventUser.for_tags(self.project.id, ["id:myminion"]) == {
"id:myminion": EventUser.from_event(self.event_3)
}
assert EventUser.for_tags(self.project.id, ["id:doesnotexist"]) == {}
assert EventUser.for_tags(self.project.id, ["id:myminion", "id:doesnotexist"]) == {
"id:myminion": EventUser.from_event(self.event_3)
}

0 comments on commit e59edf3

Please sign in to comment.