fix(ci_visibility): don't rely on git in pytest plugin [backport 2.8] (…

…#10024)

Backport c460441 from #9989 to 2.8.

Fixes an issue introduced by #9586 and reported in #9981 where the
pytest plugin would crash if the `git` binary was absent.

The workspace is instead grabbed from the `CIVisibility` service's tags
(via a new getter classmethod).

In order for the variable to persist through to the
`pytest_terminal_summary` hook, it is moved from being stashed on the
pytest `session` object to the nested `config` object (which itself is
passed to `pytest_terminal_summary`).

For improved testability, a function to get the location of the `git`
binary's passed, using `@cached()` to avoid re-fetching the path each
time we call `git`.

## Checklist
- [x] PR author has checked that all the criteria below are met
- The PR description includes an overview of the change
- The PR description articulates the motivation for the change
- The change includes tests OR the PR description describes a testing
strategy
- The PR description notes risks associated with the change, if any
- Newly-added code is easy to change
- The change follows the [library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
- The change includes or references documentation updates if necessary
- Backport labels are set (if
[applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))

## Reviewer Checklist
- [x] Reviewer has checked that all the criteria below are met 
- Title is accurate
- All changes are related to the pull request's stated goal
- Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes
- Testing strategy adequately addresses listed risks
- Newly-added code is easy to change
- Release note makes sense to a user of the library
- If necessary, author has acknowledged and discussed the performance
implications of this PR as reported in the benchmarks PR comment
- 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.py

@@ -37,7 +37,6 @@
 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
@@ -462,11 +461,8 @@ 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
+
+        workspace_path = _CIVisibility.get_workspace_path() or session.config.rootdir
 
         session._dd_workspace_path = workspace_path
 

ddtrace/ext/git.py

@@ -6,6 +6,7 @@
 import os
 import random
 import re
+from shutil import which
 import subprocess
 from typing import Dict  # noqa:F401
 from typing import Generator  # noqa:F401
@@ -18,6 +19,7 @@
 
 from ddtrace.internal import compat
 from ddtrace.internal.logger import get_logger
+from ddtrace.internal.utils.cache import cached
 from ddtrace.internal.utils.time import StopWatch
 
 
@@ -80,6 +82,16 @@ def is_ref_a_tag(ref):
     return "tags/" in ref if ref else False
 
 
+@cached()
+def _get_executable_path(executable_name: str) -> Optional[str]:
+    """Return the path to an executable.
+
+    NOTE: cached() requires an argument which is why executable_name is passed in, even though it's really only ever
+    used to find the git executable at this point.
+    """
+    return which(executable_name, mode=os.X_OK)
+
+
 def _git_subprocess_cmd_with_details(*cmd, cwd=None, std_in=None):
     # type: (str, Optional[str], Optional[bytes]) -> _GitSubprocessDetails
     """Helper for invoking the git CLI binary
@@ -90,7 +102,10 @@ def _git_subprocess_cmd_with_details(*cmd, cwd=None, std_in=None):
         - the time it took to execute the command, in milliseconds
         - the exit code
     """
-    git_cmd = ["git"]
+    git_cmd = _get_executable_path("git")
+    if git_cmd is None:
+        raise FileNotFoundError("Git executable not found")
+    git_cmd = [git_cmd]
     git_cmd.extend(cmd)
 
     log.debug("Executing git command: %s", git_cmd)

ddtrace/internal/ci_visibility/recorder.py

@@ -750,6 +750,17 @@ def get_service(cls) -> Optional[str]:
             return None
         return instance._service
 
+    @classmethod
+    def get_workspace_path(cls) -> Optional[str]:
+        if not cls.enabled:
+            error_msg = "CI Visibility is not enabled"
+            log.warning(error_msg)
+            raise CIVisibilityError(error_msg)
+        instance = cls.get_instance()
+        if instance is None:
+            return None
+        return instance._tags.get(ci.WORKSPACE_PATH)
+
 
 def _requires_civisibility_enabled(func):
     def wrapper(*args, **kwargs):

releasenotes/notes/ci_visibility-fix-dont_make_pytest_contrib_depend_on_git-d922e9326c981f38.yaml

@@ -0,0 +1,4 @@
+---
+fixes:
+  - |
+    CI Visibility: Fixes an issue where the pytest plugin would crash if the git binary was absent

tests/contrib/pytest/test_pytest.py

@@ -3774,3 +3774,59 @@ def test_grand_slam():
         }
 
         assert expected_source_info == test_names_to_source_info
+
+    def test_pytest_without_git_does_not_crash(self):
+        with open("tools.py", "w+") as fd:
+            fd.write(
+                textwrap.dedent(
+                    (
+                        """
+                    def add_two_number_list(list_1, list_2):
+                        output_list = []
+                        for number_a, number_b in zip(list_1, list_2):
+                            output_list.append(number_a + number_b)
+                        return output_list
+
+                    def multiply_two_number_list(list_1, list_2):
+                        output_list = []
+                        for number_a, number_b in zip(list_1, list_2):
+                            output_list.append(number_a * number_b)
+                        return output_list
+                    """
+                    )
+                )
+            )
+
+        with open("test_tools.py", "w+") as fd:
+            fd.write(
+                textwrap.dedent(
+                    (
+                        """
+                    from tools import add_two_number_list
+
+                    def test_add_two_number_list():
+                        a_list = [1,2,3,4,5,6,7,8]
+                        b_list = [2,3,4,5,6,7,8,9]
+                        actual_output = add_two_number_list(a_list, b_list)
+
+                        assert actual_output == [3,5,7,9,11,13,15,17]
+                    """
+                    )
+                )
+            )
+
+        self.testdir.chdir()
+        with mock.patch("ddtrace.ext.git._get_executable_path", return_value=None):
+            self.inline_run("--ddtrace")
+
+            spans = self.pop_spans()
+            assert len(spans) == 4
+            test_span = spans[0]
+            test_session_span = spans[1]
+            test_module_span = spans[2]
+            test_suite_span = spans[3]
+
+            assert test_session_span.get_metric("test.code_coverage.lines_pct") is None
+            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