Skip to content

Commit

Permalink
Moving logging filters from edx-platform (#65)
Browse files Browse the repository at this point in the history
* Moving logging filters from edx-platform

Moving logging filters for user and remote
IP to this repo, so they can be re-used by
newly created IDAs through the cookie-cutter
template.

SEG-34

Co-authored-by: Robert Raposa <[email protected]>
  • Loading branch information
moconnell1453 and robrap authored Oct 29, 2020
1 parent a6a4704 commit 13faebe
Show file tree
Hide file tree
Showing 16 changed files with 156 additions and 32 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,14 @@ Change Log
Unreleased
~~~~~~~~~~

[3.10.0] - 2020-10-28
~~~~~~~~~~~~~~~~~~~~~

Added
_______

* Added logging filter classes for users and remote IP addresses to be used by all IDAs. These were moved here from edx-platform.

[3.9.0] - 2020-10-21
~~~~~~~~~~~~~~~~~~~~

Expand Down
27 changes: 27 additions & 0 deletions docs/decisions/0003-logging-filters-for-user-and-ip.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
Logging filters for user and IP
================================================

Status
------

Accepted

Context
-------

As part of the Security Working Group's work on SEG-34, we recognized that the `logging filters`_ for
LMS users and remote IP addresses were not reusable by other IDAs from inside LMS.

.. _logging filters: https://github.com/edx/edx-platform/blob/11e4cab6220c8c503787142f48a352410191de0a/openedx/core/djangoapps/util/log_utils.py#L16

Decision
--------

We decided to move the LMS users and remote IP addresses to this library, so these filters may be re-used by any edx component. Of particular use, we can update the logging settings in the IDA cookie-cutter repo to reference these filters in the standard logging context that is created from the repo for new IDAs.

Consequences
------------

We will need to:
* Update the IDA cookie-cutter once these are available.
* Remove these classes from edx-platform.
2 changes: 1 addition & 1 deletion edx_django_utils/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
EdX utilities for Django Application development..
"""

__version__ = "3.9.0"
__version__ = "3.10.0"

default_app_config = (
"edx_django_utils.apps.EdxDjangoUtilsConfig"
Expand Down
6 changes: 6 additions & 0 deletions edx_django_utils/logging/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
"""
Logging utilities public api
See README.rst for details.
"""
from .internal.filters import RemoteIpFilter, UserIdFilter
Empty file.
33 changes: 33 additions & 0 deletions edx_django_utils/logging/internal/filters.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
"""
Django-based logging filters
"""

from logging import Filter

from crum import get_current_request, get_current_user


class RemoteIpFilter(Filter):
"""
A logging filter that adds the remote IP to the logging context
"""
def filter(self, record):
request = get_current_request()
if request and 'REMOTE_ADDR' in request.META:
record.remoteip = request.META['REMOTE_ADDR']
else:
record.remoteip = None
return True


class UserIdFilter(Filter):
"""
A logging filter that adds userid to the logging context
"""
def filter(self, record):
user = get_current_user()
if user and user.pk:
record.userid = user.pk
else:
record.userid = None
return True
Empty file.
61 changes: 61 additions & 0 deletions edx_django_utils/logging/tests/test_logging.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
"""
Tests for logging.
"""

from django.test import TestCase
from mock import MagicMock, patch

from edx_django_utils.logging import RemoteIpFilter, UserIdFilter


class MockRecord:
"""
Mocks a logging construct to receive data to be interpolated.
"""
def __init__(self):
self.userid = None
self.remoteip = None


class TestLoggingFilters(TestCase):
"""
Test the logging filters for users and IP addresses
"""

@patch('edx_django_utils.logging.internal.filters.get_current_user')
def test_userid_filter(self, mock_get_user):
mock_user = MagicMock()
mock_user.pk = '1234'
mock_get_user.return_value = mock_user

user_filter = UserIdFilter()
test_record = MockRecord()
user_filter.filter(test_record)

self.assertEqual(test_record.userid, '1234')

def test_userid_filter_no_user(self):
user_filter = UserIdFilter()
test_record = MockRecord()
user_filter.filter(test_record)

self.assertEqual(test_record.userid, None)

@patch('edx_django_utils.logging.internal.filters.get_current_request')
def test_remoteip_filter(self, mock_get_request):
mock_request = MagicMock()
mock_request.META = {'REMOTE_ADDR': '192.168.1.1'}
mock_get_request.return_value = mock_request

ip_filter = RemoteIpFilter()
test_record = MockRecord()
ip_filter.filter(test_record)

self.assertEqual(test_record.remoteip, '192.168.1.1')

def test_remoteip_filter_no_request(self):
ip_filter = RemoteIpFilter()
test_record = MockRecord()
ip_filter.filter(test_record)

self.assertEqual(test_record.remoteip, None)
1 change: 1 addition & 0 deletions requirements/base.in
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@ newrelic # New Relic agent for performance monitoring
psutil # Library for retrieving information on running processes and system utilization
# NOTE: psutil pinned to match edx-platform version. Will need to be updated at the same time.
stevedore # Used by plugins to make importing easier
django-crum # Used by logging filters
3 changes: 2 additions & 1 deletion requirements/base.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@
#
# make upgrade
#
django-crum==0.7.7 # via -r requirements/base.in
django-waffle==2.0.0 # via -r requirements/base.in
django==2.2.16 # via -c requirements/constraints.txt, -r requirements/base.in
django==2.2.16 # via -c requirements/constraints.txt, -r requirements/base.in, django-crum
newrelic==5.22.0.151 # via -r requirements/base.in
pbr==5.5.1 # via stevedore
psutil==5.7.3 # via -r requirements/base.in
Expand Down
3 changes: 2 additions & 1 deletion requirements/constraints.txt
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,5 @@ zipp<2.0 # newer versions require python>3.5

# stevedore 2.0.0 requires python >= 3.6
stevedore<2.0.0

keyring==20.0.1 # latest support by python 3.5
twine==1.15.0 # latest support by python 3.5
14 changes: 5 additions & 9 deletions requirements/dev.txt
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,14 @@ coverage==5.3 # via -r requirements/quality.txt, -r requirements/tra
ddt==1.4.1 # via -r requirements/quality.txt
diff-cover==4.0.1 # via -r requirements/dev.in
distlib==0.3.1 # via -r requirements/travis.txt, virtualenv
django-crum==0.7.7 # via -r requirements/quality.txt
django-waffle==2.0.0 # via -r requirements/quality.txt
django==2.2.16 # via -c requirements/constraints.txt, -r requirements/quality.txt, edx-i18n-tools
django==2.2.16 # via -c requirements/constraints.txt, -r requirements/quality.txt, django-crum, edx-i18n-tools
edx-i18n-tools==0.5.3 # via -r requirements/dev.in
edx-lint==1.5.2 # via -r requirements/quality.txt
filelock==3.0.12 # via -r requirements/travis.txt, tox, virtualenv
idna==2.10 # via -r requirements/travis.txt, requests
importlib-metadata==2.0.0 # via -r requirements/quality.txt, -r requirements/travis.txt, inflect, path, pluggy, pytest, tox, virtualenv
importlib-resources==3.2.0 # via -r requirements/travis.txt, virtualenv
inflect==3.0.2 # via jinja2-pluralize
inflect==4.1.0 # via jinja2-pluralize
iniconfig==1.1.1 # via -r requirements/quality.txt, pytest
isort==4.3.21 # via -r requirements/quality.txt, pylint
jinja2-pluralize==0.3.0 # via diff-cover
Expand All @@ -36,8 +35,7 @@ mock==3.0.5 # via -c requirements/constraints.txt, -r requirements
newrelic==5.22.0.151 # via -r requirements/quality.txt
packaging==20.4 # via -r requirements/quality.txt, -r requirements/travis.txt, pytest, tox
path.py==12.5.0 # via edx-i18n-tools
path==13.1.0 # via path.py
pathlib2==2.3.5 # via -r requirements/quality.txt, pytest
path==15.0.0 # via path.py
pbr==5.5.1 # via -r requirements/quality.txt, stevedore
pip-tools==5.3.1 # via -r requirements/pip-tools.txt
pluggy==0.13.1 # via -r requirements/quality.txt, -r requirements/travis.txt, diff-cover, pytest, tox
Expand All @@ -58,18 +56,16 @@ pytest==6.1.1 # via -r requirements/quality.txt, pytest-cov, pytest-
pytz==2020.1 # via -r requirements/quality.txt, django
pyyaml==5.3.1 # via edx-i18n-tools
requests==2.24.0 # via -r requirements/travis.txt, codecov
six==1.15.0 # via -r requirements/pip-tools.txt, -r requirements/quality.txt, -r requirements/travis.txt, astroid, edx-i18n-tools, edx-lint, mock, packaging, pathlib2, pip-tools, stevedore, tox, virtualenv
six==1.15.0 # via -r requirements/pip-tools.txt, -r requirements/quality.txt, -r requirements/travis.txt, astroid, edx-i18n-tools, edx-lint, mock, packaging, pip-tools, stevedore, tox, virtualenv
snowballstemmer==2.0.0 # via -r requirements/quality.txt, pydocstyle
sqlparse==0.4.1 # via -r requirements/quality.txt, django
stevedore==1.32.0 # via -c requirements/constraints.txt, -r requirements/quality.txt
toml==0.10.1 # via -r requirements/quality.txt, -r requirements/travis.txt, pytest, tox
tox-battery==0.6.1 # via -r requirements/travis.txt
tox==3.20.1 # via -r requirements/travis.txt, tox-battery
typed-ast==1.4.1 # via -r requirements/quality.txt, astroid
urllib3==1.25.11 # via -r requirements/travis.txt, requests
virtualenv==20.1.0 # via -r requirements/travis.txt, tox
wrapt==1.11.2 # via -r requirements/quality.txt, astroid
zipp==1.2.0 # via -c requirements/constraints.txt, -r requirements/quality.txt, -r requirements/travis.txt, importlib-metadata, importlib-resources

# The following packages are considered to be unsafe in a requirements file:
# pip
10 changes: 4 additions & 6 deletions requirements/doc.txt
Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,20 @@ certifi==2020.6.20 # via requests
chardet==3.0.4 # via doc8, requests
coverage==5.3 # via -r requirements/test.txt, pytest-cov
ddt==1.4.1 # via -r requirements/test.txt
django-crum==0.7.7 # via -r requirements/test.txt
django-waffle==2.0.0 # via -r requirements/test.txt
django==2.2.16 # via -c requirements/constraints.txt, -r requirements/test.txt
django==2.2.16 # via -c requirements/constraints.txt, -r requirements/test.txt, django-crum
doc8==0.8.1 # via -r requirements/doc.in
docutils==0.16 # via doc8, readme-renderer, restructuredtext-lint, sphinx
edx-sphinx-theme==1.5.0 # via -r requirements/doc.in
idna==2.10 # via requests
imagesize==1.2.0 # via sphinx
importlib-metadata==2.0.0 # via -r requirements/test.txt, pluggy, pytest
iniconfig==1.1.1 # via -r requirements/test.txt, pytest
jinja2==2.11.2 # via sphinx
markupsafe==1.1.1 # via jinja2
mock==3.0.5 # via -c requirements/constraints.txt, -r requirements/test.txt
newrelic==5.22.0.151 # via -r requirements/test.txt
packaging==20.4 # via -r requirements/test.txt, bleach, pytest, sphinx
pathlib2==2.3.5 # via -r requirements/test.txt, pytest
pbr==5.5.1 # via -r requirements/test.txt, stevedore
pkginfo==1.6.1 # via twine
pluggy==0.13.1 # via -r requirements/test.txt, pytest
Expand All @@ -42,7 +41,7 @@ readme-renderer==28.0 # via -r requirements/doc.in, twine
requests-toolbelt==0.9.1 # via twine
requests==2.24.0 # via requests-toolbelt, sphinx, twine
restructuredtext-lint==1.3.1 # via doc8
six==1.15.0 # via -r requirements/test.txt, bleach, doc8, edx-sphinx-theme, mock, packaging, pathlib2, readme-renderer, stevedore
six==1.15.0 # via -r requirements/test.txt, bleach, doc8, edx-sphinx-theme, mock, packaging, readme-renderer, stevedore
snowballstemmer==2.0.0 # via sphinx
sphinx==3.2.1 # via -r requirements/doc.in, edx-sphinx-theme
sphinxcontrib-applehelp==1.0.2 # via sphinx
Expand All @@ -55,10 +54,9 @@ sqlparse==0.4.1 # via -r requirements/test.txt, django
stevedore==1.32.0 # via -c requirements/constraints.txt, -r requirements/test.txt, doc8
toml==0.10.1 # via -r requirements/test.txt, pytest
tqdm==4.51.0 # via twine
twine==1.15.0 # via -r requirements/doc.in
twine==1.15.0 # via -c requirements/constraints.txt, -r requirements/doc.in
urllib3==1.25.11 # via requests
webencodings==0.5.1 # via bleach
zipp==1.2.0 # via -c requirements/constraints.txt, -r requirements/test.txt, importlib-metadata

# The following packages are considered to be unsafe in a requirements file:
# setuptools
9 changes: 3 additions & 6 deletions requirements/quality.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,17 @@ click-log==0.3.2 # via edx-lint
click==7.1.2 # via click-log, edx-lint
coverage==5.3 # via -r requirements/test.txt, pytest-cov
ddt==1.4.1 # via -r requirements/test.txt
django-crum==0.7.7 # via -r requirements/test.txt
django-waffle==2.0.0 # via -r requirements/test.txt
django==2.2.16 # via -c requirements/constraints.txt, -r requirements/test.txt
django==2.2.16 # via -c requirements/constraints.txt, -r requirements/test.txt, django-crum
edx-lint==1.5.2 # via -r requirements/quality.in
importlib-metadata==2.0.0 # via -r requirements/test.txt, pluggy, pytest
iniconfig==1.1.1 # via -r requirements/test.txt, pytest
isort==4.3.21 # via -r requirements/quality.in, pylint
lazy-object-proxy==1.4.3 # via astroid
mccabe==0.6.1 # via pylint
mock==3.0.5 # via -c requirements/constraints.txt, -r requirements/test.txt
newrelic==5.22.0.151 # via -r requirements/test.txt
packaging==20.4 # via -r requirements/test.txt, pytest
pathlib2==2.3.5 # via -r requirements/test.txt, pytest
pbr==5.5.1 # via -r requirements/test.txt, stevedore
pluggy==0.13.1 # via -r requirements/test.txt, pytest
psutil==5.7.3 # via -r requirements/test.txt
Expand All @@ -37,11 +36,9 @@ pytest-cov==2.10.1 # via -r requirements/test.txt
pytest-django==4.1.0 # via -r requirements/test.txt
pytest==6.1.1 # via -r requirements/test.txt, pytest-cov, pytest-django
pytz==2020.1 # via -r requirements/test.txt, django
six==1.15.0 # via -r requirements/test.txt, astroid, edx-lint, mock, packaging, pathlib2, stevedore
six==1.15.0 # via -r requirements/test.txt, astroid, edx-lint, mock, packaging, stevedore
snowballstemmer==2.0.0 # via pydocstyle
sqlparse==0.4.1 # via -r requirements/test.txt, django
stevedore==1.32.0 # via -c requirements/constraints.txt, -r requirements/test.txt
toml==0.10.1 # via -r requirements/test.txt, pytest
typed-ast==1.4.1 # via astroid
wrapt==1.11.2 # via astroid
zipp==1.2.0 # via -c requirements/constraints.txt, -r requirements/test.txt, importlib-metadata
6 changes: 2 additions & 4 deletions requirements/test.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,12 @@
attrs==20.2.0 # via pytest
coverage==5.3 # via pytest-cov
ddt==1.4.1 # via -r requirements/test.in
django-crum==0.7.7 # via -r requirements/base.txt
django-waffle==2.0.0 # via -r requirements/base.txt
importlib-metadata==2.0.0 # via pluggy, pytest
iniconfig==1.1.1 # via pytest
mock==3.0.5 # via -c requirements/constraints.txt, -r requirements/test.in
newrelic==5.22.0.151 # via -r requirements/base.txt
packaging==20.4 # via pytest
pathlib2==2.3.5 # via pytest
pbr==5.5.1 # via -r requirements/base.txt, stevedore
pluggy==0.13.1 # via pytest
psutil==5.7.3 # via -r requirements/base.txt
Expand All @@ -23,8 +22,7 @@ pytest-cov==2.10.1 # via -r requirements/test.in
pytest-django==4.1.0 # via -r requirements/test.in
pytest==6.1.1 # via pytest-cov, pytest-django
pytz==2020.1 # via -r requirements/base.txt, django
six==1.15.0 # via -r requirements/base.txt, mock, packaging, pathlib2, stevedore
six==1.15.0 # via -r requirements/base.txt, mock, packaging, stevedore
sqlparse==0.4.1 # via -r requirements/base.txt, django
stevedore==1.32.0 # via -c requirements/constraints.txt, -r requirements/base.txt
toml==0.10.1 # via pytest
zipp==1.2.0 # via -c requirements/constraints.txt, importlib-metadata
5 changes: 1 addition & 4 deletions requirements/travis.txt
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@ coverage==5.3 # via codecov
distlib==0.3.1 # via virtualenv
filelock==3.0.12 # via tox, virtualenv
idna==2.10 # via requests
importlib-metadata==2.0.0 # via pluggy, tox, virtualenv
importlib-resources==3.2.0 # via virtualenv
mock==3.0.5 # via -c requirements/constraints.txt, -r requirements/travis.in
packaging==20.4 # via tox
pluggy==0.13.1 # via tox
Expand All @@ -25,5 +23,4 @@ toml==0.10.1 # via tox
tox-battery==0.6.1 # via -r requirements/travis.in
tox==3.20.1 # via -r requirements/travis.in, tox-battery
urllib3==1.25.11 # via requests
virtualenv==20.1.0 # via tox
zipp==1.2.0 # via -c requirements/constraints.txt, importlib-metadata, importlib-resources
virtualenv==20.1.0 # via tox

0 comments on commit 13faebe

Please sign in to comment.