fix(ci_visibility): properly resolve decorated functions for test sou…

…rce file information (#9586)

Fixes an issue where decorated test functions could resolve to the wrong
location when certain decorators (eg: @mock.patch) were used.

This also fixes the fact that source file info paths might not always be
relative to the current repo root.

The test added in this PR verifies the above by using a variety of
decorators as well as executing from the `nested_dir` directory instead
of the git repo root, with the source file info properly showing the
path as `nested_dir/test_mydecorators.py`.

## Checklist

- [x] Change(s) are motivated and described in the PR description
- [x] Testing strategy is described if automated tests are not included
in the PR
- [x] Risks are described (performance impact, potential for breakage,
maintainability)
- [x] Change is maintainable (easy to change, telemetry, documentation)
- [x] [Library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
are followed or label `changelog/no-changelog` is set
- [x] Documentation is included (in-code, generated user docs, [public
corp docs](https://github.com/DataDog/documentation/))
- [x] Backport labels are set (if
[applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))
- [x] If this PR changes the public interface, I've notified
`@DataDog/apm-tees`.

## Reviewer Checklist

- [x] Title is accurate
- [x] All changes are related to the pull request's stated goal
- [x] Description motivates each change
- [x] Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes
- [x] Testing strategy adequately addresses listed risks
- [x] Change is maintainable (easy to change, telemetry, documentation)
- [x] Release note makes sense to a user of the library
- [x] Author has acknowledged and discussed the performance implications
of this PR as reported in the benchmarks PR comment
- [x] Backport labels are set in a manner that is consistent with the
[release branch maintenance
policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)

ddtrace/contrib/pytest/_plugin_v1.py

@@ -37,6 +37,7 @@
 from ddtrace.contrib.unittest import unpatch as unpatch_unittest
 from ddtrace.ext import SpanTypes
 from ddtrace.ext import test
+from ddtrace.ext.git import extract_workspace_path
 from ddtrace.internal.ci_visibility import CIVisibility as _CIVisibility
 from ddtrace.internal.ci_visibility.constants import EVENT_TYPE as _EVENT_TYPE
 from ddtrace.internal.ci_visibility.constants import ITR_CORRELATION_ID_TAG_NAME
@@ -67,6 +68,7 @@
 from ddtrace.internal.coverage.code import ModuleCodeCollector
 from ddtrace.internal.logger import get_logger
 from ddtrace.internal.utils.formats import asbool
+from ddtrace.internal.utils.inspection import undecorated
 
 
 log = get_logger(__name__)
@@ -97,7 +99,7 @@ def _is_pytest_cov_enabled(config) -> bool:
     nocov_option = config.getoption("--no-cov", default=False)
     if nocov_option is True:
         return False
-    if type(cov_option) == list and cov_option == [True] and not nocov_option:
+    if isinstance(cov_option, list) and cov_option == [True] and not nocov_option:
         return True
     return cov_option
 
@@ -449,6 +451,14 @@ def pytest_sessionstart(session):
             log.debug("CI Visibility enabled - starting test session")
             global _global_skipped_elements
             _global_skipped_elements = 0
+            try:
+                workspace_path = extract_workspace_path()
+            except ValueError:
+                log.debug("Couldn't extract workspace path from git, reverting to config rootdir")
+                workspace_path = session.config.rootdir
+
+            session._dd_workspace_path = workspace_path
+
             test_session_span = _CIVisibility._instance.tracer.trace(
                 "pytest.test_session",
                 service=_CIVisibility._instance._service,
@@ -661,8 +671,14 @@ def pytest_runtest_protocol(item, nextitem):
             if item.location and item.location[0]:
                 _CIVisibility.set_codeowners_of(item.location[0], span=span)
             if hasattr(item, "_obj"):
-                test_method_object = item._obj
-                _add_start_end_source_file_path_data_to_span(span, test_method_object, test_name, item.config.rootdir)
+                item_path = Path(item.path if hasattr(item, "path") else item.fspath)
+                test_method_object = undecorated(item._obj, item.name, item_path)
+                _add_start_end_source_file_path_data_to_span(
+                    span,
+                    test_method_object,
+                    test_name,
+                    getattr(item.session, "_dd_workspace_path", item.config.rootdir),
+                )
 
             # We preemptively set FAIL as a status, because if pytest_runtest_makereport is not called
             # (where the actual test status is set), it means there was a pytest error

ddtrace/internal/ci_visibility/utils.py

@@ -79,7 +79,7 @@ def _add_pct_covered_to_span(coverage_data: dict, span: ddtrace.Span):
         log.warning("Tried to add total covered percentage to session span but no data was found")
         return
     lines_pct_value = coverage_data[PCT_COVERED_KEY]
-    if type(lines_pct_value) != float:
+    if not isinstance(lines_pct_value, float):
         log.warning("Tried to add total covered percentage to session span but the format was unexpected")
         return
     span.set_tag(test.TEST_LINES_PCT, lines_pct_value)

releasenotes/notes/fix-ci_visibility-properly_unwrap_functions-7c631b68720adab2.yaml

@@ -0,0 +1,5 @@
+---
+fixes:
+  - |
+    CI Visibility: fixes source file information that would be incorrect in certain decorated / wrapped scenarios and forces
+    paths to be relative to the repository root, if present.

tests/contrib/pytest/test_pytest.py

@@ -67,7 +67,7 @@ def subprocess_run(self, *args):
 
     def test_and_emit_get_version(self):
         version = get_version()
-        assert type(version) == str
+        assert isinstance(version, str)
         assert version != ""
 
         emit_integration_and_version_to_test_agent("pytest", version)
@@ -3494,7 +3494,7 @@ def test_add_two_number_list():
         lines_pct_value = test_session_span.get_metric("test.code_coverage.lines_pct")
 
         assert lines_pct_value is not None
-        assert type(lines_pct_value) == float
+        assert isinstance(lines_pct_value, float)
         assert test_module_span.get_metric("test.code_coverage.lines_pct") is None
         assert test_suite_span.get_metric("test.code_coverage.lines_pct") is None
         assert test_span.get_metric("test.code_coverage.lines_pct") is None
@@ -3663,3 +3663,114 @@ def test_add_two_number_list():
         assert test_module_span.get_metric("test.code_coverage.lines_pct") is None
         assert test_suite_span.get_metric("test.code_coverage.lines_pct") is None
         assert test_span.get_metric("test.code_coverage.lines_pct") is None
+
+    def test_pytest_reports_correct_source_info(self):
+        """Tests that decorated functions are reported with correct source file information and with relative to
+        repo root
+        """
+        os.chdir(self.git_repo)
+        os.mkdir("nested_dir")
+        os.chdir("nested_dir")
+        with open("my_decorators.py", "w+") as fd:
+            fd.write(
+                textwrap.dedent(
+                    (
+                        """
+                    def outer_decorator(func):
+                         def wrapper(*args, **kwargs):
+                            return func(*args, **kwargs)
+                         return wrapper
+
+                    @outer_decorator
+                    def inner_decorator(func):
+                         def wrapper(*args, **kwargs):
+                            return func(*args, **kwargs)
+                         return wrapper
+                    """
+                    )
+                )
+            )
+
+        with open("test_mydecorators.py", "w+") as fd:
+            fd.write(
+                textwrap.dedent(
+                    (
+                        """
+                    # this comment is line 2 and if you didn't know that it'd be easy to miscount below
+                    from my_decorators import outer_decorator, inner_decorator
+                    from unittest.mock import patch
+
+                    def local_decorator(func):
+                        def wrapper(*args, **kwargs):
+                            return func(*args, **kwargs)
+                        return wrapper
+
+                    def test_one_decorator():  # line 11
+                        str1 = "string 1"
+                        str2 = "string 2"
+                        assert str1 != str2
+
+                    @local_decorator  # line 16
+                    def test_local_decorated():
+                        str1 = "string 1"
+                        str2 = "string 2"
+                        assert str1 == str2
+
+                    @patch("ddtrace.config._potato", "potato")  # line 22
+                    def test_patched_undecorated():
+                        str1 = "string 1"
+                        str2 = "string 2"
+                        assert str1 != str2
+
+                    @patch("ddtrace.config._potato", "potato")  # line 28
+                    @inner_decorator
+                    def test_patched_single_decorated():
+                        str1 = "string 1"
+                        str2 = "string 2"
+                        assert str1 == str2
+
+                    @patch("ddtrace.config._potato", "potato")  # line 35
+                    @outer_decorator
+                    def test_patched_double_decorated():
+                        str1 = "string 1"
+                        str2 = "string 2"
+                        assert str1 != str2
+
+                    @outer_decorator  # line 42
+                    @patch("ddtrace.config._potato", "potato")
+                    @local_decorator
+                    def test_grand_slam():
+                        str1 = "string 1"
+                        str2 = "string 2"
+                        assert str1 == str2
+                    """
+                    )
+                )
+            )
+
+        self.inline_run("--ddtrace")
+
+        spans = self.pop_spans()
+        assert len(spans) == 9
+        test_names_to_source_info = {
+            span.get_tag("test.name"): (
+                span.get_tag("test.source.file"),
+                span.get_metric("test.source.start"),
+                span.get_metric("test.source.end"),
+            )
+            for span in spans
+            if span.get_tag("type") == "test"
+        }
+        assert len(test_names_to_source_info) == 6
+
+        expected_path = "nested_dir/test_mydecorators.py"
+        expected_source_info = {
+            "test_one_decorator": (expected_path, 11, 15),
+            "test_local_decorated": (expected_path, 16, 21),
+            "test_patched_undecorated": (expected_path, 22, 27),
+            "test_patched_single_decorated": (expected_path, 28, 34),
+            "test_patched_double_decorated": (expected_path, 35, 41),
+            "test_grand_slam": (expected_path, 42, 49),
+        }
+
+        assert expected_source_info == test_names_to_source_info