From caad080df41374ef7fc70a7c7c763cdc115d9764 Mon Sep 17 00:00:00 2001 From: dimitraparaskevopoulou Date: Fri, 23 Apr 2021 08:38:43 +0200 Subject: [PATCH 1/7] adding path templating feature for django --- instana/instrumentation/django/middleware.py | 52 +++++++++++++++++++- 1 file changed, 50 insertions(+), 2 deletions(-) diff --git a/instana/instrumentation/django/middleware.py b/instana/instrumentation/django/middleware.py index 6562e299..40747389 100644 --- a/instana/instrumentation/django/middleware.py +++ b/instana/instrumentation/django/middleware.py @@ -24,6 +24,7 @@ class InstanaMiddleware(MiddlewareMixin): """ Django Middleware to provide request tracing for Instana """ + def __init__(self, get_response=None): super(InstanaMiddleware, self).__init__(get_response) self.get_response = get_response @@ -46,7 +47,8 @@ def process_request(self, request): if 'PATH_INFO' in env: request.iscope.span.set_tag(ext.HTTP_URL, env['PATH_INFO']) if 'QUERY_STRING' in env and len(env['QUERY_STRING']): - scrubbed_params = strip_secrets_from_query(env['QUERY_STRING'], agent.options.secrets_matcher, agent.options.secrets_list) + scrubbed_params = strip_secrets_from_query(env['QUERY_STRING'], agent.options.secrets_matcher, + agent.options.secrets_list) request.iscope.span.set_tag("http.params", scrubbed_params) if 'HTTP_HOST' in env: request.iscope.span.set_tag("http.host", env['HTTP_HOST']) @@ -58,7 +60,22 @@ def process_response(self, request, response): if request.iscope is not None: if 500 <= response.status_code <= 511: request.iscope.span.assure_errored() - + # for django >= 2.2 + if request.resolver_match is not None and hasattr(request.resolver_match, 'route'): + path_tpl = request.resolver_match.route + # django < 2.2 or in case of 404 + else: + try: + from django.urls import resolve + view_name = resolve(request.path)._func_path + path_tpl = self.__url_pattern_route(view_name) + except Exception: + # the resolve method can fire a Resolver404 exception, in this case there is no matching route + # so the path_tpl is set to None in order not to be added as a tag + path_tpl = None + if path_tpl: + path_tpl = path_tpl.replace(">", "}").replace("<", "{").replace("^", "") + request.iscope.span.set_tag("http.path_tpl", path_tpl) request.iscope.span.set_tag(ext.HTTP_STATUS_CODE, response.status_code) tracer.inject(request.iscope.span.context, ot.Format.HTTP_HEADERS, response) response['Server-Timing'] = "intid;desc=%s" % request.iscope.span.context.trace_id @@ -79,6 +96,37 @@ def process_exception(self, request, exception): if request.iscope is not None: request.iscope.span.log_exception(exception) + def __url_pattern_route(self, view_name): + from django.conf import settings + try: + from django.urls import RegexURLPattern as URLPattern + from django.urls import RegexURLResolver as URLResolver + except ImportError: + from django.urls import URLPattern, URLResolver + + urlconf = __import__(settings.ROOT_URLCONF, {}, {}, ['']) + + def list_urls(lis, parent_pattern=None): + if parent_pattern is None: + parent_pattern = [] + if not lis: + return + l = lis[0] + if isinstance(l, URLPattern): + if l.lookup_str == view_name: + if hasattr(l, "regex"): + return parent_pattern + [str(l.regex.pattern)] + else: + return parent_pattern + [str(l.pattern)] + elif isinstance(l, URLResolver): + if hasattr(l, "regex"): + return list_urls(l.url_patterns, parent_pattern + [str(l.regex.pattern)]) + else: + return list_urls(l.url_patterns, parent_pattern + [str(l.pattern)]) + return "".join(list_urls(lis[1:], parent_pattern)) + + return list_urls(urlconf.urlpatterns) + def load_middleware_wrapper(wrapped, instance, args, kwargs): try: From 5030ca9c940e525a23ef954e9e5f7ec90c33334a Mon Sep 17 00:00:00 2001 From: dimitraparaskevopoulou Date: Fri, 23 Apr 2021 12:40:35 +0200 Subject: [PATCH 2/7] fixing bug --- instana/instrumentation/django/middleware.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/instana/instrumentation/django/middleware.py b/instana/instrumentation/django/middleware.py index 40747389..f1f985b6 100644 --- a/instana/instrumentation/django/middleware.py +++ b/instana/instrumentation/django/middleware.py @@ -68,7 +68,7 @@ def process_response(self, request, response): try: from django.urls import resolve view_name = resolve(request.path)._func_path - path_tpl = self.__url_pattern_route(view_name) + path_tpl = "".join(self.__url_pattern_route(view_name)) except Exception: # the resolve method can fire a Resolver404 exception, in this case there is no matching route # so the path_tpl is set to None in order not to be added as a tag @@ -123,7 +123,7 @@ def list_urls(lis, parent_pattern=None): return list_urls(l.url_patterns, parent_pattern + [str(l.regex.pattern)]) else: return list_urls(l.url_patterns, parent_pattern + [str(l.pattern)]) - return "".join(list_urls(lis[1:], parent_pattern)) + return list_urls(lis[1:], parent_pattern) return list_urls(urlconf.urlpatterns) From 5f0abba06d2bdc6164ecece1f34b612bce4f156d Mon Sep 17 00:00:00 2001 From: dimitraparaskevopoulou Date: Mon, 26 Apr 2021 10:12:36 +0200 Subject: [PATCH 3/7] adding unittests --- tests/frameworks/test_django.py | 41 ++++++++++++++++++++++++++++----- 1 file changed, 35 insertions(+), 6 deletions(-) diff --git a/tests/frameworks/test_django.py b/tests/frameworks/test_django.py index cb54c905..edab2b29 100644 --- a/tests/frameworks/test_django.py +++ b/tests/frameworks/test_django.py @@ -28,7 +28,7 @@ def tearDown(self): def test_basic_request(self): with tracer.start_active_span('test'): - response = self.http.request('GET', self.live_server_url + '/') + response = self.http.request('GET', self.live_server_url + '/', fields={"test": 1}) assert response self.assertEqual(200, response.status) @@ -70,10 +70,12 @@ def test_basic_request(self): self.assertIsNone(test_span.sy) self.assertEqual(None, django_span.ec) - self.assertEqual('/', django_span.data["http"]["url"]) self.assertEqual('GET', django_span.data["http"]["method"]) self.assertEqual(200, django_span.data["http"]["status"]) + self.assertEqual('test=1', django_span.data["http"]["params"]) + self.assertEqual('$', django_span.data["http"]["path_tpl"]) + self.assertIsNone(django_span.stack) def test_synthetic_request(self): @@ -94,6 +96,8 @@ def test_synthetic_request(self): urllib3_span = spans[1] django_span = spans[0] + self.assertEqual('$', django_span.data["http"]["path_tpl"]) + self.assertTrue(django_span.sy) self.assertIsNone(urllib3_span.sy) self.assertIsNone(test_span.sy) @@ -115,15 +119,15 @@ def test_request_with_error(self): filter = lambda span: span.n == 'sdk' and span.data['sdk']['name'] == 'test' test_span = get_first_span_by_filter(spans, filter) - assert(test_span) + assert (test_span) filter = lambda span: span.n == 'urllib3' urllib3_span = get_first_span_by_filter(spans, filter) - assert(urllib3_span) + assert (urllib3_span) filter = lambda span: span.n == 'django' django_span = get_first_span_by_filter(spans, filter) - assert(django_span) + assert (django_span) assert ('X-INSTANA-T' in response.headers) assert (int(response.headers['X-INSTANA-T'], 16)) @@ -156,6 +160,7 @@ def test_request_with_error(self): self.assertEqual('GET', django_span.data["http"]["method"]) self.assertEqual(500, django_span.data["http"]["status"]) self.assertEqual('This is a fake error: /cause-error', django_span.data["http"]["error"]) + self.assertEqual('cause_error$', django_span.data["http"]["path_tpl"]) self.assertIsNone(django_span.stack) def test_request_with_not_found(self): @@ -175,8 +180,30 @@ def test_request_with_not_found(self): filter = lambda span: span.n == 'django' django_span = get_first_span_by_filter(spans, filter) - assert(django_span) + assert (django_span) + + self.assertIsNone(django_span.ec) + self.assertEqual(404, django_span.data["http"]["status"]) + + def test_request_with_not_found_no_route(self): + with tracer.start_active_span('test'): + response = self.http.request('GET', self.live_server_url + '/no_route') + + assert response + self.assertEqual(404, response.status) + spans = self.recorder.queued_spans() + spans = drop_log_spans_from_list(spans) + + span_count = len(spans) + if span_count != 3: + msg = "Expected 3 spans but got %d" % span_count + fail_with_message_and_span_dump(msg, spans) + + filter = lambda span: span.n == 'django' + django_span = get_first_span_by_filter(spans, filter) + assert (django_span) + self.assertIsNone(django_span.data["http"]["path_tpl"]) self.assertIsNone(django_span.ec) self.assertEqual(404, django_span.data["http"]["status"]) @@ -232,6 +259,7 @@ def test_complex_request(self): self.assertEqual('/complex', django_span.data["http"]["url"]) self.assertEqual('GET', django_span.data["http"]["method"]) self.assertEqual(200, django_span.data["http"]["status"]) + self.assertEqual('complex$', django_span.data["http"]["path_tpl"]) def test_custom_header_capture(self): # Hack together a manual custom headers list @@ -271,6 +299,7 @@ def test_custom_header_capture(self): self.assertEqual('/', django_span.data["http"]["url"]) self.assertEqual('GET', django_span.data["http"]["method"]) self.assertEqual(200, django_span.data["http"]["status"]) + self.assertEqual('$', django_span.data["http"]["path_tpl"]) assert "X-Capture-This" in django_span.data["http"]["header"] self.assertEqual("this", django_span.data["http"]["header"]["X-Capture-This"]) From 238bcd8b6fe7cab563d9ce15a0bd21aa12b641b8 Mon Sep 17 00:00:00 2001 From: dimitraparaskevopoulou Date: Mon, 26 Apr 2021 10:16:09 +0200 Subject: [PATCH 4/7] deprecating django vesrions less than 1.11 --- instana/instrumentation/django/middleware.py | 6 +----- tests/requirements-27.txt | 2 +- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/instana/instrumentation/django/middleware.py b/instana/instrumentation/django/middleware.py index f1f985b6..d3e2f9f7 100644 --- a/instana/instrumentation/django/middleware.py +++ b/instana/instrumentation/django/middleware.py @@ -13,14 +13,10 @@ from ...log import logger from ...singletons import agent, tracer from ...util.secrets import strip_secrets_from_query +from django.utils.deprecation import MiddlewareMixin DJ_INSTANA_MIDDLEWARE = 'instana.instrumentation.django.middleware.InstanaMiddleware' -try: - from django.utils.deprecation import MiddlewareMixin -except ImportError: - MiddlewareMixin = object - class InstanaMiddleware(MiddlewareMixin): """ Django Middleware to provide request tracing for Instana """ diff --git a/tests/requirements-27.txt b/tests/requirements-27.txt index da2e5264..ced683c6 100644 --- a/tests/requirements-27.txt +++ b/tests/requirements-27.txt @@ -3,7 +3,7 @@ aiohttp>=3.5.4;python_version>="3.5" asynqp>=0.4;python_version>="3.5" boto3>=1.10.0 celery>=4.1.1 -django<2.0.0 +django>=1.11,<2.0.0 fastapi>=0.61.1;python_version>="3.6" flask>=0.12.2 grpcio>=1.18.0 From 7c041701069d0ba1c03b617eb44b929a82dd72ca Mon Sep 17 00:00:00 2001 From: dimitraparaskevopoulou Date: Mon, 26 Apr 2021 10:48:52 +0200 Subject: [PATCH 5/7] required for initialization of the module without django installation --- instana/instrumentation/django/middleware.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/instana/instrumentation/django/middleware.py b/instana/instrumentation/django/middleware.py index d3e2f9f7..f1f985b6 100644 --- a/instana/instrumentation/django/middleware.py +++ b/instana/instrumentation/django/middleware.py @@ -13,10 +13,14 @@ from ...log import logger from ...singletons import agent, tracer from ...util.secrets import strip_secrets_from_query -from django.utils.deprecation import MiddlewareMixin DJ_INSTANA_MIDDLEWARE = 'instana.instrumentation.django.middleware.InstanaMiddleware' +try: + from django.utils.deprecation import MiddlewareMixin +except ImportError: + MiddlewareMixin = object + class InstanaMiddleware(MiddlewareMixin): """ Django Middleware to provide request tracing for Instana """ From 8da328b249c9ad83afc1b19338bb8a4118b09659 Mon Sep 17 00:00:00 2001 From: dimitraparaskevopoulou Date: Thu, 29 Apr 2021 15:05:19 +0200 Subject: [PATCH 6/7] take out removal of special characters and removing identical test case from asynqp --- instana/instrumentation/django/middleware.py | 1 - tests/clients/test_asynqp.py | 39 -------------------- tests/frameworks/test_django.py | 10 ++--- 3 files changed, 5 insertions(+), 45 deletions(-) diff --git a/instana/instrumentation/django/middleware.py b/instana/instrumentation/django/middleware.py index f1f985b6..ddbeca6d 100644 --- a/instana/instrumentation/django/middleware.py +++ b/instana/instrumentation/django/middleware.py @@ -74,7 +74,6 @@ def process_response(self, request, response): # so the path_tpl is set to None in order not to be added as a tag path_tpl = None if path_tpl: - path_tpl = path_tpl.replace(">", "}").replace("<", "{").replace("^", "") request.iscope.span.set_tag("http.path_tpl", path_tpl) request.iscope.span.set_tag(ext.HTTP_STATUS_CODE, response.status_code) tracer.inject(request.iscope.span.context, ot.Format.HTTP_HEADERS, response) diff --git a/tests/clients/test_asynqp.py b/tests/clients/test_asynqp.py index 8b6fc0f4..a7262ad9 100644 --- a/tests/clients/test_asynqp.py +++ b/tests/clients/test_asynqp.py @@ -109,45 +109,6 @@ def test(): self.assertTrue(type(rabbitmq_span.stack) is list) self.assertGreater(len(rabbitmq_span.stack), 0) - def test_publish_alternative(self): - @asyncio.coroutine - def test(): - with async_tracer.start_active_span('test'): - msg = asynqp.Message({'hello': 'world'}, content_type='application/json') - self.exchange.publish(msg, routing_key='routing.key') - - self.loop.run_until_complete(test()) - - spans = self.recorder.queued_spans() - self.assertEqual(2, len(spans)) - - rabbitmq_span = spans[0] - test_span = spans[1] - - self.assertIsNone(async_tracer.active_span) - - # Same traceId - self.assertEqual(test_span.t, rabbitmq_span.t) - - # Parent relationships - self.assertEqual(rabbitmq_span.p, test_span.s) - - # Error logging - self.assertIsNone(test_span.ec) - self.assertIsNone(rabbitmq_span.ec) - - # Span type - self.assertEqual(rabbitmq_span.k, 2) # exit - - # Rabbitmq - self.assertEqual('test.exchange', rabbitmq_span.data["rabbitmq"]["exchange"]) - self.assertEqual('publish', rabbitmq_span.data["rabbitmq"]["sort"]) - self.assertIsNotNone(rabbitmq_span.data["rabbitmq"]["address"]) - self.assertEqual('routing.key', rabbitmq_span.data["rabbitmq"]["key"]) - self.assertIsNotNone(rabbitmq_span.stack) - self.assertTrue(type(rabbitmq_span.stack) is list) - self.assertGreater(len(rabbitmq_span.stack), 0) - def test_many_publishes(self): @asyncio.coroutine def test(): diff --git a/tests/frameworks/test_django.py b/tests/frameworks/test_django.py index edab2b29..bd743c91 100644 --- a/tests/frameworks/test_django.py +++ b/tests/frameworks/test_django.py @@ -74,7 +74,7 @@ def test_basic_request(self): self.assertEqual('GET', django_span.data["http"]["method"]) self.assertEqual(200, django_span.data["http"]["status"]) self.assertEqual('test=1', django_span.data["http"]["params"]) - self.assertEqual('$', django_span.data["http"]["path_tpl"]) + self.assertEqual('^$', django_span.data["http"]["path_tpl"]) self.assertIsNone(django_span.stack) @@ -96,7 +96,7 @@ def test_synthetic_request(self): urllib3_span = spans[1] django_span = spans[0] - self.assertEqual('$', django_span.data["http"]["path_tpl"]) + self.assertEqual('^$', django_span.data["http"]["path_tpl"]) self.assertTrue(django_span.sy) self.assertIsNone(urllib3_span.sy) @@ -160,7 +160,7 @@ def test_request_with_error(self): self.assertEqual('GET', django_span.data["http"]["method"]) self.assertEqual(500, django_span.data["http"]["status"]) self.assertEqual('This is a fake error: /cause-error', django_span.data["http"]["error"]) - self.assertEqual('cause_error$', django_span.data["http"]["path_tpl"]) + self.assertEqual('^cause_error$', django_span.data["http"]["path_tpl"]) self.assertIsNone(django_span.stack) def test_request_with_not_found(self): @@ -259,7 +259,7 @@ def test_complex_request(self): self.assertEqual('/complex', django_span.data["http"]["url"]) self.assertEqual('GET', django_span.data["http"]["method"]) self.assertEqual(200, django_span.data["http"]["status"]) - self.assertEqual('complex$', django_span.data["http"]["path_tpl"]) + self.assertEqual('^complex$', django_span.data["http"]["path_tpl"]) def test_custom_header_capture(self): # Hack together a manual custom headers list @@ -299,7 +299,7 @@ def test_custom_header_capture(self): self.assertEqual('/', django_span.data["http"]["url"]) self.assertEqual('GET', django_span.data["http"]["method"]) self.assertEqual(200, django_span.data["http"]["status"]) - self.assertEqual('$', django_span.data["http"]["path_tpl"]) + self.assertEqual('^$', django_span.data["http"]["path_tpl"]) assert "X-Capture-This" in django_span.data["http"]["header"] self.assertEqual("this", django_span.data["http"]["header"]["X-Capture-This"]) From 4d855d8e77f5661bf4bf4d8c6e184ce445579ac4 Mon Sep 17 00:00:00 2001 From: dimitraparaskevopoulou Date: Fri, 30 Apr 2021 11:55:22 +0200 Subject: [PATCH 7/7] renaming variables for clarity --- instana/instrumentation/django/middleware.py | 28 ++++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/instana/instrumentation/django/middleware.py b/instana/instrumentation/django/middleware.py index ddbeca6d..21b5a5c4 100644 --- a/instana/instrumentation/django/middleware.py +++ b/instana/instrumentation/django/middleware.py @@ -105,24 +105,24 @@ def __url_pattern_route(self, view_name): urlconf = __import__(settings.ROOT_URLCONF, {}, {}, ['']) - def list_urls(lis, parent_pattern=None): + def list_urls(urlpatterns, parent_pattern=None): + if not urlpatterns: + return if parent_pattern is None: parent_pattern = [] - if not lis: - return - l = lis[0] - if isinstance(l, URLPattern): - if l.lookup_str == view_name: - if hasattr(l, "regex"): - return parent_pattern + [str(l.regex.pattern)] + first = urlpatterns[0] + if isinstance(first, URLPattern): + if first.lookup_str == view_name: + if hasattr(first, "regex"): + return parent_pattern + [str(first.regex.pattern)] else: - return parent_pattern + [str(l.pattern)] - elif isinstance(l, URLResolver): - if hasattr(l, "regex"): - return list_urls(l.url_patterns, parent_pattern + [str(l.regex.pattern)]) + return parent_pattern + [str(first.pattern)] + elif isinstance(first, URLResolver): + if hasattr(first, "regex"): + return list_urls(first.url_patterns, parent_pattern + [str(first.regex.pattern)]) else: - return list_urls(l.url_patterns, parent_pattern + [str(l.pattern)]) - return list_urls(lis[1:], parent_pattern) + return list_urls(first.url_patterns, parent_pattern + [str(first.pattern)]) + return list_urls(urlpatterns[1:], parent_pattern) return list_urls(urlconf.urlpatterns)