fix[pytest]: Default parameter JSON encoding to use repr (backport #2684) (#2691)

* fix[pytest]: Default parameter JSON encoding to use repr (#2684)

* Remove JSON encoding for pytest parameters, default to string representation of parameters

* Added type hinting

(cherry picked from commit f5bc986)

* Added logger import

Co-authored-by: Yun Kim <[email protected]>
Co-authored-by: Yun Kim <[email protected]>

Author: mergify[bot]

ddtrace/contrib/pytest/plugin.py

@@ -1,4 +1,5 @@
 import json
+from typing import Dict
 
 import pytest
 
@@ -8,13 +9,18 @@
 from ...ext import SpanTypes
 from ...ext import ci
 from ...ext import test
+from ...internal.logger import get_logger
 from ...pin import Pin
 from ..trace_utils import int_service
 from .constants import FRAMEWORK
 from .constants import HELP_MSG
 from .constants import KIND
 
 
+PATCH_ALL_HELP_MSG = "Call ddtrace.patch_all before running tests."
+log = get_logger(__name__)
+
+
 def is_enabled(config):
     """Check if the ddtrace plugin is enabled."""
     return config.getoption("ddtrace") or config.getini("ddtrace")
@@ -30,9 +36,6 @@ def _store_span(item, span):
     setattr(item, "_datadog_span", span)
 
 
-PATCH_ALL_HELP_MSG = "Call ddtrace.patch_all before running tests."
-
-
 def pytest_addoption(parser):
     """Add ddtrace options."""
     group = parser.getgroup("ddtrace")
@@ -107,8 +110,14 @@ def pytest_runtest_protocol(item, nextitem):
         # Parameterized test cases will have a `callspec` attribute attached to the pytest Item object.
         # Pytest docs: https://docs.pytest.org/en/6.2.x/reference.html#pytest.Function
         if getattr(item, "callspec", None):
-            params = {"arguments": item.callspec.params, "metadata": {}}
-            span.set_tag(test.PARAMETERS, json.dumps(params, default=repr))
+            parameters = {"arguments": {}, "metadata": {}}  # type: Dict[str, Dict[str, str]]
+            for param_name, param_val in item.callspec.params.items():
+                try:
+                    parameters["arguments"][param_name] = repr(param_val)
+                except Exception:
+                    parameters["arguments"][param_name] = "Could not encode"
+                    log.warning("Failed to encode %r", param_name, exc_info=True)
+            span.set_tag(test.PARAMETERS, json.dumps(parameters))
 
         markers = [marker.kwargs for marker in item.iter_markers(name="dd_tags")]
         for tags in markers:

releasenotes/notes/fix-pytest-parameterize-encoding-b8711d2d7dbb282a.yaml

@@ -0,0 +1,5 @@
+---
+fixes:
+  - |
+    Fixed JSON encoding errors in the pytest plugin for parameterized tests with dictionary parameters with tuple keys.
+    The pytest plugin now always JSON encodes the string representations of test parameters.

tests/contrib/pytest/test_pytest.py

@@ -123,8 +123,10 @@ def test_1(self, item):
         expected_params = [1, 2, 3, 4, [1, 2, 3]]
         assert len(spans) == 5
         for i in range(len(expected_params)):
-            extracted_params = json.loads(spans[i].meta[test.PARAMETERS])
-            assert extracted_params == {"arguments": {"item": expected_params[i]}, "metadata": {}}
+            assert json.loads(spans[i].meta[test.PARAMETERS]) == {
+                "arguments": {"item": str(expected_params[i])},
+                "metadata": {},
+            }
 
     def test_parameterize_case_complex_objects(self):
         """Test parametrize case with complex objects."""
@@ -153,6 +155,7 @@ def item_param():
                 pytest.param({"a": A("test_name", "value"), "b": [1, 2, 3]}, marks=pytest.mark.skip),
                 pytest.param(MagicMock(value=MagicMock()), marks=pytest.mark.skip),
                 pytest.param(circular_reference, marks=pytest.mark.skip),
+                pytest.param({("x", "y"): 12345}, marks=pytest.mark.skip),
             ]
             )
             class Test1(object):
@@ -162,7 +165,7 @@ def test_1(self, item):
         )
         file_name = os.path.basename(py_file.strpath)
         rec = self.inline_run("--ddtrace", file_name)
-        rec.assertoutcome(skipped=6)
+        rec.assertoutcome(skipped=7)
         spans = self.pop_spans()
 
         # Since object will have arbitrary addresses, only need to ensure that
@@ -171,14 +174,40 @@ def test_1(self, item):
             "test_parameterize_case_complex_objects.A",
             "test_parameterize_case_complex_objects.A",
             "<function item_param at 0x",
-            '"a": "<test_parameterize_case_complex_objects.A',
+            "'a': <test_parameterize_case_complex_objects.A",
             "<MagicMock id=",
             "test_parameterize_case_complex_objects.A",
+            "{('x', 'y'): 12345}",
         ]
-        assert len(spans) == 6
+        assert len(spans) == 7
         for i in range(len(expected_params_contains)):
             assert expected_params_contains[i] in spans[i].meta[test.PARAMETERS]
 
+    def test_parameterize_case_encoding_error(self):
+        """Test parametrize case with complex objects that cannot be JSON encoded."""
+        py_file = self.testdir.makepyfile(
+            """
+            from mock import MagicMock
+            import pytest
+
+            class A:
+                def __repr__(self):
+                    raise Exception("Cannot __repr__")
+
+            @pytest.mark.parametrize('item',[A()])
+            class Test1(object):
+                def test_1(self, item):
+                    assert True
+        """
+        )
+        file_name = os.path.basename(py_file.strpath)
+        rec = self.inline_run("--ddtrace", file_name)
+        rec.assertoutcome(passed=1)
+        spans = self.pop_spans()
+
+        assert len(spans) == 1
+        assert json.loads(spans[0].meta[test.PARAMETERS]) == {"arguments": {"item": "Could not encode"}, "metadata": {}}
+
     def test_skip(self):
         """Test parametrize case."""
         py_file = self.testdir.makepyfile(