Repository: scoutapp/scout_apm_python
Pull request #420 (2019-12-13 21:31:19)

Use Python version checking for all compat imports
We were doing this inconsistently.  I prefer checking versions as it avoids burying errors. See https://twitter.com/nicoddemus/status/1146196274309414912 .


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 179ece1..691370a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -9,6 +9,11 @@
   - Standardized a `language_version` key in Metadata
   - Autocloses spans that are still running when Request completes
 
+### Fixed
+
+- Fix recording of path for unicode query parameters on Python 2.7
+  ([PR #419](https://github.com/scoutapp/scout_apm_python/pull/419)).
+
 ## [2.9.0] 2019-11-29
 
 ### Added
diff --git a/src/scout_apm/compat.py b/src/scout_apm/compat.py
index 7b25393..afdbf20 100644
--- a/src/scout_apm/compat.py
+++ b/src/scout_apm/compat.py
@@ -10,8 +10,9 @@ string_type = str if sys.version_info[0] >= 3 else basestring  # noqa: F821
 text_type = str if sys.version_info[0] >= 3 else unicode  # noqa: F821
 string_types = tuple({string_type, text_type})
 
-# Python 2 (and very early 3.x) didn't have ContextDecorator, so define it for ourselves
-if sys.version_info < (3, 2):
+if sys.version_info >= (3, 2):
+    from contextlib import ContextDecorator
+else:
     import functools
 
     class ContextDecorator(object):
@@ -24,14 +25,9 @@ if sys.version_info < (3, 2):
             return decorated
 
 
-else:
-    from contextlib import ContextDecorator
-
-try:
-    # Python 3.x
+if sys.version_info >= (3, 0):
     import queue
-except ImportError:
-    # Python 2.x
+else:
     import Queue as queue
 
 # datetime_to_timestamp converts a naive UTC datetime to a unix timestamp
@@ -60,9 +56,9 @@ def text(value, encoding="utf-8", errors="strict"):
         return text_type(value)
 
 
-try:
+if sys.version_info >= (3, 0):
     from urllib.parse import urlencode
-except ImportError:
+else:
     from urllib import urlencode
 
 
diff --git a/src/scout_apm/core/web_requests.py b/src/scout_apm/core/web_requests.py
index 1767bb1..c16f779 100644
--- a/src/scout_apm/core/web_requests.py
+++ b/src/scout_apm/core/web_requests.py
@@ -42,10 +42,18 @@ FILTER_PARAMETERS = frozenset(
 def create_filtered_path(path, query_params):
     if scout_config.value("uri_reporting") == "path":
         return path
+    # Python 2 unicode compatibility: force all keys and values to bytes
     filtered_params = sorted(
         (
-            (k, "[FILTERED]" if k.lower() in FILTER_PARAMETERS else v)
-            for k, v in query_params
+            (
+                key.encode("utf-8"),
+                (
+                    b"[FILTERED]"
+                    if key.lower() in FILTER_PARAMETERS
+                    else value.encode("utf-8")
+                ),
+            )
+            for key, value in query_params
         )
     )
     if not filtered_params:
diff --git a/tox.ini b/tox.ini
index 3adc919..7f52408 100644
--- a/tox.ini
+++ b/tox.ini
@@ -54,7 +54,7 @@ deps =
     httpretty
     jinja2
     nameko<3
-    py27: mock
+    mock ; python_version < "3.0"
     psutil
     pymongo
     pyramid


Test patch:
diff --git a/pytest.ini b/pytest.ini
index 20c05e1..473b935 100644
--- a/pytest.ini
+++ b/pytest.ini
@@ -33,3 +33,6 @@ addopts = --cov=scout_apm
           # E               TypeError: wrap_socket() got an unexpected keyword argument '_context'
           -p no:pytest_nameko
 norecursedirs = src
+filterwarnings =
+    error:::scout_apm
+    error:::tests
diff --git a/tests/compat.py b/tests/compat.py
index 65f744f..80e72a3 100644
--- a/tests/compat.py
+++ b/tests/compat.py
@@ -3,15 +3,14 @@ from __future__ import absolute_import, division, print_function, unicode_litera
 
 import sys
 
-try:
+if sys.version_info >= (3, 0):
     from unittest import mock
-except ImportError:
-    # Python 2
+else:
     import mock
 
-try:
+if sys.version_info >= (3, 2):
     from tempfile import TemporaryDirectory
-except ImportError:  # Python < 3.2
+else:
     from contextlib import contextmanager
     from tempfile import mkdtemp
     from shutil import rmtree
diff --git a/tests/unit/core/test_util.py b/tests/unit/core/test_util.py
index f6a3074..90769b8 100644
--- a/tests/unit/core/test_util.py
+++ b/tests/unit/core/test_util.py
@@ -1,6 +1,8 @@
 # coding=utf-8
 from __future__ import absolute_import, division, print_function, unicode_literals
 
+import pytest
+
 from scout_apm.core.util import octal
 
 
@@ -13,7 +15,5 @@ def test_octal_with_valid_string():
 
 
 def test_octal_raises_valueerror_on_invalid_value():
-    try:
+    with pytest.raises(ValueError):
         octal("THIS IS INVALID")
-    except ValueError:
-        pass
diff --git a/tests/unit/core/test_web_requests.py b/tests/unit/core/test_web_requests.py
index 8bd8c3c..bbd66e0 100644
--- a/tests/unit/core/test_web_requests.py
+++ b/tests/unit/core/test_web_requests.py
@@ -24,7 +24,9 @@ from scout_apm.core.web_requests import (
         ("/", [], "/"),
         ("/foo/", [], "/foo/"),
         ("/", [("bar", "1")], "/?bar=1"),
-        ("/", [("baz", 2), ("bar", "1")], "/?bar=1&baz=2"),
+        ("/", [("bar", "unicØde")], "/?bar=unic%C3%98de"),
+        ("/", [("unicØde", "foo")], "/?unic%C3%98de=foo"),
+        ("/", [("baz", "2"), ("bar", "1")], "/?bar=1&baz=2"),
         ("/", [("bar", "1"), ("bar", "2")], "/?bar=1&bar=2"),
         ("/", [("password", "hunter2")], "/?password=%5BFILTERED%5D"),
         ("/", [("PASSWORD", "hunter2")], "/?PASSWORD=%5BFILTERED%5D"),