Skip to content

feat(snuba): Support start/end filter in get_adjacent_event_ids{,_snql} #93272

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/sentry/eventstore/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,8 @@ def get_adjacent_event_ids_snql(
group_id: int,
environments: list[str],
event: Event | GroupEvent,
start: datetime | None = None,
end: datetime | None = None,
conditions: list[Condition] | None = None,
):
raise NotImplementedError
Expand Down
22 changes: 14 additions & 8 deletions src/sentry/eventstore/snuba/backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -460,14 +460,16 @@ def get_adjacent_event_ids_snql(
group_id: int | None,
environments: Sequence[str],
event: Event | GroupEvent,
start: datetime | None = None,
end: datetime | None = None,
conditions: Sequence[Condition] | None = None,
):
"""
Utility function for grabbing an event's adjascent events,
Utility function for grabbing an event's adjacent events,
which are the ones with the closest timestamps before and after.
This function is only used in project_event_details at the moment,
so it's interface is tailored to that. We use SnQL and use the project_id
and toStartOfDay(timestamp) to efficently scan our table
and toStartOfDay(timestamp) to efficiently scan our table
"""
dataset = self._get_dataset_for_event(event)
app_id = "eventstore"
Expand All @@ -492,12 +494,15 @@ def make_constant_conditions():
*project_conditions,
]

lower_bound = start or (event.datetime - timedelta(days=100))
upper_bound = end or (event.datetime + timedelta(days=100))
Comment on lines +497 to +498
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should clamp start and end here just to be safe.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can let snuba handle this. Will ask around though.


def make_prev_timestamp_conditions(event):
return [
Condition(
Column(DATASETS[dataset][Columns.TIMESTAMP.value.alias]),
Op.GTE,
event.datetime - timedelta(days=100),
lower_bound,
),
Condition(
Column(DATASETS[dataset][Columns.TIMESTAMP.value.alias]),
Expand All @@ -521,7 +526,7 @@ def make_next_timestamp_conditions(event):
Condition(
Column(DATASETS[dataset][Columns.TIMESTAMP.value.alias]),
Op.LT,
event.datetime + timedelta(days=100),
upper_bound,
),
Condition(
Column(DATASETS[dataset][Columns.TIMESTAMP.value.alias]),
Expand Down Expand Up @@ -600,9 +605,9 @@ def get_adjacent_event_ids(self, event, filter):
prev_filter = deepcopy(filter)
prev_filter.conditions = prev_filter.conditions or []
prev_filter.conditions.extend(get_before_event_condition(event))

# We only store 90 days of data, add a few extra days just in case
prev_filter.start = event.datetime - timedelta(days=100)
if not prev_filter.start:
# We only store 90 days of data, add a few extra days just in case
prev_filter.start = event.datetime - timedelta(days=100)
# the previous event can have the same timestamp, add 1 second
# to the end condition since it uses a less than condition
prev_filter.end = event.datetime + timedelta(seconds=1)
Expand All @@ -612,7 +617,8 @@ def get_adjacent_event_ids(self, event, filter):
next_filter.conditions = next_filter.conditions or []
next_filter.conditions.extend(get_after_event_condition(event))
next_filter.start = event.datetime
next_filter.end = datetime.utcnow()
if not next_filter.end:
next_filter.end = datetime.utcnow()
next_filter.orderby = ASC_ORDERING

dataset = self._get_dataset_for_event(event)
Expand Down
131 changes: 82 additions & 49 deletions tests/sentry/eventstore/snuba/test_backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
class SnubaEventStorageTest(TestCase, SnubaTestCase, PerformanceIssueTestCase):
def setUp(self):
super().setUp()
self.min_ago = before_now(minutes=1).isoformat()
self.two_min_ago = before_now(minutes=2).isoformat()
self.min_ago = before_now(minutes=1)
self.two_min_ago = before_now(minutes=2)
self.project1 = self.create_project()
self.project2 = self.create_project()

Expand All @@ -26,7 +26,7 @@ def setUp(self):
"type": "default",
"platform": "python",
"fingerprint": ["group1"],
"timestamp": self.two_min_ago,
"timestamp": self.two_min_ago.isoformat(),
"tags": {"foo": "1"},
},
project_id=self.project1.id,
Expand All @@ -37,7 +37,7 @@ def setUp(self):
"type": "default",
"platform": "python",
"fingerprint": ["group1"],
"timestamp": self.min_ago,
"timestamp": self.min_ago.isoformat(),
"tags": {"foo": "1"},
},
project_id=self.project2.id,
Expand All @@ -48,7 +48,7 @@ def setUp(self):
"type": "default",
"platform": "python",
"fingerprint": ["group2"],
"timestamp": self.min_ago,
"timestamp": self.min_ago.isoformat(),
"tags": {"foo": "1"},
},
project_id=self.project2.id,
Expand Down Expand Up @@ -176,7 +176,7 @@ def test_get_event_by_id_cached(self):
dummy_event = Event(
project_id=self.project2.id,
event_id="1" * 32,
data={"something": "hi", "timestamp": self.min_ago, "type": "error"},
data={"something": "hi", "timestamp": self.min_ago.isoformat(), "type": "error"},
)
mock_event.return_value = dummy_event
event = self.eventstore.get_event_by_id(self.project2.id, "1" * 32)
Expand All @@ -185,7 +185,7 @@ def test_get_event_by_id_cached(self):

# Now we store the event properly, so it will exist in Snuba.
self.store_event(
data={"event_id": "1" * 32, "timestamp": self.min_ago, "type": "error"},
data={"event_id": "1" * 32, "timestamp": self.min_ago.isoformat(), "type": "error"},
project_id=self.project2.id,
)

Expand Down Expand Up @@ -217,33 +217,47 @@ def test_get_event_beyond_retention(self):
def test_get_adjacent_event_ids(self):
event = self.eventstore.get_event_by_id(self.project2.id, "b" * 32)

_filter = Filter(project_ids=[self.project1.id, self.project2.id])
filter = Filter(project_ids=[self.project1.id, self.project2.id])

prev_event, next_event = self.eventstore.get_adjacent_event_ids(event, filter=_filter)
prev_event, next_event = self.eventstore.get_adjacent_event_ids(event, filter=filter)

assert prev_event == (str(self.project1.id), "a" * 32)

# Events with the same timestamp are sorted by event_id
assert next_event == (str(self.project2.id), "c" * 32)

# Returns None if no event
prev_event_none, next_event_none = self.eventstore.get_adjacent_event_ids(
None, filter=_filter
# Returns None if the prev event is outside the start window
period_filter = Filter(
project_ids=[self.project1.id, self.project2.id],
start=self.min_ago,
)
prev_event, next_event = self.eventstore.get_adjacent_event_ids(event, filter=period_filter)
assert prev_event is None
assert next_event == (str(self.project2.id), "c" * 32)

# Returns None if the next event is outside the end window
period_filter = Filter(
project_ids=[self.project1.id, self.project2.id],
end=self.min_ago,
)
prev_event, next_event = self.eventstore.get_adjacent_event_ids(event, filter=period_filter)
assert prev_event == (str(self.project1.id), "a" * 32)
assert next_event is None

assert prev_event_none is None
assert next_event_none is None
# Returns None if no event
prev_event, next_event = self.eventstore.get_adjacent_event_ids(None, filter=filter)

assert prev_event is None
assert next_event is None

# Returns None if the query fails for a known reason
with mock.patch(
"sentry.utils.snuba.bulk_raw_query", side_effect=snuba.QueryOutsideRetentionError()
):
prev_event_none, next_event_none = self.eventstore.get_adjacent_event_ids(
event, filter=_filter
)
prev_event, next_event = self.eventstore.get_adjacent_event_ids(event, filter=filter)

assert prev_event_none is None
assert next_event_none is None
assert prev_event is None
assert next_event is None

def test_adjacent_event_ids_same_timestamp(self):
project = self.create_project()
Expand All @@ -254,7 +268,7 @@ def test_adjacent_event_ids_same_timestamp(self):
"type": "default",
"platform": "python",
"fingerprint": ["group"],
"timestamp": self.min_ago,
"timestamp": self.min_ago.isoformat(),
},
project_id=project.id,
)
Expand All @@ -264,7 +278,7 @@ def test_adjacent_event_ids_same_timestamp(self):
"type": "default",
"platform": "python",
"fingerprint": ["group"],
"timestamp": self.min_ago,
"timestamp": self.min_ago.isoformat(),
},
project_id=project.id,
)
Expand Down Expand Up @@ -316,36 +330,33 @@ def test_transaction_get_next_prev_event_id(self):

def test_get_adjacent_event_ids_snql(self):
project = self.create_project()
data = {
"type": "default",
"platform": "python",
"fingerprint": ["group"],
}
self.store_event(
data={**data, "event_id": "a" * 32, "timestamp": before_now(minutes=10).isoformat()},
project_id=project.id,
)
event1 = self.store_event(
data={
"event_id": "a" * 32,
"type": "default",
"platform": "python",
"fingerprint": ["group"],
"timestamp": self.two_min_ago,
},
data={**data, "event_id": "b" * 32, "timestamp": before_now(minutes=4).isoformat()},
project_id=project.id,
)
event2 = self.store_event(
data={
"event_id": "b" * 32,
"type": "default",
"platform": "python",
"fingerprint": ["group"],
"timestamp": self.min_ago,
},
data={**data, "event_id": "c" * 32, "timestamp": before_now(minutes=3).isoformat()},
project_id=project.id,
)
event3 = self.store_event(
data={
"event_id": "c" * 32,
"type": "default",
"platform": "python",
"fingerprint": ["group"],
"timestamp": before_now(minutes=0).isoformat(),
},
data={**data, "event_id": "d" * 32, "timestamp": before_now(minutes=2).isoformat()},
project_id=project.id,
)
self.store_event(
data={**data, "event_id": "e" * 32, "timestamp": before_now(minutes=0).isoformat()},
project_id=project.id,
)

# Finds next and previous IDs
prev_ids, next_ids = self.eventstore.get_adjacent_event_ids_snql(
organization_id=event2.organization.id,
project_id=event2.project_id,
Expand All @@ -357,6 +368,28 @@ def test_get_adjacent_event_ids_snql(self):
assert prev_ids == (str(event1.project_id), event1.event_id)
assert next_ids == (str(event3.project_id), event3.event_id)

# Filter previous events that are outside of the start window
prev_ids, _ = self.eventstore.get_adjacent_event_ids_snql(
organization_id=event1.organization.id,
project_id=event1.project_id,
group_id=event1.group_id,
environments=[],
event=event1,
start=before_now(minutes=5),
)
assert prev_ids is None

# Filter next events that are outside of the end window
_, next_ids = self.eventstore.get_adjacent_event_ids_snql(
organization_id=event3.organization.id,
project_id=event3.project_id,
group_id=event3.group_id,
environments=[],
event=event3,
end=before_now(minutes=1),
)
assert next_ids is None

def test_get_adjacent_event_ids_snql_order_of_event_ids(self):
project = self.create_project()
event1 = self.store_event(
Expand All @@ -365,7 +398,7 @@ def test_get_adjacent_event_ids_snql_order_of_event_ids(self):
"type": "default",
"platform": "python",
"fingerprint": ["group"],
"timestamp": self.two_min_ago,
"timestamp": self.two_min_ago.isoformat(),
},
project_id=project.id,
)
Expand All @@ -375,7 +408,7 @@ def test_get_adjacent_event_ids_snql_order_of_event_ids(self):
"type": "default",
"platform": "python",
"fingerprint": ["group"],
"timestamp": self.min_ago,
"timestamp": self.min_ago.isoformat(),
},
project_id=project.id,
)
Expand Down Expand Up @@ -408,7 +441,7 @@ def test_adjacent_event_ids_same_timestamp_snql(self):
"type": "default",
"platform": "python",
"fingerprint": ["group"],
"timestamp": self.min_ago,
"timestamp": self.min_ago.isoformat(),
},
project_id=project.id,
)
Expand All @@ -418,7 +451,7 @@ def test_adjacent_event_ids_same_timestamp_snql(self):
"type": "default",
"platform": "python",
"fingerprint": ["group"],
"timestamp": self.min_ago,
"timestamp": self.min_ago.isoformat(),
},
project_id=project.id,
)
Expand Down Expand Up @@ -458,7 +491,7 @@ def test_adjacent_event_ids_with_query_conditions(self):
"type": "default",
"platform": "python",
"fingerprint": ["group"],
"timestamp": self.min_ago,
"timestamp": self.min_ago.isoformat(),
"tags": {"organization.slug": "sentry"},
},
project_id=project.id,
Expand All @@ -469,7 +502,7 @@ def test_adjacent_event_ids_with_query_conditions(self):
"type": "default",
"platform": "python",
"fingerprint": ["group"],
"timestamp": self.min_ago,
"timestamp": self.min_ago.isoformat(),
},
project_id=project.id,
)
Expand All @@ -479,7 +512,7 @@ def test_adjacent_event_ids_with_query_conditions(self):
"type": "default",
"platform": "python",
"fingerprint": ["group"],
"timestamp": self.min_ago,
"timestamp": self.min_ago.isoformat(),
"tags": {"organization.slug": "sentry"},
},
project_id=project.id,
Expand Down
Loading