fix(pyramid): use correct package (backport #2959) (#2965)

* fix(pyramid): use correct package (#2959)

* Add pserve pyramid application

This should reproduce a bug exposed by including routes from another
module in the package.

* Fix Pyramid package

The Pyramid package needs to be set to the frame above what it normally
is because our patching introduces an additional frame in the call
stack.

* Add comments

(cherry picked from commit 5391e58)

* Fix snapshot conflict

error field is no longer encoded when zero

Co-authored-by: Kyle Verhoog <[email protected]>
Co-authored-by: Kyle Verhoog <[email protected]>

Author: mergify[bot]

ddtrace/contrib/pyramid/patch.py

@@ -62,6 +62,15 @@ def traced_init(wrapped, instance, args, kwargs):
     # If the tweens are explicitly set with 'pyramid.tweens', we need to
     # explicitly set our tween too since `add_tween` will be ignored.
     insert_tween_if_needed(trace_settings)
+
+    # The original Configurator.__init__ looks up two levels to find the package
+    # name if it is not provided. This has to be replicated here since this patched
+    # call will occur at the same level in the call stack.
+    if not kwargs.get("package", None):
+        from pyramid.path import caller_package
+
+        kwargs["package"] = caller_package(level=2)
+
     kwargs["settings"] = trace_settings
     wrapped(*args, **kwargs)
     trace_pyramid(instance)

releasenotes/notes/pyramid-init-ea34d2717aab6aab.yaml

@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    Set the correct package name in the Pyramid instrumentation. This should
+    fix an issue where the incorrect package name was being used which would
+    crash the application when trying to do relative imports within Pyramid
+    (e.g. when including routes from a relative path).

riotfile.py

@@ -944,6 +944,7 @@ def select_pys(min_version=MIN_PYTHON_VERSION, max_version=MAX_PYTHON_VERSION):
                     pkgs={
                         "requests": [latest],
                         "webtest": [latest],
+                        "tests/contrib/pyramid/pserve_app": [latest],
                         "pyramid": [
                             "~=1.7",
                             "~=1.8",

tests/conftest.py

@@ -1,4 +1,5 @@
 import contextlib
+import os
 import sys
 
 import pytest
@@ -61,7 +62,7 @@ def snapshot(request):
     assert len(marks) < 2, "Multiple snapshot marks detected"
     if marks:
         snap = marks[0]
-        token = _request_token(request)
+        token = _request_token(request).replace(" ", "_").replace(os.path.sep, "_")
         with _snapshot_context(token, *snap.args, **snap.kwargs) as snapshot:
             yield snapshot
     else:

tests/contrib/pyramid/pserve_app/app/__init__.py

@@ -0,0 +1,37 @@
+from pyramid.config import Configurator
+from pyramid.response import Response
+
+from ddtrace import tracer
+from ddtrace.filters import TraceFilter
+
+
+class PingFilter(TraceFilter):
+    def process_trace(self, trace):
+        # Filter out all traces with trace_id = 1
+        # This is done to prevent certain traces from being included in snapshots and
+        # accomplished by propagating an http trace id of 1 with the request to the webserver.
+        return None if trace and trace[0].trace_id == 1 else trace
+
+
+tracer.configure(
+    settings={
+        "FILTERS": [PingFilter()],
+    }
+)
+
+
+def tracer_shutdown(request):
+    tracer.shutdown()
+    return Response("shutdown")
+
+
+def main(global_config, **settings):
+    """This function returns a Pyramid WSGI application."""
+    with Configurator(settings=settings) as config:
+        config.add_route("tracer-shutdown", "/shutdown-tracer")
+        config.add_view(tracer_shutdown, route_name="tracer-shutdown")
+        # This will trigger usage of the package name reproing an issue
+        # reported in #2942.
+        config.include(".routes")
+        config.scan()
+    return config.make_wsgi_app()

tests/contrib/pyramid/pserve_app/app/routes.py

@@ -0,0 +1,10 @@
+from pyramid.response import Response
+
+
+def hello_world(request):
+    return Response("Hello World!")
+
+
+def includeme(config):
+    config.add_route("hello", "/")
+    config.add_view(hello_world, route_name="hello")

tests/contrib/pyramid/pserve_app/development.ini

@@ -0,0 +1,11 @@
+###
+# app configuration
+# https://docs.pylonsproject.org/projects/pyramid/en/latest/narr/environment.html
+###
+
+[app:main]
+use = egg:pserve_test_app
+
+[server:main]
+use = egg:waitress#main
+listen = localhost:8000

tests/contrib/pyramid/pserve_app/setup.py

@@ -0,0 +1,13 @@
+from setuptools import setup, find_packages
+
+setup(
+    name="pserve_test_app",
+    packages=find_packages(),
+    include_package_data=True,
+    zip_safe=False,
+    entry_points={
+        "paste.app_factory": [
+            "main = app:main",
+        ],
+    },
+)

tests/contrib/pyramid/test_pyramid.py

@@ -113,8 +113,13 @@ def test_distributed_tracing_disabled(self):
         assert span.get_tag(ORIGIN_KEY) != "synthetics"
 
 
+@pytest.fixture
+def pyramid_app():
+    return "ddtrace-run python tests/contrib/pyramid/app/app.py"
+
+
 @pytest.fixture(scope="function")
-def pyramid_client(snapshot):
+def pyramid_client(snapshot, pyramid_app):
     """Runs a Pyramid app in a subprocess and returns a client which can be used to query it.
 
     Traces are flushed by invoking a tracer.shutdown() using a /shutdown-tracer route
@@ -124,7 +129,7 @@ def pyramid_client(snapshot):
     env = os.environ.copy()
     env["SERVER_PORT"] = str(SERVER_PORT)
 
-    cmd = ["ddtrace-run", "python", "tests/contrib/pyramid/app/app.py"]
+    cmd = pyramid_app.split(" ")
     proc = subprocess.Popen(
         cmd,
         stdout=subprocess.PIPE,
@@ -146,6 +151,13 @@ def pyramid_client(snapshot):
         proc.terminate()
 
 
+@pytest.mark.parametrize(
+    "pyramid_app",
+    [
+        "ddtrace-run pserve tests/contrib/pyramid/pserve_app/development.ini",
+        "ddtrace-run python tests/contrib/pyramid/app/app.py",
+    ],
+)
 @pytest.mark.snapshot()
 def test_simple_pyramid_app_endpoint(pyramid_client):
     r = pyramid_client.get("/")

tests/snapshots/tests.contrib.pyramid.test_pyramid.test_simple_pyramid_app_endpoint[ddtrace-run_pserve_tests_contrib_pyramid_pserve_app_development.ini].json

@@ -0,0 +1,27 @@
+[[
+  {
+    "name": "pyramid.request",
+    "service": "pyramid",
+    "resource": "GET hello",
+    "trace_id": 0,
+    "span_id": 1,
+    "parent_id": 0,
+    "type": "web",
+    "error": 0,
+    "meta": {
+      "http.method": "GET",
+      "http.status_code": "200",
+      "http.url": "http://localhost:8000/",
+      "pyramid.route.name": "hello",
+      "runtime-id": "3cfe7c1618be4fc4a090f2a7dab9dcc9"
+    },
+    "metrics": {
+      "_dd.agent_psr": 1.0,
+      "_dd.measured": 1,
+      "_dd.tracer_kr": 1.0,
+      "_sampling_priority_v1": 1,
+      "system.pid": 656011
+    },
+    "duration": 128359,
+    "start": 1635529254779423958
+  }]]