Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Git Metadata errors from CI Visibility despite explicit metdata configuration and disabling metadata #10983

Open
jasondamour opened this issue Oct 8, 2024 · 2 comments · Fixed by #11175
Assignees
Labels

Comments

@jasondamour
Copy link

jasondamour commented Oct 8, 2024

Hello,

Our tests run in CI without a .git/ directory intentionally. We're configuring CI Visibility git metadata with DD_GIT_REPOSITORY_URL, DD_GIT_BRANCH, and DD_GIT_COMMIT_SHA. However, the ddtrace agent is still printing lots of errors about not finding git. Our configuration is below:

ddtrace: 2.11.6
# env
DD_CIVISIBILITY_AGENTLESS_ENABLED=true
DD_TRACE_GIT_METADATA_ENABLED=false
DD_ENV=CI
DD_SERVICE=xxx
DD_API_KEY=xxx
DD_APP_KEY=xxx
DD_GIT_REPOSITORY_URL=xxx
DD_GIT_BRANCH=xxx
DD_GIT_COMMIT_SHA=xxx

And tests are executed with pytest ..... --ddtrace:

[Datadog CI Visibility] ERROR    ddtrace.ext.git:git.py:146 Error setting safe directory
[Datadog CI Visibility] ERROR    ddtrace.ext.git:git.py:339 Error extracting git metadata: fatal: No remote configured to list refs from.
Process Process-1:
Traceback (most recent call last):
  File "/Users/jasondamour/.pyenv/versions/3.10.6/lib/python3.10/multiprocessing/process.py", line 314, in _bootstrap
    self.run()
  File "/Users/jasondamour/.pyenv/versions/3.10.6/lib/python3.10/multiprocessing/process.py", line 108, in run
    self._target(*self._args, **self._kwargs)
  File "/Users/jasondamour/.cache/pants/named_caches/pex_root/venvs/s/15dd8c43/venv/lib/python3.10/site-packages/ddtrace/internal/ci_visibility/git_client.py", line 162, in _run_protocol
    latest_commits = cls._get_latest_commits(cwd=cwd)
  File "/Users/jasondamour/.cache/pants/named_caches/pex_root/venvs/s/15dd8c43/venv/lib/python3.10/site-packages/ddtrace/internal/ci_visibility/git_client.py", line 238, in _get_latest_commits
    raise ValueError(stderr)
ValueError: fatal: not a git repository (or any of the parent directories): .git
sys:1: DeprecationWarning: builtin type swigvarlink has no __module__ attribute

Asks:

  1. These errors should not occur since the metadata is explicitly provided
  2. CI Visibility should expose a variable for explicitly disabling the git client (or respect the existing DD_TRACE_GIT_METADATA_ENABLED variable) since git metadata is optional anyways
@brettdh
Copy link
Contributor

brettdh commented Oct 19, 2024

I've run into this as well. Digging into the code a bit, it turns out that this code path isn't trying to gather any metadata that can be provided (today) via environment variables. Instead, it's gathering a list of up to 1000 commits in the past month (git command), which it then uploads to Datadog. Presumably this is needed for error attribution or other features related to change detection.

I'm working on a PR that'll be either:

brettdh added a commit to brettdh/dd-trace-py that referenced this issue Oct 19, 2024
@romainkomorndatadog romainkomorndatadog self-assigned this Oct 22, 2024
brettdh added a commit to brettdh/dd-trace-py that referenced this issue Oct 22, 2024
romainkomorndatadog added a commit that referenced this issue Oct 28, 2024
)

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: Brett Higgins <[email protected]>
github-actions bot pushed a commit that referenced this issue Oct 28, 2024
)

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: Brett Higgins <[email protected]>
(cherry picked from commit 5051de3)
github-actions bot pushed a commit that referenced this issue Oct 28, 2024
)

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: Brett Higgins <[email protected]>
(cherry picked from commit 5051de3)
github-actions bot pushed a commit that referenced this issue Oct 28, 2024
)

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: Brett Higgins <[email protected]>
(cherry picked from commit 5051de3)
github-actions bot pushed a commit that referenced this issue Oct 28, 2024
)

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: Brett Higgins <[email protected]>
(cherry picked from commit 5051de3)
@romainkomorndatadog
Copy link
Collaborator

romainkomorndatadog commented Oct 28, 2024

This was merged (and I hit submit too soon...), but it still needs to go out into the patch releases for 2.12 through 2.15. It will also be in 2.16.

romainkomorndatadog added a commit that referenced this issue Oct 29, 2024
…kport 2.15] (#11198)

Backport 5051de3 from #11175 to 2.15.

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]>
romainkomorndatadog added a commit that referenced this issue Oct 29, 2024
…kport 2.14] (#11197)

Backport 5051de3 from #11175 to 2.14.

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]>
romainkomorndatadog added a commit that referenced this issue Oct 29, 2024
…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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment