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
refactor wsgi request logging, for more context #492
Conversation
self.old_statsd = talisker.statsd.get_client.raw_update( | ||
self.statsd_client) | ||
talisker.logs.add_talisker_handler(logging.NOTSET, self.handler) | ||
self.sentry_context.start() | ||
Context.new() | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FTR - this is a driveby UX improvement - the previous lines generates logs, moving the new context to here means those logs are not captured as part of the context.
WSGIMetric.errors.inc(**labels) | ||
|
||
return extra | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These two methods are virtually unchanged from their module function versions.
extra={'request_id': rid} | ||
) | ||
else: | ||
REQUESTS[rid] = request | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We now track the entire request object, rather than just the environ
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Renames WSGIResponse to TaliskerWSGIRequest, as it's not really just a response. Moves the logic of request logging/metrics into the class, so more context is available. Later changes will use this extra context to enhance sentry reports.