From ad82e40de55e6a67aab6ba91ba4f56968ce25035 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Fri, 2 Feb 2018 15:23:28 -0800 Subject: [PATCH] fix(breadcrumbs): Ensure maximum lengths on several attributes - Trim message, category, and level attributes - Update pytest support to work in other environments --- .travis.yml | 3 ++- Makefile | 2 +- raven/base.py | 4 +++- raven/breadcrumbs.py | 41 +++++++++++++++++++++++++++++--------- setup.cfg | 2 +- setup.py | 3 +++ tests/breadcrumbs/tests.py | 10 ++++++++++ 7 files changed, 52 insertions(+), 13 deletions(-) diff --git a/.travis.yml b/.travis.yml index 51d0ab76e..1e2bcbd11 100644 --- a/.travis.yml +++ b/.travis.yml @@ -143,7 +143,8 @@ jobs: script: tox install: - - pip install tox wheel codecov "coverage<4" + - make + - pip install codecov before_script: - pip freeze after_success: diff --git a/Makefile b/Makefile index f7ab43140..5d40fed6b 100644 --- a/Makefile +++ b/Makefile @@ -4,7 +4,7 @@ bootstrap: test: bootstrap lint @echo "Running Python tests" - py.test -x tests + py.test -f tests @echo "" lint: diff --git a/raven/base.py b/raven/base.py index 9be3a0f35..058013f00 100644 --- a/raven/base.py +++ b/raven/base.py @@ -498,7 +498,9 @@ def build_msg(self, event_type, data=None, date=None, # raven client internally in sentry and the alternative # submission option of a list here is not supported by the # internal sender. - data.setdefault('breadcrumbs', {'values': crumbs}) + data.setdefault('breadcrumbs', { + 'values': crumbs + }) return data diff --git a/raven/breadcrumbs.py b/raven/breadcrumbs.py index 3d626d1c5..13987f133 100644 --- a/raven/breadcrumbs.py +++ b/raven/breadcrumbs.py @@ -1,18 +1,22 @@ from __future__ import absolute_import import os -import time import logging + +from time import time from types import FunctionType -from raven.utils.compat import iteritems, get_code, text_type, string_types from raven.utils import once +from raven.utils.encoding import to_unicode +from raven.utils.compat import iteritems, get_code, text_type, string_types + +CATEGORY_MAX_LENGTH = 128 +LEVEL_MAX_LENGTH = 16 special_logging_handlers = [] special_logger_handlers = {} - logger = logging.getLogger('raven') @@ -28,9 +32,10 @@ def event_payload_considered_equal(a, b): class BreadcrumbBuffer(object): - def __init__(self, limit=100): + def __init__(self, limit=100, message_max_length=1024): self.buffer = [] self.limit = limit + self.message_max_length = message_max_length def record(self, timestamp=None, level=None, message=None, category=None, data=None, type=None, processor=None): @@ -38,20 +43,31 @@ def record(self, timestamp=None, level=None, message=None, raise ValueError('You must pass either `message`, `data`, ' 'or `processor`') if timestamp is None: - timestamp = time.time() - self.buffer.append(({ + timestamp = time() + + # we format here to ensure we dont bloat memory due to message size + result = (self.format({ 'type': type or 'default', - 'timestamp': timestamp, + 'timestamp': float(timestamp), 'level': level, + # hardcode message length to prevent huge crumbs 'message': message, 'category': category, + # TODO(dcramer): we should trim data 'data': data, - }, processor)) + }), processor) + self.buffer.append(result) del self.buffer[:-self.limit] def clear(self): del self.buffer[:] + def format(self, result): + result['message'] = to_unicode(result['message'])[:self.message_max_length] if result['message'] else None + result['category'] = to_unicode(result['category'])[:CATEGORY_MAX_LENGTH] if result['category'] else None + result['level'] = to_unicode(result['level'])[:LEVEL_MAX_LENGTH].lower() if result['level'] else None + return result + def get_buffer(self): rv = [] for idx, (payload, processor) in enumerate(self.buffer): @@ -59,9 +75,16 @@ def get_buffer(self): try: processor(payload) except Exception: + raise logger.exception('Failed to process breadcrumbs. Ignored') payload = None + else: + # we format here to ensure we dont bloat memory due to message size + payload = self.format(payload) if payload else None self.buffer[idx] = (payload, None) + elif payload is not None: + payload = self.format(payload) + if payload is not None and \ (not rv or not event_payload_considered_equal(rv[-1], payload)): rv.append(payload) @@ -92,7 +115,7 @@ def record(message=None, timestamp=None, level=None, category=None, on a specific client. """ if timestamp is None: - timestamp = time.time() + timestamp = time() for ctx in raven.context.get_active_contexts(): ctx.breadcrumbs.record(timestamp, level, message, category, data, type, processor) diff --git a/setup.cfg b/setup.cfg index 8c465d8c9..86ec0f18b 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,6 +1,6 @@ [tool:pytest] python_files=test*.py -addopts=--tb=native -p no:doctest -p no:logging --cov=raven -nauto +addopts=--tb=native -p no:doctest -p no:logging --cov=raven norecursedirs=raven build bin dist docs htmlcov hooks node_modules .* {args} DJANGO_SETTINGS_MODULE = tests.contrib.django.settings python_paths = tests diff --git a/setup.py b/setup.py index 3911020d6..c3f774cc9 100755 --- a/setup.py +++ b/setup.py @@ -59,6 +59,7 @@ tests_require = [ 'bottle', 'celery>=2.5', + 'coverage<4', 'exam>=0.5.2', 'flake8==3.5.0', 'logbook', @@ -75,8 +76,10 @@ 'pytest-flake8==0.9.1', 'requests', 'tornado>=4.1', + 'tox', 'webob', 'webtest', + 'wheel', 'anyjson', 'ZConfig', ] + ( diff --git a/tests/breadcrumbs/tests.py b/tests/breadcrumbs/tests.py index 1acc4b42d..9c68aca3b 100644 --- a/tests/breadcrumbs/tests.py +++ b/tests/breadcrumbs/tests.py @@ -34,6 +34,16 @@ def test_log_crumb_reporting(self): assert crumbs[0]['data'] == {'blah': 'baz'} assert crumbs[0]['message'] == 'This is a message with foo!' + def test_log_crumb_reporting_with_large_message(self): + client = Client('http://foo:bar@example.com/0') + with client.context: + log = logging.getLogger('whatever.foo') + log.info('a' * 4096) + crumbs = client.context.breadcrumbs.get_buffer() + + assert len(crumbs) == 1 + assert crumbs[0]['message'] == 'a' * 1024 + def test_log_location(self): out = StringIO() logger = logging.getLogger(__name__)