Skip to content
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

feat(grouping): Group chunk load errors #60885

Merged
merged 7 commits into from
Dec 11, 2023
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
33 changes: 32 additions & 1 deletion src/sentry/event_manager.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from __future__ import annotations

import copy
import hashlib
import ipaddress
import logging
import mimetypes
Expand Down Expand Up @@ -2551,7 +2552,13 @@ def _calculate_event_grouping(
# event. If that config has since been deleted (because it was an
# experimental grouping config) we fall back to the default.
try:
hashes = event.get_hashes(grouping_config)
hashes = None
# If the event platform is javascript, it could have a ChunkLoadError that we want to force group
if event.platform == "javascript":
hashes = get_chunk_load_error_hash(event)

if not hashes:
hashes = event.get_hashes(grouping_config)
except GroupingConfigNotFound:
event.data["grouping_config"] = get_grouping_config_dict_for_project(project)
hashes = event.get_hashes()
Expand Down Expand Up @@ -2593,6 +2600,30 @@ def _detect_performance_problems(jobs: Sequence[Job], projects: ProjectsMapping)
)


def get_chunk_load_error_hash(event: Event) -> Optional[CalculatedHashes]:
"""
Return the same hash if the event is a ChunkLoadError, otherwise return None
"""
try:
exception_value = event.data["exception"]["values"][0]["value"]
exception_type = event.data["exception"]["values"][0]["type"]
except KeyError:
return None

hashes = None
# Only check for the flag after it is established if it's a ChunkLoadError to avoid
# unnecessary querying
Comment on lines +2614 to +2615
Copy link
Member

Choose a reason for hiding this comment

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

Good thinking!

if "ChunkLoadError" in exception_value or exception_type == "ChunkLoadError":
organization = Project.objects.get(id=event.project_id).organization
if features.has("organizations:group-chunk-load-errors", organization):
hashes = CalculatedHashes(
hashes=[hashlib.md5(b"chunkloaderror").hexdigest()],
hierarchical_hashes=[],
tree_labels=[],
)
return hashes


class PerformanceJob(TypedDict, total=False):
performance_problems: Sequence[PerformanceProblem]
event: Event
Expand Down
136 changes: 136 additions & 0 deletions tests/sentry/event_manager/test_event_manager_grouping.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from sentry.models.grouphash import GroupHash
from sentry.testutils.cases import TestCase
from sentry.testutils.helpers.datetime import before_now, freeze_time, iso_format
from sentry.testutils.helpers.features import with_feature
from sentry.testutils.silo import region_silo_test
from sentry.testutils.skips import requires_snuba
from sentry.tsdb.base import TSDBModel
Expand Down Expand Up @@ -342,3 +343,138 @@ def test_ignores_fingerprint_on_transaction_event(self):
)[event1.group.id]
== 1
)

@with_feature("organizations:group-chunk-load-errors")
def test_group_chunk_load_errors_flag_on(self):
"""
Test that when `group-chunk-load-errors` flag is on, ChunkLoadErrors with different stack
traces and values are grouped together
"""
manager = EventManager(
make_event(
platform="javascript",
exception={
"values": [
{
"type": "Error",
"stacktrace": {
"frames": [
{
"function": "in_app_function",
}
]
},
"value": "ChunkLoadError: Loading chunk 123 failed",
"in_app": True,
}
]
},
)
)
with self.tasks():
manager.normalize()
event = manager.save(self.project.id)

manager = EventManager(
make_event(
platform="javascript",
exception={
"values": [
{
"type": "ChunkLoadError",
"stacktrace": {
"frames": [
{
"function": "different_in_app_function",
}
]
},
"value": "Loading chunk 321 failed",
"in_app": True,
}
]
},
)
)

with self.tasks():
manager.normalize()
event2 = manager.save(self.project.id)

assert event.group_id == event2.group_id

@with_feature({"organizations:group-chunk-load-errors": False})
def test_do_not_group_chunk_load_errors_flag_off(self):
"""
Test that when `group-chunk-load-errors` flag is off, ChunkLoadErrors with different stack
traces and values are not grouped together
"""
manager = EventManager(
make_event(
platform="javascript",
exception={
"values": [
{
"type": "Error",
"stacktrace": {
"frames": [
{
"function": "in_app_function",
}
]
},
"value": "ChunkLoadError: Loading chunk 123 failed",
"in_app": True,
}
]
},
)
)
with self.tasks():
manager.normalize()
event = manager.save(self.project.id)

manager = EventManager(
make_event(
platform="javascript",
exception={
"values": [
{
"type": "ChunkLoadError",
"stacktrace": {
"frames": [
{
"function": "different_in_app_function",
}
]
},
"value": "Loading chunk 321 failed",
"in_app": True,
}
]
},
)
)

with self.tasks():
manager.normalize()
event2 = manager.save(self.project.id)

assert event.group_id != event2.group_id

def test_chunk_load_errors_exception(self):
"""
Test that when an error does not have an exception, the `get_chunk_load_error_hash`
function catches the exception and does not stop execution
"""
manager = EventManager(
make_event(
platform="javascript",
exception={},
)
)
with self.tasks():
manager.normalize()
event = manager.save(self.project.id)

assert event.group