Skip to content
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

Sentry integration #34

Merged
merged 28 commits into from Mar 11, 2020
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
a5da162
Added sentry integration logic
Mar 5, 2020
3d51a4c
Added an integrations.py to house all future package integrations
sondrelg Mar 8, 2020
31e36a4
Added integration logic to config.py and middleware.py
sondrelg Mar 8, 2020
a9d2900
Finished testing
sondrelg Mar 8, 2020
d4a6134
Changed integration docstrings and added additional configuration logic
sondrelg Mar 8, 2020
b1af385
Added sentry-sdk to test requirements
sondrelg Mar 8, 2020
b1ed915
Moved integration into an integrations folder, and added a base class
sondrelg Mar 8, 2020
dec4ee5
fixed docstring
sondrelg Mar 8, 2020
cb88611
Moved base class to base.py
sondrelg Mar 8, 2020
e03c94c
Added setup and validate methods to the base class, where the setup m…
sondrelg Mar 8, 2020
7ea22ed
Added kwarg requirement for the run method, and put the validation in…
sondrelg Mar 10, 2020
1f0d5de
Final adjustments
sondrelg Mar 10, 2020
3e36cf9
Fixed tests. Bumped versions in requirements
JonasKs Mar 11, 2020
582fa93
Patched out scope in test and checked return args using pytest-mock
JonasKs Mar 11, 2020
7d6428f
Ran precommit on all files. Removed whitespace and added missing type…
JonasKs Mar 11, 2020
56a67d1
.
Mar 11, 2020
9c2fab1
.
Mar 11, 2020
9827d30
Added tear_down to the base and called it in the middleware. (So that…
JonasKs Mar 11, 2020
9dcf321
Merge branch 'sentry' of https://github.com/JonasKs/django-guid into …
Mar 11, 2020
8f07923
Added tear_down kwargs requirement.
JonasKs Mar 11, 2020
8bdfe21
Merge branch 'sentry' of https://github.com/JonasKs/django-guid into …
Mar 11, 2020
fc60ace
Changed index and readme rsts
Mar 11, 2020
a93201d
Removed validate in favor of setup.
JonasKs Mar 11, 2020
95611a5
Removed validate from the docs
JonasKs Mar 11, 2020
e56e4ac
Changelogs
JonasKs Mar 11, 2020
e4d4ac1
Added search info for sentry integratioN
Mar 11, 2020
eeedd12
Merge branch 'sentry' of https://github.com/JonasKs/django-guid into …
Mar 11, 2020
6872c24
Fixed sphinx error
JonasKs Mar 11, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 8 additions & 4 deletions django_guid/config.py
Expand Up @@ -14,9 +14,10 @@ class Settings(object):
def __init__(self) -> None:
self.GUID_HEADER_NAME = 'Correlation-ID'
self.VALIDATE_GUID = True
self.SKIP_CLEANUP = None # Deprecated - to be removed in the next major version
self.RETURN_HEADER = True
self.EXPOSE_HEADER = True
self.INTEGRATIONS = []
self.SKIP_CLEANUP = None # Deprecated - to be removed in the next major version

if hasattr(django_settings, 'DJANGO_GUID'):
_settings = django_settings.DJANGO_GUID
Expand All @@ -36,6 +37,12 @@ def __init__(self) -> None:
raise ImproperlyConfigured('RETURN_HEADER must be a boolean')
if not isinstance(self.EXPOSE_HEADER, bool):
raise ImproperlyConfigured('EXPOSE_HEADER must be a boolean')
if not isinstance(self.INTEGRATIONS, list):
raise ImproperlyConfigured('INTEGRATIONS must be an array')

for integration in self.INTEGRATIONS:
if not hasattr(integration, 'run'):
raise ImproperlyConfigured('Integration classes must be instantiated and contain a `run` method')

if 'SKIP_CLEANUP' in _settings:
warn(
Expand All @@ -44,8 +51,5 @@ def __init__(self) -> None:
DeprecationWarning,
)

else:
pass # Do nothing if DJANGO_GUID not found in settings


settings = Settings()
39 changes: 39 additions & 0 deletions django_guid/integrations.py
@@ -0,0 +1,39 @@
import logging

from django.core.exceptions import ImproperlyConfigured

logger = logging.getLogger('django_guid')


class SentryIntegration:
"""
This integrations, when added to the django_guid settings,
ensures that each requests correlation ID is passed on to Sentry exception logs.
sondrelg marked this conversation as resolved.
Show resolved Hide resolved
"""

def __init__(self):
"""
This is run when the integration is initialized in the clients settings.py.

Put all validation logic here.
"""
# Makes sure the client has installed the `sentry_sdk` package, and that the header is appropriately named.
try:
import sentry_sdk # noqa: F401
except ModuleNotFoundError:
raise ImproperlyConfigured(
'The package `sentry-sdk` is required for extending your tracing IDs to Sentry. '
'Please run `pip install sentry-sdk` if you wish to include this integration.'
)

@staticmethod
def run(self):
sondrelg marked this conversation as resolved.
Show resolved Hide resolved
"""
This method holds execution logic to be executed in the middleware.
"""
import sentry_sdk

with sentry_sdk.configure_scope() as scope:
guid = self.get_guid()
logger.debug('Setting Sentry transaction_id to %s', guid)
scope.set_tag('transaction_id', guid)
45 changes: 27 additions & 18 deletions django_guid/middleware.py
Expand Up @@ -14,10 +14,11 @@

class GuidMiddleware(object):
"""
Checks for an existing GUID (correlation ID) in a request's headers.
If a header value is found, the value is validated as a GUID and stored, before the request is passed to the
next middleware.
If no value is found, or one is found but is invalid, we generate and store a new GUID on the thread.
Checks for a GUID (correlation ID) in an incoming request's headers and assigns it to the thread if found.
If none is found, one is generated.

As every request spawns a new thread, this makes it possible to add logger IDs that only span a single request.
sondrelg marked this conversation as resolved.
Show resolved Hide resolved

Stored GUIDs are accessible from anywhere in the Django app.
"""

Expand All @@ -29,15 +30,18 @@ def __init__(self, get_response: Callable) -> None:
"""
self.get_response = get_response

# django_guid must be in installed apps for signals to work.
# Without signals there may be a memory leak
# `django_guid` must be in installed apps for signals to work and if the signals dont work,
# it creates a memory leak, since _guid is never cleaned and can theoretically keep growing infinitely.
#
sondrelg marked this conversation as resolved.
Show resolved Hide resolved
# This logic cannot be moved to config.py because apps are not yet initialized when that is executed
if not apps.is_installed('django_guid'):
raise ImproperlyConfigured('django_guid must be in installed apps')

def __call__(self, request: HttpRequest) -> Union[HttpRequest, HttpResponse]:
"""
Fetches the current thread ID from the pool and stores the GUID in the _guid class variable,
with the thread ID as the key.

sondrelg marked this conversation as resolved.
Show resolved Hide resolved
Deletes the GUID from the object unless settings are overwritten.

:param request: HttpRequest from Django
Expand All @@ -46,6 +50,10 @@ def __call__(self, request: HttpRequest) -> Union[HttpRequest, HttpResponse]:
# Process request and store the GUID on the thread
self.set_guid(self._get_id_from_header(request))

# Run all integrations
for integration in settings.INTEGRATIONS:
integration.run(self)

# ^ Code above this line is executed before the view and later middleware
response = self.get_response(request)
if settings.RETURN_HEADER:
Expand All @@ -57,10 +65,11 @@ def __call__(self, request: HttpRequest) -> Union[HttpRequest, HttpResponse]:
@classmethod
def get_guid(cls, default: str = None) -> str:
"""
Fetches the GUID of the current thread, from _guid.
If no value has been set for the current thread yet, we return a default value.
:default: Optional value to return if no GUID has been set on the current thread.
Fetches the GUID of the current thread from _guid.

sondrelg marked this conversation as resolved.
Show resolved Hide resolved
If no value has been set for the current thread yet, a default value is returned.

:param default: Optional value to return if no GUID has been set on the current thread.
:return: GUID or default.
"""
return cls._guid.get(threading.current_thread(), default)
Expand All @@ -70,15 +79,15 @@ def set_guid(cls, guid: str) -> None:
"""
Assigns a GUID to the thread.

:param guid: str
:param guid: The GUID being assigned
sondrelg marked this conversation as resolved.
Show resolved Hide resolved
:return: None
"""
cls._guid[threading.current_thread()] = guid

@classmethod
def delete_guid(cls) -> None:
"""
Delete the GUID that was stored for the current thread.
Delete the thread's GUID.

:return: None
"""
Expand Down Expand Up @@ -112,8 +121,7 @@ def _validate_guid(original_guid: str) -> bool:

def _get_correlation_id_from_header(self, request: HttpRequest) -> str:
"""
Returns either the provided GUID or a new one,
depending on if the provided GUID is valid, and the specified settings.
Returns either the provided GUID or a new one depending on if the provided GUID is valid or not.

:param request: HttpRequest object
:return: GUID
Expand All @@ -133,21 +141,22 @@ def _get_correlation_id_from_header(self, request: HttpRequest) -> str:
def _get_id_from_header(self, request: HttpRequest) -> str:
"""
Checks if the request contains the header specified in the Django settings.

sondrelg marked this conversation as resolved.
Show resolved Hide resolved
If it does, we fetch the header and attempt to validate the contents as GUID.
If no header is found, we generate a GUID to be injected instead.

:param request: HttpRequest object
:return: GUID
"""
guid_header_name = settings.GUID_HEADER_NAME
header = request.headers.get(guid_header_name) # Case insensitive headers.get added in Django2.2
header = request.headers.get(settings.GUID_HEADER_NAME) # Case insensitive headers.get added in Django2.2
if header:
logger.info('%s found in the header: %s', guid_header_name, header)
logger.info('%s found in the header: %s', settings.GUID_HEADER_NAME, header)
request.correlation_id = self._get_correlation_id_from_header(request)
else:
request.correlation_id = self._generate_guid()
logger.info(
'No %s found in the header. Added %s: %s', guid_header_name, guid_header_name, request.correlation_id
'Header `%s` was not found in the incoming request. Generated new GUID: %s',
settings.GUID_HEADER_NAME,
request.correlation_id,
)

return request.correlation_id
7 changes: 4 additions & 3 deletions django_guid/signals.py
Expand Up @@ -14,11 +14,12 @@ def delete_guid(sender: Optional[dict], **kwargs: dict) -> None:
"""
Receiver function for when a request finishes.

When a request is finished, delete a requests _guid reference to prevent memory leaks.
When a request is finished the delete_guid method is called to remove the request _guid reference.
This is important to maintain, to prevent a memory leak.
sondrelg marked this conversation as resolved.
Show resolved Hide resolved

:param sender: The sender of the signal. By documentation, we must allow this input parameter.
:param kwargs: The request_finished signal does not actually send any kwargs, but Django will throw an error
if we don't accept them. This is because at any point arguments could get added to the signal, and the reciever
:param kwargs: The request_finished signal does not actually send any kwargs, but Django will throw an error
if we don't accept them. This is because at any point arguments could get added to the signal, and the receiver
must be able to handle those new arguments.
:return: None
"""
Expand Down
10 changes: 8 additions & 2 deletions tests/functional/test_middleware.py
Expand Up @@ -21,7 +21,10 @@ def test_request_with_no_correlation_id(client, caplog, mock_uuid):
"""
response = client.get('/')
expected = [
('No Correlation-ID found in the header. Added Correlation-ID: 704ae5472cae4f8daa8f2cc5a5a8mock', None),
(
'Header `Correlation-ID` was not found in the incoming request. Generated new GUID: 704ae5472cae4f8daa8f2cc5a5a8mock',
None,
),
('This log message should have a GUID', '704ae5472cae4f8daa8f2cc5a5a8mock'),
('Some warning in a function', '704ae5472cae4f8daa8f2cc5a5a8mock'),
('Received signal `request_finished`', '704ae5472cae4f8daa8f2cc5a5a8mock'),
Expand Down Expand Up @@ -103,7 +106,10 @@ def test_no_return_header_and_drf_url(client, caplog, monkeypatch, mock_uuid):
monkeypatch.setattr(guid_settings, 'RETURN_HEADER', False)
response = client.get('/api')
expected = [
('No Correlation-ID found in the header. Added Correlation-ID: 704ae5472cae4f8daa8f2cc5a5a8mock', None),
(
'Header `Correlation-ID` was not found in the incoming request. Generated new GUID: 704ae5472cae4f8daa8f2cc5a5a8mock',
None,
),
('This is a DRF view log, and should have a GUID.', '704ae5472cae4f8daa8f2cc5a5a8mock'),
('Some warning in a function', '704ae5472cae4f8daa8f2cc5a5a8mock'),
('Received signal `request_finished`', '704ae5472cae4f8daa8f2cc5a5a8mock'),
Expand Down
42 changes: 42 additions & 0 deletions tests/functional/test_sentry_integration.py
@@ -0,0 +1,42 @@
import pytest
from django.core.exceptions import ImproperlyConfigured


def test_sentry_integration(client, monkeypatch, caplog):
"""
Tests that the package handles multiple header values by defaulting to one and logging a warning.
JonasKs marked this conversation as resolved.
Show resolved Hide resolved
"""
from django_guid.integrations import SentryIntegration
from django_guid.config import settings as guid_settings

monkeypatch.setattr(guid_settings, 'INTEGRATIONS', [SentryIntegration()])
client.get('/api', **{'HTTP_Correlation-ID': '97c304252fd14b25b72d6aee31565842'})
expected = [
(None, 'Correlation-ID found in the header: 97c304252fd14b25b72d6aee31565842'),
(None, '97c304252fd14b25b72d6aee31565842 is a valid GUID'),
('97c304252fd14b25b72d6aee31565842', 'Setting Sentry transaction_id to 97c304252fd14b25b72d6aee31565842'),
('97c304252fd14b25b72d6aee31565842', 'This is a DRF view log, and should have a GUID.'),
('97c304252fd14b25b72d6aee31565842', 'Some warning in a function'),
('97c304252fd14b25b72d6aee31565842', 'Received signal `request_finished`'),
('97c304252fd14b25b72d6aee31565842', 'Deleting 97c304252fd14b25b72d6aee31565842 from _guid'),
]
assert [(x.correlation_id, x.message) for x in caplog.records] == expected
JonasKs marked this conversation as resolved.
Show resolved Hide resolved


def test_sentry_validation(client, monkeypatch, caplog):
"""
Tests that the package handles multiple header values by defaulting to one and logging a warning.
"""
import sys
from django_guid.integrations import SentryIntegration
from django_guid.config import settings as guid_settings

# Mock away the sentry_sdk dependency
sys.modules['sentry_sdk'] = None

with pytest.raises(
ImproperlyConfigured,
match='The package `sentry-sdk` is required for extending your tracing IDs to Sentry. '
'Please run `pip install sentry-sdk` if you wish to include this integration.',
):
monkeypatch.setattr(guid_settings, 'INTEGRATIONS', [SentryIntegration()])
1 change: 1 addition & 0 deletions tests/requirements-test-base.txt
Expand Up @@ -2,3 +2,4 @@ pytest==5.3.2
pytest-cov==2.8.1
pytest-django==3.8.0
djangorestframework==3.11.0
sentry-sdk==0.14.2
18 changes: 18 additions & 0 deletions tests/unit/test_config.py
Expand Up @@ -58,3 +58,21 @@ def test_valid_settings(monkeypatch):
assert not Settings().VALIDATE_GUID
assert Settings().GUID_HEADER_NAME == 'Correlation-ID-TEST'
assert not Settings().RETURN_HEADER


def test_bad_integrations_type(monkeypatch):
for item in [{}, '', 2, None, -2]:
monkeypatch.setattr(django_settings, 'DJANGO_GUID', {'INTEGRATIONS': item})
with pytest.raises(ImproperlyConfigured, match='INTEGRATIONS must be an array'):
Settings()


def test_integration_without_run_method(monkeypatch):
class FakeIntegration:
pass

monkeypatch.setattr(django_settings, 'DJANGO_GUID', {'INTEGRATIONS': [FakeIntegration()]})
with pytest.raises(
ImproperlyConfigured, match='Integration classes must be instantiated and contain a `run` method'
):
Settings()