Skip to content

ref(grouping): Add SDK name as a tag on event_manager metrics #59379

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
vartec merged 7 commits into from
Nov 6, 2023
Merged

ref(grouping): Add SDK name as a tag on event_manager metrics #59379

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
d8ad392
Adding SDK name as a tag to event_manager grouping metrics
Nov 3, 2023
5ab4528
adding testing
Nov 3, 2023
4fe1f2b
adding test for unknown SDK
Nov 3, 2023
0b11727
Merge branch 'master' into vartec/add_sdk_to_grouping_metrics
Nov 3, 2023
24bd476
not adding SDK tag to 'next_short_id.timeout' metric
Nov 4, 2023
08f9e03
more tags
Nov 4, 2023
0c0a135
Merge branch 'master' into vartec/add_sdk_to_grouping_metrics
Nov 4, 2023
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
101 changes: 60 additions & 41 deletions src/sentry/event_manager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,10 @@ def save(

return jobs[0]["event"]
else:
metric_tags = {"platform": job["event"].platform or "unknown"}
metric_tags = {
"platform": job["event"].platform or "unknown",
"sdk": job["event"].data.get("sdk", {}).get("name") or "unknown",
}
# This metric allows differentiating from all calls to the `event_manager.save` metric
# and adds support for differentiating based on platforms
with metrics.timer("event_manager.save_error_events", tags=metric_tags):
Expand Down Expand Up @@ -776,7 +779,7 @@ def _auto_update_grouping(project: Project) -> None:
"sentry:secondary_grouping_expiry": expiry,
"sentry:grouping_config": new_grouping,
}
for (key, value) in changes.items():
for key, value in changes.items():
Copy link
Copy Markdown
Contributor Author

@vartec vartec Nov 3, 2023

Choose a reason for hiding this comment

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

^^ auto-formatting

project.update_option(key, value)
create_system_audit_entry(
organization=project.organization,
Expand All @@ -786,11 +789,16 @@ def _auto_update_grouping(project: Project) -> None:
)


@metrics.wraps("event_manager.background_grouping")
def _calculate_background_grouping(
project: Project, event: Event, config: GroupingConfig
) -> CalculatedHashes:
return _calculate_event_grouping(project, event, config)
metric_tags: MutableTags = {
"grouping_config": config["id"],
"platform": event.platform or "unknown",
"sdk": event.data.get("sdk", {}).get("name") or "unknown",
}
with metrics.timer("event_manager.background_grouping", tags=metric_tags):
return _calculate_event_grouping(project, event, config)


def _run_background_grouping(project: Project, job: Job) -> None:
Expand Down Expand Up @@ -1581,7 +1589,6 @@ def _save_aggregate(
kwargs["data"]["last_received"] = received_timestamp

if existing_grouphash is None:

if killswitch_matches_context(
"store.load-shed-group-creation-projects",
{
Expand Down Expand Up @@ -1621,7 +1628,6 @@ def _save_aggregate(
root_hierarchical_grouphash = None

if existing_grouphash is None:

group = _create_group(project, event, **kwargs)

if (
Expand Down Expand Up @@ -1654,7 +1660,10 @@ def _save_aggregate(
metrics.incr(
"group.created",
skip_internal=True,
tags={"platform": event.platform or "unknown"},
tags={
"platform": event.platform or "unknown",
"sdk": event.data.get("sdk", {}).get("name") or "unknown",
},
)

# This only applies to events with stacktraces
Expand All @@ -1665,6 +1674,7 @@ def _save_aggregate(
sample_rate=1.0,
tags={
"platform": event.platform or "unknown",
"sdk": event.data.get("sdk", {}).get("name") or "unknown",
"frame_mix": frame_mix,
},
)
Expand Down Expand Up @@ -1826,7 +1836,9 @@ def _create_group(project: Project, event: Event, **kwargs: Any) -> Group:
except OperationalError:
metrics.incr(
"next_short_id.timeout",
tags={"platform": event.platform or "unknown"},
tags={
"platform": event.platform or "unknown"
}, # TODO: remove this tag, it's nor relevant
)
sentry_sdk.capture_message("short_id.timeout")
raise HashDiscarded("Timeout when getting next_short_id", reason="timeout")
Expand Down Expand Up @@ -2207,7 +2219,10 @@ def discard_event(job: Job, attachments: Sequence[Attachment]) -> None:
metrics.incr(
"events.discarded",
skip_internal=True,
tags={"platform": job["platform"]},
tags={
"platform": job["platform"],
"sdk": job["event"].data.get("sdk", {}).get("name") or "unknown",
},
)


Expand Down Expand Up @@ -2455,7 +2470,6 @@ def _materialize_event_metrics(jobs: Sequence[Job]) -> None:
job["event_metrics"] = event_metrics


@metrics.wraps("save_event.calculate_event_grouping")
def _calculate_event_grouping(
project: Project, event: Event, grouping_config: GroupingConfig
) -> CalculatedHashes:
Expand All @@ -2466,40 +2480,42 @@ def _calculate_event_grouping(
metric_tags: MutableTags = {
"grouping_config": grouping_config["id"],
"platform": event.platform or "unknown",
"sdk": event.data.get("sdk", {}).get("name") or "unknown",
}

with metrics.timer("event_manager.normalize_stacktraces_for_grouping", tags=metric_tags):
with sentry_sdk.start_span(op="event_manager.normalize_stacktraces_for_grouping"):
event.normalize_stacktraces_for_grouping(load_grouping_config(grouping_config))

# Detect & set synthetic marker if necessary
detect_synthetic_exception(event.data, grouping_config)

with metrics.timer("event_manager.apply_server_fingerprinting"):
# 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("save_event.calculate_event_grouping", tags=metric_tags):
with metrics.timer("event_manager.normalize_stacktraces_for_grouping", tags=metric_tags):
with sentry_sdk.start_span(op="event_manager.normalize_stacktraces_for_grouping"):
event.normalize_stacktraces_for_grouping(load_grouping_config(grouping_config))

# 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,
)

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):
# 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
hashes.write_to_event(event.data)
return hashes


@metrics.wraps("save_event.calculate_span_grouping")
Expand All @@ -2518,7 +2534,10 @@ def _calculate_span_grouping(jobs: Sequence[Job], projects: ProjectsMapping) ->
metrics.incr(
"save_event.transaction.span_group_count.default",
amount=len(unique_default_hashes),
tags={"platform": job["platform"] or "unknown"},
tags={
"platform": job["platform"] or "unknown",
"sdk": event.data.get("sdk", {}).get("name") or "unknown",
},
)
except Exception:
sentry_sdk.capture_exception()
Expand Down
28 changes: 25 additions & 3 deletions tests/sentry/event_manager/test_event_manager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2301,7 +2301,6 @@ def test_perf_issue_creation(self):
@override_options({"performance.issues.all.problem-detection": 1.0})
@override_options({"performance.issues.n_plus_one_db.problem-creation": 1.0})
def test_perf_issue_update(self):

with mock.patch("sentry_sdk.tracing.Span.containing_transaction"):
event = self.create_performance_issue(
event_data=make_event(**get_event("n-plus-one-in-django-index-view"))
Expand Down Expand Up @@ -2441,7 +2440,9 @@ def attempt_to_generate_slow_db_issue() -> Event:

@patch("sentry.event_manager.metrics.incr")
def test_new_group_metrics_logging(self, mock_metrics_incr: MagicMock) -> None:
manager = EventManager(make_event(platform="javascript"))
manager = EventManager(
make_event(platform="javascript", sdk={"name": "sentry.javascript.foo"})
)
manager.normalize()
manager.save(self.project.id)

Expand All @@ -2450,12 +2451,32 @@ def test_new_group_metrics_logging(self, mock_metrics_incr: MagicMock) -> None:
skip_internal=True,
tags={
"platform": "javascript",
"sdk": "sentry.javascript.foo",
},
)

@patch("sentry.event_manager.metrics.incr")
def test_new_group_metrics_logging_no_platform_no_sdk(
self, mock_metrics_incr: MagicMock
) -> None:
manager = EventManager(make_event(platform=None, sdk=None))
manager.normalize()
manager.save(self.project.id)

mock_metrics_incr.assert_any_call(
"group.created",
skip_internal=True,
tags={
"platform": "other",
"sdk": "unknown",
},
)

def test_new_group_metrics_logging_with_frame_mix(self) -> None:
with patch("sentry.event_manager.metrics.incr") as mock_metrics_incr:
manager = EventManager(make_event(platform="javascript"))
manager = EventManager(
make_event(platform="javascript", sdk={"name": "sentry.javascript.foo"})
)
manager.normalize()
# IRL, `normalize_stacktraces_for_grouping` adds frame mix metadata to the event, but we
# can't mock that because it's imported inside its calling function to avoid circular imports
Expand All @@ -2468,6 +2489,7 @@ def test_new_group_metrics_logging_with_frame_mix(self) -> None:
tags={
"platform": "javascript",
"frame_mix": "in-app-only",
"sdk": "sentry.javascript.foo",
},
)

Expand Down