From 4db9aa9373659c607d5b250ad030556d7e3accf0 Mon Sep 17 00:00:00 2001 From: Owais Lone Date: Thu, 1 Apr 2021 13:49:52 +0530 Subject: [PATCH] [Django] Added support for traceresponse headers Added opt-in support to return traceresponse headers from Django. This allows users to configure their Django apps to inject trace context as headers in HTTP responses. This is useful when client side apps need to connect their spans with the server side spans e.g, in RUM products. Today the most practical way to do this is to use the `Server-Timing` header but in near future we might use the `traceresponse` header as described here: https://w3c.github.io/trace-context/#trace-context-http-response-headers-format As a result the implementation does not use a hard-coded header and instead let's the users pick one. This can be done by setting the `OTEL_PYTHON_TRACE_RESPONSE_HEADER` to the header name that users want to inject in HTTP responses. The option does not have a default value and the feature is disbaled when a env var is not set. --- CHANGELOG.md | 2 + .../instrumentation/django/middleware.py | 23 +++-- .../tests/test_middleware.py | 83 ++++++++++++++++++- .../tests/views.py | 9 ++ .../src/opentelemetry/util/http/__init__.py | 38 ++++++++- 5 files changed, 144 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d06238a5f2..16aca5862f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - `opentelemetry-instrumentation-urllib3` Add urllib3 instrumentation ([#299](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/299)) +- Add trace response header support for Django. + ([#395](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/395)) ## [0.19b0](https://github.com/open-telemetry/opentelemetry-python-contrib/releases/tag/v0.19b0) - 2021-03-26 diff --git a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py index d07777ff04..9a40e79fff 100644 --- a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py @@ -25,7 +25,11 @@ ) from opentelemetry.propagate import extract from opentelemetry.trace import SpanKind, get_tracer, use_span -from opentelemetry.util.http import get_excluded_urls, get_traced_request_attrs +from opentelemetry.util.http import ( + get_excluded_urls, + get_trace_response_headers, + get_traced_request_attrs, +) try: from django.core.urlresolvers import ( # pylint: disable=no-name-in-module @@ -156,18 +160,23 @@ def process_response(self, request, response): if self._excluded_urls.url_disabled(request.build_absolute_uri("?")): return response - if ( - self._environ_activation_key in request.META.keys() - and self._environ_span_key in request.META.keys() - ): + span = request.META.pop(self._environ_span_key, None) + if span and self._environ_activation_key in request.META.keys(): + # record span attributes from response add_response_attributes( - request.META[self._environ_span_key], + span, "{} {}".format(response.status_code, response.reason_phrase), response, ) - request.META.pop(self._environ_span_key) + # inject trace response headers + for header, value in get_trace_response_headers(span): + old_value = response.get(header, "") + if old_value: + value = "{0}, {1}".format(old_value, value) + response[header] = value + # record any exceptions raised while processing the request exception = request.META.pop(self._environ_exception_key, None) if exception: request.META[self._environ_activation_key].__exit__( diff --git a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py index 154e68bc15..05303d0b2e 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +from os import environ from sys import modules from unittest.mock import Mock, patch @@ -25,7 +26,12 @@ from opentelemetry.test.test_base import TestBase from opentelemetry.test.wsgitestutil import WsgiTestBase from opentelemetry.trace import SpanKind, StatusCode -from opentelemetry.util.http import get_excluded_urls, get_traced_request_attrs +from opentelemetry.util.http import ( + ENV_HTTP_TRACE_RESPONSE_HEADER, + get_excluded_urls, + get_trace_response_headers, + get_traced_request_attrs, +) # pylint: disable=import-error from .views import ( @@ -33,6 +39,7 @@ excluded, excluded_noarg, excluded_noarg2, + with_response_header, route_span_name, traced, traced_template, @@ -47,6 +54,7 @@ url(r"^excluded_arg/", excluded), url(r"^excluded_noarg/", excluded_noarg), url(r"^excluded_noarg2/", excluded_noarg2), + url(r"^response_header/", with_response_header), url(r"^span_name/([0-9]{4})/$", route_span_name), ] _django_instrumentor = DjangoInstrumentor() @@ -268,3 +276,76 @@ def test_traced_request_attrs(self): self.assertEqual(span.attributes["path_info"], "/span_name/1234/") self.assertEqual(span.attributes["content_type"], "test/ct") self.assertNotIn("non_existing_variable", span.attributes) + + def test_trace_response_header(self): + original_env_var = environ.pop(ENV_HTTP_TRACE_RESPONSE_HEADER, None) + response = Client().get("/span_name/1234/") + self.assertNotIn("Server-Timing", response._headers) + self.memory_exporter.clear() + + for header in ["Server-Timing", "traceresponse"]: + environ[ENV_HTTP_TRACE_RESPONSE_HEADER] = header + + response = Client().get("/span_name/1234/") + span = self.memory_exporter.get_finished_spans()[0] + headers = get_trace_response_headers(span) + self.assertEqual(len(headers), 2) + + access_control_header = headers[0] + response_header = headers[1] + + self.assertIn(header.lower(), response._headers) + self.assertEqual( + response._headers["access-control-expose-headers"][0], + access_control_header[0], + ) + self.assertEqual( + response._headers["access-control-expose-headers"][1], + access_control_header[1], + ) + self.assertEqual( + response._headers[header.lower()][0], response_header[0] + ) + self.assertEqual( + response._headers[header.lower()][1], response_header[1] + ) + + self.memory_exporter.clear() + del environ[ENV_HTTP_TRACE_RESPONSE_HEADER] + + if original_env_var: + environ[ENV_HTTP_TRACE_RESPONSE_HEADER] = original_env_var + + def test_trace_response_header_pre_existing_header(self): + original_env_var = environ.pop(ENV_HTTP_TRACE_RESPONSE_HEADER, None) + environ[ENV_HTTP_TRACE_RESPONSE_HEADER] = "Server-Timing" + response = Client().get("/response_header/") + + span = self.memory_exporter.get_finished_spans()[0] + headers = get_trace_response_headers(span) + self.assertEqual(len(headers), 2) + access_control_header = headers[0] + response_header = headers[1] + + self.assertIn("server-timing", response._headers) + self.assertEqual( + response._headers["access-control-expose-headers"][0], + access_control_header[0], + ) + self.assertEqual( + response._headers["access-control-expose-headers"][1], + "X-Test-Header, Server-Timing", + ) + self.assertEqual( + response._headers["server-timing"][0], response_header[0] + ) + self.assertEqual( + response._headers["server-timing"][1], + "abc; val=1, " + response_header[1], + ) + + self.memory_exporter.clear() + del environ[ENV_HTTP_TRACE_RESPONSE_HEADER] + + if original_env_var: + environ[ENV_HTTP_TRACE_RESPONSE_HEADER] = original_env_var diff --git a/instrumentation/opentelemetry-instrumentation-django/tests/views.py b/instrumentation/opentelemetry-instrumentation-django/tests/views.py index 872222a842..eaff6cf0f7 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/views.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/views.py @@ -1,5 +1,7 @@ from django.http import HttpResponse +from opentelemetry.util.http import HTTP_HEADER_ACCESS_CONTROL_EXPOSE_HEADERS + def traced(request): # pylint: disable=unused-argument return HttpResponse() @@ -29,3 +31,10 @@ def route_span_name( request, *args, **kwargs ): # pylint: disable=unused-argument return HttpResponse() + + +def with_response_header(request): # pylint: disable=unused-argument + response = HttpResponse() + response["Server-Timing"] = "abc; val=1" + response[HTTP_HEADER_ACCESS_CONTROL_EXPOSE_HEADERS] = "X-Test-Header" + return response diff --git a/util/opentelemetry-util-http/src/opentelemetry/util/http/__init__.py b/util/opentelemetry-util-http/src/opentelemetry/util/http/__init__.py index 068511010d..7a955c23ec 100644 --- a/util/opentelemetry-util-http/src/opentelemetry/util/http/__init__.py +++ b/util/opentelemetry-util-http/src/opentelemetry/util/http/__init__.py @@ -15,6 +15,14 @@ from os import environ from re import compile as re_compile from re import search +from typing import Callable, Collection, Optional, Tuple + +from opentelemetry import trace + +_root = r"OTEL_PYTHON_{}" + +HTTP_HEADER_ACCESS_CONTROL_EXPOSE_HEADERS = "Access-Control-Expose-Headers" +ENV_HTTP_TRACE_RESPONSE_HEADER = "OTEL_PYTHON_HTTP_TRACE_RESPONSE_HEADER" class ExcludeList: @@ -29,9 +37,6 @@ def url_disabled(self, url: str) -> bool: return bool(self._excluded_urls and search(self._regex, url)) -_root = r"OTEL_PYTHON_{}" - - def get_traced_request_attrs(instrumentation): traced_request_attrs = environ.get( _root.format("{}_TRACED_REQUEST_ATTRS".format(instrumentation)), [] @@ -57,3 +62,30 @@ def get_excluded_urls(instrumentation): ] return ExcludeList(excluded_urls) + + +def get_trace_response_headers( + span: trace.Span, +) -> Collection[Tuple[str, str]]: + header_name = environ.get(ENV_HTTP_TRACE_RESPONSE_HEADER, "").strip() + if not header_name: + return tuple() + + if span is trace.INVALID_SPAN: + return tuple() + + ctx = span.get_span_context() + if ctx is trace.INVALID_SPAN_CONTEXT: + return tuple() + + return ( + (HTTP_HEADER_ACCESS_CONTROL_EXPOSE_HEADERS, header_name), + ( + header_name, + 'traceparent;desc="00-{trace_id}-{span_id}-{sampled}"'.format( + trace_id=trace.format_trace_id(ctx.trace_id), + span_id=trace.format_span_id(ctx.span_id), + sampled="01" if ctx.trace_flags.sampled else "00", + ), + ), + )