From d0b90013ca0f9799e135880063cce8ddda5fd9b4 Mon Sep 17 00:00:00 2001 From: William Stewart Date: Sun, 21 Oct 2018 19:46:55 +0200 Subject: [PATCH 01/13] Use Django 2.0 DB instrumentation too Closes https://github.com/census-instrumentation/opencensus-python/issues/356 --- .../opencensus/ext/django/middleware.py | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/contrib/opencensus-ext-django/opencensus/ext/django/middleware.py b/contrib/opencensus-ext-django/opencensus/ext/django/middleware.py index 676a75fb4..2104afea7 100644 --- a/contrib/opencensus-ext-django/opencensus/ext/django/middleware.py +++ b/contrib/opencensus-ext-django/opencensus/ext/django/middleware.py @@ -23,6 +23,7 @@ from opencensus.trace import utils from opencensus.trace.samplers import probability +from django.db import connection try: from django.utils.deprecation import MiddlewareMixin except ImportError: # pragma: NO COVER @@ -237,9 +238,38 @@ def process_request(self, request): SPAN_THREAD_LOCAL_KEY, span) + connection.execute_wrappers.append(self._trace_db_call) + except Exception: # pragma: NO COVER log.error('Failed to trace request', exc_info=True) + def _trace_db_call(self, execute, sql, params, many, context): + _tracer = execution_context.get_opencensus_tracer() + if _tracer is not None: + if many: + method_name = 'executemany' + else: + method_name = 'execute' + db_type = context['connection'].vendor + # Note that although get_opencensus_tracer() returns a NoopTracer + # if no thread local has been set, set_opencensus_tracer() does NOT + # protect against setting None to the thread local - be defensive + # here + _span = _tracer.start_span() + _span.name = '{}.query'.format(db_type) + _span.span_kind = span_module.SpanKind.CLIENT + _tracer.add_attribute_to_current_span( + '{}.query'.format(db_type), sql) + _tracer.add_attribute_to_current_span( + '{}.cursor.method.name'.format(db_type), method_name) + + result = execute(sql, params, many, context) + + if _tracer is not None: + _tracer.end_span() + + return result + def process_view(self, request, view_func, *args, **kwargs): """Process view is executed before the view function, here we get the function name add set it as the span name. From 522c1441ac1dd54e270732ef27a337f4df5e36be Mon Sep 17 00:00:00 2001 From: William Stewart Date: Mon, 3 Dec 2018 17:07:36 +0100 Subject: [PATCH 02/13] Only add DB tracing on Django 2.0+ --- .../opencensus-ext-django/opencensus/ext/django/middleware.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/contrib/opencensus-ext-django/opencensus/ext/django/middleware.py b/contrib/opencensus-ext-django/opencensus/ext/django/middleware.py index 2104afea7..aed5a2aa4 100644 --- a/contrib/opencensus-ext-django/opencensus/ext/django/middleware.py +++ b/contrib/opencensus-ext-django/opencensus/ext/django/middleware.py @@ -23,6 +23,7 @@ from opencensus.trace import utils from opencensus.trace.samplers import probability +import django from django.db import connection try: from django.utils.deprecation import MiddlewareMixin @@ -238,7 +239,8 @@ def process_request(self, request): SPAN_THREAD_LOCAL_KEY, span) - connection.execute_wrappers.append(self._trace_db_call) + if django.VERSION >= (2,): + connection.execute_wrappers.append(self._trace_db_call) except Exception: # pragma: NO COVER log.error('Failed to trace request', exc_info=True) From ee45c3c2c16785ead69d0e949d05bc347df87ac6 Mon Sep 17 00:00:00 2001 From: William Stewart Date: Mon, 3 Dec 2018 19:47:24 +0100 Subject: [PATCH 03/13] WIP Django DB instrumentation tests --- tests/unit/trace/ext/django/test_db.py | 77 ++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 tests/unit/trace/ext/django/test_db.py diff --git a/tests/unit/trace/ext/django/test_db.py b/tests/unit/trace/ext/django/test_db.py new file mode 100644 index 000000000..b8572dd0b --- /dev/null +++ b/tests/unit/trace/ext/django/test_db.py @@ -0,0 +1,77 @@ +# Copyright 2017, OpenCensus Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import unittest +from collections import namedtuple + +import django +import mock +import pytest +from django.test.utils import teardown_test_environment + +from opencensus.trace import execution_context +from opencensus.trace.tracers.noop_tracer import NoopTracer + + +class TestOpencensusDatabaseMiddleware(unittest.TestCase): + def setUp(self): + from django.conf import settings as django_settings + from django.test.utils import setup_test_environment + + + if not django_settings.configured: + django_settings.configure() + setup_test_environment() + + def tearDown(self): + execution_context.clear() + teardown_test_environment() + + def test_process_request(self): + if django.VERSION < (2, 0): + pytest.skip("Wrong version of Django") + + from opencensus.trace.ext.django import middleware + + tracer = middleware._get_current_tracer() + span = tracer.current_span() + + sql = "SELECT * FROM users" + + MockConnection = namedtuple('Connection', ('vendor')) + connection = MockConnection('mysql') + + mock_execute = mock.Mock() + mock_execute.return_value = "Mock result" + + middleware_obj = middleware.OpencensusMiddleware() + + result = middleware_obj._trace_db_call( + mock_execute, sql, params=[], many=False, + context={'connection': connection}) + + mock_sql, mock_params, mock_many, mock_context = mock_execute.call_args[0] + + self.assertEqual(mock_sql, sql) + self.assertEqual(mock_params, []) + self.assertEqual(mock_many, False) + self.assertEqual(mock_context, {'connection': connection}) + self.assertEqual(result, "Mock result") + + result = middleware_obj._trace_db_call( + mock_execute, sql, params=[], many=True, + context={'connection': connection}) + + mock_sql, mock_params, mock_many, mock_context = mock_execute.call_args[0] + self.assertEqual(mock_many, True) \ No newline at end of file From 92af714e1fd9716469aa4d778eda575319655131 Mon Sep 17 00:00:00 2001 From: William Stewart Date: Mon, 3 Dec 2018 22:55:22 +0100 Subject: [PATCH 04/13] Linting fixups --- tests/unit/trace/ext/django/test_db.py | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/tests/unit/trace/ext/django/test_db.py b/tests/unit/trace/ext/django/test_db.py index b8572dd0b..18ba0a585 100644 --- a/tests/unit/trace/ext/django/test_db.py +++ b/tests/unit/trace/ext/django/test_db.py @@ -21,7 +21,6 @@ from django.test.utils import teardown_test_environment from opencensus.trace import execution_context -from opencensus.trace.tracers.noop_tracer import NoopTracer class TestOpencensusDatabaseMiddleware(unittest.TestCase): @@ -29,7 +28,6 @@ def setUp(self): from django.conf import settings as django_settings from django.test.utils import setup_test_environment - if not django_settings.configured: django_settings.configure() setup_test_environment() @@ -44,9 +42,6 @@ def test_process_request(self): from opencensus.trace.ext.django import middleware - tracer = middleware._get_current_tracer() - span = tracer.current_span() - sql = "SELECT * FROM users" MockConnection = namedtuple('Connection', ('vendor')) @@ -61,7 +56,8 @@ def test_process_request(self): mock_execute, sql, params=[], many=False, context={'connection': connection}) - mock_sql, mock_params, mock_many, mock_context = mock_execute.call_args[0] + (mock_sql, mock_params, mock_many, + mock_context) = mock_execute.call_args[0] self.assertEqual(mock_sql, sql) self.assertEqual(mock_params, []) @@ -73,5 +69,6 @@ def test_process_request(self): mock_execute, sql, params=[], many=True, context={'connection': connection}) - mock_sql, mock_params, mock_many, mock_context = mock_execute.call_args[0] - self.assertEqual(mock_many, True) \ No newline at end of file + (mock_sql, mock_params, mock_many, + mock_context) = mock_execute.call_args[0] + self.assertEqual(mock_many, True) From 0c133b148a2ee2b5bda47ad425e3d88d703e9abd Mon Sep 17 00:00:00 2001 From: William Stewart Date: Tue, 4 Dec 2018 11:29:46 +0100 Subject: [PATCH 05/13] E127 continuation line over-indented for visual indent --- tests/unit/trace/ext/django/test_db.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/trace/ext/django/test_db.py b/tests/unit/trace/ext/django/test_db.py index 18ba0a585..942b0cc9c 100644 --- a/tests/unit/trace/ext/django/test_db.py +++ b/tests/unit/trace/ext/django/test_db.py @@ -57,7 +57,7 @@ def test_process_request(self): context={'connection': connection}) (mock_sql, mock_params, mock_many, - mock_context) = mock_execute.call_args[0] + mock_context) = mock_execute.call_args[0] self.assertEqual(mock_sql, sql) self.assertEqual(mock_params, []) @@ -70,5 +70,5 @@ def test_process_request(self): context={'connection': connection}) (mock_sql, mock_params, mock_many, - mock_context) = mock_execute.call_args[0] + mock_context) = mock_execute.call_args[0] self.assertEqual(mock_many, True) From 0a60425a5c818cfbfe21881171306275a5a3b8a1 Mon Sep 17 00:00:00 2001 From: William Stewart Date: Mon, 11 Mar 2019 15:00:59 +0100 Subject: [PATCH 06/13] Django DB instrumentation is Django 2.0+ --- contrib/opencensus-ext-django/setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/opencensus-ext-django/setup.py b/contrib/opencensus-ext-django/setup.py index 333564ad2..7b95f96ab 100644 --- a/contrib/opencensus-ext-django/setup.py +++ b/contrib/opencensus-ext-django/setup.py @@ -39,7 +39,7 @@ include_package_data=True, long_description=open('README.rst').read(), install_requires=[ - 'Django >= 1.11.0, <= 1.11.20', + 'Django >= 1.11.0', 'opencensus >= 0.2.dev0, < 1.0.0', ], extras_require={}, From e26626f48b8ab19c06672d30c8898cf1dc6dbed0 Mon Sep 17 00:00:00 2001 From: William Stewart Date: Mon, 11 Mar 2019 16:27:43 +0100 Subject: [PATCH 07/13] Fix import --- .../opencensus-ext-django/tests/test_django_db_middleware.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename tests/unit/trace/ext/django/test_db.py => contrib/opencensus-ext-django/tests/test_django_db_middleware.py (97%) diff --git a/tests/unit/trace/ext/django/test_db.py b/contrib/opencensus-ext-django/tests/test_django_db_middleware.py similarity index 97% rename from tests/unit/trace/ext/django/test_db.py rename to contrib/opencensus-ext-django/tests/test_django_db_middleware.py index 942b0cc9c..47c4e5bbb 100644 --- a/tests/unit/trace/ext/django/test_db.py +++ b/contrib/opencensus-ext-django/tests/test_django_db_middleware.py @@ -40,7 +40,7 @@ def test_process_request(self): if django.VERSION < (2, 0): pytest.skip("Wrong version of Django") - from opencensus.trace.ext.django import middleware + from opencensus.ext.django import middleware sql = "SELECT * FROM users" From fe66c775e45298a858a63ffa99c9995dc925c991 Mon Sep 17 00:00:00 2001 From: William Stewart Date: Sat, 23 Mar 2019 09:10:52 +0100 Subject: [PATCH 08/13] Passing a 3-tuple to include() is no longer supported --- nox.py | 4 ++-- tests/system/trace/django/app/urls.py | 20 ++++++++++---------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/nox.py b/nox.py index 42fbca67f..c624497dc 100644 --- a/nox.py +++ b/nox.py @@ -40,7 +40,7 @@ def _install_dev_packages(session): @nox.session -@nox.parametrize('py', ['2.7', '3.4', '3.5', '3.6']) +@nox.parametrize('py', ['2.7', '3.4', '3.5', '3.6', '3.7']) def unit(session, py): """Run the unit test suite.""" @@ -69,7 +69,7 @@ def unit(session, py): @nox.session -@nox.parametrize('py', ['2.7', '3.6']) +@nox.parametrize('py', ['2.7', '3.7']) def system(session, py): """Run the system test suite.""" diff --git a/tests/system/trace/django/app/urls.py b/tests/system/trace/django/app/urls.py index b111d1894..6163b4d1c 100644 --- a/tests/system/trace/django/app/urls.py +++ b/tests/system/trace/django/app/urls.py @@ -27,20 +27,20 @@ 1. Add an import: from blog import urls as blog_urls 2. Add a URL to urlpatterns: url(r'^blog/', include(blog_urls)) """ -from django.conf.urls import include, url +from django.urls import path from django.contrib import admin import app.views urlpatterns = [ - url(r'^admin/', include(admin.site.urls)), - url(r'^$', app.views.home), - url(r'^greetings$', app.views.greetings), - url(r'^_ah/health$', app.views.health_check), - url(r'^request$', app.views.get_request_header), - url(r'^mysql$', app.views.mysql_trace), - url(r'^postgresql$', app.views.postgresql_trace), - url(r'^sqlalchemy_mysql$', app.views.sqlalchemy_mysql_trace), - url(r'^sqlalchemy_postgresql$', app.views.sqlalchemy_postgresql_trace), + path(r'^admin/', admin.site.urls), + path(r'^$', app.views.home), + path(r'^greetings$', app.views.greetings), + path(r'^_ah/health$', app.views.health_check), + path(r'^request$', app.views.get_request_header), + path(r'^mysql$', app.views.mysql_trace), + path(r'^postgresql$', app.views.postgresql_trace), + path(r'^sqlalchemy_mysql$', app.views.sqlalchemy_mysql_trace), + path(r'^sqlalchemy_postgresql$', app.views.sqlalchemy_postgresql_trace), ] From 96c378db082163f45a88657a894cb70e155340be Mon Sep 17 00:00:00 2001 From: William Stewart Date: Sat, 23 Mar 2019 09:18:03 +0100 Subject: [PATCH 09/13] googleapis/nox doesn't have Python 3.7 --- nox.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nox.py b/nox.py index c624497dc..42fbca67f 100644 --- a/nox.py +++ b/nox.py @@ -40,7 +40,7 @@ def _install_dev_packages(session): @nox.session -@nox.parametrize('py', ['2.7', '3.4', '3.5', '3.6', '3.7']) +@nox.parametrize('py', ['2.7', '3.4', '3.5', '3.6']) def unit(session, py): """Run the unit test suite.""" @@ -69,7 +69,7 @@ def unit(session, py): @nox.session -@nox.parametrize('py', ['2.7', '3.7']) +@nox.parametrize('py', ['2.7', '3.6']) def system(session, py): """Run the system test suite.""" From 2cf9dca9ba9050274106c13965740129d6818424 Mon Sep 17 00:00:00 2001 From: William Stewart Date: Mon, 25 Mar 2019 15:24:58 +0100 Subject: [PATCH 10/13] Try this instead --- tests/system/trace/django/app/urls.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/system/trace/django/app/urls.py b/tests/system/trace/django/app/urls.py index 6163b4d1c..9eda96a51 100644 --- a/tests/system/trace/django/app/urls.py +++ b/tests/system/trace/django/app/urls.py @@ -27,20 +27,20 @@ 1. Add an import: from blog import urls as blog_urls 2. Add a URL to urlpatterns: url(r'^blog/', include(blog_urls)) """ -from django.urls import path +from django.conf.urls import url from django.contrib import admin import app.views urlpatterns = [ - path(r'^admin/', admin.site.urls), - path(r'^$', app.views.home), - path(r'^greetings$', app.views.greetings), - path(r'^_ah/health$', app.views.health_check), - path(r'^request$', app.views.get_request_header), - path(r'^mysql$', app.views.mysql_trace), - path(r'^postgresql$', app.views.postgresql_trace), - path(r'^sqlalchemy_mysql$', app.views.sqlalchemy_mysql_trace), - path(r'^sqlalchemy_postgresql$', app.views.sqlalchemy_postgresql_trace), + url(r'^admin/', admin.site.urls), + url(r'^$', app.views.home), + url(r'^greetings$', app.views.greetings), + url(r'^_ah/health$', app.views.health_check), + url(r'^request$', app.views.get_request_header), + url(r'^mysql$', app.views.mysql_trace), + url(r'^postgresql$', app.views.postgresql_trace), + url(r'^sqlalchemy_mysql$', app.views.sqlalchemy_mysql_trace), + url(r'^sqlalchemy_postgresql$', app.views.sqlalchemy_postgresql_trace), ] From 877c40fc3a441f9499646f4a8cfc1ea0979400b3 Mon Sep 17 00:00:00 2001 From: William Stewart Date: Tue, 26 Mar 2019 09:54:27 +0100 Subject: [PATCH 11/13] Do Django 2.0+ middlewares properly --- .../opencensus-ext-django/examples/app/settings.py | 6 ++++-- tests/system/trace/django/app/settings.py | 13 ++++++++----- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/contrib/opencensus-ext-django/examples/app/settings.py b/contrib/opencensus-ext-django/examples/app/settings.py index bb45463d2..3df79c4c4 100644 --- a/contrib/opencensus-ext-django/examples/app/settings.py +++ b/contrib/opencensus-ext-django/examples/app/settings.py @@ -32,18 +32,20 @@ 'opencensus.trace.ext.django', ) -MIDDLEWARE_CLASSES = ( +MIDDLEWARE = ( 'django.contrib.sessions.middleware.SessionMiddleware', 'django.middleware.common.CommonMiddleware', 'django.middleware.csrf.CsrfViewMiddleware', 'django.contrib.auth.middleware.AuthenticationMiddleware', - 'django.contrib.auth.middleware.SessionAuthenticationMiddleware', 'django.contrib.messages.middleware.MessageMiddleware', 'django.middleware.clickjacking.XFrameOptionsMiddleware', 'django.middleware.security.SecurityMiddleware', 'opencensus.trace.ext.django.middleware.OpencensusMiddleware', ) +if django.VERSION < (2,): + MIDDLEWARE += ('django.contrib.auth.middleware.SessionAuthenticationMiddleware',) + ROOT_URLCONF = 'app.urls' TEMPLATES = [ diff --git a/tests/system/trace/django/app/settings.py b/tests/system/trace/django/app/settings.py index fb2db39e2..00046e4a5 100644 --- a/tests/system/trace/django/app/settings.py +++ b/tests/system/trace/django/app/settings.py @@ -34,8 +34,9 @@ 'opencensus.ext.django', ) -if django.VERSION >= (1, 10): - MIDDLEWARE = ( +if django.VERSION <= (1, 10): + # Middleware interface for Django version before 1.10 + MIDDLEWARE_CLASSES = ( 'django.contrib.sessions.middleware.SessionMiddleware', 'django.middleware.common.CommonMiddleware', 'django.middleware.csrf.CsrfViewMiddleware', @@ -47,19 +48,21 @@ 'opencensus.ext.django.middleware.OpencensusMiddleware', ) -# Middleware interface for Django version before 1.10 -MIDDLEWARE_CLASSES = ( +MIDDLEWARE = ( 'django.contrib.sessions.middleware.SessionMiddleware', 'django.middleware.common.CommonMiddleware', 'django.middleware.csrf.CsrfViewMiddleware', 'django.contrib.auth.middleware.AuthenticationMiddleware', - 'django.contrib.auth.middleware.SessionAuthenticationMiddleware', 'django.contrib.messages.middleware.MessageMiddleware', 'django.middleware.clickjacking.XFrameOptionsMiddleware', 'django.middleware.security.SecurityMiddleware', 'opencensus.ext.django.middleware.OpencensusMiddleware', ) +# SessionAuthentication is unconditionally enabled for Django versions greater than 2 +if django.VERSION < (2,): + MIDDLEWARE += ('django.contrib.auth.middleware.SessionAuthenticationMiddleware',) + ROOT_URLCONF = 'app.urls' TEMPLATES = [ From 6d29b490d2a81574bf8136b0134f80f012f3a77d Mon Sep 17 00:00:00 2001 From: William Stewart Date: Tue, 26 Mar 2019 11:09:04 +0100 Subject: [PATCH 12/13] Missing import --- contrib/opencensus-ext-django/examples/app/settings.py | 1 + 1 file changed, 1 insertion(+) diff --git a/contrib/opencensus-ext-django/examples/app/settings.py b/contrib/opencensus-ext-django/examples/app/settings.py index 3df79c4c4..e57a7aa04 100644 --- a/contrib/opencensus-ext-django/examples/app/settings.py +++ b/contrib/opencensus-ext-django/examples/app/settings.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. """Django settings for test app.""" +import django # Build paths inside the project like this: os.path.join(BASE_DIR, ...) import os From 52f1cd8ab2ec8fa6c5b40f088164d5cc1d26ac48 Mon Sep 17 00:00:00 2001 From: William Stewart Date: Tue, 26 Mar 2019 12:16:41 +0100 Subject: [PATCH 13/13] Line lengths --- contrib/opencensus-ext-django/examples/app/settings.py | 4 +++- tests/system/trace/django/app/settings.py | 7 +++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/contrib/opencensus-ext-django/examples/app/settings.py b/contrib/opencensus-ext-django/examples/app/settings.py index e57a7aa04..faa0d133d 100644 --- a/contrib/opencensus-ext-django/examples/app/settings.py +++ b/contrib/opencensus-ext-django/examples/app/settings.py @@ -45,7 +45,9 @@ ) if django.VERSION < (2,): - MIDDLEWARE += ('django.contrib.auth.middleware.SessionAuthenticationMiddleware',) + MIDDLEWARE += ( + 'django.contrib.auth.middleware.SessionAuthenticationMiddleware', + ) ROOT_URLCONF = 'app.urls' diff --git a/tests/system/trace/django/app/settings.py b/tests/system/trace/django/app/settings.py index 00046e4a5..a1bf153ad 100644 --- a/tests/system/trace/django/app/settings.py +++ b/tests/system/trace/django/app/settings.py @@ -59,9 +59,12 @@ 'opencensus.ext.django.middleware.OpencensusMiddleware', ) -# SessionAuthentication is unconditionally enabled for Django versions greater than 2 +# SessionAuthentication is unconditionally enabled for Django versions greater +# than 2 if django.VERSION < (2,): - MIDDLEWARE += ('django.contrib.auth.middleware.SessionAuthenticationMiddleware',) + MIDDLEWARE += ( + 'django.contrib.auth.middleware.SessionAuthenticationMiddleware', + ) ROOT_URLCONF = 'app.urls'