chore(tracing/aiohttp_jinja2): automatically enable aiohttp_jinja2 tracing on its own (#3450) (#3507)

aiohttp_jinja2 used to be included in the aiohttp integration. In #3356 the aiohttp_jinja2 integration was separated into its own integration but kept in the testsuite of aiohttp. For backwards compatibility aiohttp_jinja2 was still enabled using the aiohttp integration. Both of these combined this led to a bug that went uncaught by the testsuite which was fixed in #3449.

To avoid further issues and to be consistent with the rest of the library the aiohttp_jinja2 is separated further out with its test suite and is no longer enabled via aiohttp so now the two integrations are independent.

Co-authored-by: Kyle Verhoog <[email protected]>
(cherry picked from commit db50385)

Co-authored-by: Brett Langdon <[email protected]>

Author: mergify[bot]

.circleci/config.yml

@@ -480,7 +480,7 @@ jobs:
     parallelism: 6
     steps:
       - run_test:
-          pattern: 'aiohttp'
+          pattern: 'aiohttp'  # includes aiohttp_jinja2
           snapshot: true
           docker_services: 'httpbin_local'
 

ddtrace/_monkey.py

@@ -52,6 +52,7 @@
     "sqlalchemy": False,  # Prefer DB client instrumentation
     "sqlite3": True,
     "aiohttp": True,  # requires asyncio (Python 3.4+)
+    "aiohttp_jinja2": True,
     "aiopg": True,
     "aiobotocore": False,
     "httplib": False,
@@ -74,7 +75,6 @@
     "fastapi": True,
     "dogpile_cache": True,
     "yaaredis": True,
-    "aiohttp_jinja2": False,  # disabled as this is handled by aiohttp for now.
     "asyncpg": True,
 }
 

ddtrace/contrib/aiohttp/patch.py

@@ -104,11 +104,6 @@ def _patch_client(aiohttp):
 
 
 def patch():
-    # Legacy patch aiohttp_jinja2
-    from ddtrace.contrib.aiohttp_jinja2.patch import patch as aiohttp_jinja2_patch
-
-    aiohttp_jinja2_patch()
-
     import aiohttp
 
     if getattr(aiohttp, "_datadog_patch", False):
@@ -125,10 +120,6 @@ def _unpatch_client(aiohttp):
 
 
 def unpatch():
-    from ddtrace.contrib.aiohttp_jinja2.patch import unpatch as aiohttp_jinja2_unpatch
-
-    aiohttp_jinja2_unpatch()
-
     import aiohttp
 
     if not getattr(aiohttp, "_datadog_patch", False):

releasenotes/notes/deprecate-aiohttp_jinja2-patching-from-aiohttp-be87600f308ca87a.yaml

@@ -0,0 +1,5 @@
+---
+upgrade:
+  - |
+    aiohttp_jinja2: use ``patch(aiohttp_jinja2=True)`` instead of ``patch(aiohttp=True)`` for
+    enabling/disabling the integration.

riotfile.py

@@ -1404,7 +1404,6 @@ def select_pys(min_version=MIN_PYTHON_VERSION, max_version=MAX_PYTHON_VERSION):
                     pys=select_pys(min_version="3.5", max_version="3.6"),
                     pkgs={
                         "aiohttp": ["~=2.0", "~=2.1", "~=2.2", "~=2.3"],
-                        "aiohttp_jinja2": ["~=0.12", "~=0.13", "~=0.15"],
                         "async-timeout": ["<4.0.0"],
                         "yarl": "~=0.18.0",
                     },
@@ -1421,12 +1420,11 @@ def select_pys(min_version=MIN_PYTHON_VERSION, max_version=MAX_PYTHON_VERSION):
                             "~=3.8",
                             latest,
                         ],
-                        "aiohttp_jinja2": latest,
                         "yarl": "~=1.0",
                     },
                 ),
                 Venv(
-                    pys=select_pys(min_version="3.7", max_version="3.10"),
+                    pys=select_pys(min_version="3.7"),
                     pkgs={
                         "pytest-asyncio": [latest],
                         "aiohttp": [
@@ -1437,12 +1435,71 @@ def select_pys(min_version=MIN_PYTHON_VERSION, max_version=MAX_PYTHON_VERSION):
                             "~=3.8",
                             latest,
                         ],
-                        "aiohttp_jinja2": latest,
                         "yarl": "~=1.0",
                     },
                 ),
             ],
         ),
+        Venv(
+            name="aiohttp_jinja2",
+            command="pytest {cmdargs} tests/contrib/aiohttp_jinja2",
+            pkgs={
+                "pytest-aiohttp": [latest],
+            },
+            venvs=[
+                Venv(
+                    pys="3.6",
+                    pkgs={
+                        "aiohttp": [
+                            "~=3.4",
+                            "~=3.6",
+                            latest,
+                        ],
+                        "aiohttp_jinja2": [
+                            "~=1.3.0",
+                            "~=1.4.0",
+                            latest,
+                        ],
+                    },
+                ),
+                Venv(
+                    pys=select_pys(min_version="3.7"),
+                    pkgs={
+                        "pytest-asyncio": [latest],
+                        "aiohttp": [
+                            "~=3.4",
+                            "~=3.6",
+                            "~=3.8",
+                            latest,
+                        ],
+                    },
+                    venvs=[
+                        Venv(
+                            pkgs={
+                                "aiohttp_jinja2": [
+                                    "~=1.3.0",
+                                    "~=1.4.0",
+                                    latest,
+                                ],
+                                # Jinja2 makes breaking changes in 3.0.
+                                "jinja2": "<3.0",
+                                # MarkupSafe makes breaking changes in 2.1.
+                                "MarkupSafe": "<2.1",
+                            }
+                        ),
+                        Venv(
+                            pkgs={
+                                "aiohttp_jinja2": [
+                                    "~=1.5.0",
+                                    latest,
+                                ],
+                                "jinja2": latest,
+                            }
+                        ),
+                    ],
+                ),
+            ],
+        ),
         Venv(
             name="jinja2",
             venvs=[

tests/conftest.py

@@ -4,6 +4,7 @@
 
 import pytest
 
+import ddtrace
 from tests.utils import DummyTracer
 from tests.utils import TracerSpanContainer
 from tests.utils import call_program
@@ -17,8 +18,16 @@ def pytest_configure(config):
 
 
 @pytest.fixture
-def tracer():
-    return DummyTracer()
+def use_global_tracer():
+    yield False
+
+
+@pytest.fixture
+def tracer(use_global_tracer):
+    if use_global_tracer:
+        return ddtrace.tracer
+    else:
+        return DummyTracer()
 
 
 @pytest.fixture

tests/contrib/aiohttp/app/web.py

@@ -2,8 +2,14 @@
 import os
 
 from aiohttp import web
-import aiohttp_jinja2
-import jinja2
+
+
+try:
+    import aiohttp_jinja2
+except ImportError:
+    aiohttp_jinja2 = None
+else:
+    import jinja2
 
 from ddtrace.contrib.aiohttp.middlewares import CONFIG_KEY
 
@@ -77,20 +83,6 @@ async def coro_2(request):
     return "OK"
 
 
-async def template_handler(request):
-    return aiohttp_jinja2.render_template("template.jinja2", request, {"text": "OK"})
-
-
-@aiohttp_jinja2.template("template.jinja2")
-async def template_decorator(request):
-    return {"text": "OK"}
-
-
-@aiohttp_jinja2.template("error.jinja2")
-async def template_error(request):
-    return {}
-
-
 async def delayed_handler(request):
     await asyncio.sleep(0.01)
     return web.Response(text="Done")
@@ -105,6 +97,20 @@ async def middleware_handler(request):
     return middleware_handler
 
 
+if aiohttp_jinja2:
+
+    async def template_handler(request):
+        return aiohttp_jinja2.render_template("template.jinja2", request, {"text": "OK"})
+
+    @aiohttp_jinja2.template("template.jinja2")
+    async def template_decorator(request):
+        return {"text": "OK"}
+
+    @aiohttp_jinja2.template("error.jinja2")
+    async def template_error(request):
+        return {}
+
+
 def setup_app(loop=None):
     """
     Use this method to create the app. It must receive
@@ -129,11 +135,12 @@ def setup_app(loop=None):
     app.router.add_get("/uncaught_server_error", uncaught_server_error)
     app.router.add_get("/caught_server_error", caught_server_error)
     app.router.add_static("/statics", STATIC_DIR)
-    # configure templates
-    set_memory_loader(app)
-    app.router.add_get("/template/", template_handler)
-    app.router.add_get("/template_decorator/", template_decorator)
-    app.router.add_get("/template_error/", template_error)
+    if aiohttp_jinja2:
+        # configure templates
+        set_memory_loader(app)
+        app.router.add_get("/template/", template_handler)
+        app.router.add_get("/template_decorator/", template_decorator)
+        app.router.add_get("/template_error/", template_error)
     app.router.add_get("/response_headers/", response_headers)
 
     return app

tests/contrib/aiohttp/conftest.py

@@ -1,11 +1,8 @@
 import aiohttp
-import aiohttp_jinja2
 import pytest
 
 from ddtrace.contrib.aiohttp.middlewares import trace_app
-from ddtrace.contrib.aiohttp_jinja2.patch import patch as patch_jinja2
 from ddtrace.internal.utils import version
-from ddtrace.pin import Pin
 
 from .app.web import setup_app
 
@@ -24,19 +21,15 @@ def app_tracer(tracer, loop):
 
     @pytest.fixture
     def patched_app_tracer(app_tracer):
-        patch_jinja2()
         app, tracer = app_tracer
-        Pin.override(aiohttp_jinja2, tracer=tracer)
         return app, tracer
         # When Python 3.5 is dropped, rather do:
         # yield app, tracer
         # unpatch()
 
     @pytest.fixture
     def untraced_app_tracer(tracer, loop):
-        patch_jinja2()
         app = setup_app()
-        Pin.override(aiohttp_jinja2, tracer=tracer)
         return app, tracer
         # When Python 3.5 is dropped, rather do:
         # yield app, tracer
@@ -53,19 +46,15 @@ async def app_tracer(tracer, loop):
 
     @pytest.fixture
     async def patched_app_tracer(app_tracer):
-        patch_jinja2()
         app, tracer = app_tracer
-        Pin.override(aiohttp_jinja2, tracer=tracer)
         return app, tracer
         # When Python 3.5 is dropped, rather do:
         # yield app, tracer
         # unpatch()
 
     @pytest.fixture
     async def untraced_app_tracer(tracer, loop):
-        patch_jinja2()
         app = setup_app()
-        Pin.override(aiohttp_jinja2, tracer=tracer)
         return app, tracer
         # When Python 3.5 is dropped, rather do:
         # yield app, tracer

tests/contrib/aiohttp/test_middleware.py

@@ -455,7 +455,7 @@ async def test_analytics_integration_enabled(app_tracer, aiohttp_client):
     client = await aiohttp_client(app)
     app["datadog_trace"]["analytics_enabled"] = True
     app["datadog_trace"]["analytics_sample_rate"] = 0.5
-    request = await client.request("GET", "/template/")
+    request = await client.request("GET", "/")
     await request.text()
 
     # Assert root span sets the appropriate metric
@@ -467,7 +467,7 @@ async def test_analytics_integration_default(app_tracer, aiohttp_client):
     """Check trace has analytics sample rate set"""
     app, tracer = app_tracer
     client = await aiohttp_client(app)
-    request = await client.request("GET", "/template/")
+    request = await client.request("GET", "/")
     await request.text()
 
     # Assert root span does not have the appropriate metric
@@ -480,7 +480,7 @@ async def test_analytics_integration_disabled(app_tracer, aiohttp_client):
     app, tracer = app_tracer
     client = await aiohttp_client(app)
     app["datadog_trace"]["analytics_enabled"] = False
-    request = await client.request("GET", "/template/")
+    request = await client.request("GET", "/")
     await request.text()
 
     # Assert root span does not have the appropriate metric