Skip to content

Commit

Permalink
fix(ci_visibility): avoid git tracebacks when .git dir is absent [bac…
Browse files Browse the repository at this point in the history
…kport 2.13] (#11196)

Backport 5051de3 from #11175 to 2.13.

Fixes #10983 where the git metadata upload would log exceptions when
trying to upload git metadata without a working `git` environment (eg:
missing `git` executable or no `.git` directory).

## 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)

---------

Co-authored-by: Romain Komorn <[email protected]>
Co-authored-by: Romain Komorn <[email protected]>
  • Loading branch information
3 people authored Oct 29, 2024
1 parent 52f51e8 commit fdb023c
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 1 deletion.
13 changes: 13 additions & 0 deletions ddtrace/internal/ci_visibility/git_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
from ddtrace.ext.git import extract_commit_sha
from ddtrace.ext.git import extract_git_version
from ddtrace.ext.git import extract_remote_url
from ddtrace.ext.git import extract_workspace_path
from ddtrace.internal.agent import get_trace_url
from ddtrace.internal.compat import JSONDecodeError
from ddtrace.internal.logger import get_logger
Expand Down Expand Up @@ -94,8 +95,20 @@ def __init__(
elif self._requests_mode == REQUESTS_MODE.AGENTLESS_EVENTS:
self._base_url = "https://api.{}{}".format(os.getenv("DD_SITE", AGENTLESS_DEFAULT_SITE), GIT_API_BASE_PATH)

def _get_git_dir(self, cwd=None):
# type: (Optional[str]) -> Optional[str]
try:
return extract_workspace_path(cwd=cwd)
except (FileNotFoundError, ValueError):
return None

def upload_git_metadata(self, cwd=None):
# type: (Optional[str]) -> None
if not self._get_git_dir(cwd=cwd):
log.debug("Missing .git directory; skipping git metadata upload")
self._metadata_upload_status.value = METADATA_UPLOAD_STATUS.FAILED # type: ignore[attr-defined]
return

self._tags = ci.tags(cwd=cwd)
if self._worker is None:
self._worker = Process(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
fixes:
- CI Visibility: fixes unnecessary logging of an exception that would appear when trying to upload git metadata in
an environment without functioning git (eg: missing ``git`` binary or ``.git`` directory)
10 changes: 10 additions & 0 deletions tests/ci_visibility/test_ci_visibility.py
Original file line number Diff line number Diff line change
Expand Up @@ -999,6 +999,7 @@ def mock_git_client(self):
_get_filtered_revisions=mock.Mock(return_value="latest3\nlatest4"),
_upload_packfiles=mock.Mock(side_effec=NotImplementedError),
_do_request=mock.Mock(side_effect=NotImplementedError),
_get_git_dir=mock.Mock(return_value="does_not_matter"),
), mock.patch(
"ddtrace.internal.ci_visibility.git_client._build_git_packfiles_with_details"
) as mock_build_packfiles:
Expand Down Expand Up @@ -1082,6 +1083,15 @@ def test_upload_git_metadata_upload_unnecessary(self, api_key, requests_mode):
git_client.upload_git_metadata()
assert git_client.wait_for_metadata_upload_status() == METADATA_UPLOAD_STATUS.UNNECESSARY

@pytest.mark.parametrize("api_key, requests_mode", api_key_requests_mode_parameters)
def test_upload_git_metadata_upload_no_git_dir(self, api_key, requests_mode):
with mock.patch.object(CIVisibilityGitClient, "_get_git_dir", mock.Mock(return_value=None)):
git_client = CIVisibilityGitClient(api_key, requests_mode)
git_client.upload_git_metadata()

# Notably, this should _not_ raise ValueError
assert git_client.wait_for_metadata_upload_status() == METADATA_UPLOAD_STATUS.FAILED


def test_get_filtered_revisions():
with mock.patch(
Expand Down
6 changes: 5 additions & 1 deletion tests/contrib/pytest/test_pytest.py
Original file line number Diff line number Diff line change
Expand Up @@ -3940,7 +3940,11 @@ def test_add_two_number_list():
)

self.testdir.chdir()
with mock.patch("ddtrace.ext.git._get_executable_path", return_value=None):
with mock.patch("ddtrace.ext.git._get_executable_path", return_value=None), mock.patch(
"ddtrace.internal.ci_visibility.recorder.ddconfig",
_get_default_civisibility_ddconfig(),
) as mock_ddconfig:
mock_ddconfig._ci_visibility_agentless_enabled = True
self.inline_run("--ddtrace")

spans = self.pop_spans()
Expand Down

0 comments on commit fdb023c

Please sign in to comment.