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 3 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
74 changes: 53 additions & 21 deletions 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 @@ -2561,28 +2562,34 @@ def _calculate_event_grouping(
# Detect & set synthetic marker if necessary
detect_synthetic_exception(event.data, grouping_config)

with metrics.timer("event_manager.apply_server_fingerprinting", tags=metric_tags):
# The active grouping config was put into the event in the
# normalize step before. We now also make sure that the
# fingerprint was set to `'{{ default }}' just in case someone
# removed it from the payload. The call to get_hashes will then
# look at `grouping_config` to pick the right parameters.
event.data["fingerprint"] = event.data.data.get("fingerprint") or ["{{ default }}"]
apply_server_fingerprinting(
event.data.data,
get_fingerprinting_config_for_project(project),
allow_custom_title=True,
)
hashes = None
# If the event platform is node, it could have a ChunkLoadError that we want to force group
if event.platform == "node":
jangjodi marked this conversation as resolved.
Show resolved Hide resolved
hashes = handle_chunk_load_error_hash(event)
jangjodi marked this conversation as resolved.
Show resolved Hide resolved

if not hashes:
with metrics.timer("event_manager.apply_server_fingerprinting", tags=metric_tags):
# The active grouping config was put into the event in the
# normalize step before. We now also make sure that the
# fingerprint was set to `'{{ default }}' just in case someone
# removed it from the payload. The call to get_hashes will then
# look at `grouping_config` to pick the right parameters.
event.data["fingerprint"] = event.data.data.get("fingerprint") or ["{{ default }}"]
apply_server_fingerprinting(
event.data.data,
get_fingerprinting_config_for_project(project),
allow_custom_title=True,
)

with metrics.timer("event_manager.event.get_hashes", tags=metric_tags):
# Here we try to use the grouping config that was requested in the
# 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)
except GroupingConfigNotFound:
event.data["grouping_config"] = get_grouping_config_dict_for_project(project)
hashes = event.get_hashes()
with metrics.timer("event_manager.event.get_hashes", tags=metric_tags):
jangjodi marked this conversation as resolved.
Show resolved Hide resolved
# Here we try to use the grouping config that was requested in the
# 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)
except GroupingConfigNotFound:
event.data["grouping_config"] = get_grouping_config_dict_for_project(project)
hashes = event.get_hashes()

hashes.write_to_event(event.data)
return hashes
Expand Down Expand Up @@ -2621,6 +2628,31 @@ def _detect_performance_problems(jobs: Sequence[Job], projects: ProjectsMapping)
)


def handle_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"]
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:
organization = Project.objects.get(id=event.project_id).organization
if features.has("organizations:group-chunk-load-errors", organization):
hashes = get_chunk_load_error_hash()
return hashes


def get_chunk_load_error_hash() -> CalculatedHashes:
return CalculatedHashes(
hashes=hashlib.md5(b"chunkloaderror").hexdigest(), hierarchical_hashes=[], tree_labels=[]
jangjodi marked this conversation as resolved.
Show resolved Hide resolved
)


class PerformanceJob(TypedDict, total=False):
performance_problems: Sequence[PerformanceProblem]
event: Event
Expand Down
134 changes: 134 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,136 @@ 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(self):
manager = EventManager(
make_event(
title="something",
message="ChunkLoadError: Loading chunk 123 failed",
event_id="a" * 32,
jangjodi marked this conversation as resolved.
Show resolved Hide resolved
platform="node",
exception={
"values": [
{
"type": "Error",
"stacktrace": {
"frames": [
{
"function": "in_app_function",
jangjodi marked this conversation as resolved.
Show resolved Hide resolved
}
]
},
"value": "ChunkLoadError: Loading chunk 123 failed",
}
]
},
)
)
with self.tasks():
manager.normalize()
event = manager.save(self.project.id)

manager = EventManager(
make_event(
title="else",
message="ChunkLoadError: Loading chunk 321 failed",
event_id="b" * 32,
platform="node",
exception={
"values": [
{
"type": "Error",
"stacktrace": {
"frames": [
{
"function": "different_in_app_function",
jangjodi marked this conversation as resolved.
Show resolved Hide resolved
}
]
},
"value": "ChunkLoadError: Loading chunk 321 failed",
}
]
},
)
)

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

assert event.group_id == event2.group_id

def test_do_not_group_chunk_load_errors(self):
jangjodi marked this conversation as resolved.
Show resolved Hide resolved
manager = EventManager(
make_event(
title="something",
message="ChunkLoadError: Loading chunk 123 failed",
event_id="a" * 32,
platform="node",
exception={
"values": [
{
"type": "Error",
"stacktrace": {
"frames": [
{
"function": "in_app_function",
}
]
},
"value": "ChunkLoadError: Loading chunk 123 failed",
}
]
},
)
)
with self.tasks():
manager.normalize()
event = manager.save(self.project.id)

manager = EventManager(
make_event(
title="else",
message="ChunkLoadError: Loading chunk 321 failed",
event_id="b" * 32,
platform="node",
exception={
"values": [
{
"type": "Error",
"stacktrace": {
"frames": [
{
"function": "different_in_app_function",
}
]
},
"value": "ChunkLoadError: Loading chunk 321 failed",
}
]
},
)
)

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):
lobsterkatie marked this conversation as resolved.
Show resolved Hide resolved
manager = EventManager(
make_event(
title="something",
message="",
event_id="a" * 32,
platform="node",
exception={},
)
)
with self.tasks():
manager.normalize()
event = manager.save(self.project.id)

assert event.group
Loading