Skip to content

ref(grouping): Rewrite hierarchical grouping tests #74144

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.

Sign up for GitHub

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
merged 9 commits into from
Jul 12, 2024
Merged

ref(grouping): Rewrite hierarchical grouping tests #74144

Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
84129d1
WIP minor refactor
armenzg Jul 11, 2024
10e593a
More changes
armenzg Jul 11, 2024
eec5fc2
Refactor
armenzg Jul 11, 2024
fdf97ec
Add hashes logic
armenzg Jul 11, 2024
b90de58
Minor change
armenzg Jul 11, 2024
740fa2a
Re-order functions
armenzg Jul 11, 2024
5b59591
Reduce the diff
armenzg Jul 11, 2024
6b932c1
Reduce the diff
armenzg Jul 11, 2024
91c244c
Reduce the diff
armenzg Jul 11, 2024
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
146 changes: 56 additions & 90 deletions tests/sentry/event_manager/test_hierarchical_hashes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,54 +202,19 @@ def test_partial_move(default_project, fast_save):


class EventManagerGroupingTest(TestCase):
def test_applies_secondary_grouping_hierarchical(self):
Copy link
Member Author

Choose a reason for hiding this comment

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

This is easier to review with the split view
image

project = self.project
project.update_option("sentry:grouping_config", "legacy:2019-03-12")
project.update_option("sentry:secondary_grouping_expiry", 0)
Copy link
Member Author

Choose a reason for hiding this comment

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

These 3 lines now live in the shared function set_options:
image


timestamp = time.time() - 300
Copy link
Member Author

Choose a reason for hiding this comment

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

save_event will not use timestamp anymore.


def save_event(ts_offset):
Copy link
Member Author

Choose a reason for hiding this comment

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

This function has been converted into a helper function. See down below.

ts = timestamp + ts_offset
manager = EventManager(
make_event(
message="foo 123",
event_id=hex(2**127 + int(ts))[-32:],
timestamp=ts,
exception={
"values": [
{
"type": "Hello",
"stacktrace": {
"frames": [
{
"function": "not_in_app_function",
},
{
"function": "in_app_function",
},
]
},
}
]
},
)
)
manager.normalize()
with self.tasks():
return manager.save(project.id)
def test_can_upgrade_to_hierarchical_config(self):
Copy link
Member Author

Choose a reason for hiding this comment

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

More meaningful naming.

self.set_options("legacy:2019-03-12") # Starting configuration

event = save_event(0)
event = self.save_event()

project.update_option("sentry:grouping_config", "mobile:2021-02-12")
project.update_option("sentry:secondary_grouping_config", "legacy:2019-03-12")
project.update_option("sentry:secondary_grouping_expiry", time.time() + (24 * 90 * 3600))
Copy link
Member Author

Choose a reason for hiding this comment

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

These 3 lines have been moved into a helper function.
image

self.transition_to_new_config("mobile:2021-02-12")

# Switching to newstyle grouping changes hashes as 123 will be removed
event2 = save_event(2)
# This event will have two sets of hashes
event2 = self.save_event()

# make sure that events did get into same group because of fallback grouping, not because of hashes which come from primary grouping only
# The hashes property between the two events do not intersect
assert not set(event.get_hashes().hashes) & set(event2.get_hashes().hashes)
# They are both grouped together
assert event.group_id == event2.group_id

group = Group.objects.get(id=event.group_id)
Expand All @@ -258,58 +223,23 @@ def save_event(ts_offset):
assert group.last_seen == event2.datetime

# After expiry, new events are still assigned to the same group:
project.update_option("sentry:secondary_grouping_expiry", 0)
event3 = save_event(4)
self.project.update_option("sentry:secondary_grouping_expiry", 0)
event3 = self.save_event()
assert event3.group_id == event2.group_id

def test_applies_downgrade_hierarchical(self):
project = self.project
project.update_option("sentry:grouping_config", "mobile:2021-02-12")
project.update_option("sentry:secondary_grouping_expiry", 0)

timestamp = time.time() - 300

def save_event(ts_offset):
ts = timestamp + ts_offset
manager = EventManager(
make_event(
message="foo 123",
event_id=hex(2**127 + int(ts))[-32:],
timestamp=ts,
exception={
"values": [
{
"type": "Hello",
"stacktrace": {
"frames": [
{
"function": "not_in_app_function",
},
{
"function": "in_app_function",
},
]
},
}
]
},
)
)
manager.normalize()
with self.tasks():
return manager.save(project.id)
def test_can_downgrade_from_hierarchical_config(self):
self.set_options("mobile:2021-02-12") # Starting configuration

event = save_event(0)
event = self.save_event()

project.update_option("sentry:grouping_config", "legacy:2019-03-12")
project.update_option("sentry:secondary_grouping_config", "mobile:2021-02-12")
project.update_option("sentry:secondary_grouping_expiry", time.time() + (24 * 90 * 3600))
self.transition_to_new_config("legacy:2019-03-12")

# Switching to newstyle grouping changes hashes as 123 will be removed
event2 = save_event(2)
# This event will have two sets of hashes
event2 = self.save_event()

# make sure that events did get into same group because of fallback grouping, not because of hashes which come from primary grouping only
# The hashes property between the two events do not intersect
assert not set(event.get_hashes().hashes) & set(event2.get_hashes().hashes)
# They are both grouped together
assert event.group_id == event2.group_id

group = Group.objects.get(id=event.group_id)
Expand All @@ -325,6 +255,42 @@ def save_event(ts_offset):
assert group.last_seen == event2.datetime

# After expiry, new events are still assigned to the same group:
project.update_option("sentry:secondary_grouping_expiry", 0)
event3 = save_event(4)
self.project.update_option("sentry:secondary_grouping_expiry", 0)
event3 = self.save_event()
assert event3.group_id == event2.group_id

def save_event(self):
manager = EventManager(
make_event(
message="foo 123",
event_id=hex(2**127)[-32:],
Copy link
Member Author

Choose a reason for hiding this comment

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

Using the timestamp offset was not necessary. Two events can still have the same ID.
image

exception={
"values": [
{
"type": "Hello",
"stacktrace": {
"frames": [
{"function": "not_in_app_function"},
{"function": "in_app_function"},
]
},
}
]
},
)
)
manager.normalize()
with self.tasks():
return manager.save(self.project.id)

def set_options(self, primary_config):
self.project.update_option("sentry:grouping_config", primary_config)
self.project.update_option("sentry:secondary_grouping_expiry", 0)

def transition_to_new_config(self, new_config):
original_config = self.project.get_option("sentry:grouping_config")
self.project.update_option("sentry:grouping_config", new_config)
self.project.update_option("sentry:secondary_grouping_config", original_config)
self.project.update_option(
"sentry:secondary_grouping_expiry", time.time() + (24 * 90 * 3600)
)
Loading