Skip to content

Commit

Permalink
ref: Remove sampling code from eventstream (#14829)
Browse files Browse the repository at this point in the history
Since we no longer sample events, we can remove sampling related code
from the eventstream.
  • Loading branch information
lynnagara authored Sep 26, 2019
1 parent 37d6b90 commit 5c629a3
Show file tree
Hide file tree
Showing 7 changed files with 28 additions and 135 deletions.
12 changes: 2 additions & 10 deletions src/sentry/eventstream/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ def _dispatch_post_process_group_task(
self,
event,
is_new,
is_sample,
is_regression,
is_new_group_environment,
primary_hash,
Expand All @@ -47,7 +46,6 @@ def _dispatch_post_process_group_task(
post_process_group.delay(
event=event,
is_new=is_new,
is_sample=is_sample,
is_regression=is_regression,
is_new_group_environment=is_new_group_environment,
primary_hash=primary_hash,
Expand All @@ -58,20 +56,14 @@ def insert(
group,
event,
is_new,
is_sample,
is_regression,
is_new_group_environment,
primary_hash,
skip_consume=False,
is_sample=False, # TODO: Remove once this is no longer passed
):
self._dispatch_post_process_group_task(
event,
is_new,
is_sample,
is_regression,
is_new_group_environment,
primary_hash,
skip_consume,
event, is_new, is_regression, is_new_group_environment, primary_hash, skip_consume
)

def start_delete_groups(self, project_id, group_ids):
Expand Down
2 changes: 1 addition & 1 deletion src/sentry/eventstream/kafka/protocol.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ def get_task_kwargs_for_insert(operation, event_data, task_state=None):
"primary_hash": event_data["primary_hash"],
}

for name in ("is_new", "is_sample", "is_regression", "is_new_group_environment"):
for name in ("is_new", "is_regression", "is_new_group_environment"):
kwargs[name] = task_state[name]

return kwargs
Expand Down
14 changes: 3 additions & 11 deletions src/sentry/eventstream/snuba.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,11 @@ def insert(
group,
event,
is_new,
is_sample,
is_regression,
is_new_group_environment,
primary_hash,
skip_consume=False,
is_sample=False, # TODO: Remove once this is no longer passed
):
project = event.project
retention_days = quotas.get_event_retention(organization=project.organization)
Expand Down Expand Up @@ -121,7 +121,6 @@ def insert(
},
{
"is_new": is_new,
"is_sample": is_sample,
"is_regression": is_regression,
"is_new_group_environment": is_new_group_environment,
"skip_consume": skip_consume,
Expand Down Expand Up @@ -240,28 +239,21 @@ def insert(
group,
event,
is_new,
is_sample,
is_regression,
is_new_group_environment,
primary_hash,
skip_consume=False,
is_sample=False, # TODO: Remove once this is no longer passed
):
super(SnubaEventStream, self).insert(
group,
event,
is_new,
is_sample,
is_regression,
is_new_group_environment,
primary_hash,
skip_consume,
)
self._dispatch_post_process_group_task(
event,
is_new,
is_sample,
is_regression,
is_new_group_environment,
primary_hash,
skip_consume,
event, is_new, is_regression, is_new_group_environment, primary_hash, skip_consume
)
1 change: 0 additions & 1 deletion src/sentry/management/commands/backfill_eventstream.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ def _attach_related(_events):
group=event.group,
event=event,
is_new=False,
is_sample=False,
is_regression=False,
is_new_group_environment=False,
primary_hash=primary_hash,
Expand Down
8 changes: 1 addition & 7 deletions tests/sentry/eventstream/kafka/test_protocol.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,7 @@ def test_get_task_kwargs_for_message_version_1():
"primary_hash": "49f68a5c8493ec2c0bf489821c21fc3b",
}

task_state = {
"is_new": True,
"is_sample": False,
"is_regression": False,
"is_new_group_environment": True,
}
task_state = {"is_new": True, "is_regression": False, "is_new_group_environment": True}

kwargs = get_task_kwargs_for_message(json.dumps([1, "insert", event_data, task_state]))
event = kwargs.pop("event")
Expand All @@ -56,7 +51,6 @@ def test_get_task_kwargs_for_message_version_1():
assert kwargs.pop("primary_hash") == "49f68a5c8493ec2c0bf489821c21fc3b"

assert kwargs.pop("is_new") is True
assert kwargs.pop("is_sample") is False
assert kwargs.pop("is_regression") is False
assert kwargs.pop("is_new_group_environment") is True

Expand Down
114 changes: 19 additions & 95 deletions tests/sentry/tasks/post_process/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,7 @@ def test_issueless(
):
event = self.create_issueless_event(project=self.project)
post_process_group(
event=event,
is_new=True,
is_regression=False,
is_sample=False,
is_new_group_environment=True,
event=event, is_new=True, is_regression=False, is_new_group_environment=True
)

mock_processor.assert_not_called() # NOQA
Expand All @@ -56,11 +52,7 @@ def test_rule_processor(self, mock_processor):
mock_processor.return_value.apply.return_value = [(mock_callback, mock_futures)]

post_process_group(
event=event,
is_new=True,
is_regression=False,
is_sample=False,
is_new_group_environment=True,
event=event, is_new=True, is_regression=False, is_new_group_environment=True
)

mock_processor.assert_called_once_with(event, True, False, True, False)
Expand All @@ -86,11 +78,7 @@ def test_group_refresh(self, mock_processor):
mock_processor.return_value.apply.return_value = [(mock_callback, mock_futures)]

post_process_group(
event=event,
is_new=True,
is_regression=False,
is_sample=False,
is_new_group_environment=True,
event=event, is_new=True, is_regression=False, is_new_group_environment=True
)

assert event.group == group2
Expand All @@ -103,11 +91,7 @@ def test_invalidates_snooze(self, mock_processor):
snooze = GroupSnooze.objects.create(group=group, until=timezone.now() - timedelta(hours=1))

post_process_group(
event=event,
is_new=True,
is_regression=False,
is_sample=False,
is_new_group_environment=True,
event=event, is_new=True, is_regression=False, is_new_group_environment=True
)

mock_processor.assert_called_with(event, True, False, True, True)
Expand All @@ -124,11 +108,7 @@ def test_maintains_valid_snooze(self, mock_processor):
snooze = GroupSnooze.objects.create(group=group, until=timezone.now() + timedelta(hours=1))

post_process_group(
event=event,
is_new=True,
is_regression=False,
is_sample=False,
is_new_group_environment=True,
event=event, is_new=True, is_regression=False, is_new_group_environment=True
)

mock_processor.assert_called_with(event, True, False, True, False)
Expand Down Expand Up @@ -158,11 +138,7 @@ def test_owner_assignment_path_precedence(self):
project_id=self.project.id,
)
post_process_group(
event=event,
is_new=False,
is_regression=False,
is_sample=False,
is_new_group_environment=False,
event=event, is_new=False, is_regression=False, is_new_group_environment=False
)
assignee = event.group.assignee_set.first()
assert assignee.user is None
Expand All @@ -179,11 +155,7 @@ def test_owner_assignment_assign_user(self):
project_id=self.project.id,
)
post_process_group(
event=event,
is_new=False,
is_regression=False,
is_sample=False,
is_new_group_environment=False,
event=event, is_new=False, is_regression=False, is_new_group_environment=False
)
assignee = event.group.assignee_set.first()
assert assignee.user == self.user
Expand All @@ -199,11 +171,7 @@ def test_owner_assignment_ownership_no_matching_owners(self):
project_id=self.project.id,
)
post_process_group(
event=event,
is_new=False,
is_regression=False,
is_sample=False,
is_new_group_environment=False,
event=event, is_new=False, is_regression=False, is_new_group_environment=False
)
assert not event.group.assignee_set.exists()

Expand All @@ -219,11 +187,7 @@ def test_owner_assignment_existing_assignment(self):
)
event.group.assignee_set.create(team=self.team, project=self.project)
post_process_group(
event=event,
is_new=False,
is_regression=False,
is_sample=False,
is_new_group_environment=False,
event=event, is_new=False, is_regression=False, is_new_group_environment=False
)
assignee = event.group.assignee_set.first()
assert assignee.user is None
Expand All @@ -243,11 +207,7 @@ def test_owner_assignment_owner_is_gone(self):
project_id=self.project.id,
)
post_process_group(
event=event,
is_new=False,
is_regression=False,
is_sample=False,
is_new_group_environment=False,
event=event, is_new=False, is_regression=False, is_new_group_environment=False
)
assignee = event.group.assignee_set.first()
assert assignee is None
Expand All @@ -266,11 +226,7 @@ def test_service_hook_fires_on_new_event(self, mock_process_service_hook):

with self.feature("projects:servicehooks"):
post_process_group(
event=event,
is_new=False,
is_regression=False,
is_sample=False,
is_new_group_environment=False,
event=event, is_new=False, is_regression=False, is_new_group_environment=False
)

mock_process_service_hook.delay.assert_called_once_with(servicehook_id=hook.id, event=event)
Expand All @@ -295,11 +251,7 @@ def test_service_hook_fires_on_alert(self, mock_processor, mock_process_service_

with self.feature("projects:servicehooks"):
post_process_group(
event=event,
is_new=False,
is_regression=False,
is_sample=False,
is_new_group_environment=False,
event=event, is_new=False, is_regression=False, is_new_group_environment=False
)

mock_process_service_hook.delay.assert_called_once_with(servicehook_id=hook.id, event=event)
Expand All @@ -323,11 +275,7 @@ def test_service_hook_does_not_fire_without_alert(

with self.feature("projects:servicehooks"):
post_process_group(
event=event,
is_new=False,
is_regression=False,
is_sample=False,
is_new_group_environment=False,
event=event, is_new=False, is_regression=False, is_new_group_environment=False
)

assert not mock_process_service_hook.delay.mock_calls
Expand All @@ -343,11 +291,7 @@ def test_service_hook_does_not_fire_without_event(self, mock_process_service_hoo

with self.feature("projects:servicehooks"):
post_process_group(
event=event,
is_new=True,
is_regression=False,
is_sample=False,
is_new_group_environment=False,
event=event, is_new=True, is_regression=False, is_new_group_environment=False
)

assert not mock_process_service_hook.delay.mock_calls
Expand All @@ -358,11 +302,7 @@ def test_processes_resource_change_task_on_new_group(self, delay):
event = self.create_event(group=group)

post_process_group(
event=event,
is_new=True,
is_regression=False,
is_sample=False,
is_new_group_environment=False,
event=event, is_new=True, is_regression=False, is_new_group_environment=False
)

delay.assert_called_once_with(action="created", sender="Group", instance_id=group.id)
Expand All @@ -389,11 +329,7 @@ def test_processes_resource_change_task_on_error_events(self, delay):
)

post_process_group(
event=event,
is_new=False,
is_regression=False,
is_sample=False,
is_new_group_environment=False,
event=event, is_new=False, is_regression=False, is_new_group_environment=False
)

kwargs = {"instance": event}
Expand All @@ -411,11 +347,7 @@ def test_processes_resource_change_task_not_called_for_non_errors(self, delay):
)

post_process_group(
event=event,
is_new=False,
is_regression=False,
is_sample=False,
is_new_group_environment=False,
event=event, is_new=False, is_regression=False, is_new_group_environment=False
)

assert not delay.called
Expand All @@ -429,11 +361,7 @@ def test_processes_resource_change_task_not_called_without_feature_flag(self, de
)

post_process_group(
event=event,
is_new=False,
is_regression=False,
is_sample=False,
is_new_group_environment=False,
event=event, is_new=False, is_regression=False, is_new_group_environment=False
)

assert not delay.called
Expand All @@ -457,11 +385,7 @@ def test_processes_resource_change_task_not_called_without_error_created(self, d
)

post_process_group(
event=event,
is_new=False,
is_regression=False,
is_sample=False,
is_new_group_environment=False,
event=event, is_new=False, is_regression=False, is_new_group_environment=False
)

assert not delay.called
Expand Down
Loading

0 comments on commit 5c629a3

Please sign in to comment.