Merge branch 'master' into grpcio-1-62-2

Author: slicklash

.github/workflows/build-test-deploy.yml

@@ -35,7 +35,7 @@ jobs:
         cp output/gprofiler_x86_64 output/gprofiler  # for backwards compatibility, we upload both with arch suffix and without
 
      - name: Upload the executables as job artifacts
-       uses: actions/upload-artifact@v2
+       uses: actions/upload-artifact@v4
        with:
          name: gprofiler_x86_64
          path: output/
@@ -90,7 +90,7 @@ jobs:
           submodules: true
 
       - name: Download the executable from previous job
-        uses: actions/download-artifact@v2
+        uses: actions/download-artifact@v4
         with:
           name: gprofiler_x86_64
           path: dist/
@@ -130,7 +130,7 @@ jobs:
          mv build/aarch64/gprofiler output/gprofiler_aarch64
 
      - name: Upload the executables as job artifacts
-       uses: actions/upload-artifact@v2
+       uses: actions/upload-artifact@v4
        with:
          name: gprofiler_aarch64
          path: output/
@@ -155,13 +155,13 @@ jobs:
         run: ./scripts/verify_tag.sh
 
       - name: Download x86_64 executable from a previous job
-        uses: actions/download-artifact@v2
+        uses: actions/download-artifact@v4
         with:
           name: gprofiler_x86_64
           path: output/
 
       - name: Download aarch64 executable from a previous job
-        uses: actions/download-artifact@v2
+        uses: actions/download-artifact@v4
         with:
           name: gprofiler_aarch64
           path: output/
@@ -189,7 +189,7 @@ jobs:
         fetch-depth: 0
 
     - name: Download executables from the previous job
-      uses: actions/download-artifact@v2
+      uses: actions/download-artifact@v4
       with:
         name: gprofiler_x86_64
         path: output/
@@ -205,7 +205,7 @@ jobs:
       run: mkdir -p output && docker image save gprofiler_x86_64 > output/gprofiler_x86_64.img
 
     - name: Upload the image artifact
-      uses: actions/upload-artifact@v2
+      uses: actions/upload-artifact@v4
       with:
         name: gprofiler_x86_64.img
         path: output/
@@ -250,13 +250,13 @@ jobs:
         submodules: true
 
     - name: Download the executable from previous job
-      uses: actions/download-artifact@v2
+      uses: actions/download-artifact@v4
       with:
         name: gprofiler_x86_64
         path: output/
 
     - name: Download the image from previous job
-      uses: actions/download-artifact@v2
+      uses: actions/download-artifact@v4
       with:
         name: gprofiler_x86_64.img
         path: output/
@@ -309,7 +309,7 @@ jobs:
         uses: docker/setup-buildx-action@v1
 
       - name: Download executables from the previous job
-        uses: actions/download-artifact@v2
+        uses: actions/download-artifact@v4
         with:
           name: gprofiler_aarch64
           path: output/
@@ -347,7 +347,7 @@ jobs:
       # build-container-aarch64 has pushed the image to DockerHub, so we'll pull it later when creating
       # the manifest.
       - name: Download the x86_64 image from previous job
-        uses: actions/download-artifact@v2
+        uses: actions/download-artifact@v4
         with:
           name: gprofiler_x86_64.img
           path: output/

gprofiler/__init__.py

@@ -13,4 +13,4 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 #
-__version__ = "1.49.0"
+__version__ = "1.50.0"

gprofiler/profilers/python_ebpf.py

@@ -17,10 +17,11 @@
 import os
 import resource
 import selectors
+import shutil
 import signal
 from pathlib import Path
 from subprocess import Popen
-from typing import Dict, List, Optional, Tuple, cast
+from typing import Dict, List, Optional, Tuple, Union, cast
 
 from granulate_utils.linux.ns import is_running_in_init_pid
 from psutil import NoSuchProcess, Process
@@ -86,6 +87,10 @@ def __init__(
         self._kernel_offsets: Dict[str, int] = {}
         self._metadata = python.PythonMetadata(self._profiler_state.stop_event)
         self._verbose = verbose
+        self._pyperf_staticx_tmpdir: Optional[Path] = None
+        if os.environ.get("TMPDIR", None) is not None:
+            # We want to create a new level of hirerachy in our current staticx tempdir.
+            self._pyperf_staticx_tmpdir = Path(os.environ["TMPDIR"]) / ("pyperf_" + random_prefix())
 
     @classmethod
     def _check_output(cls, process: Popen, output_path: Path) -> None:
@@ -153,6 +158,14 @@ def _pyperf_base_command(self) -> List[str]:
             cmd.extend(["-v", "4"])
         return cmd
 
+    def _staticx_cleanup(self) -> None:
+        """
+        We experienced issues with PyPerf's staticx'd directory, so we want to clean it up on termination.
+        """
+        assert not self.is_running(), "_staticx_cleanup is impossible while PyPerf is running"
+        if self._pyperf_staticx_tmpdir is not None and self._pyperf_staticx_tmpdir.exists():
+            shutil.rmtree(self._pyperf_staticx_tmpdir.as_posix())
+
     def test(self) -> None:
         self._ebpf_environment()
 
@@ -170,14 +183,16 @@ def test(self) -> None:
             "--duration",
             "1",
         ]
-        process = start_process(cmd)
+        process = start_process(cmd, tmpdir=self._pyperf_staticx_tmpdir)
         try:
             poll_process(process, self._POLL_TIMEOUT, self._profiler_state.stop_event)
         except TimeoutError:
             process.kill()
             raise
         else:
             self._check_output(process, self.output_path)
+        finally:
+            self._staticx_cleanup()
 
     def start(self) -> None:
         logger.info("Starting profiling of Python processes with PyPerf")
@@ -201,7 +216,7 @@ def start(self) -> None:
         for f in glob.glob(f"{str(self.output_path)}.*"):
             os.unlink(f)
 
-        process = start_process(cmd)
+        process = start_process(cmd, tmpdir=self._pyperf_staticx_tmpdir)
         # wait until the transient data file appears - because once returning from here, PyPerf may
         # be polled via snapshot() and we need it to finish installing its signal handler.
         try:
@@ -212,6 +227,7 @@ def start(self) -> None:
             stdout = process.stdout.read()
             stderr = process.stderr.read()
             logger.error("PyPerf failed to start", stdout=stdout, stderr=stderr)
+            self._staticx_cleanup()
             raise
         else:
             self.process = process
@@ -303,16 +319,23 @@ def snapshot(self) -> ProcessToProfileData:
         return profiles
 
     def _terminate(self) -> Tuple[Optional[int], str, str]:
+        exit_status: Optional[int] = None
+        stdout: Union[str, bytes] = ""
+        stderr: Union[str, bytes] = ""
+
         self._unregister_process_selectors()
         if self.is_running():
             assert self.process is not None  # for mypy
             self.process.terminate()  # okay to call even if process is already dead
             exit_status, stdout, stderr = reap_process(self.process)
             self.process = None
-            return exit_status, stdout.decode(), stderr.decode()
+
+        stdout = stdout.decode() if isinstance(stdout, bytes) else stdout
+        stderr = stderr.decode() if isinstance(stderr, bytes) else stderr
 
         assert self.process is None  # means we're not running
-        return None, "", ""
+        self._staticx_cleanup()
+        return exit_status, stdout, stderr
 
     def stop(self) -> None:
         exit_status, stdout, stderr = self._terminate()

gprofiler/utils/__init__.py

@@ -2,7 +2,6 @@
 # Copyright (C) 2022 Intel Corporation
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
-
 # you may not use this file except in compliance with the License.
 # You may obtain a copy of the License at
 #
@@ -99,7 +98,13 @@ def is_root() -> bool:
         return os.geteuid() == 0
 
 
-def start_process(cmd: Union[str, List[str]], via_staticx: bool = False, **kwargs: Any) -> Popen:
+def start_process(
+    cmd: Union[str, List[str]],
+    via_staticx: bool = False,
+    term_on_parent_death: bool = True,
+    tmpdir: Optional[Path] = None,
+    **kwargs: Any,
+) -> Popen:
     global _processes
 
     if isinstance(cmd, str):
@@ -122,9 +127,16 @@ def start_process(cmd: Union[str, List[str]], via_staticx: bool = False, **kwarg
             # see https://github.com/JonathonReinhart/staticx#run-time-information
             cmd = [f"{staticx_dir}/.staticx.interp", "--library-path", staticx_dir] + cmd
         else:
-            # explicitly remove our directory from LD_LIBRARY_PATH
             env = env if env is not None else os.environ.copy()
-            env.update({"LD_LIBRARY_PATH": ""})
+            if tmpdir is not None:
+                tmpdir.mkdir(exist_ok=True)
+                env["TMPDIR"] = tmpdir.as_posix()
+            elif "TMPDIR" not in env and "TMPDIR" in os.environ:
+                # ensure `TMPDIR` env is propagated to the child processes (used by staticx)
+                env["TMPDIR"] = os.environ["TMPDIR"]
+
+            # explicitly remove our directory from LD_LIBRARY_PATH
+            env["LD_LIBRARY_PATH"] = ""
 
     process = Popen(
         cmd,