fix(ci_visibility): use ||| as git show format separator when fetching user info [backport 1.20] (#8605)

Backport b887ed9 from #8559 to 1.20.

Fixes an issue where git committer or author names containing a comma
(eg: "Lastname, Firstname" instead of "Firstname Lastname") would cause
the user info function to fail to unpack properly.

This changes the separator for the git show command format to "|||"
which should be far less common in committer or author names.

## 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`.
- [x] If change touches code that signs or publishes builds or packages,
or handles credentials of any kind, I've requested a review from
`@DataDog/security-design-and-guidance`.

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

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

Author: github-actions[bot]

ddtrace/ext/git.py

@@ -140,8 +140,10 @@ 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``."""
     # Note: `git show -s --format... --date...` is supported since git 2.1.4 onwards
-    stdout = _git_subprocess_cmd("show -s --format=%an,%ae,%ad,%cn,%ce,%cd --date=format:%Y-%m-%dT%H:%M:%S%z", cwd=cwd)
-    author_name, author_email, author_date, committer_name, committer_email, committer_date = stdout.split(",")
+    stdout = _git_subprocess_cmd(
+        "show -s --format=%an|||%ae|||%ad|||%cn|||%ce|||%cd --date=format:%Y-%m-%dT%H:%M:%S%z", cwd=cwd
+    )
+    author_name, author_email, author_date, committer_name, committer_email, committer_date = stdout.split("|||")
     return {
         "author": (author_name, author_email, author_date),
         "committer": (committer_name, committer_email, committer_date),

docs/spelling_wordlist.txt

@@ -5,10 +5,12 @@ autoreloading
 CPython
 Fargate
 Firehose
+Firstname
 Gunicorn
 HTTPPropagator
 INfo
 IPv
+Lastname
 MySQL
 OpenTracing
 Runtimes
@@ -79,6 +81,7 @@ dogpile.cache
 dogstatsd
 dunder
 dsn
+eg
 elasticsearch
 elasticsearch1
 elasticsearch7

releasenotes/notes/ci_visibility-fix-comma_separation_issue_in_git_format-76bf32d68ba6c164.yaml

@@ -0,0 +1,5 @@
+---
+fixes:
+  - |
+    CI Visibility: fixes an issue where git author or committer names containing commas (eg: "Lastname, Firstname")
+    would not work (and log an error) due to the use of comma as a separator.

tests/tracer/test_ci.py

@@ -61,6 +61,16 @@ def test_git_extract_user_info(git_repo):
     assert extracted_users["committer"] == expected_committer
 
 
+def test_git_extract_user_info_with_commas():
+    with mock.patch(
+        "ddtrace.ext.git._git_subprocess_cmd",
+        return_value="Do, Jo|||[email protected]|||2021-01-19T09:24:53-0400|||Do, Ja|||[email protected]|||2021-01-20T04:37:21-0400",
+    ):
+        extracted_users = git.extract_user_info()
+        assert extracted_users["author"] == ("Do, Jo", "[email protected]", "2021-01-19T09:24:53-0400")
+        assert extracted_users["committer"] == ("Do, Ja", "[email protected]", "2021-01-20T04:37:21-0400")
+
+
 def test_git_extract_user_info_error(git_repo_empty):
     """On error, the author/committer tags should not be extracted, and should internally raise an error."""
     with pytest.raises(ValueError):