Browse Source

Merge pull request #1237 from jaysoffian/fix_truncate

Remove redundant truncation of {culprit, message}
David Cramer 10 years ago
parent
commit
4f4e222d09

+ 1 - 1
src/sentry/conf/server.py

@@ -661,7 +661,7 @@ SENTRY_MAX_EXTRA_VARIABLE_SIZE = 4096
 # keys
 SENTRY_MAX_DICTIONARY_ITEMS = 50
 
-SENTRY_MAX_MESSAGE_LENGTH = 1024 * 10
+SENTRY_MAX_MESSAGE_LENGTH = 1024 * 2
 SENTRY_MAX_STACKTRACE_FRAMES = 25
 
 # Gravatar service base url

+ 2 - 13
src/sentry/coreapi.py

@@ -16,14 +16,13 @@ import zlib
 from gzip import GzipFile
 
 from datetime import datetime, timedelta
-from django.conf import settings
 from django.utils.encoding import smart_str
 
 import six
 
 from sentry.app import cache, env
 from sentry.constants import (
-    DEFAULT_LOG_LEVEL, LOG_LEVELS, MAX_CULPRIT_LENGTH, MAX_TAG_VALUE_LENGTH,
+    DEFAULT_LOG_LEVEL, LOG_LEVELS, MAX_TAG_VALUE_LENGTH,
     MAX_TAG_KEY_LENGTH)
 from sentry.exceptions import InvalidTimestamp
 from sentry.interfaces.base import get_interface
@@ -32,7 +31,7 @@ from sentry.tasks.store import preprocess_event
 from sentry.utils import is_float, json
 from sentry.utils.auth import parse_auth_header
 from sentry.utils.compat import StringIO
-from sentry.utils.strings import decompress, truncatechars
+from sentry.utils.strings import decompress
 
 
 logger = logging.getLogger('sentry.coreapi.errors')
@@ -248,20 +247,10 @@ def validate_data(project, data, client=None):
         data['message'] = '<no message value>'
     elif not isinstance(data['message'], six.string_types):
         raise APIError('Invalid value for message')
-    elif len(data['message']) > settings.SENTRY_MAX_MESSAGE_LENGTH:
-        logger.info(
-            'Truncated value for message due to length (%d chars)',
-            len(data['message']), **client_metadata(client, project))
-        data['message'] = truncatechars(
-            data['message'], settings.SENTRY_MAX_MESSAGE_LENGTH)
 
     if data.get('culprit'):
         if not isinstance(data['culprit'], six.string_types):
             raise APIError('Invalid value for culprit')
-        logger.info(
-            'Truncated value for culprit due to length (%d chars)',
-            len(data['culprit']), **client_metadata(client, project))
-        data['culprit'] = truncatechars(data['culprit'], MAX_CULPRIT_LENGTH)
 
     if not data.get('event_id'):
         data['event_id'] = uuid.uuid4().hex

+ 2 - 1
src/sentry/event_manager.py

@@ -197,7 +197,8 @@ class EventManager(object):
             data['culprit'] = trim(data['culprit'], MAX_CULPRIT_LENGTH)
 
         if data['message']:
-            data['message'] = trim(data['message'], 2048)
+            data['message'] = trim(
+                data['message'], settings.SENTRY_MAX_MESSAGE_LENGTH)
 
         return data
 

+ 0 - 7
tests/sentry/coreapi/tests.py

@@ -9,7 +9,6 @@ from uuid import UUID
 
 from sentry.models import Project, User
 from sentry.exceptions import InvalidTimestamp
-from sentry.constants import MAX_CULPRIT_LENGTH
 from sentry.coreapi import (
     extract_auth_vars, project_from_auth_vars, APIForbidden, ensure_has_ip,
     process_data_timestamp, validate_data, get_interface, APIError
@@ -238,12 +237,6 @@ class ValidateDataTest(BaseAPITest):
             'culprit': 1
         })
 
-    def test_long_culprit(self):
-        data = validate_data(self.project, {
-            'culprit': 'x' * (MAX_CULPRIT_LENGTH + 1)
-        })
-        assert len(data['culprit']) == MAX_CULPRIT_LENGTH
-
 
 class GetInterfaceTest(TestCase):
     def test_does_not_let_through_disallowed_name(self):

+ 17 - 0
tests/sentry/test_event_manager.py

@@ -6,6 +6,9 @@ import logging
 
 from mock import patch
 
+from django.conf import settings
+
+from sentry.constants import MAX_CULPRIT_LENGTH
 from sentry.event_manager import EventManager, get_hashes_for_event
 from sentry.models import Event, Group, Project, EventMapping
 from sentry.testutils import TestCase
@@ -105,6 +108,20 @@ class EventManagerTest(TestCase):
         assert group.last_seen.replace(microsecond=0) == event.datetime.replace(microsecond=0)
         assert group.message == event2.message
 
+    def test_long_culprit(self):
+        manager = EventManager(self.make_event(
+            culprit='x' * (MAX_CULPRIT_LENGTH + 1),
+        ))
+        data = manager.normalize()
+        assert len(data['culprit']) == MAX_CULPRIT_LENGTH
+
+    def test_long_message(self):
+        manager = EventManager(self.make_event(
+            message='x' * (settings.SENTRY_MAX_MESSAGE_LENGTH + 1),
+        ))
+        data = manager.normalize()
+        assert len(data['message']) == settings.SENTRY_MAX_MESSAGE_LENGTH
+
 
 class GetHashesFromEventTest(TestCase):
     @patch('sentry.interfaces.stacktrace.Stacktrace.compute_hashes')