chore(ci-visibility): unshallow repository when git >= 2.27 [backport 1.17] (#6495)

Backport d4d2f78 from
#6268 to 1.17.

An unshallow command compatible with git >= 2.27 is now used when a
shallow repository is detected.

Additionally, packfile upload is improved by searching for commits found
in the repo that are not known to the ITR backend, rather than just
HEAD.

For tests, a shallow clone was created using `git clone --depth=1
https://github.com/pallets/flask.git`, with git log showing only one
result:
```
% git log --pretty=oneline
cb825687a592709f902f3d320d93987a0546fd28 (grafted, HEAD -> main, origin/main, origin/HEAD) Bump actions/checkout from 3.5.2 to 3.5.3 (#5186)
```

After:
```
cb825687a592709f902f3d320d93987a0546fd28 (HEAD -> main, origin/main, origin/HEAD) Bump actions/checkout from 3.5.2 to 3.5.3 (#5186)
51bf0fdd9018d63b7bc09db52a66b726c2c4bad8 Bump slsa-framework/slsa-github-generator from 1.6.0 to 1.7.0 (#5185)
0da5788efbda92b6b6239352c2b9a835efe0888f Bump dessant/lock-threads from 4.0.0 to 4.0.1 (#5184)
# ... ~20 lines
32d2f47ed1bf3f27fd46e1a54dd136688de9ab81 [pre-commit.ci] pre-commit autoupdate
c9b6110dec52ff483b42b1428221f903bf143788 (grafted) Merge branch '2.3.x'
7dbb2f7e05d2c0963e90016b1f7a3a45e98a865a (grafted) retarget pre-commit.ci
```

This is gated behind the unreleased Intelligent Test Runner (ITR)
functionality, so there is no risk of unexpectedly unshallowing
repositories.

## 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] Risk is outlined (performance impact, potential for breakage,
maintainability, etc).
- [x] Change is maintainable (easy to change, telemetry, documentation).
- [x] [Library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
are followed. If no release note is required, add label
`changelog/no-changelog`.
- [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))

## Reviewer Checklist

- [x] Title is accurate.
- [x] No unnecessary changes are introduced.
- [x] Description motivates each change.
- [x] Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes unless absolutely necessary.
- [x] Testing strategy adequately addresses listed risk(s).
- [x] Change is maintainable (easy to change, telemetry, documentation).
- [x] Release note makes sense to a user of the library.
- [x] Reviewer has explicitly 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)

Co-authored-by: Romain Komorn <[email protected]>

Author: gnufede

ddtrace/contrib/pytest/plugin.py

@@ -272,6 +272,7 @@ def pytest_configure(config):
 
 def pytest_sessionstart(session):
     if _CIVisibility.enabled:
+        log.debug("CI Visibility enabled - starting test session")
         test_session_span = _CIVisibility._instance.tracer.trace(
             "pytest.test_session",
             service=_CIVisibility._instance._service,
@@ -289,6 +290,7 @@ def pytest_sessionstart(session):
 
 def pytest_sessionfinish(session, exitstatus):
     if _CIVisibility.enabled:
+        log.debug("CI Visibility enabled - finishing test session")
         test_session_span = _extract_span(session)
         if test_session_span is not None:
             _mark_test_status(session, test_session_span)

ddtrace/ext/git.py

@@ -91,6 +91,35 @@ def _git_subprocess_cmd(cmd, cwd=None, std_in=None):
     raise ValueError(compat.ensure_text(stderr).strip())
 
 
+def _extract_clone_defaultremotename(cwd=None):
+    output = _git_subprocess_cmd("config --default origin --get clone.defaultRemoteName")
+    return output
+
+
+def _is_shallow_repository(cwd=None):
+    # type: (Optional[str]) -> bool
+    output = _git_subprocess_cmd("rev-parse --is-shallow-repository", cwd=cwd)
+    return output.strip() == "true"
+
+
+def _unshallow_repository(cwd=None):
+    # type (Optional[str]) -> None
+    remote_name = _extract_clone_defaultremotename(cwd)
+    head_sha = extract_commit_sha(cwd)
+
+    cmd = [
+        "fetch",
+        "--update-shallow",
+        "--filter=blob:none",
+        "--recurse-submodules=no",
+        '--shallow-since="1 month ago"',
+        remote_name,
+        head_sha,
+    ]
+
+    _git_subprocess_cmd(cmd, cwd=cwd)
+
+
 def extract_user_info(cwd=None):
     # type: (Optional[str]) -> Dict[str, Tuple[str, str, str]]
     """Extract commit author info from the git repository in the current directory or one specified by ``cwd``."""
@@ -120,13 +149,22 @@ def extract_latest_commits(cwd=None):
 
 
 def get_rev_list_excluding_commits(commit_shas, cwd=None):
+    return _get_rev_list(excluded_commit_shas=commit_shas, cwd=cwd)
+
+
+def _get_rev_list(excluded_commit_shas=None, included_commit_shas=None, cwd=None):
+    # type: (Optional[list[str]], Optional[list[str]], Optional[str]) -> str
     command = ["rev-list", "--objects", "--filter=blob:none"]
     if extract_git_version(cwd=cwd) >= (2, 23, 0):
         command.append('--since="1 month ago"')
         command.append("--no-object-names")
     command.append("HEAD")
-    exclusions = ["^%s" % sha for sha in commit_shas]
-    command.extend(exclusions)
+    if excluded_commit_shas:
+        exclusions = ["^%s" % sha for sha in excluded_commit_shas]
+        command.extend(exclusions)
+    if included_commit_shas:
+        inclusions = ["%s" % sha for sha in included_commit_shas]
+        command.extend(inclusions)
     commits = _git_subprocess_cmd(command, cwd=cwd)
     return commits
 

ddtrace/internal/ci_visibility/git_client.py

@@ -7,11 +7,14 @@
 from typing import Tuple  # noqa
 
 from ddtrace.ext import ci
+from ddtrace.ext.git import _get_rev_list
+from ddtrace.ext.git import _is_shallow_repository
+from ddtrace.ext.git import _unshallow_repository
 from ddtrace.ext.git import build_git_packfiles
 from ddtrace.ext.git import extract_commit_sha
+from ddtrace.ext.git import extract_git_version
 from ddtrace.ext.git import extract_latest_commits
 from ddtrace.ext.git import extract_remote_url
-from ddtrace.ext.git import get_rev_list_excluding_commits
 from ddtrace.internal.agent import get_trace_url
 from ddtrace.internal.compat import JSONDecodeError
 from ddtrace.internal.logger import get_logger
@@ -76,9 +79,22 @@ def shutdown(self, timeout=None):
     def _run_protocol(cls, serializer, requests_mode, base_url, _tags={}, _response=None, cwd=None):
         # type: (CIVisibilityGitClientSerializerV1, int, str, Dict[str, str], Optional[Response], Optional[str]) -> None
         repo_url = cls._get_repository_url(tags=_tags, cwd=cwd)
+
+        if cls._is_shallow_repository(cwd=cwd) and extract_git_version(cwd=cwd) >= (2, 27, 0):
+            log.debug("Shallow repository detected on git > 2.27 detected, unshallowing")
+            try:
+                cls._unshallow_repository(cwd=cwd)
+                log.debug("Unshallowing done")
+            except ValueError:
+                log.warning("Failed to unshallow repository, continuing to send pack data", exc_info=True)
+
         latest_commits = cls._get_latest_commits(cwd=cwd)
         backend_commits = cls._search_commits(requests_mode, base_url, repo_url, latest_commits, serializer, _response)
-        rev_list = cls._get_filtered_revisions(backend_commits, cwd=cwd)
+        commits_not_in_backend = list(set(latest_commits) - set(backend_commits))
+
+        rev_list = cls._get_filtered_revisions(
+            excluded_commits=backend_commits, included_commits=commits_not_in_backend, cwd=cwd
+        )
         if rev_list:
             with cls._build_packfiles(rev_list, cwd=cwd) as packfiles_prefix:
                 cls._upload_packfiles(
@@ -128,9 +144,9 @@ def _do_request(cls, requests_mode, base_url, endpoint, payload, serializer, hea
         return result
 
     @classmethod
-    def _get_filtered_revisions(cls, excluded_commits, cwd=None):
-        # type: (list[str], Optional[str]) -> list[str]
-        return get_rev_list_excluding_commits(excluded_commits, cwd=cwd)
+    def _get_filtered_revisions(cls, excluded_commits, included_commits=None, cwd=None):
+        # type: (list[str], Optional[list[str]], Optional[str]) -> str
+        return _get_rev_list(excluded_commits, included_commits, cwd=cwd)
 
     @classmethod
     def _build_packfiles(cls, revisions, cwd=None):
@@ -156,6 +172,16 @@ def _upload_packfiles(cls, requests_mode, base_url, repo_url, packfiles_prefix,
                 return False
         return True
 
+    @classmethod
+    def _is_shallow_repository(cls, cwd=None):
+        # type () -> bool
+        return _is_shallow_repository(cwd=cwd)
+
+    @classmethod
+    def _unshallow_repository(cls, cwd=None):
+        # type () -> None
+        _unshallow_repository(cwd=cwd)
+
 
 class CIVisibilityGitClientSerializerV1(object):
     def __init__(self, api_key, app_key):

ddtrace/internal/ci_visibility/recorder.py

@@ -292,6 +292,8 @@ def _fetch_tests_to_skip(self):
             self._test_suites_to_skip = []
             return
 
+        self._test_suites_to_skip = []
+
         if response.status >= 400:
             log.warning("Test skips request responded with status %d", response.status)
             return
@@ -301,7 +303,6 @@ def _fetch_tests_to_skip(self):
             log.warning("Test skips request responded with invalid JSON '%s'", response.body)
             return
 
-        self._test_suites_to_skip = []
         for item in parsed["data"]:
             if item["type"] == TEST_SKIPPING_LEVEL and "suite" in item["attributes"]:
                 module = item["attributes"].get("configurations", {}).get("test.bundle", "").replace(".", "/")

tests/ci_visibility/test_ci_visibility.py

@@ -59,7 +59,6 @@ def _patch_dummy_writer():
 
 
 def test_ci_visibility_service_enable():
-
     with override_env(
         dict(
             DD_API_KEY="foobar.baz",
@@ -716,3 +715,73 @@ def test_civisibility_check_enabled_features_itr_enabled_malformed_response(_do_
 
         mock_log.warning.assert_called_with("Settings request responded with invalid JSON '%s'", "}")
         CIVisibility.disable()
+
+
+def test_run_protocol_unshallow_git_ge_227():
+    with mock.patch("ddtrace.internal.ci_visibility.git_client.extract_git_version", return_value=(2, 27, 0)):
+        with mock.patch.multiple(
+            CIVisibilityGitClient,
+            _get_repository_url=mock.DEFAULT,
+            _is_shallow_repository=classmethod(lambda *args, **kwargs: True),
+            _get_latest_commits=classmethod(lambda *args, **kwwargs: ["latest1", "latest2"]),
+            _search_commits=classmethod(lambda *args: ["latest1", "searched1", "searched2"]),
+            _get_filtered_revisions=mock.DEFAULT,
+            _build_packfiles=mock.DEFAULT,
+            _upload_packfiles=mock.DEFAULT,
+        ):
+            with mock.patch.object(CIVisibilityGitClient, "_unshallow_repository") as mock_unshallow_repository:
+                CIVisibilityGitClient._run_protocol(None, None, None)
+
+            mock_unshallow_repository.assert_called_once_with(cwd=None)
+
+
+def test_run_protocol_does_not_unshallow_git_lt_227():
+    with mock.patch("ddtrace.internal.ci_visibility.git_client.extract_git_version", return_value=(2, 26, 0)):
+        with mock.patch.multiple(
+            CIVisibilityGitClient,
+            _get_repository_url=mock.DEFAULT,
+            _is_shallow_repository=classmethod(lambda *args, **kwargs: True),
+            _get_latest_commits=classmethod(lambda *args, **kwargs: ["latest1", "latest2"]),
+            _search_commits=classmethod(lambda *args: ["latest1", "searched1", "searched2"]),
+            _get_filtered_revisions=mock.DEFAULT,
+            _build_packfiles=mock.DEFAULT,
+            _upload_packfiles=mock.DEFAULT,
+        ):
+            with mock.patch.object(CIVisibilityGitClient, "_unshallow_repository") as mock_unshallow_repository:
+                CIVisibilityGitClient._run_protocol(None, None, None)
+
+            mock_unshallow_repository.assert_not_called()
+
+
+def test_get_filtered_revisions():
+    with mock.patch(
+        "ddtrace.internal.ci_visibility.git_client._get_rev_list", return_value=["rev1", "rev2"]
+    ) as mock_get_rev_list:
+        assert CIVisibilityGitClient._get_filtered_revisions(
+            ["excluded1", "excluded2"], included_commits=["included1", "included2"], cwd="/path/to/repo"
+        ) == ["rev1", "rev2"]
+        mock_get_rev_list.assert_called_once_with(
+            ["excluded1", "excluded2"], ["included1", "included2"], cwd="/path/to/repo"
+        )
+
+
+def test_is_shallow_repository_true():
+    with mock.patch(
+        "ddtrace.internal.ci_visibility.git_client._is_shallow_repository", return_value=True
+    ) as mock_is_shallow_repository:
+        assert CIVisibilityGitClient._is_shallow_repository(cwd="/path/to/repo") is True
+        mock_is_shallow_repository.assert_called_once_with(cwd="/path/to/repo")
+
+
+def test_is_shallow_repository_false():
+    with mock.patch(
+        "ddtrace.internal.ci_visibility.git_client._is_shallow_repository", return_value=False
+    ) as mock_is_shallow_repository:
+        assert CIVisibilityGitClient._is_shallow_repository(cwd="/path/to/repo") is False
+        mock_is_shallow_repository.assert_called_once_with(cwd="/path/to/repo")
+
+
+def test_unshallow_repository():
+    with mock.patch("ddtrace.internal.ci_visibility.git_client._unshallow_repository") as mock_unshallow_repository:
+        CIVisibilityGitClient._unshallow_repository(cwd="/path/to/repo")
+        mock_unshallow_repository.assert_called_once_with(cwd="/path/to/repo")

tests/tracer/test_ci.py

@@ -227,3 +227,73 @@ def test_os_runtime_metadata_tagging():
     assert extracted_tags.get(ci.OS_VERSION) is not None
     assert extracted_tags.get(ci.RUNTIME_NAME) is not None
     assert extracted_tags.get(ci.RUNTIME_VERSION) is not None
+
+
+def test_get_rev_list_no_args_git_ge_223(git_repo):
+    with mock.patch("ddtrace.ext.git._git_subprocess_cmd") as mock_git_subprocess, mock.patch(
+        "ddtrace.ext.git.extract_git_version", return_value=(2, 23, 0)
+    ):
+        mock_git_subprocess.return_value = ["commithash1", "commithash2"]
+        assert git._get_rev_list(cwd=git_repo) == ["commithash1", "commithash2"]
+        mock_git_subprocess.assert_called_once_with(
+            ["rev-list", "--objects", "--filter=blob:none", '--since="1 month ago"', "--no-object-names", "HEAD"],
+            cwd=git_repo,
+        )
+
+
+def test_get_rev_list_git_lt_223(git_repo):
+    with mock.patch("ddtrace.ext.git._git_subprocess_cmd") as mock_git_subprocess, mock.patch(
+        "ddtrace.ext.git.extract_git_version", return_value=(2, 22, 0)
+    ):
+        mock_git_subprocess.return_value = ["commithash1", "commithash2"]
+        assert git._get_rev_list(
+            excluded_commit_shas=["exclude1", "exclude2"], included_commit_shas=["include1", "include2"], cwd=git_repo
+        ) == ["commithash1", "commithash2"]
+        mock_git_subprocess.assert_called_once_with(
+            ["rev-list", "--objects", "--filter=blob:none", "HEAD", "^exclude1", "^exclude2", "include1", "include2"],
+            cwd=git_repo,
+        )
+
+
+def test_is_shallow_repository_true(git_repo):
+    with mock.patch("ddtrace.ext.git._git_subprocess_cmd", return_value="true") as mock_git_subprocess:
+        assert git._is_shallow_repository(cwd=git_repo) is True
+        mock_git_subprocess.assert_called_once_with("rev-parse --is-shallow-repository", cwd=git_repo)
+
+
+def test_is_shallow_repository_false(git_repo):
+    with mock.patch("ddtrace.ext.git._git_subprocess_cmd", return_value="false") as mock_git_subprocess:
+        assert git._is_shallow_repository(cwd=git_repo) is False
+        mock_git_subprocess.assert_called_once_with("rev-parse --is-shallow-repository", cwd=git_repo)
+
+
+def test_unshallow_repository(git_repo):
+    with mock.patch(
+        "ddtrace.ext.git._extract_clone_defaultremotename", return_value="myremote"
+    ) as mock_defaultremotename:
+        with mock.patch(
+            "ddtrace.ext.git.extract_commit_sha", return_value="mycommitshaaaaaaaaaaaa123"
+        ) as mock_extract_sha:
+            with mock.patch("ddtrace.ext.git._git_subprocess_cmd") as mock_git_subprocess:
+                git._unshallow_repository(cwd=git_repo)
+
+                mock_defaultremotename.assert_called_once_with(git_repo)
+                mock_extract_sha.assert_called_once_with(git_repo)
+                mock_git_subprocess.assert_called_once_with(
+                    [
+                        "fetch",
+                        "--update-shallow",
+                        "--filter=blob:none",
+                        "--recurse-submodules=no",
+                        '--shallow-since="1 month ago"',
+                        "myremote",
+                        "mycommitshaaaaaaaaaaaa123",
+                    ],
+                    cwd=git_repo,
+                )
+
+
+def test_extract_clone_defaultremotename():
+    with mock.patch("ddtrace.ext.git._git_subprocess_cmd", return_value="default_remote_name") as mock_git_subprocess:
+        assert git._extract_clone_defaultremotename(cwd=git_repo) == "default_remote_name"
+        mock_git_subprocess.assert_called_once_with("config --default origin --get clone.defaultRemoteName")