Add detailed upload context to Trace2 telemetry

This patch intercepts the output during a successful upload
execution to capture the generated CL URLs. It logs a dynamically
constructed "repo.uploadstate" data event to the active trace2
log containing:
- Uploaded CL URLs
- Target remote name
- Source branch
- Modified files

Test:
1. ./run_tests
2. pytest tests/test_project.py
3. Manual verification:
- Created a dummy branch `test_upload_branch` in a project with local file changes.
- Invoked repo upload passing an explicit trace output directory:
  `repo --git-trace2-event-log=/tmp/trace2out upload --dry-run --no-verify art`
- Verified the injected payload successfully appeared on disk within the generated log:
  ```json
  {"event":"data",..."key":"repo.uploadstate/cls","value":""}
  {"event":"data",..."key":"repo.uploadstate/remote","value":"goog"}
  {"event":"data",..."key":"repo.uploadstate/branch","value":"test_upload_branch"}
  {"event":"data",..."key":"repo.uploadstate/files","value":"dummy_file.txt"}
  ```
  (cls is correctly blank on --dry-run but populates on real HTTP pushes)

Bug: 543953499
Change-Id: I9c402a32d01d156d42cf24eaa2e60e22710b5e6f
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/616581
Reviewed-by: Gavin Mak <gavinmak@google.com>
Tested-by: Ram Peri <ramperi@google.com>
Commit-Queue: Ram Peri <ramperi@google.com>

project.py

@@ -239,6 +239,12 @@ class ReviewableBranch:
             "--pretty=format:%cd", "-n", "1", R_HEADS + self.name, "--"
         )
 
+    @property
+    def modified_files(self) -> List[str]:
+        return self.project.bare_git.diff(
+            "--name-only", f"{self.base}...{R_HEADS}{self.name}"
+        ).splitlines()
+
     @property
     def base_exists(self):
         """Whether the branch we're tracking exists.
@@ -271,7 +277,8 @@ class ReviewableBranch:
         validate_certs=True,
         push_options=None,
         patchset_description=None,
-    ):
+        git_event_log: Optional[EventLog] = None,
+    ) -> None:
         self.project.UploadForReview(
             branch=self.name,
             people=people,
@@ -287,6 +294,7 @@ class ReviewableBranch:
             validate_certs=validate_certs,
             push_options=push_options,
             patchset_description=patchset_description,
+            git_event_log=git_event_log,
         )
 
     def GetPublishedRefs(self):
@@ -1193,7 +1201,8 @@ class Project:
         validate_certs=True,
         push_options=None,
         patchset_description=None,
-    ):
+        git_event_log: Optional[EventLog] = None,
+    ) -> None:
         """Uploads the named branch for code review."""
         if branch is None:
             branch = self.CurrentBranch
@@ -1282,14 +1291,47 @@ class Project:
             ref_spec = ref_spec + "%" + ",".join(opts)
         cmd.append(ref_spec)
 
-        GitCommand(self, cmd, bare=True, verify_command=True).Wait()
+        push_cmd = GitCommand(
+            self,
+            cmd,
+            bare=True,
+            verify_command=True,
+        )
+        push_cmd.Wait()
 
+        cls_urls = self._FindGerritUrls(push_cmd.stderr)
+
+        try:
+            rb = ReviewableBranch(self, branch, branch.LocalMerge)
+            modified_files_list = rb.modified_files
+            if git_event_log:
+                git_event_log.LogDataConfigEvents(
+                    {
+                        "cls": ",".join(cls_urls),
+                        "remote": branch.remote.name,
+                        "branch": branch.name,
+                        "files": ",".join(modified_files_list),
+                    },
+                    "repo.uploadstate",
+                )
+        except Exception as e:
+            logger.error("Tracing failed: %s", str(e))
         if not dryrun:
             msg = f"posted to {branch.remote.review} for {dest_branch}"
             self.bare_git.UpdateRef(
                 R_PUB + branch.name, R_HEADS + branch.name, message=msg
             )
 
+    @staticmethod
+    def _FindGerritUrls(stderr: Optional[str]) -> List[str]:
+        """Extracts Gerrit review URLs from git push output."""
+        if not stderr:
+            return []
+        return [
+            match.group(1)
+            for match in re.finditer(r"(https?://[^/]+/c/.+?/\+/\d+)", stderr)
+        ]
+
     @staticmethod
     def _encode_patchset_description(original):
         """Applies percent-encoding for strings sent as patchset description.

subcmds/upload.py

@@ -649,6 +649,7 @@ Gerrit Code Review:  https://www.gerritcodereview.com/
             validate_certs=opt.validate_certs,
             push_options=push_options,
             patchset_description=opt.patchset_description,
+            git_event_log=self.git_event_log,
         )
 
         branch.uploaded = True

tests/test_project.py

@@ -29,6 +29,7 @@ import utils_for_test
 
 import error
 import git_config
+import git_trace2_event_log
 import manifest_xml
 import platform_utils
 import project
@@ -82,6 +83,7 @@ class ReviewableBranchTests(unittest.TestCase):
             short, long = next(iter(d.items()))
             self.assertTrue(long.startswith(short))
             self.assertTrue(rb.base_exists)
+            self.assertEqual(["readme"], rb.modified_files)
             # Hard to assert anything useful about this.
             self.assertTrue(rb.date)
 
@@ -95,6 +97,20 @@ class ReviewableBranchTests(unittest.TestCase):
             # Hard to assert anything useful about this.
             self.assertTrue(rb.date)
 
+    def test_upload_for_review_forwards_git_event_log(self) -> None:
+        """Check UploadForReview passes git_event_log to project."""
+        proj = mock.MagicMock(spec=project.Project)
+        branch = mock.MagicMock()
+        branch.name = "work"
+        rb = project.ReviewableBranch(proj, branch, "main")
+        mock_event_log = mock.MagicMock()
+
+        rb.UploadForReview(people=([], []), git_event_log=mock_event_log)
+
+        proj.UploadForReview.assert_called_once()
+        _, kwargs = proj.UploadForReview.call_args
+        self.assertEqual(kwargs.get("git_event_log"), mock_event_log)
+
 
 class ProjectTests(unittest.TestCase):
     """Check Project behavior."""
@@ -105,6 +121,249 @@ class ProjectTests(unittest.TestCase):
             "abcd00%21%21_%2b",
         )
 
+    def test_find_gerrit_urls(self) -> None:
+        """Check _FindGerritUrls extracts review URLs from stderr."""
+        # Single CL URL from standard push output.
+        stderr = (
+            "remote:\n"
+            "remote: Processing changes: new: 1, refs: 1, done\n"
+            "remote:\n"
+            "remote: SUCCESS\n"
+            "remote:\n"
+            "remote:   https://gerrit.example.com/c/git-repo/+/616581"
+            " Add telemetry [NEW]\n"
+            "remote:\n"
+            "To sso://gerrit/git-repo\n"
+            " * [new reference]   HEAD -> refs/for/main\n"
+        )
+        self.assertEqual(
+            project.Project._FindGerritUrls(stderr),
+            ["https://gerrit.example.com/c/git-repo/+/616581"],
+        )
+
+        # Project names containing slashes and nested paths.
+        url1 = "https://example.com/c/platform/base/+/12345"
+        url2 = "https://example.com/c/vendor/device/raviole/prebuilts/+/987654"
+        stderr_nested = f"remote:   {url1}\nremote:   {url2} [NEW]\n"
+        self.assertEqual(
+            project.Project._FindGerritUrls(stderr_nested),
+            [url1, url2],
+        )
+
+        # Multiple URLs on same line or custom ports / http schemas.
+        stderr_custom = (
+            "remote:   https://review.corp:8443/c/platform/manifest/+/4321\n"
+            "remote:   http://localhost:8080/c/test-project/+/555\n"
+        )
+        self.assertEqual(
+            project.Project._FindGerritUrls(stderr_custom),
+            [
+                "https://review.corp:8443/c/platform/manifest/+/4321",
+                "http://localhost:8080/c/test-project/+/555",
+            ],
+        )
+
+        # Non-matching outputs.
+        self.assertEqual(project.Project._FindGerritUrls(None), [])
+        self.assertEqual(project.Project._FindGerritUrls(""), [])
+        self.assertEqual(
+            project.Project._FindGerritUrls("Everything up-to-date\n"), []
+        )
+        self.assertEqual(
+            project.Project._FindGerritUrls(
+                "https://example.com/not/a/gerrit/url"
+            ),
+            [],
+        )
+
+    def _create_project_for_upload_test(
+        self,
+    ) -> Tuple[mock.MagicMock, mock.MagicMock]:
+        proj = mock.MagicMock(spec=project.Project)
+        proj.name = "test-project"
+        proj.UserEmail = "test@example.com"
+        proj.dest_branch = "refs/heads/main"
+        proj.bare_git = mock.MagicMock()
+        proj._FindGerritUrls = project.Project._FindGerritUrls
+
+        mock_branch = mock.MagicMock()
+        mock_branch.name = "test-branch"
+        mock_branch.LocalMerge = "refs/heads/main"
+        mock_branch.merge = "refs/heads/main"
+        mock_branch.remote.review = "http://review.example.com"
+        mock_branch.remote.name = "origin"
+        mock_branch.remote.projectname = "test-project"
+        mock_branch.remote.ReviewUrl.return_value = (
+            "https://review.example.com/test-project"
+        )
+
+        proj.GetBranch.return_value = mock_branch
+        return proj, mock_branch
+
+    def test_upload_for_review_event_emission(self) -> None:
+        """Check UploadForReview emits repo.uploadstate trace2 data events."""
+        proj, _ = self._create_project_for_upload_test()
+
+        with mock.patch("project.GitCommand") as mock_git_cmd, mock.patch(
+            "project.ReviewableBranch"
+        ) as mock_rb_cls:
+            mock_cmd = mock.MagicMock()
+            mock_cmd.Wait.return_value = 0
+            mock_cmd.stderr = (
+                "remote:   https://example.com/c/test/+/123 [NEW]\n"
+                "remote:   https://example.com/c/test/+/124 [NEW]\n"
+            )
+            mock_git_cmd.return_value = mock_cmd
+
+            mock_rb = mock.MagicMock()
+            mock_rb.modified_files = ["file1.txt", "file2.txt"]
+            mock_rb_cls.return_value = mock_rb
+
+            mock_event_log = mock.MagicMock()
+            project.Project.UploadForReview(
+                proj,
+                branch="test-branch",
+                dryrun=True,
+                git_event_log=mock_event_log,
+            )
+
+            mock_event_log.LogDataConfigEvents.assert_called_once_with(
+                {
+                    "cls": (
+                        "https://example.com/c/test/+/123,"
+                        "https://example.com/c/test/+/124"
+                    ),
+                    "remote": "origin",
+                    "branch": "test-branch",
+                    "files": "file1.txt,file2.txt",
+                },
+                "repo.uploadstate",
+            )
+
+    def test_upload_for_review_event_emission_no_cls(self) -> None:
+        """Check event emission when stderr contains no review URLs."""
+        proj, _ = self._create_project_for_upload_test()
+
+        with mock.patch("project.GitCommand") as mock_git_cmd, mock.patch(
+            "project.ReviewableBranch"
+        ) as mock_rb_cls:
+            mock_cmd = mock.MagicMock()
+            mock_cmd.Wait.return_value = 0
+            mock_cmd.stderr = "Everything up-to-date\n"
+            mock_git_cmd.return_value = mock_cmd
+
+            mock_rb = mock.MagicMock()
+            mock_rb.modified_files = ["dummy.txt"]
+            mock_rb_cls.return_value = mock_rb
+
+            mock_event_log = mock.MagicMock()
+            project.Project.UploadForReview(
+                proj,
+                branch="test-branch",
+                dryrun=True,
+                git_event_log=mock_event_log,
+            )
+
+            mock_event_log.LogDataConfigEvents.assert_called_once_with(
+                {
+                    "cls": "",
+                    "remote": "origin",
+                    "branch": "test-branch",
+                    "files": "dummy.txt",
+                },
+                "repo.uploadstate",
+            )
+
+    def test_upload_for_review_no_event_log(self) -> None:
+        """Check UploadForReview succeeds when git_event_log is None."""
+        proj, _ = self._create_project_for_upload_test()
+
+        with mock.patch("project.GitCommand") as mock_git_cmd, mock.patch(
+            "project.ReviewableBranch"
+        ) as mock_rb_cls:
+            mock_cmd = mock.MagicMock()
+            mock_cmd.Wait.return_value = 0
+            mock_cmd.stderr = "remote:   https://example.com/c/test/+/1\n"
+            mock_git_cmd.return_value = mock_cmd
+
+            mock_rb = mock.MagicMock()
+            mock_rb.modified_files = ["file.txt"]
+            mock_rb_cls.return_value = mock_rb
+
+            project.Project.UploadForReview(
+                proj,
+                branch="test-branch",
+                dryrun=True,
+                git_event_log=None,
+            )
+
+    def test_upload_for_review_tracing_exception_handled(self) -> None:
+        """Check exceptions during tracing are caught and do not fail upload."""
+        proj, _ = self._create_project_for_upload_test()
+
+        with mock.patch("project.GitCommand") as mock_git_cmd, mock.patch(
+            "project.ReviewableBranch"
+        ) as mock_rb_cls, mock.patch("project.logger") as mock_logger:
+            mock_cmd = mock.MagicMock()
+            mock_cmd.Wait.return_value = 0
+            mock_cmd.stderr = "remote:   https://example.com/c/test/+/1\n"
+            mock_git_cmd.return_value = mock_cmd
+
+            mock_rb_cls.side_effect = Exception("failed to inspect branch")
+
+            mock_event_log = mock.MagicMock()
+            project.Project.UploadForReview(
+                proj,
+                branch="test-branch",
+                dryrun=True,
+                git_event_log=mock_event_log,
+            )
+
+            mock_logger.error.assert_called_once()
+            mock_event_log.LogDataConfigEvents.assert_not_called()
+
+    def test_upload_for_review_real_event_log(self) -> None:
+        """Check integration with real git_trace2_event_log.EventLog."""
+        proj, _ = self._create_project_for_upload_test()
+
+        with mock.patch("project.GitCommand") as mock_git_cmd, mock.patch(
+            "project.ReviewableBranch"
+        ) as mock_rb_cls:
+            mock_cmd = mock.MagicMock()
+            mock_cmd.Wait.return_value = 0
+            mock_cmd.stderr = (
+                "remote:   https://example.com/c/test/+/456 [NEW]\n"
+            )
+            mock_git_cmd.return_value = mock_cmd
+
+            mock_rb = mock.MagicMock()
+            mock_rb.modified_files = ["file1.py", "file2.py"]
+            mock_rb_cls.return_value = mock_rb
+
+            event_log = git_trace2_event_log.EventLog(env={})
+            project.Project.UploadForReview(
+                proj,
+                branch="test-branch",
+                dryrun=True,
+                git_event_log=event_log,
+            )
+
+            data_events = [
+                e for e in event_log._log if e.get("event") == "data"
+            ]
+            data_map = {e["key"]: e["value"] for e in data_events}
+            self.assertEqual(
+                data_map.get("repo.uploadstate/cls"),
+                "https://example.com/c/test/+/456",
+            )
+            self.assertEqual(data_map.get("repo.uploadstate/remote"), "origin")
+            self.assertEqual(
+                data_map.get("repo.uploadstate/branch"), "test-branch"
+            )
+            self.assertEqual(
+                data_map.get("repo.uploadstate/files"), "file1.py,file2.py"
+            )
+
     def test_get_head_revision_id(self):
         """Check GetHeadRevisionId behavior."""
         with utils_for_test.TempGitTree() as tempdir: