From bfc10975713d2de243ba8096e584f84e073db2de Mon Sep 17 00:00:00 2001 From: David Cramer Date: Tue, 5 Apr 2016 09:49:35 -0700 Subject: [PATCH 1/2] Handle GroupTagKey and GroupTagValue when merging issues This reverts commit ef9986a9563a6cc44f9a7c3aca15cc3e67dfb404. Fixes SENTRY-10F --- src/sentry/tasks/merge.py | 23 ++++++++++++++++++++++- tests/sentry/tasks/test_merge.py | 31 ++++++++++++++++++++++++++++++- 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/src/sentry/tasks/merge.py b/src/sentry/tasks/merge.py index bc3a48cb486a0a..bb76f52790cb0f 100644 --- a/src/sentry/tasks/merge.py +++ b/src/sentry/tasks/merge.py @@ -153,13 +153,15 @@ def _rehash_group_events(group, limit=100): def merge_objects(models, group, new_group, limit=1000, logger=None): + from sentry.models import GroupTagKey, GroupTagValue has_more = False for model in models: if logger is not None: logger.info('Merging %r objects where %r into %r', model, group, new_group) - has_group = 'group' in model._meta.get_all_field_names() + all_fields = model._meta.get_all_field_names() + has_group = 'group' in all_fields if has_group: queryset = model.objects.filter(group=group) else: @@ -179,7 +181,26 @@ def merge_objects(models, group, new_group, limit=1000, delete = True else: delete = False + if delete: + # Before deleting, we want to merge in counts + try: + if model == GroupTagKey: + with transaction.atomic(using=router.db_for_write(model)): + model.objects.filter( + group=new_group, + key=obj.key, + ).update(values_seen=F('values_seen') + obj.values_seen) + elif model == GroupTagValue: + with transaction.atomic(using=router.db_for_write(model)): + model.objects.filter( + group=new_group, + key=obj.key, + value=obj.value, + ).update(times_seen=F('times_seen') + obj.times_seen) + except DataError: + # it's possible to hit an out of range value for counters + pass obj.delete() has_more = True diff --git a/tests/sentry/tasks/test_merge.py b/tests/sentry/tasks/test_merge.py index 4fadf85c90a5cb..317e5391419bb6 100644 --- a/tests/sentry/tasks/test_merge.py +++ b/tests/sentry/tasks/test_merge.py @@ -1,7 +1,7 @@ from __future__ import absolute_import from sentry.tasks.merge import merge_group, rehash_group_events -from sentry.models import Event, Group, GroupRedirect +from sentry.models import Event, Group, GroupRedirect, GroupTagKey from sentry.testutils import TestCase @@ -51,6 +51,35 @@ def test_merge_creates_redirect(self): group_id=groups[2].id, ).count() == 2 + def test_merge_updates_tag_values_seen(self): + project = self.create_project() + groups = [self.create_group(project) for _ in xrange(0, 2)] + + for group in groups: + GroupTagKey.objects.create( + project=project, + group=group, + key='sentry:user', + values_seen=1, + ) + GroupTagKey.objects.create( + project=project, + group=group, + key='foo', + values_seen=5, + ) + + with self.tasks(): + merge_group(groups[0].id, groups[1].id) + + assert not Group.objects.filter(id=groups[0].id).exists() + assert not GroupTagKey.objects.filter(group_id=groups[0].id).exists() + + assert GroupTagKey.objects.get( + group_id=groups[1].id, + key='sentry:user', + ).values_seen == 2 + class RehashGroupEventsTest(TestCase): def test_simple(self): From f8986e0b252977ea788920f6da1504000e2fa57f Mon Sep 17 00:00:00 2001 From: Matt Robenolt Date: Tue, 5 Apr 2016 12:16:03 -0700 Subject: [PATCH 2/2] Add more test coverage for merging --- tests/sentry/tasks/test_merge.py | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/tests/sentry/tasks/test_merge.py b/tests/sentry/tasks/test_merge.py index 317e5391419bb6..df785a91a1f9e0 100644 --- a/tests/sentry/tasks/test_merge.py +++ b/tests/sentry/tasks/test_merge.py @@ -1,7 +1,7 @@ from __future__ import absolute_import from sentry.tasks.merge import merge_group, rehash_group_events -from sentry.models import Event, Group, GroupRedirect, GroupTagKey +from sentry.models import Event, Group, GroupRedirect, GroupTagKey, GroupTagValue from sentry.testutils import TestCase @@ -68,17 +68,43 @@ def test_merge_updates_tag_values_seen(self): key='foo', values_seen=5, ) + GroupTagValue.objects.create( + project=project, + group=group, + key='key1', + times_seen=1, + ) + GroupTagValue.objects.create( + project=project, + group=group, + key='key2', + times_seen=5, + ) with self.tasks(): merge_group(groups[0].id, groups[1].id) assert not Group.objects.filter(id=groups[0].id).exists() assert not GroupTagKey.objects.filter(group_id=groups[0].id).exists() + assert not GroupTagValue.objects.filter(group_id=groups[0].id).exists() assert GroupTagKey.objects.get( group_id=groups[1].id, key='sentry:user', ).values_seen == 2 + assert GroupTagKey.objects.get( + group_id=groups[1].id, + key='foo', + ).values_seen == 10 + + assert GroupTagValue.objects.get( + group_id=groups[1].id, + key='key1', + ).times_seen == 2 + assert GroupTagValue.objects.get( + group_id=groups[1].id, + key='key2', + ).times_seen == 10 class RehashGroupEventsTest(TestCase):