Skip to content

fix: Renormalize once #12991

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.

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
merged 2 commits into from
Apr 30, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions src/sentry/event_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,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 @@ -512,7 +512,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.exceptions import PluginError
Expand Down Expand Up @@ -113,6 +114,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