From 4d4bda696d7b871024be0f76b77ac1713f2221f7 Mon Sep 17 00:00:00 2001 From: saartochner <47108628+saartochner@users.noreply.github.com> Date: Wed, 20 Sep 2023 17:35:59 +0300 Subject: [PATCH] add django and modernize flask with content (#489) * add django, and modernize flask with content --- noxfile.py | 50 +++++++ setup.py | 1 + .../instrumentations/django/__init__.py | 45 +++++++ .../django/tested_versions/django | 1 + .../instrumentations/flask/__init__.py | 40 ++++-- .../instrumentations/instrumentations.py | 4 + .../instrumentations/redis/__init__.py | 2 +- src/test/integration/django/__init__.py | 0 src/test/integration/django/app/README.md | 4 + src/test/integration/django/app/__init__.py | 0 .../integration/django/app/app/__init__.py | 0 src/test/integration/django/app/app/asgi.py | 16 +++ .../integration/django/app/app/settings.py | 123 ++++++++++++++++++ src/test/integration/django/app/app/urls.py | 22 ++++ src/test/integration/django/app/app/wsgi.py | 16 +++ src/test/integration/django/app/manage.py | 23 ++++ src/test/integration/django/conftest.py | 8 ++ .../django/requirements_others.txt | 4 + .../integration/django/scripts/start_django | 4 + src/test/integration/django/tested_versions | 1 + src/test/integration/django/tests/__init__.py | 0 .../integration/django/tests/test_django.py | 34 +++++ .../integration/flask/tests/test_flask.py | 4 + .../integration/redis/tests/test_redis.py | 2 +- 24 files changed, 389 insertions(+), 15 deletions(-) create mode 100644 src/lumigo_opentelemetry/instrumentations/django/__init__.py create mode 100644 src/lumigo_opentelemetry/instrumentations/django/tested_versions/django create mode 100644 src/test/integration/django/__init__.py create mode 100644 src/test/integration/django/app/README.md create mode 100644 src/test/integration/django/app/__init__.py create mode 100644 src/test/integration/django/app/app/__init__.py create mode 100644 src/test/integration/django/app/app/asgi.py create mode 100644 src/test/integration/django/app/app/settings.py create mode 100644 src/test/integration/django/app/app/urls.py create mode 100644 src/test/integration/django/app/app/wsgi.py create mode 100755 src/test/integration/django/app/manage.py create mode 100644 src/test/integration/django/conftest.py create mode 100644 src/test/integration/django/requirements_others.txt create mode 100755 src/test/integration/django/scripts/start_django create mode 120000 src/test/integration/django/tested_versions create mode 100644 src/test/integration/django/tests/__init__.py create mode 100644 src/test/integration/django/tests/test_django.py diff --git a/noxfile.py b/noxfile.py index a3479d23..83859043 100755 --- a/noxfile.py +++ b/noxfile.py @@ -515,6 +515,56 @@ def integration_tests_flask(session, flask_version): kill_process_and_clean_outputs(temp_file, "flask", session) +@nox.session(python=python_versions()) +@nox.parametrize( + "django_version", + dependency_versions_to_be_tested( + directory="django", + dependency_name="django", + test_untested_versions=should_test_only_untested_versions(), + ), +) +def integration_tests_django(session, django_version): + with TestedVersions.save_tests_result("django", "django", django_version): + install_package("django", django_version, session) + + session.install(".") + + temp_file = create_it_tempfile("django") + with session.chdir("src/test/integration/django"): + session.install("-r", OTHER_REQUIREMENTS) + + try: + session.run( + "sh", + "./scripts/start_django", + env={ + "LUMIGO_DEBUG_SPANDUMP": temp_file, + "OTEL_SERVICE_NAME": "app", + }, + external=True, + ) # One happy day we will have https://github.com/wntrblm/nox/issues/198 + + # TODO Make this deterministic + # Wait 1s to give time for app to start + time.sleep(8) + + session.run( + "pytest", + "--tb", + "native", + "--log-cli-level=INFO", + "--color=yes", + "-v", + "./tests/test_django.py", + env={ + "LUMIGO_DEBUG_SPANDUMP": temp_file, + }, + ) + finally: + kill_process_and_clean_outputs(temp_file, "django", session) + + @nox.session(python=python_versions()) @nox.parametrize( "grpcio_version", diff --git a/setup.py b/setup.py index d5cac44e..a64999de 100644 --- a/setup.py +++ b/setup.py @@ -35,5 +35,6 @@ "opentelemetry-instrumentation-pymysql==0.36b0", "opentelemetry-instrumentation-requests==0.36b0", "opentelemetry-instrumentation-redis==0.36b0", + "opentelemetry-instrumentation-django==0.36b0", ], ) diff --git a/src/lumigo_opentelemetry/instrumentations/django/__init__.py b/src/lumigo_opentelemetry/instrumentations/django/__init__.py new file mode 100644 index 00000000..1fcf73ce --- /dev/null +++ b/src/lumigo_opentelemetry/instrumentations/django/__init__.py @@ -0,0 +1,45 @@ +from opentelemetry.trace.span import Span + +from lumigo_opentelemetry.instrumentations import AbstractInstrumentor +from lumigo_opentelemetry.libs.general_utils import lumigo_safe_execute +from lumigo_opentelemetry.instrumentations.instrumentation_utils import ( + add_body_attribute, +) +from lumigo_opentelemetry.libs.json_utils import dump_with_context + + +class DjangoInstrumentorWrapper(AbstractInstrumentor): + def __init__(self) -> None: + super().__init__("django") + + def check_if_applicable(self) -> None: + import django # noqa + + def install_instrumentation(self) -> None: + from opentelemetry.instrumentation.django import DjangoInstrumentor + from django.http import HttpRequest, HttpResponse + + def request_hook(span: Span, request: HttpRequest) -> None: + with lumigo_safe_execute("django request_hook"): + span.set_attribute( + "http.request.headers", + dump_with_context("requestHeaders", request.headers), + ) + add_body_attribute(span, request.body, "http.request.body") + + def response_hook( + span: Span, request: HttpRequest, response: HttpResponse + ) -> None: + with lumigo_safe_execute("django response_hook"): + span.set_attribute( + "http.response.headers", + dump_with_context("responseHeaders", response.headers), + ) + add_body_attribute(span, response.content, "http.response.body") + + DjangoInstrumentor().instrument( + request_hook=request_hook, response_hook=response_hook + ) + + +instrumentor: AbstractInstrumentor = DjangoInstrumentorWrapper() diff --git a/src/lumigo_opentelemetry/instrumentations/django/tested_versions/django b/src/lumigo_opentelemetry/instrumentations/django/tested_versions/django new file mode 100644 index 00000000..ad35fe0d --- /dev/null +++ b/src/lumigo_opentelemetry/instrumentations/django/tested_versions/django @@ -0,0 +1 @@ +4.2.5 \ No newline at end of file diff --git a/src/lumigo_opentelemetry/instrumentations/flask/__init__.py b/src/lumigo_opentelemetry/instrumentations/flask/__init__.py index 7c8be92c..be9a705e 100644 --- a/src/lumigo_opentelemetry/instrumentations/flask/__init__.py +++ b/src/lumigo_opentelemetry/instrumentations/flask/__init__.py @@ -1,4 +1,10 @@ +from typing import Any, Dict + +from opentelemetry.trace.span import Span + from lumigo_opentelemetry.instrumentations import AbstractInstrumentor +from lumigo_opentelemetry.libs.general_utils import lumigo_safe_execute +from lumigo_opentelemetry.libs.json_utils import dump_with_context class FlaskInstrumentorWrapper(AbstractInstrumentor): @@ -9,19 +15,27 @@ def check_if_applicable(self) -> None: import flask # noqa def install_instrumentation(self) -> None: - import wrapt - from lumigo_opentelemetry import logger - - @wrapt.patch_function_wrapper("flask", "Flask.__init__") - def init_otel_flask_instrumentation(wrapped, instance, args, kwargs): # type: ignore - try: - from opentelemetry.instrumentation.flask import FlaskInstrumentor - - return_value = wrapped(*args, **kwargs) - FlaskInstrumentor().instrument_app(instance) - return return_value - except Exception as e: - logger.exception("failed instrumenting Flask", exc_info=e) + from opentelemetry.instrumentation.flask import FlaskInstrumentor + + def request_hook(span: Span, flask_request_environ: Dict[str, Any]) -> None: + with lumigo_safe_execute("flask_request_hook"): + span.set_attribute( + "http.request.headers", + dump_with_context("requestHeaders", flask_request_environ), + ) + + def response_hook( + span: Span, status: int, response_headers: Dict[str, Any] + ) -> None: + with lumigo_safe_execute("flask_response_hook"): + span.set_attribute( + "http.response.headers", + dump_with_context("responseHeaders", response_headers), + ) + + FlaskInstrumentor().instrument( + request_hook=request_hook, response_hook=response_hook + ) instrumentor: AbstractInstrumentor = FlaskInstrumentorWrapper() diff --git a/src/lumigo_opentelemetry/instrumentations/instrumentations.py b/src/lumigo_opentelemetry/instrumentations/instrumentations.py index 16040021..3eab1922 100644 --- a/src/lumigo_opentelemetry/instrumentations/instrumentations.py +++ b/src/lumigo_opentelemetry/instrumentations/instrumentations.py @@ -9,6 +9,7 @@ from .botocore import instrumentor as botocore_instrumentor from .fastapi import instrumentor as fastapi_instrumentor from .flask import instrumentor as flask_instrumentor +from .django import instrumentor as django_instrumentor from .grpcio import instrumentor as grpc_instrumentor from .kafka_python import instrumentor as kafka_python_instrumentor from .pika import instrumentor as pika_instrumentor @@ -23,6 +24,7 @@ botocore_instrumentor, fastapi_instrumentor, flask_instrumentor, + django_instrumentor, grpc_instrumentor, kafka_python_instrumentor, pika_instrumentor, @@ -46,6 +48,7 @@ "An error occurred while applying the '%s' instrumentation: %s", instrumentor.instrumentation_id, str(e), + exc_info=True, ) logger.debug( @@ -58,6 +61,7 @@ in [ fastapi_instrumentor.instrumentation_id, flask_instrumentor.instrumentation_id, + django_instrumentor.instrumentation_id, ], installed_instrumentations, ) diff --git a/src/lumigo_opentelemetry/instrumentations/redis/__init__.py b/src/lumigo_opentelemetry/instrumentations/redis/__init__.py index 21c64a4e..87a2a7ac 100644 --- a/src/lumigo_opentelemetry/instrumentations/redis/__init__.py +++ b/src/lumigo_opentelemetry/instrumentations/redis/__init__.py @@ -27,7 +27,7 @@ def request_hook( pass def response_hook(span: Span, instance: Connection, response: Any) -> None: - add_body_attribute(span, response, "redis.response.body") + add_body_attribute(span, response, "db.response.body") RedisInstrumentor().instrument( request_hook=request_hook, diff --git a/src/test/integration/django/__init__.py b/src/test/integration/django/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/src/test/integration/django/app/README.md b/src/test/integration/django/app/README.md new file mode 100644 index 00000000..6b87bb53 --- /dev/null +++ b/src/test/integration/django/app/README.md @@ -0,0 +1,4 @@ +THIS DIRECTORY WAS CREATED BY THE DJANGO TEST APP GENERATOR: +```shell +django-admin startproject app +``` \ No newline at end of file diff --git a/src/test/integration/django/app/__init__.py b/src/test/integration/django/app/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/src/test/integration/django/app/app/__init__.py b/src/test/integration/django/app/app/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/src/test/integration/django/app/app/asgi.py b/src/test/integration/django/app/app/asgi.py new file mode 100644 index 00000000..3296932f --- /dev/null +++ b/src/test/integration/django/app/app/asgi.py @@ -0,0 +1,16 @@ +""" +ASGI config for app project. + +It exposes the ASGI callable as a module-level variable named ``application``. + +For more information on this file, see +https://docs.djangoproject.com/en/4.2/howto/deployment/asgi/ +""" + +import os + +from django.core.asgi import get_asgi_application + +os.environ.setdefault("DJANGO_SETTINGS_MODULE", "app.settings") + +application = get_asgi_application() diff --git a/src/test/integration/django/app/app/settings.py b/src/test/integration/django/app/app/settings.py new file mode 100644 index 00000000..e01c1ba1 --- /dev/null +++ b/src/test/integration/django/app/app/settings.py @@ -0,0 +1,123 @@ +""" +Django settings for app project. + +Generated by 'django-admin startproject' using Django 4.2.4. + +For more information on this file, see +https://docs.djangoproject.com/en/4.2/topics/settings/ + +For the full list of settings and their values, see +https://docs.djangoproject.com/en/4.2/ref/settings/ +""" + +from pathlib import Path + +# Build paths inside the project like this: BASE_DIR / 'subdir'. +BASE_DIR = Path(__file__).resolve().parent.parent + + +# Quick-start development settings - unsuitable for production +# See https://docs.djangoproject.com/en/4.2/howto/deployment/checklist/ + +# SECURITY WARNING: keep the secret key used in production secret! +SECRET_KEY = "django-insecure-(^(jp%=9%fu8o5_u1w(!a))bwy1uls0=v@cchpze%_fgur_ac2" + +# SECURITY WARNING: don't run with debug turned on in production! +DEBUG = True + +ALLOWED_HOSTS = [] + + +# Application definition + +INSTALLED_APPS = [ + "django.contrib.admin", + "django.contrib.auth", + "django.contrib.contenttypes", + "django.contrib.sessions", + "django.contrib.messages", + "django.contrib.staticfiles", +] + +MIDDLEWARE = [ + "django.middleware.security.SecurityMiddleware", + "django.contrib.sessions.middleware.SessionMiddleware", + "django.middleware.common.CommonMiddleware", + "django.middleware.csrf.CsrfViewMiddleware", + "django.contrib.auth.middleware.AuthenticationMiddleware", + "django.contrib.messages.middleware.MessageMiddleware", + "django.middleware.clickjacking.XFrameOptionsMiddleware", +] + +ROOT_URLCONF = "app.urls" + +TEMPLATES = [ + { + "BACKEND": "django.template.backends.django.DjangoTemplates", + "DIRS": [], + "APP_DIRS": True, + "OPTIONS": { + "context_processors": [ + "django.template.context_processors.debug", + "django.template.context_processors.request", + "django.contrib.auth.context_processors.auth", + "django.contrib.messages.context_processors.messages", + ], + }, + }, +] + +WSGI_APPLICATION = "app.wsgi.application" + + +# Database +# https://docs.djangoproject.com/en/4.2/ref/settings/#databases + +DATABASES = { + "default": { + "ENGINE": "django.db.backends.sqlite3", + "NAME": BASE_DIR / "db.sqlite3", + } +} + + +# Password validation +# https://docs.djangoproject.com/en/4.2/ref/settings/#auth-password-validators + +AUTH_PASSWORD_VALIDATORS = [ + { + "NAME": "django.contrib.auth.password_validation.UserAttributeSimilarityValidator", + }, + { + "NAME": "django.contrib.auth.password_validation.MinimumLengthValidator", + }, + { + "NAME": "django.contrib.auth.password_validation.CommonPasswordValidator", + }, + { + "NAME": "django.contrib.auth.password_validation.NumericPasswordValidator", + }, +] + + +# Internationalization +# https://docs.djangoproject.com/en/4.2/topics/i18n/ + +LANGUAGE_CODE = "en-us" + +TIME_ZONE = "UTC" + +USE_I18N = True + +USE_TZ = True + + +# Static files (CSS, JavaScript, Images) +# https://docs.djangoproject.com/en/4.2/howto/static-files/ + +STATIC_URL = "static/" + +# Default primary key field type +# https://docs.djangoproject.com/en/4.2/ref/settings/#default-auto-field + +DEFAULT_AUTO_FIELD = "django.db.models.BigAutoField" diff --git a/src/test/integration/django/app/app/urls.py b/src/test/integration/django/app/app/urls.py new file mode 100644 index 00000000..65cf0d69 --- /dev/null +++ b/src/test/integration/django/app/app/urls.py @@ -0,0 +1,22 @@ +""" +URL configuration for app project. + +The `urlpatterns` list routes URLs to views. For more information please see: + https://docs.djangoproject.com/en/4.2/topics/http/urls/ +Examples: +Function views + 1. Add an import: from my_app import views + 2. Add a URL to urlpatterns: path('', views.home, name='home') +Class-based views + 1. Add an import: from other_app.views import Home + 2. Add a URL to urlpatterns: path('', Home.as_view(), name='home') +Including another URLconf + 1. Import the include() function: from django.urls import include, path + 2. Add a URL to urlpatterns: path('blog/', include('blog.urls')) +""" +from django.contrib import admin +from django.urls import path + +urlpatterns = [ + path("admin/", admin.site.urls), +] diff --git a/src/test/integration/django/app/app/wsgi.py b/src/test/integration/django/app/app/wsgi.py new file mode 100644 index 00000000..cbdf4342 --- /dev/null +++ b/src/test/integration/django/app/app/wsgi.py @@ -0,0 +1,16 @@ +""" +WSGI config for app project. + +It exposes the WSGI callable as a module-level variable named ``application``. + +For more information on this file, see +https://docs.djangoproject.com/en/4.2/howto/deployment/wsgi/ +""" + +import os + +from django.core.wsgi import get_wsgi_application + +os.environ.setdefault("DJANGO_SETTINGS_MODULE", "app.settings") + +application = get_wsgi_application() diff --git a/src/test/integration/django/app/manage.py b/src/test/integration/django/app/manage.py new file mode 100755 index 00000000..da396ac5 --- /dev/null +++ b/src/test/integration/django/app/manage.py @@ -0,0 +1,23 @@ +#!/usr/bin/env python +"""Django's command-line utility for administrative tasks.""" +import os + + +def main(): + """Run administrative tasks.""" + os.environ.setdefault("DJANGO_SETTINGS_MODULE", "app.settings") + import lumigo_opentelemetry # noqa: F401 + + try: + from django.core.management import execute_from_command_line + except ImportError as exc: + raise ImportError( + "Couldn't import Django. Are you sure it's installed and " + "available on your PYTHONPATH environment variable? Did you " + "forget to activate a virtual environment?" + ) from exc + execute_from_command_line(["manage.py", "runserver", "5003"]) + + +if __name__ == "__main__": + main() diff --git a/src/test/integration/django/conftest.py b/src/test/integration/django/conftest.py new file mode 100644 index 00000000..988cb3d9 --- /dev/null +++ b/src/test/integration/django/conftest.py @@ -0,0 +1,8 @@ +import pytest + +from test.test_utils.spans_parser import SpansContainer + + +@pytest.fixture(autouse=True) +def increment_spans_counter(): + SpansContainer.increment_spans() diff --git a/src/test/integration/django/requirements_others.txt b/src/test/integration/django/requirements_others.txt new file mode 100644 index 00000000..679beab0 --- /dev/null +++ b/src/test/integration/django/requirements_others.txt @@ -0,0 +1,4 @@ +pytest +json-stream +requests +psutil diff --git a/src/test/integration/django/scripts/start_django b/src/test/integration/django/scripts/start_django new file mode 100755 index 00000000..13880da5 --- /dev/null +++ b/src/test/integration/django/scripts/start_django @@ -0,0 +1,4 @@ +#!/bin/sh + +cd app || exit 1 +python manage.py runserver 5003 & \ No newline at end of file diff --git a/src/test/integration/django/tested_versions b/src/test/integration/django/tested_versions new file mode 120000 index 00000000..919077b8 --- /dev/null +++ b/src/test/integration/django/tested_versions @@ -0,0 +1 @@ +../../../lumigo_opentelemetry/instrumentations/django/tested_versions \ No newline at end of file diff --git a/src/test/integration/django/tests/__init__.py b/src/test/integration/django/tests/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/src/test/integration/django/tests/test_django.py b/src/test/integration/django/tests/test_django.py new file mode 100644 index 00000000..3945d805 --- /dev/null +++ b/src/test/integration/django/tests/test_django.py @@ -0,0 +1,34 @@ +import unittest +from test.test_utils.span_exporter import wait_for_exporter +from test.test_utils.spans_parser import SpansContainer + +import requests + + +class TestDjangoSpans(unittest.TestCase): + def test_200_OK(self): + response = requests.get("http://localhost:5003/", data='{"support": "django"}') + response.raise_for_status() + + body = response.text + + self.assertIsNotNone(body) + + wait_for_exporter() + + spans_container = SpansContainer.get_spans_from_file() + self.assertEqual(1, len(spans_container.spans)) + + # assert root + root = spans_container.get_root() + self.assertIsNotNone(root) + self.assertEqual(root["kind"], "SpanKind.SERVER") + self.assertEqual(root["attributes"]["http.status_code"], 200) + self.assertEqual( + root["attributes"]["http.request.body"], '{"support": "django"}' + ) + self.assertIsNotNone(root["attributes"]["http.request.headers"]) + self.assertIsNotNone(root["attributes"]["http.response.headers"]) + self.assertIsNotNone(root["attributes"]["http.response.body"]) + self.assertEqual(root["attributes"]["http.method"], "GET") + self.assertEqual(root["attributes"]["http.host"], "localhost:5003") diff --git a/src/test/integration/flask/tests/test_flask.py b/src/test/integration/flask/tests/test_flask.py index 975ab88e..2a44a545 100644 --- a/src/test/integration/flask/tests/test_flask.py +++ b/src/test/integration/flask/tests/test_flask.py @@ -24,6 +24,8 @@ def test_200_OK(self): self.assertIsNotNone(root) self.assertEqual(root["kind"], "SpanKind.SERVER") self.assertEqual(root["attributes"]["http.status_code"], 200) + self.assertIsNotNone(root["attributes"]["http.request.headers"]) + self.assertIsNotNone(root["attributes"]["http.response.headers"]) self.assertEqual(root["attributes"]["http.method"], "GET") self.assertEqual(root["attributes"]["http.host"], "localhost:5000") self.assertEqual(root["attributes"]["http.route"], "/") @@ -48,6 +50,8 @@ def test_requests_instrumentation(self): self.assertEqual(root["attributes"]["http.status_code"], 200) self.assertEqual(root["attributes"]["http.host"], "localhost:5000") self.assertEqual(root["attributes"]["http.route"], "/invoke-requests") + self.assertIsNotNone(root["attributes"]["http.request.headers"]) + self.assertIsNotNone(root["attributes"]["http.response.headers"]) # assert child spans children = spans_container.get_non_internal_children() diff --git a/src/test/integration/redis/tests/test_redis.py b/src/test/integration/redis/tests/test_redis.py index 5dc99531..9616f267 100644 --- a/src/test/integration/redis/tests/test_redis.py +++ b/src/test/integration/redis/tests/test_redis.py @@ -11,7 +11,7 @@ ATTRIBUTES = "attributes" DB_STATEMENT = "db.statement" DB_SYSTEM = "db.system" -REDIS_RESPONSE_BODY = "redis.response.body" +REDIS_RESPONSE_BODY = "db.response.body" def run_redis_sample(sample_name: str, redis_host: str, redis_port: int):