-
Notifications
You must be signed in to change notification settings - Fork 4
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
[Fix niteoweb/pyramid_heroku#22] Add request_id... #38
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
"""Set request ID in Sentry logs if sentry is imported, and in structlog logs | ||
if structlog is imported. | ||
""" | ||
|
||
|
||
try: | ||
import sentry_sdk | ||
IS_SENTRY_ENABLED = True | ||
except ImportError: | ||
IS_SENTRY_ENABLED = False | ||
|
||
try: | ||
import structlog | ||
IS_STRUCTLOG_ENABLED = True | ||
except ImportError: | ||
IS_STRUCTLOG_ENABLED = False | ||
|
||
|
||
def includeme(config): | ||
config.add_tween("pyramid_heroku.request_id.RequestIDLogger") | ||
|
||
|
||
class RequestIDLogger(object): | ||
"""Set request ID in sentry and structlog logs. | ||
|
||
If the request headers contain a request ID (X-Request-ID), it should be | ||
logged for better debugging. In case sentry is being used, log the request | ||
ID along with other info. Similarly, if structlog is being used, add the | ||
request ID in the logs. | ||
""" | ||
|
||
def __init__(self, handler, _): | ||
self.handler = handler | ||
|
||
def __call__(self, request): | ||
request_id = request.headers.get("X-Request-ID") | ||
|
||
if request_id is not None: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Less nesting is better. So it's better to have if not request_id:
return self.handler(request)
... business logic here ... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
if IS_SENTRY_ENABLED: | ||
with sentry_sdk.configure_scope() as scope: | ||
scope.set_tag("request_id", request_id) | ||
|
||
if IS_STRUCTLOG_ENABLED: | ||
WrappedDictClass = structlog.threadlocal.wrap_dict(dict) | ||
wrapped_request_id = WrappedDictClass({ | ||
"request_id": request_id | ||
}) | ||
structlog.configure(context_class=WrappedDictClass) | ||
|
||
return self.handler(request) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
import unittest | ||
|
||
from .test_herokuapp_access import TestHerokuappAccessTween | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please use absolute, not relative, imports. Easier to sort, and to move around, fever problems with circular dependencies, easier for IDEs to parse, etc. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed this file, since pytest discovers the test files. I was trying to do it with |
||
from .test_migrate import TestHerokuMigrate | ||
from .test_request_id import TestRequestIDLogger | ||
from .test_utils import test_expandvars_dict, test_safe_eval | ||
|
||
if __name__ == "__main__": | ||
unittest.main() |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,92 @@ | ||
"""Tests for RequestIDLogger tween""" | ||
|
||
from contextlib import contextmanager | ||
from pyramid import request | ||
from pyramid import testing | ||
|
||
import unittest | ||
from unittest import mock | ||
import structlog | ||
import sys | ||
|
||
|
||
SENTRY_TAGS = {} | ||
|
||
class MockScope(object): | ||
|
||
def __init__(self): | ||
# reset the tags for every new test | ||
SENTRY_TAGS = {} | ||
|
||
def set_tag(self, key, value): | ||
SENTRY_TAGS[key] = value | ||
|
||
|
||
class MockSentrySDK: | ||
@classmethod | ||
def configure_scope(cls): | ||
@contextmanager | ||
def inner(): | ||
yield MockScope() | ||
return inner() | ||
|
||
|
||
sys.modules['sentry_sdk'] = MockSentrySDK | ||
|
||
|
||
class TestRequestIDLogger(unittest.TestCase): | ||
def setUp(self): | ||
self.config = testing.setUp() | ||
self.request = request.Request({}) | ||
|
||
self.handler = mock.Mock() | ||
self.registry = None | ||
|
||
self.request_id = "some-random-requestid" | ||
|
||
def tearDown(self): | ||
testing.tearDown() | ||
|
||
def test_request_id_in_sentry(self): | ||
from pyramid_heroku.request_id import RequestIDLogger | ||
|
||
self.request.headers["X-Request-ID"] = self.request_id | ||
RequestIDLogger(self.handler, self.registry)(self.request) | ||
self.handler.assert_called_with(self.request) | ||
self.assertEqual(SENTRY_TAGS, {"request_id": self.request_id}) | ||
|
||
def test_request_id_in_structlog(self): | ||
structlog.reset_defaults() | ||
from pyramid_heroku.request_id import RequestIDLogger | ||
|
||
context_class = structlog.get_config().get('context_class') | ||
self.assertEqual(context_class, dict) | ||
|
||
self.request.headers["X-Request-ID"] = self.request_id | ||
RequestIDLogger(self.handler, self.registry)(self.request) | ||
self.handler.assert_called_with(self.request) | ||
|
||
context_class = structlog.get_config().get('context_class') | ||
self.assertEqual( | ||
context_class._tl.dict_, {"request_id": self.request_id} | ||
) | ||
|
||
def test_no_request_id_in_sentry(self): | ||
from pyramid_heroku.request_id import RequestIDLogger | ||
|
||
RequestIDLogger(self.handler, self.registry)(self.request) | ||
self.handler.assert_called_with(self.request) | ||
self.assertEqual(SENTRY_TAGS, {}) | ||
|
||
def test_no_request_id_in_structlog(self): | ||
structlog.reset_defaults() | ||
from pyramid_heroku.request_id import RequestIDLogger | ||
|
||
context_class = structlog.get_config().get('context_class') | ||
self.assertEqual(context_class, dict) | ||
|
||
RequestIDLogger(self.handler, self.registry)(self.request) | ||
self.handler.assert_called_with(self.request) | ||
|
||
context_class = structlog.get_config().get('context_class') | ||
self.assertEqual(context_class, dict) |
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.
This should be
SENTRY_INSTALLED
, not "enabled".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.
Done.