Skip to content

Commit

Permalink
fix: Renormalize once (#12991)
Browse files Browse the repository at this point in the history
  • Loading branch information
untitaker authored and BYK committed Jul 12, 2019
1 parent 0a9d43d commit 08ac2c6
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 4 deletions.
4 changes: 2 additions & 2 deletions src/sentry/event_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
)
from sentry.interfaces.base import get_interface
from sentry.models import (
Activity, Environment, Event, EventError, EventMapping, EventUser, Group,
Activity, Environment, Event, EventDict, EventError, EventMapping, EventUser, Group,
GroupEnvironment, GroupHash, GroupLink, GroupRelease, GroupResolution, GroupStatus,
Project, Release, ReleaseEnvironment, ReleaseProject,
ReleaseProjectEnvironment, UserReport, Organization,
Expand Down Expand Up @@ -515,7 +515,7 @@ def _get_event_instance(self, project_id=None):
return Event(
project_id=project_id or self._project.id,
event_id=event_id,
data=data,
data=EventDict(data, skip_renormalization=True),
time_spent=time_spent,
datetime=date,
platform=platform
Expand Down
12 changes: 11 additions & 1 deletion src/sentry/models/event.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,17 @@ class EventDict(CanonicalKeyDict):
"""

def __init__(self, data, skip_renormalization=False, **kwargs):
if not skip_renormalization and not isinstance(data, EventDict):
is_renormalized = (
isinstance(data, EventDict) or
(isinstance(data, NodeData) and isinstance(data.data, EventDict))
)

with configure_scope() as scope:
scope.set_tag("rust.is_renormalized", is_renormalized)
scope.set_tag("rust.skip_renormalization", skip_renormalization)
scope.set_tag("rust.renormalized", "null")

if not skip_renormalization and not is_renormalized:
rust_renormalized = _should_skip_to_python(data.get('event_id'))
if rust_renormalized:
normalizer = StoreNormalizer(is_renormalize=True)
Expand Down
5 changes: 5 additions & 0 deletions src/sentry/tasks/post_process.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
from django.conf import settings

from sentry import features
from sentry.models import EventDict
from sentry.utils import snuba
from sentry.utils.cache import cache
from sentry.plugins import plugins
Expand Down Expand Up @@ -100,6 +101,10 @@ def post_process_group(event, is_new, is_regression, is_sample, is_new_group_env
from sentry.rules.processor import RuleProcessor
from sentry.tasks.servicehooks import process_service_hook

# Re-bind node data to avoid renormalization. We only want to
# renormalize when loading old data from the database.
event.data = EventDict(event.data, skip_renormalization=True)

# Re-bind Group since we're pickling the whole Event object
# which may contain a stale Group.
event.group, _ = get_group_with_redirect(event.group_id)
Expand Down
6 changes: 6 additions & 0 deletions src/sentry/utils/pytest/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,12 @@ def factories():
return Factories


@pytest.fixture
def task_runner():
from sentry.testutils.helpers.task_runner import TaskRunner
return TaskRunner


@pytest.fixture(scope='function')
def session():
return factories.create_session()
Expand Down
39 changes: 38 additions & 1 deletion tests/sentry/models/test_event.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
from __future__ import absolute_import

import pytest
import pickle

from sentry.models import Environment
from sentry.testutils import TestCase
from sentry.db.models.fields.node import NodeData
from sentry.event_manager import EventManager
from sentry.testutils import TestCase


class EventTest(TestCase):
Expand Down Expand Up @@ -162,6 +163,42 @@ def test_ip_address(self):
assert event.ip_address is None


@pytest.mark.django_db
def test_renormalization(monkeypatch, factories, task_runner, default_project):
from semaphore.processing import StoreNormalizer

old_normalize = StoreNormalizer.normalize_event
normalize_mock_calls = []

def normalize(*args, **kwargs):
normalize_mock_calls.append(1)
return old_normalize(*args, **kwargs)

monkeypatch.setattr('semaphore.processing.StoreNormalizer.normalize_event',
normalize)

sample_mock_calls = []

def sample(*args, **kwargs):
sample_mock_calls.append(1)
return False

with task_runner():
factories.store_event(
data={
'event_id': 'a' * 32,
'environment': 'production',
},
project_id=default_project.id
)

# Assert we only renormalize this once. If this assertion fails it's likely
# that you will encounter severe performance issues during event processing
# or postprocessing.
assert len(normalize_mock_calls) == 1
assert len(sample_mock_calls) == 0


class EventGetLegacyMessageTest(TestCase):
def test_message(self):
event = self.create_event(message='foo bar')
Expand Down

0 comments on commit 08ac2c6

Please sign in to comment.