Skip to content

Commit

Permalink
Sentry logging breadcrumb fixes
Browse files Browse the repository at this point in the history
Ensure that there is an active sentry context by activating it before
each request. Previously, there was no active context for the very first
request a worker served.

Also improves logging breadcrumbs:
 - remove duplicated information
 - condense file/lineno/funcName into one item
 - stringify keys so breadcrumb displays properly

Cleans up the depreacted use of LogRecord._structured
  • Loading branch information
Simon Davy committed Dec 11, 2019
1 parent 10923a1 commit 70097f3
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 17 deletions.
2 changes: 1 addition & 1 deletion talisker/context.py
Expand Up @@ -65,7 +65,7 @@
else:
early_log(
__name__,
'error',
'warning',
'aiocontextvars is installed, but it does not function with '
'python {}. Please use python >= 3.5.3 if you wish to use '
'talisker with asyncio.'.format(
Expand Down
3 changes: 1 addition & 2 deletions talisker/logs.py
Expand Up @@ -342,7 +342,6 @@ def format(self, record):
# we never want sentry to capture DEBUG logs in its breadcrumbs, as
# they may be sensitive
if record.levelno > logging.DEBUG:

talisker.sentry.record_log_breadcrumb(record)

if len(record.message) > self.MAX_MSG_SIZE:
Expand All @@ -356,7 +355,7 @@ def format(self, record):
s = self._fmt % record.__dict__

# add our structured tags *before* exception info is added
structured = getattr(record, '_structured', {})
structured = getattr(record, 'extra', {})
if record.exc_info and 'errno' not in structured:
structured.update(get_errno_fields(record.exc_info[1]))
if structured:
Expand Down
31 changes: 24 additions & 7 deletions talisker/sentry.py
Expand Up @@ -54,6 +54,7 @@
'get_client',
'record_breadcrumb',
'report_wsgi',
'new_context',
'set_client',
]

Expand All @@ -75,6 +76,9 @@
import raven.handlers.logging
except ImportError:
# dummy APIs that do nothing
def new_context():
pass

def record_breadcrumb(*args, **kwargs):
pass

Expand Down Expand Up @@ -112,6 +116,14 @@ def __exit__(self, exc_type, exc, traceback):
raven.breadcrumbs.ignore_logger('talisker.slowqueries')
raven.breadcrumbs.ignore_logger('talisker.requests')

FILTERED_EXTRAS = {
'request_id', # do not need request_id for each log breadcrumb
'exc_type', # exc info already displayed
}

def new_context():
get_client().context.activate()

def record_breadcrumb(*args, **kwargs):
raven.breadcrumbs.record(*args, **kwargs)

Expand All @@ -124,7 +136,7 @@ def record_log_breadcrumb(record):
record.message,
record.args,
{
'extra': getattr(record, '_structured', {}),
'extra': getattr(record, 'extra', {}),
'exc_info': record.exc_info,
'stack_info': getattr(record, 'stack_info', None)
},
Expand All @@ -141,14 +153,19 @@ def record_log_breadcrumb(record):

def processor(data):
metadata = {
'path': record.pathname,
'lineno': record.lineno,
'location': '{}:{}:{}'.format(
record.pathname,
record.lineno,
getattr(record, 'funcName', ''),
),
}
if hasattr(record, 'func'):
metadata['func'] = record.func
metadata.update(getattr(record, '_structured', {}))
extra = getattr(record, 'extra', {})
metadata.update(
(k, str(v)) for k, v in extra.items()
if k not in FILTERED_EXTRAS
)
data.update({
'message': record.message,
'message': record.getMessage(),
'category': record.name,
'level': record.levelname.lower(),
'data': metadata,
Expand Down
1 change: 1 addition & 0 deletions talisker/wsgi.py
Expand Up @@ -512,6 +512,7 @@ def __init__(self, app, environ=None, headers=None):

def __call__(self, environ, start_response):
Context.new()
talisker.sentry.new_context()
config = talisker.get_config()
# populate default values
Context.current.soft_timeout = config.soft_request_timeout
Expand Down
20 changes: 13 additions & 7 deletions tests/test_logging.py
Expand Up @@ -187,21 +187,27 @@ def test_logger_collects_raven_breadcrumbs():

fmt = logs.StructuredFormatter()
with raven.context.Context() as ctx:
fmt.format(make_record({'foo': 'bar'}, 'info'))
record = make_record({'foo': 'bar'}, 'info')
record.levelno = logging.DEBUG
record.levelname = 'debug'
record = make_record(
{
'foo': 'bar',
'number': 1,
},
'msg',
)
record.levelno = logging.INFO
record.levelname = 'info'
record.funcName = 'func'
fmt.format(record)
breadcrumbs = ctx.breadcrumbs.get_buffer()

assert len(breadcrumbs) == 1
assert breadcrumbs[0]['message'] == 'info'
assert breadcrumbs[0]['message'] == 'msg'
assert breadcrumbs[0]['level'] == 'info'
assert breadcrumbs[0]['category'] == 'name'
assert breadcrumbs[0]['data'] == {
'foo': 'bar',
'lineno': 'lno',
'path': 'fn',
'location': 'fn:lno:func',
'number': '1',
}


Expand Down

0 comments on commit 70097f3

Please sign in to comment.