Repository: scoutapp/scout_apm_python
Pull request #290 (2019-10-23 11:46:21)

Deduplicate Werkzeug request data tracking
Flask and Nameko both use Werkzeug requests, and had basically copy-pasted blocks of code to call the underlying pieces. DRY that out with a single function.


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 6f41917..c277c34 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,6 +6,8 @@
 
 - Fix warning emitted from using deprecated 'warn' method
   ([PR #283](https://github.com/scoutapp/scout_apm_python/pull/283)).
+- Improve warning message for deprecated `log_level` configuration option
+  ([PR #288](https://github.com/scoutapp/scout_apm_python/pull/288)).
 
 ## [2.6.0] 2019-10-22
 
diff --git a/setup.py b/setup.py
index a97bedc..66bdbfa 100644
--- a/setup.py
+++ b/setup.py
@@ -51,7 +51,7 @@ setup(
             "core-agent-manager = scout_apm.core.cli.core_agent_manager:main"
         ]
     },
-    install_requires=["psutil", "requests"],
+    install_requires=["psutil>=5,<6", "requests>=2,<3"],
     keywords="apm performance monitoring development",
     classifiers=[
         "Development Status :: 5 - Production/Stable",
diff --git a/src/scout_apm/core/core_agent_manager.py b/src/scout_apm/core/core_agent_manager.py
index 04e53cd..c760cef 100644
--- a/src/scout_apm/core/core_agent_manager.py
+++ b/src/scout_apm/core/core_agent_manager.py
@@ -91,7 +91,9 @@ class CoreAgentManager(object):
         if log_level is not None:
             logger.warning(
                 "The config name 'log_level' is deprecated - "
-                + "please use the new name 'core_agent_log_level' instead."
+                + "please use the new name 'core_agent_log_level' instead. "
+                + "This might be configured in your environment variables or "
+                + "framework settings as SCOUT_LOG_LEVEL."
             )
         else:
             log_level = AgentContext.instance.config.value("core_agent_log_level")
diff --git a/src/scout_apm/core/web_requests.py b/src/scout_apm/core/web_requests.py
index e885813..9791efa 100644
--- a/src/scout_apm/core/web_requests.py
+++ b/src/scout_apm/core/web_requests.py
@@ -163,3 +163,34 @@ def convert_ambiguous_timestamp_to_ns(timestamp):
     else:
         return 0.0
     return converted_timestamp
+
+
+def werkzeug_track_request_data(werkzeug_request, tracked_request):
+    """
+    Several integrations use Werkzeug requests, so share the code for
+    extracting common data here.
+    """
+    path = werkzeug_request.path
+    tracked_request.tag(
+        "path", create_filtered_path(path, werkzeug_request.args.items(multi=True))
+    )
+    if ignore_path(path):
+        tracked_request.tag("ignore_transaction", True)
+
+    # Determine a remote IP to associate with the request. The value is
+    # spoofable by the requester so this is not suitable to use in any
+    # security sensitive context.
+    user_ip = (
+        werkzeug_request.headers.get("x-forwarded-for", default="").split(",")[0]
+        or werkzeug_request.headers.get("client-ip", default="").split(",")[0]
+        or werkzeug_request.remote_addr
+    )
+    tracked_request.tag("user_ip", user_ip)
+
+    queue_time = werkzeug_request.headers.get(
+        "x-queue-start", default=""
+    ) or werkzeug_request.headers.get("x-request-start", default="")
+    tracked_queue_time = track_request_queue_time(queue_time, tracked_request)
+    if not tracked_queue_time:
+        amazon_queue_time = werkzeug_request.headers.get("x-amzn-trace-id", default="")
+        track_amazon_request_queue_time(amazon_queue_time, tracked_request)
diff --git a/src/scout_apm/flask/__init__.py b/src/scout_apm/flask/__init__.py
index d4a7850..5e7ad7b 100644
--- a/src/scout_apm/flask/__init__.py
+++ b/src/scout_apm/flask/__init__.py
@@ -8,12 +8,7 @@ import scout_apm.core
 from scout_apm.core.config import ScoutConfig
 from scout_apm.core.monkey import CallableProxy
 from scout_apm.core.tracked_request import TrackedRequest
-from scout_apm.core.web_requests import (
-    create_filtered_path,
-    ignore_path,
-    track_amazon_request_queue_time,
-    track_request_queue_time,
-)
+from scout_apm.core.web_requests import werkzeug_track_request_data
 
 
 class ScoutApm(object):
@@ -72,30 +67,7 @@ class ScoutApm(object):
         span = tracked_request.start_span(operation=operation)
         span.tag("name", name)
 
-        path = request.path
-        tracked_request.tag(
-            "path", create_filtered_path(path, request.args.items(multi=True))
-        )
-        if ignore_path(path):
-            tracked_request.tag("ignore_transaction", True)
-
-        # Determine a remote IP to associate with the request. The value is
-        # spoofable by the requester so this is not suitable to use in any
-        # security sensitive context.
-        user_ip = (
-            request.headers.get("x-forwarded-for", default="").split(",")[0]
-            or request.headers.get("client-ip", default="").split(",")[0]
-            or request.remote_addr
-        )
-        tracked_request.tag("user_ip", user_ip)
-
-        queue_time = request.headers.get(
-            "x-queue-start", default=""
-        ) or request.headers.get("x-request-start", default="")
-        tracked = track_request_queue_time(queue_time, tracked_request)
-        if not tracked:
-            amazon_queue_time = request.headers.get("x-amzn-trace-id", default="")
-            track_amazon_request_queue_time(amazon_queue_time, tracked_request)
+        werkzeug_track_request_data(request, tracked_request)
 
         try:
             return wrapped(*args, **kwargs)
diff --git a/src/scout_apm/nameko.py b/src/scout_apm/nameko.py
index b223de3..cf8a64f 100644
--- a/src/scout_apm/nameko.py
+++ b/src/scout_apm/nameko.py
@@ -7,12 +7,7 @@ from werkzeug.wrappers import Request
 
 import scout_apm.core
 from scout_apm.core.tracked_request import TrackedRequest
-from scout_apm.core.web_requests import (
-    create_filtered_path,
-    ignore_path,
-    track_amazon_request_queue_time,
-    track_request_queue_time,
-)
+from scout_apm.core.web_requests import werkzeug_track_request_data
 
 
 class ScoutReporter(DependencyProvider):
@@ -47,30 +42,7 @@ class ScoutReporter(DependencyProvider):
         if not isinstance(request, Request):
             return
 
-        path = request.path
-        tracked_request.tag(
-            "path", create_filtered_path(path, request.args.items(multi=True))
-        )
-        if ignore_path(path):
-            tracked_request.tag("ignore_transaction", True)
-
-        # Determine a remote IP to associate with the request. The value is
-        # spoofable by the requester so this is not suitable to use in any
-        # security sensitive context.
-        user_ip = (
-            request.headers.get("x-forwarded-for", default="").split(",")[0]
-            or request.headers.get("client-ip", default="").split(",")[0]
-            or request.remote_addr
-        )
-        tracked_request.tag("user_ip", user_ip)
-
-        queue_time = request.headers.get(
-            "x-queue-start", default=""
-        ) or request.headers.get("x-request-start", default="")
-        tracked_queue_time = track_request_queue_time(queue_time, tracked_request)
-        if not tracked_queue_time:
-            amazon_queue_time = request.headers.get("x-amzn-trace-id", default="")
-            track_amazon_request_queue_time(amazon_queue_time, tracked_request)
+        werkzeug_track_request_data(request, tracked_request)
 
     def worker_result(self, worker_ctx, result=None, exc_info=None):
         if self._do_nothing:


Test patch:
diff --git a/tests/integration/core/test_core_agent_manager.py b/tests/integration/core/test_core_agent_manager.py
index 1be3430..c95196d 100644
--- a/tests/integration/core/test_core_agent_manager.py
+++ b/tests/integration/core/test_core_agent_manager.py
@@ -161,7 +161,8 @@ def test_log_level_deprecated(caplog, core_agent_manager):
             logging.WARNING,
             (
                 "The config name 'log_level' is deprecated - please use the new name "
-                + "'core_agent_log_level' instead."
+                + "'core_agent_log_level' instead. This might be configured in your "
+                + "environment variables or framework settings as SCOUT_LOG_LEVEL."
             ),
         )
     ]