Skip to content

Commit cafa5df

Browse files
authored
feat(grouping): Add metrics for issue merging and unmerging (#52919)
This adds DataDog metrics for instances of issues being merged and unmerged. Both metrics include the issues' platform, and the merge metric additionally includes referer (since you can merge from either the issue stream or the Similar Issues tab). A few missing merge-related tests were also added.
1 parent 7e13772 commit cafa5df

File tree

4 files changed

+139
-4
lines changed

4 files changed

+139
-4
lines changed

Diff for: src/sentry/api/endpoints/group_hashes.py

+8
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
from sentry.api.serializers import EventSerializer, serialize
1111
from sentry.models import GroupHash
1212
from sentry.tasks.unmerge import unmerge
13+
from sentry.utils import metrics
1314
from sentry.utils.snuba import raw_query
1415

1516

@@ -61,6 +62,13 @@ def delete(self, request: Request, group) -> Response:
6162
if not hash_list:
6263
return Response()
6364

65+
metrics.incr(
66+
"grouping.unmerge_issues",
67+
sample_rate=1.0,
68+
# We assume that if someone's merged groups, they were all from the same platform
69+
tags={"platform": group.platform or "unknown"},
70+
)
71+
6472
unmerge.delay(
6573
group.project_id, group.id, None, hash_list, request.user.id if request.user else None
6674
)

Diff for: src/sentry/api/helpers/group_index/update.py

+23
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
from __future__ import annotations
22

3+
import re
34
from collections import defaultdict
45
from datetime import datetime, timedelta
56
from typing import Any, Dict, Mapping, MutableMapping, Sequence
7+
from urllib.parse import urlparse
68

79
import rest_framework
810
from django.db import IntegrityError, transaction
@@ -650,6 +652,27 @@ def update_groups(
650652
if len(projects) > 1:
651653
return Response({"detail": "Merging across multiple projects is not supported"})
652654

655+
referer = urlparse(request.META.get("HTTP_REFERER", "")).path
656+
issue_stream_regex = r"^(\/organizations\/[^\/]+)?\/issues\/$"
657+
similar_issues_tab_regex = r"^(\/organizations\/[^\/]+)?\/issues\/\d+\/similar\/$"
658+
659+
metrics.incr(
660+
"grouping.merge_issues",
661+
sample_rate=1.0,
662+
tags={
663+
# We assume that if someone's merging groups, they're from the same platform
664+
"platform": group_list[0].platform or "unknown",
665+
# TODO: It's probably cleaner to just send this value from the front end
666+
"referer": (
667+
"issue stream"
668+
if re.search(issue_stream_regex, referer)
669+
else "similar issues tab"
670+
if re.search(similar_issues_tab_regex, referer)
671+
else "unknown"
672+
),
673+
},
674+
)
675+
653676
result["merge"] = handle_merge(group_list, project_lookup, acting_user)
654677

655678
inbox = result.get("inbox", None)

Diff for: tests/sentry/api/endpoints/test_group_hashes.py

+11-3
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import copy
2+
from unittest.mock import patch
23
from urllib.parse import urlencode
34

45
from sentry.eventstream.snuba import SnubaEventStream
@@ -94,7 +95,7 @@ def test_return_multiple_hashes(self):
9495
def test_unmerge(self):
9596
self.login_as(user=self.user)
9697

97-
group = self.create_group()
98+
group = self.create_group(platform="javascript")
9899

99100
hashes = [
100101
GroupHash.objects.create(project=group.project, group=group, hash=hash)
@@ -108,5 +109,12 @@ def test_unmerge(self):
108109
]
109110
)
110111

111-
response = self.client.delete(url, format="json")
112-
assert response.status_code == 202, response.content
112+
with patch("sentry.api.endpoints.group_hashes.metrics.incr") as mock_metrics_incr:
113+
response = self.client.delete(url, format="json")
114+
115+
assert response.status_code == 202, response.content
116+
mock_metrics_incr.assert_any_call(
117+
"grouping.unmerge_issues",
118+
sample_rate=1.0,
119+
tags={"platform": "javascript"},
120+
)

Diff for: tests/sentry/api/helpers/test_group_index.py

+97-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
from datetime import datetime, timedelta
2-
from unittest.mock import Mock, patch
2+
from unittest.mock import MagicMock, Mock, patch
33

44
import pytest
55
from django.http import QueryDict
@@ -264,6 +264,102 @@ def test_ignore_with_substatus_archived_until_escalating(self, send_robust: Mock
264264
assert not GroupInbox.objects.filter(group=group).exists()
265265

266266

267+
class MergeGroupsTest(TestCase):
268+
@patch("sentry.api.helpers.group_index.update.handle_merge")
269+
def test_simple(self, mock_handle_merge: MagicMock):
270+
group_ids = [self.create_group().id, self.create_group().id]
271+
project = self.project
272+
273+
request = self.make_request(method="PUT")
274+
request.user = self.user
275+
request.data = {"merge": 1}
276+
request.GET = {"id": group_ids, "project": [project.id]}
277+
278+
update_groups(request, group_ids, [project], self.organization.id, search_fn=Mock())
279+
280+
call_args = mock_handle_merge.call_args.args
281+
282+
assert len(call_args) == 3
283+
# Have to convert to ids because first argument is a queryset
284+
assert [group.id for group in call_args[0]] == group_ids
285+
assert call_args[1] == {project.id: project}
286+
assert call_args[2] == self.user
287+
288+
@patch("sentry.api.helpers.group_index.update.handle_merge")
289+
def test_multiple_projects(self, mock_handle_merge: MagicMock):
290+
project1 = self.create_project()
291+
project2 = self.create_project()
292+
projects = [project1, project2]
293+
project_ids = [project.id for project in projects]
294+
295+
group_ids = [
296+
self.create_group(project1).id,
297+
self.create_group(project2).id,
298+
]
299+
300+
request = self.make_request(method="PUT")
301+
request.user = self.user
302+
request.data = {"merge": 1}
303+
request.GET = {"id": group_ids, "project": project_ids}
304+
305+
response = update_groups(
306+
request, group_ids, projects, self.organization.id, search_fn=Mock()
307+
)
308+
309+
assert response.data == {"detail": "Merging across multiple projects is not supported"}
310+
assert mock_handle_merge.call_count == 0
311+
312+
def test_metrics(self):
313+
for referer, expected_referer_tag in [
314+
("https://sentry.io/organizations/dogsaregreat/issues/", "issue stream"),
315+
("https://dogsaregreat.sentry.io/issues/", "issue stream"),
316+
(
317+
"https://sentry.io/organizations/dogsaregreat/issues/12311121/similar/",
318+
"similar issues tab",
319+
),
320+
(
321+
"https://dogsaregreat.sentry.io/issues/12311121/similar/",
322+
"similar issues tab",
323+
),
324+
(
325+
"https://sentry.io/organizations/dogsaregreat/some/other/path/",
326+
"unknown",
327+
),
328+
(
329+
"https://dogsaregreat.sentry.io/some/other/path/",
330+
"unknown",
331+
),
332+
(
333+
"",
334+
"unknown",
335+
),
336+
]:
337+
338+
group_ids = [
339+
self.create_group(platform="javascript").id,
340+
self.create_group(platform="javascript").id,
341+
]
342+
project = self.project
343+
344+
request = self.make_request(method="PUT")
345+
request.user = self.user
346+
request.data = {"merge": 1}
347+
request.GET = {"id": group_ids, "project": [project.id]}
348+
request.META = {"HTTP_REFERER": referer}
349+
350+
with patch("sentry.api.helpers.group_index.update.metrics.incr") as mock_metrics_incr:
351+
update_groups(request, group_ids, [project], self.organization.id, search_fn=Mock())
352+
353+
mock_metrics_incr.assert_any_call(
354+
"grouping.merge_issues",
355+
sample_rate=1.0,
356+
tags={
357+
"platform": "javascript",
358+
"referer": expected_referer_tag,
359+
},
360+
)
361+
362+
267363
class TestHandleIsSubscribed(TestCase):
268364
def setUp(self) -> None:
269365
self.group = self.create_group()

0 commit comments

Comments
 (0)