From 4aad6223ab0afd95004dc71f3378864f2f526877 Mon Sep 17 00:00:00 2001 From: "Wisniewski, Krzysztof2" Date: Mon, 29 Aug 2022 15:59:26 +0200 Subject: [PATCH 1/9] Make NodeJS profiling attachable --- Dockerfile | 24 ++- README.md | 2 + gprofiler/main.py | 12 +- gprofiler/metadata/__init__.py | 21 ++ gprofiler/metadata/application_metadata.py | 22 +- gprofiler/profilers/perf.py | 8 +- gprofiler/utils/__init__.py | 8 + gprofiler/utils/node.py | 223 +++++++++++++++++++++ pyi.Dockerfile | 22 ++ requirements.txt | 2 + scripts/build_aarch64_container.sh | 5 +- scripts/build_aarch64_executable.sh | 5 +- scripts/build_node_package.sh | 35 ++++ scripts/build_x86_64_executable.sh | 4 +- tests/test_perf.py | 1 + tests/test_sanity.py | 3 +- 16 files changed, 368 insertions(+), 29 deletions(-) create mode 100644 gprofiler/utils/node.py create mode 100755 scripts/build_node_package.sh diff --git a/Dockerfile b/Dockerfile index d6d52826f..42fd1eec0 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,5 +1,4 @@ # these need to be defined before any FROM - otherwise, the ARGs expand to empty strings. - # pyspy & rbspy, using the same builder for both pyspy and rbspy since they share build dependencies - rust:latest 1.52.1 ARG RUST_BUILDER_VERSION=@sha256:9c106c1222abe1450f45774273f36246ebf257623ed51280dbc458632d14c9fc # pyperf - ubuntu 20.04 @@ -21,6 +20,10 @@ ARG AP_BUILDER_ALPINE=@sha256:69704ef328d05a9f806b6b8502915e6a0a4faa4d72018dc423 ARG BURN_BUILDER_GOLANG=@sha256:f7d3519759ba6988a2b73b5874b17c5958ac7d0aa48a8b1d84d66ef25fa345f1 # gprofiler - ubuntu 20.04 ARG GPROFILER_BUILDER_UBUNTU=@sha256:cf31af331f38d1d7158470e095b132acd126a7180a54f263d386da88eb681d93 +# node-package-builder-musl alpine +ARG NODE_PACKAGE_BUILDER_MUSL=@sha256:69704ef328d05a9f806b6b8502915e6a0a4faa4d72018dc42343f511490daf8a +# node-package-builder-glibc - ubuntu:20.04 +ARG NODE_PACKAGE_BUILDER_GLIBC=@sha256:cf31af331f38d1d7158470e095b132acd126a7180a54f263d386da88eb681d93 # pyspy & rbspy builder base FROM rust${RUST_BUILDER_VERSION} AS pyspy-rbspy-builder-common @@ -148,6 +151,23 @@ COPY scripts/async_profiler_build_shared.sh . COPY scripts/async_profiler_build_musl.sh . RUN ./async_profiler_build_shared.sh /tmp/async_profiler_build_musl.sh +# node-package-builder-musl +FROM alpine${NODE_PACKAGE_BUILDER_MUSL} AS node-package-builder-musl +WORKDIR /tmp +COPY scripts/build_node_package.sh . +RUN apk add --no-cache curl g++ python3 make gcc git bash nodejs npm +RUN ./build_node_package.sh + +# node-package-builder-glibc +FROM ubuntu${NODE_PACKAGE_BUILDER_GLIBC} AS node-package-builder-glibc +WORKDIR /tmp +COPY scripts/build_node_package.sh . +ENV DEBIAN_FRONTEND=noninteractive +RUN apt update -y && apt install -y curl g++ python3 make gcc git +RUN curl -fsSL https://deb.nodesource.com/setup_16.x | bash - +RUN apt install -y nodejs +RUN ./build_node_package.sh + # burn FROM golang${BURN_BUILDER_GOLANG} AS burn-builder WORKDIR /tmp @@ -191,6 +211,8 @@ COPY --from=async-profiler-builder-glibc /tmp/async-profiler/build/async-profile COPY --from=async-profiler-builder-glibc /tmp/async-profiler/build/libasyncProfiler.so gprofiler/resources/java/glibc/libasyncProfiler.so COPY --from=async-profiler-builder-musl /tmp/async-profiler/build/libasyncProfiler.so gprofiler/resources/java/musl/libasyncProfiler.so COPY --from=async-profiler-builder-glibc /tmp/async-profiler/build/fdtransfer gprofiler/resources/java/fdtransfer +COPY --from=node-package-builder-musl /tmp/module_build gprofiler/resources/node/module/musl +COPY --from=node-package-builder-glibc /tmp/module_build gprofiler/resources/node/module/glibc COPY --from=rbspy-builder /tmp/rbspy/rbspy gprofiler/resources/ruby/rbspy diff --git a/README.md b/README.md index 1e66b4c9d..d839443ac 100644 --- a/README.md +++ b/README.md @@ -75,6 +75,7 @@ Profiling using eBPF incurs lower overhead & provides kernel & native stacks. * `--nodejs-mode`: Controls which profiler is used for NodeJS. * `none` - (default) no profiler is used. * `perf` - augment the system profiler (`perf`) results with jitdump files generated by NodeJS. This requires running your `node` processes with `--perf-prof` (and for Node >= 10, with `--interpreted-frames-native-stack`). See this [NodeJS page](https://nodejs.org/en/docs/guides/diagnostics-flamegraph/) for more information. + * `attachable` - Generates perf map using [node-linux-perf module](https://github.com/mmarchini-oss/node-linux-perf). This module is injected at runtime. Requires entrypoint of application to be CommonJS script. (Doesn't work for ES modules) ### System profiling options @@ -367,6 +368,7 @@ Alongside `perf`, gProfiler invokes runtime-specific profilers for processes bas * Uses [Granulate's fork](https://github.com/Granulate/rbspy) of the [rbspy](https://github.com/rbspy/rbspy) profiler. * NodeJS (version >= 10 for functioning `--perf-prof`): * Uses `perf inject --jit` and NodeJS's ability to generate jitdump files. See [NodeJS profiling options](#nodejs-profiling-options). + * Can also generate perf maps at runtime. * .NET runtime * Uses dotnet-trace. diff --git a/gprofiler/main.py b/gprofiler/main.py index 1d2e40c16..4c7b8fb16 100644 --- a/gprofiler/main.py +++ b/gprofiler/main.py @@ -433,9 +433,10 @@ def parse_cmd_args() -> configargparse.Namespace: "--nodejs-mode", dest="nodejs_mode", default="disabled", - choices=["perf", "disabled", "none"], - help="Select the NodeJS profiling mode: perf (run 'perf inject --jit' on perf results, to augment them" - " with jitdump files of NodeJS processes, if present) or disabled (no runtime-specific profilers for NodeJS)", + choices=["attachable", "perf", "disabled", "none"], + help="Select the NodeJS profiling mode: attachable (generates perf-maps at runtime), perf (run 'perf inject --jit'" + " on perf results, to augment them with jitdump files of NodeJS processes, if present) or disabled" + " (no runtime-specific profilers for NodeJS)", ) nodejs_options.add_argument( @@ -601,6 +602,7 @@ def parse_cmd_args() -> configargparse.Namespace: args = parser.parse_args() args.perf_inject = args.nodejs_mode == "perf" + args.perf_node_attach = args.nodejs_mode == "attachable" if args.upload_results: if not args.server_token: @@ -617,8 +619,8 @@ def parse_cmd_args() -> configargparse.Namespace: if args.perf_mode in ("dwarf", "smart") and args.frequency > 100: parser.error("--profiling-frequency|-f can't be larger than 100 when using --perf-mode 'smart' or 'dwarf'") - if args.nodejs_mode == "perf" and args.perf_mode not in ("fp", "smart"): - parser.error("--nodejs-mode perf requires --perf-mode 'fp' or 'smart'") + if (args.nodejs_mode == "perf" or args.nodejs_mode == "attachable") and args.perf_mode not in ("fp", "smart"): + parser.error("--nodejs-mode perf or attachable requires --perf-mode 'fp' or 'smart'") return args diff --git a/gprofiler/metadata/__init__.py b/gprofiler/metadata/__init__.py index e69de29bb..32cda0712 100644 --- a/gprofiler/metadata/__init__.py +++ b/gprofiler/metadata/__init__.py @@ -0,0 +1,21 @@ +from granulate_utils.linux.ns import get_process_nspid, run_in_ns +from gprofiler.utils import run_process +from psutil import Process +from subprocess import CompletedProcess + +def get_exe_version(self, process: Process, version_arg: str = "--version", try_stderr: bool = False) -> str: + """ + Runs {process.exe()} --version in the appropriate namespace + """ + exe_path = f"/proc/{get_process_nspid(process.pid)}/exe" + + def _run_get_version() -> "CompletedProcess[bytes]": + return run_process([exe_path, version_arg], stop_event=self._stop_event, timeout=self._GET_VERSION_TIMEOUT) + + cp = run_in_ns(["pid", "mnt"], _run_get_version, process.pid) + stdout = cp.stdout.decode().strip() + # return stderr if stdout is empty, some apps print their version to stderr. + if try_stderr and not stdout: + return cp.stderr.decode().strip() + + return stdout \ No newline at end of file diff --git a/gprofiler/metadata/application_metadata.py b/gprofiler/metadata/application_metadata.py index 5d0567224..1e55b0bde 100644 --- a/gprofiler/metadata/application_metadata.py +++ b/gprofiler/metadata/application_metadata.py @@ -4,16 +4,15 @@ # import functools -from subprocess import CompletedProcess from threading import Event, Lock from typing import Any, Dict, Optional +from . import get_exe_version -from granulate_utils.linux.ns import get_process_nspid, run_in_ns from granulate_utils.linux.process import is_process_running, read_process_execfn from psutil import NoSuchProcess, Process from gprofiler.log import get_logger_adapter -from gprofiler.utils import run_process + logger = get_logger_adapter(__name__) @@ -37,22 +36,9 @@ def _clear_cache(self) -> None: if not is_process_running(process): del self._cache[process] - def get_exe_version(self, process: Process, version_arg: str = "--version", try_stderr: bool = False) -> str: - """ - Runs {process.exe()} --version in the appropriate namespace - """ - exe_path = f"/proc/{get_process_nspid(process.pid)}/exe" - - def _run_get_version() -> "CompletedProcess[bytes]": - return run_process([exe_path, version_arg], stop_event=self._stop_event, timeout=self._GET_VERSION_TIMEOUT) - cp = run_in_ns(["pid", "mnt"], _run_get_version, process.pid) - stdout = cp.stdout.decode().strip() - # return stderr if stdout is empty, some apps print their version to stderr. - if try_stderr and not stdout: - return cp.stderr.decode().strip() - - return stdout + def get_exe_version(self, process: Process, version_arg: str = "--version", try_stderr: bool = False) -> str: + return get_exe_version(process, version_arg, try_stderr) @functools.lru_cache(4096) def get_exe_version_cached(self, process: Process, version_arg: str = "--version", try_stderr: bool = False) -> str: diff --git a/gprofiler/profilers/perf.py b/gprofiler/profilers/perf.py index 667375e8f..c28bc4602 100644 --- a/gprofiler/profilers/perf.py +++ b/gprofiler/profilers/perf.py @@ -25,6 +25,7 @@ from gprofiler.utils import run_process, start_process, wait_event, wait_for_file_by_prefix from gprofiler.utils.perf import perf_path from gprofiler.utils.process import is_process_basename_matching +from gprofiler.utils.node import clean_up_node_maps, generate_map_for_node_processes logger = get_logger_adapter(__name__) @@ -173,6 +174,7 @@ def __init__( perf_mode: str, perf_dwarf_stack_size: int, perf_inject: bool, + perf_node_attach: bool, ): super().__init__(frequency, duration, stop_event, storage_dir) _ = profile_spawned_processes # Required for mypy unused argument warning @@ -204,14 +206,18 @@ def __init__( self._perfs.append(self._perf_dwarf) else: self._perf_dwarf = None - + self.perf_node_attach = perf_node_attach assert self._perf_fp is not None or self._perf_dwarf is not None def start(self) -> None: + if self.perf_node_attach: + generate_map_for_node_processes() for perf in self._perfs: perf.start() def stop(self) -> None: + if self.perf_node_attach: + clean_up_node_maps() for perf in reversed(self._perfs): perf.stop() diff --git a/gprofiler/utils/__init__.py b/gprofiler/utils/__init__.py index 14eac5560..b6765beda 100644 --- a/gprofiler/utils/__init__.py +++ b/gprofiler/utils/__init__.py @@ -457,3 +457,11 @@ def is_pyinstaller() -> bool: def get_staticx_dir() -> Optional[str]: return os.getenv("STATICX_BUNDLE_DIR") + +def add_permission_dir(path: str, permission_for_file: int, permission_for_dir: int): + os.chmod(path, os.stat(path).st_mode | permission_for_dir) + for subpath in glob.glob(path + "/*"): + if os.path.isdir(subpath): + add_permission_dir(subpath, permission_for_file, permission_for_dir) + else: + os.chmod(subpath, os.stat(subpath).st_mode | permission_for_file) diff --git a/gprofiler/utils/node.py b/gprofiler/utils/node.py new file mode 100644 index 000000000..9b727ce31 --- /dev/null +++ b/gprofiler/utils/node.py @@ -0,0 +1,223 @@ +from functools import lru_cache +import shutil +import traceback +from typing import Callable +from gprofiler.metadata import get_exe_version +from gprofiler.log import get_logger_adapter +from granulate_utils.linux.ns import get_proc_root_path, is_same_ns, NsType, run_in_ns +from granulate_utils.linux.process import is_musl + + +from . import add_permission_dir, pgrep_maps, resource_path, pgrep_exe + +import json +import requests +import os +import signal +import psutil +import stat + +from retry import retry +from websocket import create_connection +from websocket._core import WebSocket + +logger = get_logger_adapter(__name__) + + +class NodeDebuggerUrlNotFound(Exception): + pass + + +class NodeDebuggerUnexpectedResponse(Exception): + pass + + +def _start_debugger(pid): + os.kill(pid, signal.SIGUSR1) + + +def get_node_processes(): + return pgrep_maps(r"(?:^.+/node[^/]*$)") + + +@retry(NodeDebuggerUrlNotFound, 5, 1) +def _get_debugger_url() -> str: + # when killing process with SIGUSR1 it will open new debugger session on port 9229, + # so it will always the same + port = 9229 + debugger_url_response = requests.get(f"http://127.0.0.1:{port}/json/list") + if ( + debugger_url_response.status_code != 200 + or not "application/json" in debugger_url_response.headers.get("Content-Type") + ): + raise NodeDebuggerUrlNotFound + + response_json = debugger_url_response.json() + if ( + not isinstance(response_json, list) + or len(response_json) == 0 + or not isinstance(response_json[0], dict) + or not "webSocketDebuggerUrl" in response_json[0].keys() + ): + raise NodeDebuggerUrlNotFound + + return response_json[0]["webSocketDebuggerUrl"] + + +@retry(NodeDebuggerUnexpectedResponse, 5, 1) +def _load_dso(sock: WebSocket, module_path: str): + cdp_request = { + "id": 1, + "method": "Runtime.evaluate", + "params": { + "expression": f'process.mainModule.require("{os.path.join(module_path, "linux-perf.js")}").start()', + "replMode": True, + }, + } + sock.send(json.dumps(cdp_request)) + message = sock.recv() + try: + message = json.loads(message) + + except json.JSONDecodeError: + raise NodeDebuggerUnexpectedResponse(message) + + if ( + "result" not in message.keys() + or "result" not in message["result"].keys() + or "type" not in message["result"]["result"].keys() + or message["result"]["result"]["type"] != "boolean" + ): + raise NodeDebuggerUnexpectedResponse(message) + + +@retry(NodeDebuggerUnexpectedResponse, 5, 1) +def _stop_dso(sock: WebSocket, module_path: str): + cdp_request = { + "id": 1, + "method": "Runtime.evaluate", + "params": { + "expression": f'process.mainModule.require("{os.path.join(module_path, "linux-perf.js")}").stop()', + "replMode": True, + }, + } + sock.send(json.dumps(cdp_request)) + message = sock.recv() + try: + message = json.loads(message) + except json.JSONDecodeError: + raise NodeDebuggerUnexpectedResponse(message) + + if ( + "result" not in message.keys() + or "result" not in message["result"].keys() + or "type" not in message["result"]["result"].keys() + or message["result"]["result"]["type"] != "boolean" + ): + raise NodeDebuggerUnexpectedResponse(message) + + +def _copy_module_into_process_ns(process: psutil.Process, musl: bool, version: str): + proc_root = get_proc_root_path(process) + libc = "musl" if musl else "glibc" + dest_inside_container = os.path.join("tmp", "node_module", libc, version) + dest = os.path.join(proc_root, dest_inside_container) + if os.path.exists(dest): + return "/" + dest_inside_container + src = resource_path(os.path.join("node", "module", libc, version)) + shutil.copytree(src, dest) + add_permission_dir(dest, stat.S_IROTH, stat.S_IXOTH | stat.S_IROTH) + return "/" + dest_inside_container + + +def _create_symlink_for_map(process: psutil.Process, pid_inside_ns: int): + proc_root = get_proc_root_path(process) + map_file_name = f"perf-{process.pid}.map" + src = os.path.join(proc_root, "tmp", f"perf-{pid_inside_ns}.map") + dest = os.path.join("/tmp", map_file_name) + os.symlink(src, dest) + + +@lru_cache(1024) +def _get_pid_inside_ns(pid: int) -> int: + with open(f"/proc/{pid}/status") as f: + for line in f.readlines(): + if line.startswith("NSpid"): + return line.split("\t")[-1].strip("\n") + + +def _generate_perf_map_wrapper(module_path: str) -> Callable: + def generate_perf_map(): + debugger_url = _get_debugger_url() + sock = create_connection(debugger_url) + _load_dso(sock, module_path) + + return generate_perf_map + + +def _clean_up_wrapper(module_path: str, pid: int) -> Callable: + def clean_up(): + debugger_url = _get_debugger_url() + sock = create_connection(debugger_url) + _stop_dso(sock, module_path) + shutil.rmtree(os.path.join("/tmp", "node_module")) + os.remove(os.path.join("/tmp", f"perf-{pid}.map")) + + return clean_up + + +def generate_map_for_node_processes(): + """Iterates over all NodeJS processes, starts debugger for it, finds debugger URL, + copies node-linux-perf module into process' namespace, loads module and starts it. + After that it links it into gProfiler namespace's /tmp. This lets perf load it""" + processes = get_node_processes() + for process in processes: + try: + musl = is_musl(process) + node_version = get_exe_version(process) + node_major_version = node_version[1:].split(".")[0] + dest = _copy_module_into_process_ns(process, musl, node_major_version) + _start_debugger(process.pid) + pid_inside_ns = _get_pid_inside_ns(process.pid) + cp = run_in_ns( + ["pid", "mnt", "net"], _generate_perf_map_wrapper(dest), process.pid + ) + if hasattr(cp, "stdout"): + logger.debug(cp.stdout.decode().strip()) + if hasattr(cp, "stderr"): + logger.debug(cp.stderr.decode().strip()) + if not is_same_ns(process, NsType.mnt.name): + _create_symlink_for_map(process, pid_inside_ns) + except Exception as e: + logger.warn( + f"Could not create debug symbols for pid {process.pid}. Reason: {e}" + ) + logger.debug(traceback.format_exc()) + + +def clean_up_node_maps(): + """Stops generating perf maps for each NodeJS process and cleans up generated maps""" + processes = get_node_processes() + for process in processes: + try: + node_version = get_exe_version(process) + node_major_version = node_version[1:].split(".")[0] + pid_inside_ns = _get_pid_inside_ns(process.pid) + libc = "musl" if is_musl(process) else "glibc" + dest = os.path.join("/tmp", "node_module", libc, node_major_version) + cp = run_in_ns( + ["pid", "mnt", "net"], + _clean_up_wrapper(dest, pid_inside_ns), + process.pid, + ) + if hasattr(cp, "stdout"): + logger.debug(cp.stdout.decode().strip()) + if hasattr(cp, "stderr"): + logger.debug(cp.stderr.decode().strip()) + if not is_same_ns(process, NsType.mnt.name): + os.remove(os.path.join("/tmp", f"perf-{process.pid}.map")) + except Exception as e: + logger.warn( + f"Could not clean up debug symbols for pid {process.pid}. Reason: {e}" + ) + logger.debug(traceback.format_exc()) diff --git a/pyi.Dockerfile b/pyi.Dockerfile index 5f07c7f3b..829ae8d3e 100644 --- a/pyi.Dockerfile +++ b/pyi.Dockerfile @@ -12,6 +12,9 @@ ARG BURN_BUILDER_GOLANG ARG GPROFILER_BUILDER ARG PYPERF_BUILDER_UBUNTU ARG DOTNET_BUILDER +ARG DOTNET_BUILDER_UBUNTU +ARG NODE_PACKAGE_BUILDER_MUSL +ARG NODE_PACKAGE_BUILDER_GLIBC # pyspy & rbspy builder base FROM rust${RUST_BUILDER_VERSION} AS pyspy-rbspy-builder-common @@ -98,6 +101,23 @@ WORKDIR /tmp COPY scripts/burn_build.sh . RUN ./burn_build.sh +# node-package-builder-musl +FROM alpine${NODE_PACKAGE_BUILDER_MUSL} AS node-package-builder-musl +WORKDIR /tmp +RUN apk add --no-cache curl g++ python3 make gcc git bash nodejs npm +COPY scripts/build_node_package.sh . +RUN ./build_node_package.sh + +# node-package-builder-glibc +FROM ubuntu${NODE_PACKAGE_BUILDER_GLIBC} AS node-package-builder-glibc +WORKDIR /tmp +ENV DEBIAN_FRONTEND=noninteractive +RUN apt update -y && apt install -y curl g++ python3 make gcc git +RUN curl -fsSL https://deb.nodesource.com/setup_16.x | bash - +RUN apt install -y nodejs +COPY scripts/build_node_package.sh . +RUN ./build_node_package.sh + # bcc helpers # built on newer Ubuntu because they require new clang (newer than available in GPROFILER_BUILDER's CentOS 7) # these are only relevant for modern kernels, so there's no real reason to build them on CentOS 7 anyway. @@ -260,6 +280,8 @@ COPY --from=async-profiler-builder-glibc /tmp/async-profiler/build/async-profile COPY --from=async-profiler-centos-min-test-glibc /libasyncProfiler.so gprofiler/resources/java/glibc/libasyncProfiler.so COPY --from=async-profiler-builder-musl /tmp/async-profiler/build/libasyncProfiler.so gprofiler/resources/java/musl/libasyncProfiler.so COPY --from=async-profiler-builder-glibc /tmp/async-profiler/build/fdtransfer gprofiler/resources/java/fdtransfer +COPY --from=node-package-builder-musl /tmp/module_build gprofiler/resources/node/module/musl +COPY --from=node-package-builder-glibc /tmp/module_build gprofiler/resources/node/module/glibc COPY --from=burn-builder /tmp/burn/burn gprofiler/resources/burn diff --git a/requirements.txt b/requirements.txt index 32bbe0bb6..31344e950 100644 --- a/requirements.txt +++ b/requirements.txt @@ -9,4 +9,6 @@ dataclasses==0.8; python_version < '3.7' packaging==21.2 pyelftools==0.28 curlify==2.2.1 +retry==0.9.2 +websocket-client==1.3.1 ./granulate-utils/ diff --git a/scripts/build_aarch64_container.sh b/scripts/build_aarch64_container.sh index e5fb0173d..d203749df 100755 --- a/scripts/build_aarch64_container.sh +++ b/scripts/build_aarch64_container.sh @@ -28,4 +28,7 @@ docker buildx build --platform=linux/arm64 \ --build-arg BURN_BUILDER_GOLANG=$GOLANG_VERSION \ --build-arg GPROFILER_BUILDER_UBUNTU=$UBUNTU_VERSION \ --build-arg DOTNET_BUILDER=$DOTNET_BUILDER \ - . "$@" + --build-arg NODE_PACKAGE_BUILDER_MUSL=$ALPINE_VERSION \ + --build-arg NODE_PACKAGE_BUILDER_GLIBC=$UBUNTU_VERSION \ + . $@ + diff --git a/scripts/build_aarch64_executable.sh b/scripts/build_aarch64_executable.sh index 4ba43502c..56a5ad1dc 100755 --- a/scripts/build_aarch64_executable.sh +++ b/scripts/build_aarch64_executable.sh @@ -20,6 +20,7 @@ ALPINE_VERSION=@sha256:b06a5cf61b2956088722c4f1b9a6f71dfe95f0b1fe285d44195452b8a # mcr.microsoft.com/dotnet/sdk:6.0-focal DOTNET_BUILDER=@sha256:749439ff7a431ab4bc38d43cea453dff9ae1ed89a707c318b5082f9b2b25fa22 + mkdir -p build/aarch64 docker buildx build --platform=linux/arm64 \ --build-arg RUST_BUILDER_VERSION=$RUST_VERSION \ @@ -32,4 +33,6 @@ docker buildx build --platform=linux/arm64 \ --build-arg BURN_BUILDER_GOLANG=$GOLANG_VERSION \ --build-arg GPROFILER_BUILDER=$CENTOS8_VERSION \ --build-arg DOTNET_BUILDER=$DOTNET_BUILDER \ - . -f pyi.Dockerfile --output type=local,dest=build/aarch64/ "$@" + --build-arg NODE_PACKAGE_BUILDER_MUSL=$ALPINE_VERSION \ + --build-arg NODE_PACKAGE_BUILDER_GLIBC=$UBUNTU_VERSION \ + . -f pyi.Dockerfile --output type=local,dest=build/aarch64/ $@ diff --git a/scripts/build_node_package.sh b/scripts/build_node_package.sh new file mode 100755 index 000000000..60d3f12cc --- /dev/null +++ b/scripts/build_node_package.sh @@ -0,0 +1,35 @@ +#!/usr/bin/env bash + +set -euo pipefail + +MODULE_PATH=/tmp/module +BUILD_TARGET_DIR=/tmp/module_build + +git clone https://github.com/mmarchini-oss/node-linux-perf.git $MODULE_PATH +npm install -g node-gyp + +cd $MODULE_PATH +curl -L https://github.com/nodejs/nan/archive/refs/tags/v2.16.0.tar.gz -o nan.tar.gz +tar -vxzf nan.tar.gz -C /tmp +export NAN_PATH=$(realpath /tmp/nan-*) +sed -i 's/node \-e \\"require('\''nan'\'')\\"/echo $NAN_PATH/g' binding.gyp +rm -rf nan.tar.gz +mkdir $BUILD_TARGET_DIR +node_versions=( "10.10.0" "11.0.0" ) +for node_version in ${node_versions[@]}; do + node-gyp configure --target=$node_version --build_v8_with_gn=false + node-gyp build --target=$node_version + t=(${node_version//./ }) + node_major_version=${t[0]} + mkdir $BUILD_TARGET_DIR/$node_major_version + cp -r $MODULE_PATH/* $BUILD_TARGET_DIR/$node_major_version + rm -rf $MODULE_PATH/build +done +for node_major_version in {12..16}; do + node-gyp configure --target=$node_major_version.0.0 + node-gyp build --target=$node_major_version.0.0 + mkdir $BUILD_TARGET_DIR/$node_major_version + cp -r $MODULE_PATH/* $BUILD_TARGET_DIR/$node_major_version + rm -rf $MODULE_PATH/build +done +rm -rf $NAN_PATH \ No newline at end of file diff --git a/scripts/build_x86_64_executable.sh b/scripts/build_x86_64_executable.sh index c2d984e37..fb392c991 100755 --- a/scripts/build_x86_64_executable.sh +++ b/scripts/build_x86_64_executable.sh @@ -39,4 +39,6 @@ DOCKER_BUILDKIT=1 docker build -f pyi.Dockerfile --output type=local,dest=build/ --build-arg BURN_BUILDER_GOLANG=$BURN_BUILDER_GOLANG \ --build-arg GPROFILER_BUILDER=$GPROFILER_BUILDER \ --build-arg DOTNET_BUILDER=$DOTNET_BUILDER \ - . "$@" + --build-arg NODE_PACKAGE_BUILDER_MUSL=$AP_BUILDER_ALPINE \ + --build-arg NODE_PACKAGE_BUILDER_GLIBC=$UBUNTU_VERSION \ + . $@ diff --git a/tests/test_perf.py b/tests/test_perf.py index fe13a9dc0..6d0169e3e 100644 --- a/tests/test_perf.py +++ b/tests/test_perf.py @@ -25,6 +25,7 @@ def system_profiler(tmp_path: Path, perf_mode: str) -> SystemProfiler: perf_mode=perf_mode, perf_inject=False, perf_dwarf_stack_size=DEFAULT_PERF_DWARF_STACK_SIZE, + perf_node_attach=False, ) diff --git a/tests/test_sanity.py b/tests/test_sanity.py index 0a5abf98c..9a049ace9 100644 --- a/tests/test_sanity.py +++ b/tests/test_sanity.py @@ -116,7 +116,8 @@ def test_nodejs( gprofiler_docker_image: Image, ) -> None: with SystemProfiler( - 1000, 6, Event(), str(tmp_path), False, perf_mode="fp", perf_inject=True, perf_dwarf_stack_size=0 + 1000, 6, Event(), str(tmp_path), False, perf_mode="fp", perf_inject=True, perf_dwarf_stack_size=0, + perf_node_attach=False, ) as profiler: process_collapsed = snapshot_pid_collapsed(profiler, application_pid) assert_collapsed(process_collapsed) From c72ac23e2f358a29acda9b58de48d1f76653226e Mon Sep 17 00:00:00 2001 From: "Wisniewski, Krzysztof2" Date: Thu, 8 Sep 2022 17:17:37 +0200 Subject: [PATCH 2/9] Review changes --- Dockerfile | 10 +- README.md | 2 +- dev-requirements.txt | 1 + gprofiler/main.py | 14 +- gprofiler/metadata/__init__.py | 20 ++- gprofiler/metadata/application_metadata.py | 6 +- gprofiler/profilers/perf.py | 15 +- gprofiler/utils/__init__.py | 7 +- gprofiler/utils/node.py | 187 ++++++++------------ mypy.ini | 3 + pyi.Dockerfile | 10 +- scripts/build_aarch64_executable.sh | 1 - scripts/build_node_package.sh | 36 ++-- scripts/node_builder_glibc_env.sh | 12 ++ scripts/node_builder_musl_env.sh | 8 + tests/conftest.py | 8 +- tests/containers/nodejs-attach/Dockerfile | 10 ++ tests/containers/nodejs-attach/fibonacci.js | 11 ++ tests/test_sanity.py | 34 +++- 19 files changed, 225 insertions(+), 170 deletions(-) create mode 100755 scripts/node_builder_glibc_env.sh create mode 100755 scripts/node_builder_musl_env.sh create mode 100644 tests/containers/nodejs-attach/Dockerfile create mode 100644 tests/containers/nodejs-attach/fibonacci.js diff --git a/Dockerfile b/Dockerfile index 42fd1eec0..6e55dd2ec 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,5 @@ # these need to be defined before any FROM - otherwise, the ARGs expand to empty strings. + # pyspy & rbspy, using the same builder for both pyspy and rbspy since they share build dependencies - rust:latest 1.52.1 ARG RUST_BUILDER_VERSION=@sha256:9c106c1222abe1450f45774273f36246ebf257623ed51280dbc458632d14c9fc # pyperf - ubuntu 20.04 @@ -154,18 +155,17 @@ RUN ./async_profiler_build_shared.sh /tmp/async_profiler_build_musl.sh # node-package-builder-musl FROM alpine${NODE_PACKAGE_BUILDER_MUSL} AS node-package-builder-musl WORKDIR /tmp +COPY scripts/node_builder_musl_env.sh . +RUN ./node_builder_musl_env.sh COPY scripts/build_node_package.sh . -RUN apk add --no-cache curl g++ python3 make gcc git bash nodejs npm RUN ./build_node_package.sh # node-package-builder-glibc FROM ubuntu${NODE_PACKAGE_BUILDER_GLIBC} AS node-package-builder-glibc WORKDIR /tmp +COPY scripts/node_builder_glibc_env.sh . +RUN ./node_builder_glibc_env.sh COPY scripts/build_node_package.sh . -ENV DEBIAN_FRONTEND=noninteractive -RUN apt update -y && apt install -y curl g++ python3 make gcc git -RUN curl -fsSL https://deb.nodesource.com/setup_16.x | bash - -RUN apt install -y nodejs RUN ./build_node_package.sh # burn diff --git a/README.md b/README.md index d839443ac..fc476e3bb 100644 --- a/README.md +++ b/README.md @@ -75,7 +75,7 @@ Profiling using eBPF incurs lower overhead & provides kernel & native stacks. * `--nodejs-mode`: Controls which profiler is used for NodeJS. * `none` - (default) no profiler is used. * `perf` - augment the system profiler (`perf`) results with jitdump files generated by NodeJS. This requires running your `node` processes with `--perf-prof` (and for Node >= 10, with `--interpreted-frames-native-stack`). See this [NodeJS page](https://nodejs.org/en/docs/guides/diagnostics-flamegraph/) for more information. - * `attachable` - Generates perf map using [node-linux-perf module](https://github.com/mmarchini-oss/node-linux-perf). This module is injected at runtime. Requires entrypoint of application to be CommonJS script. (Doesn't work for ES modules) + * `attach-maps` - Generates perf map using [node-linux-perf module](https://github.com/mmarchini-oss/node-linux-perf). This module is injected at runtime. Requires entrypoint of application to be CommonJS script. (Doesn't work for ES modules) ### System profiling options diff --git a/dev-requirements.txt b/dev-requirements.txt index d169bb28b..9a2cc206f 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -10,3 +10,4 @@ types-PyYAML==6.0.3 types-pkg-resources==0.1.3 types-protobuf==3.19.22 types-toml==0.10.8 +types-retry==0.9.9 diff --git a/gprofiler/main.py b/gprofiler/main.py index 4c7b8fb16..01880e52d 100644 --- a/gprofiler/main.py +++ b/gprofiler/main.py @@ -433,10 +433,10 @@ def parse_cmd_args() -> configargparse.Namespace: "--nodejs-mode", dest="nodejs_mode", default="disabled", - choices=["attachable", "perf", "disabled", "none"], - help="Select the NodeJS profiling mode: attachable (generates perf-maps at runtime), perf (run 'perf inject --jit'" - " on perf results, to augment them with jitdump files of NodeJS processes, if present) or disabled" - " (no runtime-specific profilers for NodeJS)", + choices=["attach-maps", "perf", "disabled", "none"], + help="Select the NodeJS profiling mode: attach-maps (generates perf-maps at runtime)," + " perf (run 'perf inject --jit' on perf results, to augment them with jitdump files" + " of NodeJS processes, if present) or disabled (no runtime-specific profilers for NodeJS)", ) nodejs_options.add_argument( @@ -602,7 +602,7 @@ def parse_cmd_args() -> configargparse.Namespace: args = parser.parse_args() args.perf_inject = args.nodejs_mode == "perf" - args.perf_node_attach = args.nodejs_mode == "attachable" + args.perf_node_attach = args.nodejs_mode == "attach-maps" if args.upload_results: if not args.server_token: @@ -619,8 +619,8 @@ def parse_cmd_args() -> configargparse.Namespace: if args.perf_mode in ("dwarf", "smart") and args.frequency > 100: parser.error("--profiling-frequency|-f can't be larger than 100 when using --perf-mode 'smart' or 'dwarf'") - if (args.nodejs_mode == "perf" or args.nodejs_mode == "attachable") and args.perf_mode not in ("fp", "smart"): - parser.error("--nodejs-mode perf or attachable requires --perf-mode 'fp' or 'smart'") + if args.nodejs_mode in ("perf", "attach-maps") and args.perf_mode not in ("fp", "smart"): + parser.error("--nodejs-mode perf or attach-maps requires --perf-mode 'fp' or 'smart'") return args diff --git a/gprofiler/metadata/__init__.py b/gprofiler/metadata/__init__.py index 32cda0712..65d2b5bff 100644 --- a/gprofiler/metadata/__init__.py +++ b/gprofiler/metadata/__init__.py @@ -1,16 +1,26 @@ +from subprocess import CompletedProcess +from threading import Event + from granulate_utils.linux.ns import get_process_nspid, run_in_ns -from gprofiler.utils import run_process from psutil import Process -from subprocess import CompletedProcess -def get_exe_version(self, process: Process, version_arg: str = "--version", try_stderr: bool = False) -> str: +from gprofiler.utils import run_process + + +def get_exe_version( + process: Process, + stop_event: Event = None, + get_version_timeout: int = None, + version_arg: str = "--version", + try_stderr: bool = False, +) -> str: """ Runs {process.exe()} --version in the appropriate namespace """ exe_path = f"/proc/{get_process_nspid(process.pid)}/exe" def _run_get_version() -> "CompletedProcess[bytes]": - return run_process([exe_path, version_arg], stop_event=self._stop_event, timeout=self._GET_VERSION_TIMEOUT) + return run_process([exe_path, version_arg], stop_event=stop_event, timeout=get_version_timeout) cp = run_in_ns(["pid", "mnt"], _run_get_version, process.pid) stdout = cp.stdout.decode().strip() @@ -18,4 +28,4 @@ def _run_get_version() -> "CompletedProcess[bytes]": if try_stderr and not stdout: return cp.stderr.decode().strip() - return stdout \ No newline at end of file + return stdout diff --git a/gprofiler/metadata/application_metadata.py b/gprofiler/metadata/application_metadata.py index 1e55b0bde..af70179e8 100644 --- a/gprofiler/metadata/application_metadata.py +++ b/gprofiler/metadata/application_metadata.py @@ -6,13 +6,12 @@ import functools from threading import Event, Lock from typing import Any, Dict, Optional -from . import get_exe_version from granulate_utils.linux.process import is_process_running, read_process_execfn from psutil import NoSuchProcess, Process from gprofiler.log import get_logger_adapter - +from gprofiler.metadata import get_exe_version logger = get_logger_adapter(__name__) @@ -36,9 +35,8 @@ def _clear_cache(self) -> None: if not is_process_running(process): del self._cache[process] - def get_exe_version(self, process: Process, version_arg: str = "--version", try_stderr: bool = False) -> str: - return get_exe_version(process, version_arg, try_stderr) + return get_exe_version(process, self._stop_event, self._GET_VERSION_TIMEOUT, version_arg, try_stderr) @functools.lru_cache(4096) def get_exe_version_cached(self, process: Process, version_arg: str = "--version", try_stderr: bool = False) -> str: diff --git a/gprofiler/profilers/perf.py b/gprofiler/profilers/perf.py index c28bc4602..4cc7e0765 100644 --- a/gprofiler/profilers/perf.py +++ b/gprofiler/profilers/perf.py @@ -23,9 +23,9 @@ from gprofiler.profilers.profiler_base import ProfilerBase from gprofiler.profilers.registry import ProfilerArgument, register_profiler from gprofiler.utils import run_process, start_process, wait_event, wait_for_file_by_prefix +from gprofiler.utils.node import clean_up_node_maps, generate_map_for_node_processes, get_node_processes from gprofiler.utils.perf import perf_path from gprofiler.utils.process import is_process_basename_matching -from gprofiler.utils.node import clean_up_node_maps, generate_map_for_node_processes logger = get_logger_adapter(__name__) @@ -180,7 +180,7 @@ def __init__( _ = profile_spawned_processes # Required for mypy unused argument warning self._perfs: List[PerfProcess] = [] self._metadata_collectors: List[PerfMetadata] = [GolangPerfMetadata(stop_event), NodePerfMetadata(stop_event)] - + self._node_processes: List[Process] = [] if perf_mode in ("fp", "smart"): self._perf_fp: Optional[PerfProcess] = PerfProcess( self._frequency, @@ -206,18 +206,20 @@ def __init__( self._perfs.append(self._perf_dwarf) else: self._perf_dwarf = None + self.perf_node_attach = perf_node_attach assert self._perf_fp is not None or self._perf_dwarf is not None def start(self) -> None: if self.perf_node_attach: - generate_map_for_node_processes() + self._node_processes = get_node_processes() + generate_map_for_node_processes(self._node_processes) for perf in self._perfs: perf.start() def stop(self) -> None: if self.perf_node_attach: - clean_up_node_maps() + clean_up_node_maps(self._node_processes) for perf in reversed(self._perfs): perf.stop() @@ -235,6 +237,11 @@ def _get_metadata(self, pid: int) -> Optional[AppMetadata]: return None def snapshot(self) -> ProcessToProfileData: + if self.perf_node_attach: + new_processes = [process for process in get_node_processes() if process not in self._node_processes] + generate_map_for_node_processes(new_processes) + self._node_processes.extend(new_processes) + if self._stop_event.wait(self._duration): raise StopEventSetException diff --git a/gprofiler/utils/__init__.py b/gprofiler/utils/__init__.py index b6765beda..d43a112a5 100644 --- a/gprofiler/utils/__init__.py +++ b/gprofiler/utils/__init__.py @@ -26,7 +26,7 @@ import importlib_resources import psutil -from granulate_utils.exceptions import CouldNotAcquireMutex +from granulate_utils.exceptions import CouldNotAcquireMutex, MissingExePath from granulate_utils.linux.mutex import try_acquire_mutex from granulate_utils.linux.ns import run_in_ns from granulate_utils.linux.process import process_exe @@ -273,7 +273,7 @@ def pgrep_exe(match: str) -> List[Process]: try: if pattern.match(process_exe(process)): procs.append(process) - except psutil.NoSuchProcess: # process might have died meanwhile + except (psutil.NoSuchProcess, MissingExePath): # process might have died meanwhile continue return procs @@ -458,7 +458,8 @@ def is_pyinstaller() -> bool: def get_staticx_dir() -> Optional[str]: return os.getenv("STATICX_BUNDLE_DIR") -def add_permission_dir(path: str, permission_for_file: int, permission_for_dir: int): + +def add_permission_dir(path: str, permission_for_file: int, permission_for_dir: int) -> None: os.chmod(path, os.stat(path).st_mode | permission_for_dir) for subpath in glob.glob(path + "/*"): if os.path.isdir(subpath): diff --git a/gprofiler/utils/node.py b/gprofiler/utils/node.py index 9b727ce31..0b5dc7ee7 100644 --- a/gprofiler/utils/node.py +++ b/gprofiler/utils/node.py @@ -1,28 +1,26 @@ -from functools import lru_cache -import shutil -import traceback -from typing import Callable -from gprofiler.metadata import get_exe_version -from gprofiler.log import get_logger_adapter -from granulate_utils.linux.ns import get_proc_root_path, is_same_ns, NsType, run_in_ns -from granulate_utils.linux.process import is_musl - - -from . import add_permission_dir, pgrep_maps, resource_path, pgrep_exe - import json -import requests import os +import shutil import signal -import psutil import stat +from typing import Dict, List +import psutil +import requests +from granulate_utils.linux.ns import get_proc_root_path, get_process_nspid, resolve_proc_root_links, run_in_ns +from granulate_utils.linux.process import is_musl from retry import retry from websocket import create_connection from websocket._core import WebSocket +from gprofiler.log import get_logger_adapter +from gprofiler.metadata import get_exe_version +from gprofiler.utils import add_permission_dir, pgrep_exe, resource_path + logger = get_logger_adapter(__name__) +DSO_GIT_REVISION = "20eb88a" + class NodeDebuggerUrlNotFound(Exception): pass @@ -32,12 +30,14 @@ class NodeDebuggerUnexpectedResponse(Exception): pass -def _start_debugger(pid): - os.kill(pid, signal.SIGUSR1) +def get_dest_inside_container(musl: bool, node_version: str) -> str: + libc = "musl" if musl else "glibc" + return os.path.join("/", "tmp", "node_module", DSO_GIT_REVISION, libc, node_version) -def get_node_processes(): - return pgrep_maps(r"(?:^.+/node[^/]*$)") +def _start_debugger(pid: int) -> None: + # for windows: in shell node -e "process._debugProcess(PID)" + os.kill(pid, signal.SIGUSR1) @retry(NodeDebuggerUrlNotFound, 5, 1) @@ -48,37 +48,32 @@ def _get_debugger_url() -> str: debugger_url_response = requests.get(f"http://127.0.0.1:{port}/json/list") if ( debugger_url_response.status_code != 200 - or not "application/json" in debugger_url_response.headers.get("Content-Type") + or not debugger_url_response.headers.get("Content-Type") + or "application/json" not in debugger_url_response.headers.get("Content-Type") # type: ignore ): - raise NodeDebuggerUrlNotFound + raise NodeDebuggerUrlNotFound( + {"status_code": debugger_url_response.status_code, "text": debugger_url_response.text} + ) response_json = debugger_url_response.json() if ( not isinstance(response_json, list) or len(response_json) == 0 or not isinstance(response_json[0], dict) - or not "webSocketDebuggerUrl" in response_json[0].keys() + or "webSocketDebuggerUrl" not in response_json[0] ): - raise NodeDebuggerUrlNotFound + raise NodeDebuggerUrlNotFound(response_json) - return response_json[0]["webSocketDebuggerUrl"] + return response_json[0]["webSocketDebuggerUrl"] # type: ignore @retry(NodeDebuggerUnexpectedResponse, 5, 1) -def _load_dso(sock: WebSocket, module_path: str): - cdp_request = { - "id": 1, - "method": "Runtime.evaluate", - "params": { - "expression": f'process.mainModule.require("{os.path.join(module_path, "linux-perf.js")}").start()', - "replMode": True, - }, - } +def _send_socket_request(sock: WebSocket, cdp_request: Dict) -> None: + sock.settimeout(2) sock.send(json.dumps(cdp_request)) message = sock.recv() try: message = json.loads(message) - except json.JSONDecodeError: raise NodeDebuggerUnexpectedResponse(message) @@ -91,86 +86,66 @@ def _load_dso(sock: WebSocket, module_path: str): raise NodeDebuggerUnexpectedResponse(message) -@retry(NodeDebuggerUnexpectedResponse, 5, 1) -def _stop_dso(sock: WebSocket, module_path: str): +def _load_dso(sock: WebSocket, module_path: str) -> None: cdp_request = { "id": 1, "method": "Runtime.evaluate", "params": { - "expression": f'process.mainModule.require("{os.path.join(module_path, "linux-perf.js")}").stop()', + "expression": f'process.mainModule.require("{os.path.join(module_path, "linux-perf.js")}").start()', "replMode": True, }, } - sock.send(json.dumps(cdp_request)) - message = sock.recv() - try: - message = json.loads(message) - except json.JSONDecodeError: - raise NodeDebuggerUnexpectedResponse(message) + _send_socket_request(sock, cdp_request) - if ( - "result" not in message.keys() - or "result" not in message["result"].keys() - or "type" not in message["result"]["result"].keys() - or message["result"]["result"]["type"] != "boolean" - ): - raise NodeDebuggerUnexpectedResponse(message) +def _stop_dso(sock: WebSocket, module_path: str) -> None: + cdp_request = { + "id": 1, + "method": "Runtime.evaluate", + "params": { + "expression": f'process.mainModule.require("{os.path.join(module_path, "linux-perf.js")}").stop()', + "replMode": True, + }, + } + _send_socket_request(sock, cdp_request) -def _copy_module_into_process_ns(process: psutil.Process, musl: bool, version: str): + +def _copy_module_into_process_ns(process: psutil.Process, musl: bool, version: str) -> str: proc_root = get_proc_root_path(process) libc = "musl" if musl else "glibc" - dest_inside_container = os.path.join("tmp", "node_module", libc, version) - dest = os.path.join(proc_root, dest_inside_container) + dest_inside_container = get_dest_inside_container(musl, version) + dest = resolve_proc_root_links(proc_root, dest_inside_container) if os.path.exists(dest): - return "/" + dest_inside_container + return dest_inside_container src = resource_path(os.path.join("node", "module", libc, version)) shutil.copytree(src, dest) add_permission_dir(dest, stat.S_IROTH, stat.S_IXOTH | stat.S_IROTH) - return "/" + dest_inside_container + return dest_inside_container -def _create_symlink_for_map(process: psutil.Process, pid_inside_ns: int): - proc_root = get_proc_root_path(process) - map_file_name = f"perf-{process.pid}.map" - src = os.path.join(proc_root, "tmp", f"perf-{pid_inside_ns}.map") - dest = os.path.join("/tmp", map_file_name) - os.symlink(src, dest) +def _generate_perf_map(module_path: str) -> None: + debugger_url = _get_debugger_url() + sock = create_connection(debugger_url) + sock.settimeout(2) + _load_dso(sock, module_path) -@lru_cache(1024) -def _get_pid_inside_ns(pid: int) -> int: - with open(f"/proc/{pid}/status") as f: - for line in f.readlines(): - if line.startswith("NSpid"): - return line.split("\t")[-1].strip("\n") +def _clean_up(module_path: str, pid: int) -> None: + debugger_url = _get_debugger_url() + sock = create_connection(debugger_url) + _stop_dso(sock, module_path) + sock.settimeout(2) + os.remove(os.path.join("/tmp", f"perf-{pid}.map")) -def _generate_perf_map_wrapper(module_path: str) -> Callable: - def generate_perf_map(): - debugger_url = _get_debugger_url() - sock = create_connection(debugger_url) - _load_dso(sock, module_path) +def get_node_processes() -> List[psutil.Process]: + return pgrep_exe(r"(?:^.+/node[^/]*$)") - return generate_perf_map - -def _clean_up_wrapper(module_path: str, pid: int) -> Callable: - def clean_up(): - debugger_url = _get_debugger_url() - sock = create_connection(debugger_url) - _stop_dso(sock, module_path) - shutil.rmtree(os.path.join("/tmp", "node_module")) - os.remove(os.path.join("/tmp", f"perf-{pid}.map")) - - return clean_up - - -def generate_map_for_node_processes(): +def generate_map_for_node_processes(processes: List[psutil.Process]) -> None: """Iterates over all NodeJS processes, starts debugger for it, finds debugger URL, copies node-linux-perf module into process' namespace, loads module and starts it. After that it links it into gProfiler namespace's /tmp. This lets perf load it""" - processes = get_node_processes() for process in processes: try: musl = is_musl(process) @@ -178,46 +153,24 @@ def generate_map_for_node_processes(): node_major_version = node_version[1:].split(".")[0] dest = _copy_module_into_process_ns(process, musl, node_major_version) _start_debugger(process.pid) - pid_inside_ns = _get_pid_inside_ns(process.pid) - cp = run_in_ns( - ["pid", "mnt", "net"], _generate_perf_map_wrapper(dest), process.pid - ) - if hasattr(cp, "stdout"): - logger.debug(cp.stdout.decode().strip()) - if hasattr(cp, "stderr"): - logger.debug(cp.stderr.decode().strip()) - if not is_same_ns(process, NsType.mnt.name): - _create_symlink_for_map(process, pid_inside_ns) + run_in_ns(["pid", "mnt", "net"], lambda: _generate_perf_map(dest), process.pid, passthrough_exception=True) except Exception as e: - logger.warn( - f"Could not create debug symbols for pid {process.pid}. Reason: {e}" - ) - logger.debug(traceback.format_exc()) + logger.warning(f"Could not create debug symbols for pid {process.pid}. Reason: {e}", exc_info=True) -def clean_up_node_maps(): +def clean_up_node_maps(processes: List[psutil.Process]) -> None: """Stops generating perf maps for each NodeJS process and cleans up generated maps""" - processes = get_node_processes() for process in processes: try: node_version = get_exe_version(process) node_major_version = node_version[1:].split(".")[0] - pid_inside_ns = _get_pid_inside_ns(process.pid) - libc = "musl" if is_musl(process) else "glibc" - dest = os.path.join("/tmp", "node_module", libc, node_major_version) - cp = run_in_ns( + pid_inside_ns = get_process_nspid(process.pid) + dest = get_dest_inside_container(is_musl(process), node_major_version) + run_in_ns( ["pid", "mnt", "net"], - _clean_up_wrapper(dest, pid_inside_ns), + lambda: _clean_up(dest, pid_inside_ns), process.pid, + passthrough_exception=True, ) - if hasattr(cp, "stdout"): - logger.debug(cp.stdout.decode().strip()) - if hasattr(cp, "stderr"): - logger.debug(cp.stderr.decode().strip()) - if not is_same_ns(process, NsType.mnt.name): - os.remove(os.path.join("/tmp", f"perf-{process.pid}.map")) except Exception as e: - logger.warn( - f"Could not clean up debug symbols for pid {process.pid}. Reason: {e}" - ) - logger.debug(traceback.format_exc()) + logger.warning(f"Could not clean up debug symbols for pid {process.pid}. Reason: {e}", exc_info=True) diff --git a/mypy.ini b/mypy.ini index b04acc747..8e983c177 100644 --- a/mypy.ini +++ b/mypy.ini @@ -19,3 +19,6 @@ ignore_missing_imports = True # no types in package / types- package :( [mypy-docker.*] ignore_missing_imports = True +# no types in package / types- package :( +[mypy-websocket.*] +ignore_missing_imports = True diff --git a/pyi.Dockerfile b/pyi.Dockerfile index 829ae8d3e..3c47023e4 100644 --- a/pyi.Dockerfile +++ b/pyi.Dockerfile @@ -12,7 +12,6 @@ ARG BURN_BUILDER_GOLANG ARG GPROFILER_BUILDER ARG PYPERF_BUILDER_UBUNTU ARG DOTNET_BUILDER -ARG DOTNET_BUILDER_UBUNTU ARG NODE_PACKAGE_BUILDER_MUSL ARG NODE_PACKAGE_BUILDER_GLIBC @@ -104,17 +103,16 @@ RUN ./burn_build.sh # node-package-builder-musl FROM alpine${NODE_PACKAGE_BUILDER_MUSL} AS node-package-builder-musl WORKDIR /tmp -RUN apk add --no-cache curl g++ python3 make gcc git bash nodejs npm +COPY scripts/node_builder_musl_env.sh . +RUN ./node_builder_musl_env.sh COPY scripts/build_node_package.sh . RUN ./build_node_package.sh # node-package-builder-glibc FROM ubuntu${NODE_PACKAGE_BUILDER_GLIBC} AS node-package-builder-glibc WORKDIR /tmp -ENV DEBIAN_FRONTEND=noninteractive -RUN apt update -y && apt install -y curl g++ python3 make gcc git -RUN curl -fsSL https://deb.nodesource.com/setup_16.x | bash - -RUN apt install -y nodejs +COPY scripts/node_builder_glibc_env.sh . +RUN ./node_builder_glibc_env.sh COPY scripts/build_node_package.sh . RUN ./build_node_package.sh diff --git a/scripts/build_aarch64_executable.sh b/scripts/build_aarch64_executable.sh index 56a5ad1dc..f3f47ddfb 100755 --- a/scripts/build_aarch64_executable.sh +++ b/scripts/build_aarch64_executable.sh @@ -20,7 +20,6 @@ ALPINE_VERSION=@sha256:b06a5cf61b2956088722c4f1b9a6f71dfe95f0b1fe285d44195452b8a # mcr.microsoft.com/dotnet/sdk:6.0-focal DOTNET_BUILDER=@sha256:749439ff7a431ab4bc38d43cea453dff9ae1ed89a707c318b5082f9b2b25fa22 - mkdir -p build/aarch64 docker buildx build --platform=linux/arm64 \ --build-arg RUST_BUILDER_VERSION=$RUST_VERSION \ diff --git a/scripts/build_node_package.sh b/scripts/build_node_package.sh index 60d3f12cc..804bcb37d 100755 --- a/scripts/build_node_package.sh +++ b/scripts/build_node_package.sh @@ -6,30 +6,36 @@ MODULE_PATH=/tmp/module BUILD_TARGET_DIR=/tmp/module_build git clone https://github.com/mmarchini-oss/node-linux-perf.git $MODULE_PATH -npm install -g node-gyp - cd $MODULE_PATH +git reset --hard 20eb88a35ab256313dfb0f14645456ebf046ac1b + +npm install -g node-gyp curl -L https://github.com/nodejs/nan/archive/refs/tags/v2.16.0.tar.gz -o nan.tar.gz tar -vxzf nan.tar.gz -C /tmp -export NAN_PATH=$(realpath /tmp/nan-*) +NAN_PATH=$(realpath /tmp/nan-*) +export NAN_PATH sed -i 's/node \-e \\"require('\''nan'\'')\\"/echo $NAN_PATH/g' binding.gyp rm -rf nan.tar.gz mkdir $BUILD_TARGET_DIR node_versions=( "10.10.0" "11.0.0" ) -for node_version in ${node_versions[@]}; do - node-gyp configure --target=$node_version --build_v8_with_gn=false - node-gyp build --target=$node_version +for node_version in "${node_versions[@]}"; do + node-gyp configure --target="$node_version" --build_v8_with_gn=false + node-gyp build --target="$node_version" t=(${node_version//./ }) node_major_version=${t[0]} - mkdir $BUILD_TARGET_DIR/$node_major_version - cp -r $MODULE_PATH/* $BUILD_TARGET_DIR/$node_major_version - rm -rf $MODULE_PATH/build + mkdir "$BUILD_TARGET_DIR/$node_major_version" + cp "$MODULE_PATH/linux-perf.js" "$BUILD_TARGET_DIR/$node_major_version/." + mkdir -p "$BUILD_TARGET_DIR/$node_major_version/build/Release" + cp "$MODULE_PATH/build/Release/linux-perf.node" "$BUILD_TARGET_DIR/$node_major_version/build/Release/linux-perf.node" + rm -rf "$MODULE_PATH/build" done for node_major_version in {12..16}; do - node-gyp configure --target=$node_major_version.0.0 - node-gyp build --target=$node_major_version.0.0 - mkdir $BUILD_TARGET_DIR/$node_major_version - cp -r $MODULE_PATH/* $BUILD_TARGET_DIR/$node_major_version - rm -rf $MODULE_PATH/build + node-gyp configure --target="$node_major_version.0.0" + node-gyp build --target="$node_major_version.0.0" + mkdir "$BUILD_TARGET_DIR/$node_major_version" + cp "$MODULE_PATH/linux-perf.js" "$BUILD_TARGET_DIR/$node_major_version/." + mkdir -p "$BUILD_TARGET_DIR/$node_major_version/build/Release" + cp "$MODULE_PATH/build/Release/linux-perf.node" "$BUILD_TARGET_DIR/$node_major_version/build/Release/linux-perf.node" + rm -rf "$MODULE_PATH/build" done -rm -rf $NAN_PATH \ No newline at end of file +rm -rf "$NAN_PATH" \ No newline at end of file diff --git a/scripts/node_builder_glibc_env.sh b/scripts/node_builder_glibc_env.sh new file mode 100755 index 000000000..e35ab763e --- /dev/null +++ b/scripts/node_builder_glibc_env.sh @@ -0,0 +1,12 @@ +#!/usr/bin/env bash +# +# Copyright (c) Granulate. All rights reserved. +# Licensed under the AGPL3 License. See LICENSE.md in the project root for license information. +# +set -eu + +export DEBIAN_FRONTEND=noninteractive +apt update -y && apt install -y --no-install-recommends curl g++ python3 make gcc git ca-certificates npm +# apt has node v10 by default, so we need to add newer version to run node-gyp +curl -fsSL https://deb.nodesource.com/setup_16.x | bash - +apt install -y --no-install-recommends nodejs diff --git a/scripts/node_builder_musl_env.sh b/scripts/node_builder_musl_env.sh new file mode 100755 index 000000000..b23eb6832 --- /dev/null +++ b/scripts/node_builder_musl_env.sh @@ -0,0 +1,8 @@ +#!/usr/bin/env sh +# +# Copyright (c) Granulate. All rights reserved. +# Licensed under the AGPL3 License. See LICENSE.md in the project root for license information. +# +set -eu + +apk add --no-cache curl g++ python3 make gcc git bash nodejs npm \ No newline at end of file diff --git a/tests/conftest.py b/tests/conftest.py index ca642647f..586f2c76f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -124,6 +124,10 @@ def command_line(runtime: str, java_command_line: List[str], dotnet_command_line "--interpreted-frames-native-stack", str(CONTAINERS_DIRECTORY / "nodejs/fibonacci.js"), ], + "nodejs-attach": [ + "node", + str(CONTAINERS_DIRECTORY / "nodejs/fibonacci.js"), + ], }[runtime] @@ -131,7 +135,7 @@ def command_line(runtime: str, java_command_line: List[str], dotnet_command_line def application_executable(runtime: str) -> str: if runtime == "golang": return "fibonacci" - elif runtime == "nodejs": + elif runtime in ("nodejs", "nodejs-attach"): return "node" return runtime @@ -405,6 +409,7 @@ def runtime_specific_args(runtime: str) -> List[str]: ], "python": ["-d", "3"], # Burner python tests make syscalls and we want to record python + kernel stacks "nodejs": ["--nodejs-mode", "perf"], # enable NodeJS profiling + "nodejs-attach": ["--nodejs-mode", "attach-maps"], }.get(runtime, []) @@ -421,6 +426,7 @@ def assert_collapsed(runtime: str) -> AssertInCollapsed: "nodejs": "fibonacci", "golang": "fibonacci", "dotnet": "Fibonacci", + "nodejs-attach": "fibonacci", }[runtime] return partial(assert_function_in_collapsed, function_name) diff --git a/tests/containers/nodejs-attach/Dockerfile b/tests/containers/nodejs-attach/Dockerfile new file mode 100644 index 000000000..2fc1aab49 --- /dev/null +++ b/tests/containers/nodejs-attach/Dockerfile @@ -0,0 +1,10 @@ +# node:10.24.1 +FROM node@sha256:59531d2835edd5161c8f9512f9e095b1836f7a1fcb0ab73e005ec46047384911 + +# /tmp so node has permissions to write its jitdump file +WORKDIR /tmp + +RUN mkdir /app +ADD fibonacci.js /app + +CMD ["node", "/app/fibonacci.js"] diff --git a/tests/containers/nodejs-attach/fibonacci.js b/tests/containers/nodejs-attach/fibonacci.js new file mode 100644 index 000000000..f69313c41 --- /dev/null +++ b/tests/containers/nodejs-attach/fibonacci.js @@ -0,0 +1,11 @@ +// +// Copyright (c) Granulate. All rights reserved. +// Licensed under the AGPL3 License. See LICENSE.md in the project root for license information. +// +function fibonacci(n) { + return n <= 1 ? n : fibonacci(n - 1) + fibonacci(n - 2); +} + +while (true) { + fibonacci(30); +} diff --git a/tests/test_sanity.py b/tests/test_sanity.py index 9a049ace9..ed68e82ae 100644 --- a/tests/test_sanity.py +++ b/tests/test_sanity.py @@ -116,13 +116,45 @@ def test_nodejs( gprofiler_docker_image: Image, ) -> None: with SystemProfiler( - 1000, 6, Event(), str(tmp_path), False, perf_mode="fp", perf_inject=True, perf_dwarf_stack_size=0, + 1000, + 6, + Event(), + str(tmp_path), + False, + perf_mode="fp", + perf_inject=True, + perf_dwarf_stack_size=0, perf_node_attach=False, ) as profiler: process_collapsed = snapshot_pid_collapsed(profiler, application_pid) assert_collapsed(process_collapsed) +@pytest.mark.parametrize("runtime", ["nodejs-attach"]) +def test_nodejs_attach_maps( + tmp_path: Path, + application_pid: int, + assert_collapsed: AssertInCollapsed, +) -> None: + with SystemProfiler( + 1000, + 6, + Event(), + str(tmp_path), + False, + perf_mode="fp", + perf_inject=False, + perf_dwarf_stack_size=0, + perf_node_attach=True, + ) as profiler: + process_collapsed = snapshot_pid_collapsed(profiler, application_pid) + assert_collapsed(process_collapsed) + # check for node built-in functions + assert_function_in_collapsed("node::Start", process_collapsed) + # check for v8 built-in functions + assert_function_in_collapsed("v8::Function::Call", process_collapsed) + + @pytest.mark.parametrize("runtime", ["python"]) def test_python_ebpf( tmp_path: Path, From d5f15f7c3606a4466b9f2ddf4ca0ab586da8edb8 Mon Sep 17 00:00:00 2001 From: "Wisniewski, Krzysztof2" Date: Wed, 28 Sep 2022 16:29:56 +0200 Subject: [PATCH 3/9] Reveiw changes v2 --- gprofiler/metadata/__init__.py | 31 ----- gprofiler/metadata/application_metadata.py | 2 +- gprofiler/metadata/versions.py | 31 +++++ gprofiler/profilers/perf.py | 5 +- gprofiler/utils/__init__.py | 16 ++- gprofiler/utils/node.py | 143 ++++++++++++++------ scripts/build_aarch64_container.sh | 3 +- scripts/build_aarch64_executable.sh | 2 +- scripts/build_node_package.sh | 24 ++-- scripts/build_x86_64_executable.sh | 2 +- tests/conftest.py | 17 ++- tests/containers/nodejs-attach/Dockerfile | 10 -- tests/containers/nodejs-attach/fibonacci.js | 11 -- tests/containers/nodejs/Dockerfile | 6 +- tests/test_sanity.py | 34 ++++- 15 files changed, 212 insertions(+), 125 deletions(-) create mode 100644 gprofiler/metadata/versions.py delete mode 100644 tests/containers/nodejs-attach/Dockerfile delete mode 100644 tests/containers/nodejs-attach/fibonacci.js diff --git a/gprofiler/metadata/__init__.py b/gprofiler/metadata/__init__.py index 65d2b5bff..e69de29bb 100644 --- a/gprofiler/metadata/__init__.py +++ b/gprofiler/metadata/__init__.py @@ -1,31 +0,0 @@ -from subprocess import CompletedProcess -from threading import Event - -from granulate_utils.linux.ns import get_process_nspid, run_in_ns -from psutil import Process - -from gprofiler.utils import run_process - - -def get_exe_version( - process: Process, - stop_event: Event = None, - get_version_timeout: int = None, - version_arg: str = "--version", - try_stderr: bool = False, -) -> str: - """ - Runs {process.exe()} --version in the appropriate namespace - """ - exe_path = f"/proc/{get_process_nspid(process.pid)}/exe" - - def _run_get_version() -> "CompletedProcess[bytes]": - return run_process([exe_path, version_arg], stop_event=stop_event, timeout=get_version_timeout) - - cp = run_in_ns(["pid", "mnt"], _run_get_version, process.pid) - stdout = cp.stdout.decode().strip() - # return stderr if stdout is empty, some apps print their version to stderr. - if try_stderr and not stdout: - return cp.stderr.decode().strip() - - return stdout diff --git a/gprofiler/metadata/application_metadata.py b/gprofiler/metadata/application_metadata.py index af70179e8..0d23a58c9 100644 --- a/gprofiler/metadata/application_metadata.py +++ b/gprofiler/metadata/application_metadata.py @@ -11,7 +11,7 @@ from psutil import NoSuchProcess, Process from gprofiler.log import get_logger_adapter -from gprofiler.metadata import get_exe_version +from gprofiler.metadata.versions import get_exe_version logger = get_logger_adapter(__name__) diff --git a/gprofiler/metadata/versions.py b/gprofiler/metadata/versions.py new file mode 100644 index 000000000..2816fe0a6 --- /dev/null +++ b/gprofiler/metadata/versions.py @@ -0,0 +1,31 @@ +from subprocess import CompletedProcess +from threading import Event + +from granulate_utils.linux.ns import get_process_nspid, run_in_ns +from psutil import Process + +from gprofiler.utils import run_process + + +def get_exe_version( + process: Process, + stop_event: Event, + get_version_timeout: int, + version_arg: str = "--version", + try_stderr: bool = False, +) -> str: + """ + Runs {process.exe()} --version in the appropriate namespace + """ + exe_path = f"/proc/{get_process_nspid(process.pid)}/exe" + + def _run_get_version() -> "CompletedProcess[bytes]": + return run_process([exe_path, version_arg], stop_event=stop_event, timeout=get_version_timeout) + + cp = run_in_ns(["pid", "mnt"], _run_get_version, process.pid) + stdout = cp.stdout.decode().strip() + # return stderr if stdout is empty, some apps print their version to stderr. + if try_stderr and not stdout: + return cp.stderr.decode().strip() + + return stdout diff --git a/gprofiler/profilers/perf.py b/gprofiler/profilers/perf.py index 4cc7e0765..80caeedd0 100644 --- a/gprofiler/profilers/perf.py +++ b/gprofiler/profilers/perf.py @@ -12,7 +12,7 @@ from typing import Any, Dict, List, Optional from granulate_utils.linux.elf import is_statically_linked, read_elf_symbol, read_elf_va -from granulate_utils.linux.process import is_musl +from granulate_utils.linux.process import is_musl, is_process_running from psutil import NoSuchProcess, Process from gprofiler import merge @@ -181,6 +181,7 @@ def __init__( self._perfs: List[PerfProcess] = [] self._metadata_collectors: List[PerfMetadata] = [GolangPerfMetadata(stop_event), NodePerfMetadata(stop_event)] self._node_processes: List[Process] = [] + if perf_mode in ("fp", "smart"): self._perf_fp: Optional[PerfProcess] = PerfProcess( self._frequency, @@ -219,6 +220,7 @@ def start(self) -> None: def stop(self) -> None: if self.perf_node_attach: + self._node_processes = [process for process in self._node_processes if is_process_running(process)] clean_up_node_maps(self._node_processes) for perf in reversed(self._perfs): perf.stop() @@ -238,6 +240,7 @@ def _get_metadata(self, pid: int) -> Optional[AppMetadata]: def snapshot(self) -> ProcessToProfileData: if self.perf_node_attach: + self._node_processes = [process for process in self._node_processes if is_process_running(process)] new_processes = [process for process in get_node_processes() if process not in self._node_processes] generate_map_for_node_processes(new_processes) self._node_processes.extend(new_processes) diff --git a/gprofiler/utils/__init__.py b/gprofiler/utils/__init__.py index d43a112a5..c43bad0a5 100644 --- a/gprofiler/utils/__init__.py +++ b/gprofiler/utils/__init__.py @@ -26,7 +26,7 @@ import importlib_resources import psutil -from granulate_utils.exceptions import CouldNotAcquireMutex, MissingExePath +from granulate_utils.exceptions import CouldNotAcquireMutex from granulate_utils.linux.mutex import try_acquire_mutex from granulate_utils.linux.ns import run_in_ns from granulate_utils.linux.process import process_exe @@ -271,9 +271,10 @@ def pgrep_exe(match: str) -> List[Process]: procs = [] for process in psutil.process_iter(): try: - if pattern.match(process_exe(process)): + # kernel threads should be child of process with pid 2 + if process.pid != 2 and process.ppid() != 2 and pattern.match(process_exe(process)): procs.append(process) - except (psutil.NoSuchProcess, MissingExePath): # process might have died meanwhile + except psutil.NoSuchProcess: # process might have died meanwhile continue return procs @@ -461,8 +462,9 @@ def get_staticx_dir() -> Optional[str]: def add_permission_dir(path: str, permission_for_file: int, permission_for_dir: int) -> None: os.chmod(path, os.stat(path).st_mode | permission_for_dir) - for subpath in glob.glob(path + "/*"): - if os.path.isdir(subpath): - add_permission_dir(subpath, permission_for_file, permission_for_dir) + for subpath in os.listdir(path): + absolute_subpath = os.path.join(path, subpath) + if os.path.isdir(absolute_subpath): + add_permission_dir(absolute_subpath, permission_for_file, permission_for_dir) else: - os.chmod(subpath, os.stat(subpath).st_mode | permission_for_file) + os.chmod(absolute_subpath, os.stat(absolute_subpath).st_mode | permission_for_file) diff --git a/gprofiler/utils/node.py b/gprofiler/utils/node.py index 0b5dc7ee7..563bd87aa 100644 --- a/gprofiler/utils/node.py +++ b/gprofiler/utils/node.py @@ -3,24 +3,24 @@ import shutil import signal import stat -from typing import Dict, List +from functools import lru_cache +from threading import Event +from typing import Dict, List, cast import psutil import requests from granulate_utils.linux.ns import get_proc_root_path, get_process_nspid, resolve_proc_root_links, run_in_ns -from granulate_utils.linux.process import is_musl +from granulate_utils.linux.process import is_musl, is_process_running from retry import retry from websocket import create_connection from websocket._core import WebSocket from gprofiler.log import get_logger_adapter -from gprofiler.metadata import get_exe_version +from gprofiler.metadata.versions import get_exe_version from gprofiler.utils import add_permission_dir, pgrep_exe, resource_path logger = get_logger_adapter(__name__) -DSO_GIT_REVISION = "20eb88a" - class NodeDebuggerUrlNotFound(Exception): pass @@ -30,9 +30,28 @@ class NodeDebuggerUnexpectedResponse(Exception): pass -def get_dest_inside_container(musl: bool, node_version: str) -> str: +def _get_node_major_version(process: psutil.Process) -> str: + node_version = get_exe_version(process, Event(), 3) + # i. e. v16.3.2 -> 16 + return node_version[1:].split(".")[0] + + +def _get_dso_git_rev() -> str: + libc_dso_version_file = resource_path(os.path.join("node", "module", "glibc", "version")) + musl_dso_version_file = resource_path(os.path.join("node", "module", "musl", "version")) + with open(libc_dso_version_file) as f: + libc_dso_ver = f.readline() + with open(musl_dso_version_file) as f: + musl_dso_ver = f.readline() + # with no build errors this should always be the same + assert libc_dso_ver == musl_dso_ver + return libc_dso_ver + + +@lru_cache() +def _get_dest_inside_container(musl: bool, node_version: str) -> str: libc = "musl" if musl else "glibc" - return os.path.join("/", "tmp", "node_module", DSO_GIT_REVISION, libc, node_version) + return os.path.join("/", "tmp", "node_module", _get_dso_git_rev(), libc, node_version) def _start_debugger(pid: int) -> None: @@ -43,13 +62,16 @@ def _start_debugger(pid: int) -> None: @retry(NodeDebuggerUrlNotFound, 5, 1) def _get_debugger_url() -> str: # when killing process with SIGUSR1 it will open new debugger session on port 9229, - # so it will always the same + # so it will always the same. When another debugger is opened in same NS it will not open new one. + # REF: Inspector agent initialization uses host_port + # https://github.com/nodejs/node/blob/5fad0b93667ffc6e4def52996b9529ac99b26319/src/inspector_agent.cc#L668 + # host_port defaults to 9229 + # ref: https://github.com/nodejs/node/blob/2849283c4cebbfbf523cc24303941dc36df9332f/src/node_options.h#L90 + # in our case it won't be changed port = 9229 debugger_url_response = requests.get(f"http://127.0.0.1:{port}/json/list") - if ( - debugger_url_response.status_code != 200 - or not debugger_url_response.headers.get("Content-Type") - or "application/json" not in debugger_url_response.headers.get("Content-Type") # type: ignore + if debugger_url_response.status_code != 200 or "application/json" not in debugger_url_response.headers.get( + "Content-Type", "" ): raise NodeDebuggerUrlNotFound( {"status_code": debugger_url_response.status_code, "text": debugger_url_response.text} @@ -64,12 +86,10 @@ def _get_debugger_url() -> str: ): raise NodeDebuggerUrlNotFound(response_json) - return response_json[0]["webSocketDebuggerUrl"] # type: ignore + return cast(str, response_json[0]["webSocketDebuggerUrl"]) -@retry(NodeDebuggerUnexpectedResponse, 5, 1) def _send_socket_request(sock: WebSocket, cdp_request: Dict) -> None: - sock.settimeout(2) sock.send(json.dumps(cdp_request)) message = sock.recv() try: @@ -86,6 +106,27 @@ def _send_socket_request(sock: WebSocket, cdp_request: Dict) -> None: raise NodeDebuggerUnexpectedResponse(message) +def _get_node_pid(sock: WebSocket) -> int: + cdp_request = { + "id": 1, + "method": "Runtime.evaluate", + "params": { + "expression": "process.pid", + "replMode": True, + }, + } + sock.send(json.dumps(cdp_request)) + message = sock.recv() + try: + message = json.loads(message) + except json.JSONDecodeError: + raise NodeDebuggerUnexpectedResponse(message) + try: + return cast(int, message["result"]["result"]["value"]) + except KeyError: + raise NodeDebuggerUnexpectedResponse(message) + + def _load_dso(sock: WebSocket, module_path: str) -> None: cdp_request = { "id": 1, @@ -110,50 +151,74 @@ def _stop_dso(sock: WebSocket, module_path: str) -> None: _send_socket_request(sock, cdp_request) +def validate_ns(nspid: int, expected_ns_link_name: str) -> None: + actual_nspid_name = os.readlink(os.path.join("/proc", str(nspid), "ns", "pid")) + assert ( + actual_nspid_name == expected_ns_link_name + ), f"Wrong namespace, expected {expected_ns_link_name}, got {actual_nspid_name}" + + +def validate_pid(expected_pid: int, sock: WebSocket) -> None: + actual_pid = _get_node_pid(sock) + assert expected_pid == actual_pid, f"Wrong pid, expected {expected_pid}, actual {actual_pid}" + + +def create_debugger_socket(nspid: int, ns_link_name: str) -> WebSocket: + validate_ns(nspid, ns_link_name) + debugger_url = _get_debugger_url() + sock = create_connection(debugger_url) + sock.settimeout(10) + validate_pid(nspid, sock) + return sock + + def _copy_module_into_process_ns(process: psutil.Process, musl: bool, version: str) -> str: proc_root = get_proc_root_path(process) libc = "musl" if musl else "glibc" - dest_inside_container = get_dest_inside_container(musl, version) + dest_inside_container = _get_dest_inside_container(musl, version) dest = resolve_proc_root_links(proc_root, dest_inside_container) if os.path.exists(dest): return dest_inside_container - src = resource_path(os.path.join("node", "module", libc, version)) + src = resource_path(os.path.join("node", "module", libc, _get_dso_git_rev(), version)) shutil.copytree(src, dest) add_permission_dir(dest, stat.S_IROTH, stat.S_IXOTH | stat.S_IROTH) return dest_inside_container -def _generate_perf_map(module_path: str) -> None: - debugger_url = _get_debugger_url() - sock = create_connection(debugger_url) - sock.settimeout(2) +def _generate_perf_map(module_path: str, nspid: int, ns_link_name: str) -> None: + sock = create_debugger_socket(nspid, ns_link_name) _load_dso(sock, module_path) -def _clean_up(module_path: str, pid: int) -> None: - debugger_url = _get_debugger_url() - sock = create_connection(debugger_url) - _stop_dso(sock, module_path) - sock.settimeout(2) - os.remove(os.path.join("/tmp", f"perf-{pid}.map")) +def _clean_up(module_path: str, nspid: int, ns_link_name: str) -> None: + sock = create_debugger_socket(nspid, ns_link_name) + try: + _stop_dso(sock, module_path) + finally: + os.remove(os.path.join("/tmp", f"perf-{nspid}.map")) def get_node_processes() -> List[psutil.Process]: - return pgrep_exe(r"(?:^.+/node[^/]*$)") + return pgrep_exe(r".*node[^/]*$") def generate_map_for_node_processes(processes: List[psutil.Process]) -> None: """Iterates over all NodeJS processes, starts debugger for it, finds debugger URL, - copies node-linux-perf module into process' namespace, loads module and starts it. - After that it links it into gProfiler namespace's /tmp. This lets perf load it""" + copies node-linux-perf module into process' namespace, loads module and starts it.""" for process in processes: try: musl = is_musl(process) - node_version = get_exe_version(process) - node_major_version = node_version[1:].split(".")[0] + node_major_version = _get_node_major_version(process) dest = _copy_module_into_process_ns(process, musl, node_major_version) + nspid = get_process_nspid(process.pid) + ns_link_name = os.readlink(f"/proc/{process.pid}/ns/pid") _start_debugger(process.pid) - run_in_ns(["pid", "mnt", "net"], lambda: _generate_perf_map(dest), process.pid, passthrough_exception=True) + run_in_ns( + ["pid", "mnt", "net"], + lambda: _generate_perf_map(dest, nspid, ns_link_name), + process.pid, + passthrough_exception=True, + ) except Exception as e: logger.warning(f"Could not create debug symbols for pid {process.pid}. Reason: {e}", exc_info=True) @@ -162,13 +227,15 @@ def clean_up_node_maps(processes: List[psutil.Process]) -> None: """Stops generating perf maps for each NodeJS process and cleans up generated maps""" for process in processes: try: - node_version = get_exe_version(process) - node_major_version = node_version[1:].split(".")[0] - pid_inside_ns = get_process_nspid(process.pid) - dest = get_dest_inside_container(is_musl(process), node_major_version) + if not is_process_running(process): + continue + node_major_version = _get_node_major_version(process) + nspid = get_process_nspid(process.pid) + ns_link_name = os.readlink(f"/proc/{process.pid}/ns/pid") + dest = _get_dest_inside_container(is_musl(process), node_major_version) run_in_ns( ["pid", "mnt", "net"], - lambda: _clean_up(dest, pid_inside_ns), + lambda: _clean_up(dest, nspid, ns_link_name), process.pid, passthrough_exception=True, ) diff --git a/scripts/build_aarch64_container.sh b/scripts/build_aarch64_container.sh index d203749df..de73354fd 100755 --- a/scripts/build_aarch64_container.sh +++ b/scripts/build_aarch64_container.sh @@ -30,5 +30,4 @@ docker buildx build --platform=linux/arm64 \ --build-arg DOTNET_BUILDER=$DOTNET_BUILDER \ --build-arg NODE_PACKAGE_BUILDER_MUSL=$ALPINE_VERSION \ --build-arg NODE_PACKAGE_BUILDER_GLIBC=$UBUNTU_VERSION \ - . $@ - + . "$@" diff --git a/scripts/build_aarch64_executable.sh b/scripts/build_aarch64_executable.sh index f3f47ddfb..b330c33ec 100755 --- a/scripts/build_aarch64_executable.sh +++ b/scripts/build_aarch64_executable.sh @@ -34,4 +34,4 @@ docker buildx build --platform=linux/arm64 \ --build-arg DOTNET_BUILDER=$DOTNET_BUILDER \ --build-arg NODE_PACKAGE_BUILDER_MUSL=$ALPINE_VERSION \ --build-arg NODE_PACKAGE_BUILDER_GLIBC=$UBUNTU_VERSION \ - . -f pyi.Dockerfile --output type=local,dest=build/aarch64/ $@ + . -f pyi.Dockerfile --output type=local,dest=build/aarch64/ "$@" diff --git a/scripts/build_node_package.sh b/scripts/build_node_package.sh index 804bcb37d..488de2219 100755 --- a/scripts/build_node_package.sh +++ b/scripts/build_node_package.sh @@ -5,15 +5,17 @@ set -euo pipefail MODULE_PATH=/tmp/module BUILD_TARGET_DIR=/tmp/module_build +GIT_REV=20eb88a git clone https://github.com/mmarchini-oss/node-linux-perf.git $MODULE_PATH cd $MODULE_PATH -git reset --hard 20eb88a35ab256313dfb0f14645456ebf046ac1b +git reset --hard $GIT_REV npm install -g node-gyp curl -L https://github.com/nodejs/nan/archive/refs/tags/v2.16.0.tar.gz -o nan.tar.gz tar -vxzf nan.tar.gz -C /tmp NAN_PATH=$(realpath /tmp/nan-*) export NAN_PATH +# providing nan module by path, rather than npm, because of using node-gyp instead of npm sed -i 's/node \-e \\"require('\''nan'\'')\\"/echo $NAN_PATH/g' binding.gyp rm -rf nan.tar.gz mkdir $BUILD_TARGET_DIR @@ -23,19 +25,21 @@ for node_version in "${node_versions[@]}"; do node-gyp build --target="$node_version" t=(${node_version//./ }) node_major_version=${t[0]} - mkdir "$BUILD_TARGET_DIR/$node_major_version" - cp "$MODULE_PATH/linux-perf.js" "$BUILD_TARGET_DIR/$node_major_version/." - mkdir -p "$BUILD_TARGET_DIR/$node_major_version/build/Release" - cp "$MODULE_PATH/build/Release/linux-perf.node" "$BUILD_TARGET_DIR/$node_major_version/build/Release/linux-perf.node" + mkdir -p "$BUILD_TARGET_DIR/$GIT_REV/$node_major_version" + cp "$MODULE_PATH/linux-perf.js" "$BUILD_TARGET_DIR/$GIT_REV/$node_major_version/." + mkdir -p "$BUILD_TARGET_DIR/$GIT_REV/$node_major_version/build/Release" + # we need to preserve original path required by linux-perf.js + cp "$MODULE_PATH/build/Release/linux-perf.node" "$BUILD_TARGET_DIR/$GIT_REV/$node_major_version/build/Release/linux-perf.node" rm -rf "$MODULE_PATH/build" done for node_major_version in {12..16}; do node-gyp configure --target="$node_major_version.0.0" node-gyp build --target="$node_major_version.0.0" - mkdir "$BUILD_TARGET_DIR/$node_major_version" - cp "$MODULE_PATH/linux-perf.js" "$BUILD_TARGET_DIR/$node_major_version/." - mkdir -p "$BUILD_TARGET_DIR/$node_major_version/build/Release" - cp "$MODULE_PATH/build/Release/linux-perf.node" "$BUILD_TARGET_DIR/$node_major_version/build/Release/linux-perf.node" + mkdir -p "$BUILD_TARGET_DIR/$GIT_REV/$node_major_version" + cp "$MODULE_PATH/linux-perf.js" "$BUILD_TARGET_DIR/$GIT_REV/$node_major_version/." + mkdir -p "$BUILD_TARGET_DIR/$GIT_REV/$node_major_version/build/Release" + cp "$MODULE_PATH/build/Release/linux-perf.node" "$BUILD_TARGET_DIR/$GIT_REV/$node_major_version/build/Release/linux-perf.node" rm -rf "$MODULE_PATH/build" done -rm -rf "$NAN_PATH" \ No newline at end of file +rm -rf "$NAN_PATH" +echo -n "$GIT_REV" > $BUILD_TARGET_DIR/version \ No newline at end of file diff --git a/scripts/build_x86_64_executable.sh b/scripts/build_x86_64_executable.sh index fb392c991..67ec6adcc 100755 --- a/scripts/build_x86_64_executable.sh +++ b/scripts/build_x86_64_executable.sh @@ -41,4 +41,4 @@ DOCKER_BUILDKIT=1 docker build -f pyi.Dockerfile --output type=local,dest=build/ --build-arg DOTNET_BUILDER=$DOTNET_BUILDER \ --build-arg NODE_PACKAGE_BUILDER_MUSL=$AP_BUILDER_ALPINE \ --build-arg NODE_PACKAGE_BUILDER_GLIBC=$UBUNTU_VERSION \ - . $@ + . "$@" diff --git a/tests/conftest.py b/tests/conftest.py index 586f2c76f..2376b9b5a 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -124,10 +124,6 @@ def command_line(runtime: str, java_command_line: List[str], dotnet_command_line "--interpreted-frames-native-stack", str(CONTAINERS_DIRECTORY / "nodejs/fibonacci.js"), ], - "nodejs-attach": [ - "node", - str(CONTAINERS_DIRECTORY / "nodejs/fibonacci.js"), - ], }[runtime] @@ -135,7 +131,7 @@ def command_line(runtime: str, java_command_line: List[str], dotnet_command_line def application_executable(runtime: str) -> str: if runtime == "golang": return "fibonacci" - elif runtime in ("nodejs", "nodejs-attach"): + elif runtime == "nodejs": return "node" return runtime @@ -220,7 +216,8 @@ def application_docker_image_configs() -> Mapping[str, Dict[str, Any]]: "thread_comm": dict(dockerfile="thread_comm.Dockerfile"), }, "nodejs": { - "": {}, + "": dict(buildargs={"NODE_RUNTIME_FLAGS": "--perf-prof --interpreted-frames-native-stack"}), + "without-flags": dict(buildargs={"NODE_RUNTIME_FLAGS": ""}), }, "php": { "": {}, @@ -388,7 +385,11 @@ def application_pid( # Application might be run using "sh -c ...", we detect the case and return the "real" application pid process = Process(pid) - if process.cmdline()[0] == "sh" and process.cmdline()[1] == "-c" and len(process.children(recursive=False)) == 1: + if ( + process.cmdline()[0].endswith("sh") + and process.cmdline()[1] == "-c" + and len(process.children(recursive=False)) == 1 + ): pid = process.children(recursive=False)[0].pid return pid @@ -409,7 +410,6 @@ def runtime_specific_args(runtime: str) -> List[str]: ], "python": ["-d", "3"], # Burner python tests make syscalls and we want to record python + kernel stacks "nodejs": ["--nodejs-mode", "perf"], # enable NodeJS profiling - "nodejs-attach": ["--nodejs-mode", "attach-maps"], }.get(runtime, []) @@ -426,7 +426,6 @@ def assert_collapsed(runtime: str) -> AssertInCollapsed: "nodejs": "fibonacci", "golang": "fibonacci", "dotnet": "Fibonacci", - "nodejs-attach": "fibonacci", }[runtime] return partial(assert_function_in_collapsed, function_name) diff --git a/tests/containers/nodejs-attach/Dockerfile b/tests/containers/nodejs-attach/Dockerfile deleted file mode 100644 index 2fc1aab49..000000000 --- a/tests/containers/nodejs-attach/Dockerfile +++ /dev/null @@ -1,10 +0,0 @@ -# node:10.24.1 -FROM node@sha256:59531d2835edd5161c8f9512f9e095b1836f7a1fcb0ab73e005ec46047384911 - -# /tmp so node has permissions to write its jitdump file -WORKDIR /tmp - -RUN mkdir /app -ADD fibonacci.js /app - -CMD ["node", "/app/fibonacci.js"] diff --git a/tests/containers/nodejs-attach/fibonacci.js b/tests/containers/nodejs-attach/fibonacci.js deleted file mode 100644 index f69313c41..000000000 --- a/tests/containers/nodejs-attach/fibonacci.js +++ /dev/null @@ -1,11 +0,0 @@ -// -// Copyright (c) Granulate. All rights reserved. -// Licensed under the AGPL3 License. See LICENSE.md in the project root for license information. -// -function fibonacci(n) { - return n <= 1 ? n : fibonacci(n - 1) + fibonacci(n - 2); -} - -while (true) { - fibonacci(30); -} diff --git a/tests/containers/nodejs/Dockerfile b/tests/containers/nodejs/Dockerfile index b5d9f72a6..eac4c6bbc 100644 --- a/tests/containers/nodejs/Dockerfile +++ b/tests/containers/nodejs/Dockerfile @@ -1,3 +1,5 @@ +ARG NODE_RUNTIME_FLAGS + # node:10.24.1 FROM node@sha256:59531d2835edd5161c8f9512f9e095b1836f7a1fcb0ab73e005ec46047384911 @@ -7,4 +9,6 @@ WORKDIR /tmp RUN mkdir /app ADD fibonacci.js /app -CMD ["node", "--perf-prof", "--interpreted-frames-native-stack", "/app/fibonacci.js"] +ENV NODE_RUNTIME_FLAGS ${NODE_RUNTIME_FLAGS} + +CMD node $NODE_RUNTIME_FLAGS /app/fibonacci.js diff --git a/tests/test_sanity.py b/tests/test_sanity.py index ed68e82ae..f7c18a0fe 100644 --- a/tests/test_sanity.py +++ b/tests/test_sanity.py @@ -21,7 +21,7 @@ from gprofiler.profilers.python_ebpf import PythonEbpfProfiler from gprofiler.profilers.ruby import RbSpyProfiler from gprofiler.utils import wait_event -from tests import PHPSPY_DURATION +from tests import CONTAINERS_DIRECTORY, PHPSPY_DURATION from tests.conftest import AssertInCollapsed from tests.utils import ( RUNTIME_PROFILERS, @@ -130,11 +130,17 @@ def test_nodejs( assert_collapsed(process_collapsed) -@pytest.mark.parametrize("runtime", ["nodejs-attach"]) +@pytest.mark.parametrize("profiler_type", ["attach-maps"]) +@pytest.mark.parametrize("runtime", ["nodejs"]) +@pytest.mark.parametrize("application_image_tag", ["without-flags"]) +@pytest.mark.parametrize("command_line", [["node", f"{CONTAINERS_DIRECTORY}/nodejs/fibonacci.js"]]) def test_nodejs_attach_maps( tmp_path: Path, application_pid: int, assert_collapsed: AssertInCollapsed, + profiler_type: str, + command_line: List[str], + runtime_specific_args: List[str], ) -> None: with SystemProfiler( 1000, @@ -155,6 +161,30 @@ def test_nodejs_attach_maps( assert_function_in_collapsed("v8::Function::Call", process_collapsed) +@pytest.mark.parametrize("profiler_type", ["attach-maps"]) +@pytest.mark.parametrize("runtime", ["nodejs"]) +@pytest.mark.parametrize("application_image_tag", ["without-flags"]) +@pytest.mark.parametrize("command_line", [["node", f"{CONTAINERS_DIRECTORY}/nodejs/fibonacci.js"]]) +def test_nodejs_attach_maps_from_container( + docker_client: DockerClient, + application_pid: int, + runtime_specific_args: List[str], + gprofiler_docker_image: Image, + output_directory: Path, + output_collapsed: Path, + assert_collapsed: AssertInCollapsed, + assert_app_id: Callable, + profiler_flags: List[str], +) -> None: + _ = application_pid # Fixture only used for running the application. + _ = assert_app_id # Required for mypy unused argument warning + collapsed_text = run_gprofiler_in_container_for_one_session( + docker_client, gprofiler_docker_image, output_directory, output_collapsed, runtime_specific_args, profiler_flags + ) + collapsed = parse_one_collapsed(collapsed_text) + assert_collapsed(collapsed) + + @pytest.mark.parametrize("runtime", ["python"]) def test_python_ebpf( tmp_path: Path, From 14b6899a282dafd583ce2bee263c27d0fa93fad0 Mon Sep 17 00:00:00 2001 From: "Wisniewski, Krzysztof2" Date: Thu, 29 Sep 2022 14:09:30 +0200 Subject: [PATCH 4/9] Review changes v3 --- gprofiler/{utils => profilers}/node.py | 5 +- gprofiler/profilers/perf.py | 2 +- scripts/build_node_package.sh | 2 + tests/containers/nodejs/Dockerfile | 4 +- tests/test_node.py | 66 ++++++++++++++++++++++++++ tests/test_sanity.py | 57 +--------------------- 6 files changed, 75 insertions(+), 61 deletions(-) rename gprofiler/{utils => profilers}/node.py (97%) create mode 100644 tests/test_node.py diff --git a/gprofiler/utils/node.py b/gprofiler/profilers/node.py similarity index 97% rename from gprofiler/utils/node.py rename to gprofiler/profilers/node.py index 563bd87aa..248812eee 100644 --- a/gprofiler/utils/node.py +++ b/gprofiler/profilers/node.py @@ -17,7 +17,7 @@ from gprofiler.log import get_logger_adapter from gprofiler.metadata.versions import get_exe_version -from gprofiler.utils import add_permission_dir, pgrep_exe, resource_path +from gprofiler.utils import TEMPORARY_STORAGE_PATH, add_permission_dir, pgrep_exe, resource_path logger = get_logger_adapter(__name__) @@ -36,6 +36,7 @@ def _get_node_major_version(process: psutil.Process) -> str: return node_version[1:].split(".")[0] +@lru_cache(maxsize=1) def _get_dso_git_rev() -> str: libc_dso_version_file = resource_path(os.path.join("node", "module", "glibc", "version")) musl_dso_version_file = resource_path(os.path.join("node", "module", "musl", "version")) @@ -51,7 +52,7 @@ def _get_dso_git_rev() -> str: @lru_cache() def _get_dest_inside_container(musl: bool, node_version: str) -> str: libc = "musl" if musl else "glibc" - return os.path.join("/", "tmp", "node_module", _get_dso_git_rev(), libc, node_version) + return os.path.join(TEMPORARY_STORAGE_PATH, "node_module", _get_dso_git_rev(), libc, node_version) def _start_debugger(pid: int) -> None: diff --git a/gprofiler/profilers/perf.py b/gprofiler/profilers/perf.py index 80caeedd0..c0e307c92 100644 --- a/gprofiler/profilers/perf.py +++ b/gprofiler/profilers/perf.py @@ -20,10 +20,10 @@ from gprofiler.gprofiler_types import AppMetadata, ProcessToProfileData, ProfileData from gprofiler.log import get_logger_adapter from gprofiler.metadata.application_metadata import ApplicationMetadata +from gprofiler.profilers.node import clean_up_node_maps, generate_map_for_node_processes, get_node_processes from gprofiler.profilers.profiler_base import ProfilerBase from gprofiler.profilers.registry import ProfilerArgument, register_profiler from gprofiler.utils import run_process, start_process, wait_event, wait_for_file_by_prefix -from gprofiler.utils.node import clean_up_node_maps, generate_map_for_node_processes, get_node_processes from gprofiler.utils.perf import perf_path from gprofiler.utils.process import is_process_basename_matching diff --git a/scripts/build_node_package.sh b/scripts/build_node_package.sh index 488de2219..2dde04b81 100755 --- a/scripts/build_node_package.sh +++ b/scripts/build_node_package.sh @@ -15,6 +15,7 @@ curl -L https://github.com/nodejs/nan/archive/refs/tags/v2.16.0.tar.gz -o nan.ta tar -vxzf nan.tar.gz -C /tmp NAN_PATH=$(realpath /tmp/nan-*) export NAN_PATH +# shellcheck disable=SC2016 # expression shouldn't be expanded # providing nan module by path, rather than npm, because of using node-gyp instead of npm sed -i 's/node \-e \\"require('\''nan'\'')\\"/echo $NAN_PATH/g' binding.gyp rm -rf nan.tar.gz @@ -23,6 +24,7 @@ node_versions=( "10.10.0" "11.0.0" ) for node_version in "${node_versions[@]}"; do node-gyp configure --target="$node_version" --build_v8_with_gn=false node-gyp build --target="$node_version" + # shellcheck disable=SC2206 # string is expected to be splitted here t=(${node_version//./ }) node_major_version=${t[0]} mkdir -p "$BUILD_TARGET_DIR/$GIT_REV/$node_major_version" diff --git a/tests/containers/nodejs/Dockerfile b/tests/containers/nodejs/Dockerfile index eac4c6bbc..ee8d322ab 100644 --- a/tests/containers/nodejs/Dockerfile +++ b/tests/containers/nodejs/Dockerfile @@ -1,8 +1,8 @@ -ARG NODE_RUNTIME_FLAGS - # node:10.24.1 FROM node@sha256:59531d2835edd5161c8f9512f9e095b1836f7a1fcb0ab73e005ec46047384911 +ARG NODE_RUNTIME_FLAGS + # /tmp so node has permissions to write its jitdump file WORKDIR /tmp diff --git a/tests/test_node.py b/tests/test_node.py new file mode 100644 index 000000000..00c799d6e --- /dev/null +++ b/tests/test_node.py @@ -0,0 +1,66 @@ +from pathlib import Path +from threading import Event +from typing import List + +import pytest +from docker import DockerClient +from docker.models.images import Image + +from gprofiler.merge import parse_one_collapsed +from gprofiler.profilers.perf import SystemProfiler +from tests import CONTAINERS_DIRECTORY +from tests.conftest import AssertInCollapsed +from tests.utils import assert_function_in_collapsed, run_gprofiler_in_container_for_one_session, snapshot_pid_collapsed + + +@pytest.mark.parametrize("profiler_type", ["attach-maps"]) +@pytest.mark.parametrize("runtime", ["nodejs"]) +@pytest.mark.parametrize("application_image_tag", ["without-flags"]) +@pytest.mark.parametrize("command_line", [["node", f"{CONTAINERS_DIRECTORY}/nodejs/fibonacci.js"]]) +def test_nodejs_attach_maps( + tmp_path: Path, + application_pid: int, + assert_collapsed: AssertInCollapsed, + profiler_type: str, + command_line: List[str], + runtime_specific_args: List[str], +) -> None: + with SystemProfiler( + 1000, + 6, + Event(), + str(tmp_path), + False, + perf_mode="fp", + perf_inject=False, + perf_dwarf_stack_size=0, + perf_node_attach=True, + ) as profiler: + process_collapsed = snapshot_pid_collapsed(profiler, application_pid) + assert_collapsed(process_collapsed) + # check for node built-in functions + assert_function_in_collapsed("node::Start", process_collapsed) + # check for v8 built-in functions + assert_function_in_collapsed("v8::Function::Call", process_collapsed) + + +@pytest.mark.parametrize("profiler_type", ["attach-maps"]) +@pytest.mark.parametrize("runtime", ["nodejs"]) +@pytest.mark.parametrize("application_image_tag", ["without-flags"]) +@pytest.mark.parametrize("command_line", [["node", f"{CONTAINERS_DIRECTORY}/nodejs/fibonacci.js"]]) +def test_nodejs_attach_maps_from_container( + docker_client: DockerClient, + application_pid: int, + runtime_specific_args: List[str], + gprofiler_docker_image: Image, + output_directory: Path, + output_collapsed: Path, + assert_collapsed: AssertInCollapsed, + profiler_flags: List[str], +) -> None: + _ = application_pid # Fixture only used for running the application. + collapsed_text = run_gprofiler_in_container_for_one_session( + docker_client, gprofiler_docker_image, output_directory, output_collapsed, runtime_specific_args, profiler_flags + ) + collapsed = parse_one_collapsed(collapsed_text) + assert_collapsed(collapsed) diff --git a/tests/test_sanity.py b/tests/test_sanity.py index f7c18a0fe..0a2b70753 100644 --- a/tests/test_sanity.py +++ b/tests/test_sanity.py @@ -21,7 +21,7 @@ from gprofiler.profilers.python_ebpf import PythonEbpfProfiler from gprofiler.profilers.ruby import RbSpyProfiler from gprofiler.utils import wait_event -from tests import CONTAINERS_DIRECTORY, PHPSPY_DURATION +from tests import PHPSPY_DURATION from tests.conftest import AssertInCollapsed from tests.utils import ( RUNTIME_PROFILERS, @@ -130,61 +130,6 @@ def test_nodejs( assert_collapsed(process_collapsed) -@pytest.mark.parametrize("profiler_type", ["attach-maps"]) -@pytest.mark.parametrize("runtime", ["nodejs"]) -@pytest.mark.parametrize("application_image_tag", ["without-flags"]) -@pytest.mark.parametrize("command_line", [["node", f"{CONTAINERS_DIRECTORY}/nodejs/fibonacci.js"]]) -def test_nodejs_attach_maps( - tmp_path: Path, - application_pid: int, - assert_collapsed: AssertInCollapsed, - profiler_type: str, - command_line: List[str], - runtime_specific_args: List[str], -) -> None: - with SystemProfiler( - 1000, - 6, - Event(), - str(tmp_path), - False, - perf_mode="fp", - perf_inject=False, - perf_dwarf_stack_size=0, - perf_node_attach=True, - ) as profiler: - process_collapsed = snapshot_pid_collapsed(profiler, application_pid) - assert_collapsed(process_collapsed) - # check for node built-in functions - assert_function_in_collapsed("node::Start", process_collapsed) - # check for v8 built-in functions - assert_function_in_collapsed("v8::Function::Call", process_collapsed) - - -@pytest.mark.parametrize("profiler_type", ["attach-maps"]) -@pytest.mark.parametrize("runtime", ["nodejs"]) -@pytest.mark.parametrize("application_image_tag", ["without-flags"]) -@pytest.mark.parametrize("command_line", [["node", f"{CONTAINERS_DIRECTORY}/nodejs/fibonacci.js"]]) -def test_nodejs_attach_maps_from_container( - docker_client: DockerClient, - application_pid: int, - runtime_specific_args: List[str], - gprofiler_docker_image: Image, - output_directory: Path, - output_collapsed: Path, - assert_collapsed: AssertInCollapsed, - assert_app_id: Callable, - profiler_flags: List[str], -) -> None: - _ = application_pid # Fixture only used for running the application. - _ = assert_app_id # Required for mypy unused argument warning - collapsed_text = run_gprofiler_in_container_for_one_session( - docker_client, gprofiler_docker_image, output_directory, output_collapsed, runtime_specific_args, profiler_flags - ) - collapsed = parse_one_collapsed(collapsed_text) - assert_collapsed(collapsed) - - @pytest.mark.parametrize("runtime", ["python"]) def test_python_ebpf( tmp_path: Path, From 9d6175b726d47aff2edb0f7f9ab5466b14b32e8d Mon Sep 17 00:00:00 2001 From: "Wisniewski, Krzysztof2" Date: Thu, 29 Sep 2022 15:18:58 +0200 Subject: [PATCH 5/9] Add fix for missing .so in staticx --- scripts/list_needed_libs.sh | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/scripts/list_needed_libs.sh b/scripts/list_needed_libs.sh index cfa82fc50..f6fca4013 100755 --- a/scripts/list_needed_libs.sh +++ b/scripts/list_needed_libs.sh @@ -10,7 +10,14 @@ set -euo pipefail # (staticx knows to pack the libraries used by the executable we're packing. it doesn't know # which executables are to be used by it) -BINS=$(find gprofiler/resources -executable -type f) +EXCLUDED_DIRECTORIES=('"gprofiler/resources/node/*"') +FIND_BINS_CMD="find gprofiler/resources -executable -type f" + +for DIR in "${EXCLUDED_DIRECTORIES[@]}" ; do + FIND_BINS_CMD+=" -not -path $DIR" +done + +BINS=$(eval "$FIND_BINS_CMD") libs= From 152dee8f013455aed4a89dd89f91a4afaf9b04d4 Mon Sep 17 00:00:00 2001 From: "Wisniewski, Krzysztof2" Date: Thu, 29 Sep 2022 15:38:19 +0200 Subject: [PATCH 6/9] Add handling for non-zero return code from list_needed_libs.sh --- pyi.Dockerfile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pyi.Dockerfile b/pyi.Dockerfile index 3c47023e4..cdb2b3722 100644 --- a/pyi.Dockerfile +++ b/pyi.Dockerfile @@ -320,7 +320,8 @@ RUN set -e; \ if [ $(uname -m) != "aarch64" ]; then \ source scl_source enable devtoolset-8 llvm-toolset-7 ; \ fi && \ - staticx $(./scripts/list_needed_libs.sh) dist/gprofiler dist/gprofiler + LIBS=$(./scripts/list_needed_libs.sh) && \ + staticx $LIBS dist/gprofiler dist/gprofiler FROM scratch AS export-stage From db7bec9585aca7c90f206ad9a32f92a34b2c1044 Mon Sep 17 00:00:00 2001 From: "Wisniewski, Krzysztof2" Date: Fri, 30 Sep 2022 14:16:34 +0200 Subject: [PATCH 7/9] Review changes v4 --- gprofiler/metadata/versions.py | 4 +++ gprofiler/profilers/node.py | 64 ++++++++++++++++++---------------- tests/test_node.py | 3 ++ 3 files changed, 40 insertions(+), 31 deletions(-) diff --git a/gprofiler/metadata/versions.py b/gprofiler/metadata/versions.py index 2816fe0a6..907ea53e7 100644 --- a/gprofiler/metadata/versions.py +++ b/gprofiler/metadata/versions.py @@ -1,3 +1,7 @@ +# +# Copyright (c) Granulate. All rights reserved. +# Licensed under the AGPL3 License. See LICENSE.md in the project root for license information. +# from subprocess import CompletedProcess from threading import Event diff --git a/gprofiler/profilers/node.py b/gprofiler/profilers/node.py index 248812eee..4192d58ed 100644 --- a/gprofiler/profilers/node.py +++ b/gprofiler/profilers/node.py @@ -1,11 +1,16 @@ +# +# Copyright (c) Granulate. All rights reserved. +# Licensed under the AGPL3 License. See LICENSE.md in the project root for license information. +# import json import os +from pathlib import Path import shutil import signal import stat from functools import lru_cache from threading import Event -from typing import Dict, List, cast +from typing import Any, Dict, List, cast import psutil import requests @@ -40,10 +45,8 @@ def _get_node_major_version(process: psutil.Process) -> str: def _get_dso_git_rev() -> str: libc_dso_version_file = resource_path(os.path.join("node", "module", "glibc", "version")) musl_dso_version_file = resource_path(os.path.join("node", "module", "musl", "version")) - with open(libc_dso_version_file) as f: - libc_dso_ver = f.readline() - with open(musl_dso_version_file) as f: - musl_dso_ver = f.readline() + libc_dso_ver = Path(libc_dso_version_file).read_text() + musl_dso_ver = Path(musl_dso_version_file).read_text() # with no build errors this should always be the same assert libc_dso_ver == musl_dso_ver return libc_dso_ver @@ -70,7 +73,7 @@ def _get_debugger_url() -> str: # ref: https://github.com/nodejs/node/blob/2849283c4cebbfbf523cc24303941dc36df9332f/src/node_options.h#L90 # in our case it won't be changed port = 9229 - debugger_url_response = requests.get(f"http://127.0.0.1:{port}/json/list") + debugger_url_response = requests.get(f"http://127.0.0.1:{port}/json/list", timeout=3) if debugger_url_response.status_code != 200 or "application/json" not in debugger_url_response.headers.get( "Content-Type", "" ): @@ -107,12 +110,12 @@ def _send_socket_request(sock: WebSocket, cdp_request: Dict) -> None: raise NodeDebuggerUnexpectedResponse(message) -def _get_node_pid(sock: WebSocket) -> int: +def _execute_js_command(sock: WebSocket, command: str) -> Any: cdp_request = { "id": 1, "method": "Runtime.evaluate", "params": { - "expression": "process.pid", + "expression": command, "replMode": True, }, } @@ -123,53 +126,52 @@ def _get_node_pid(sock: WebSocket) -> int: except json.JSONDecodeError: raise NodeDebuggerUnexpectedResponse(message) try: - return cast(int, message["result"]["result"]["value"]) + return message["result"]["result"]["value"] except KeyError: raise NodeDebuggerUnexpectedResponse(message) -def _load_dso(sock: WebSocket, module_path: str) -> None: +def _change_dso_state(sock: WebSocket, module_path: str, action: str) -> None: + assert action in ("start", "stop"), "_change_dso_state supports only start and stop actions" cdp_request = { "id": 1, "method": "Runtime.evaluate", "params": { - "expression": f'process.mainModule.require("{os.path.join(module_path, "linux-perf.js")}").start()', + "expression": f'process.mainModule.require("{os.path.join(module_path, "linux-perf.js")}").{action}()', "replMode": True, }, } _send_socket_request(sock, cdp_request) -def _stop_dso(sock: WebSocket, module_path: str) -> None: - cdp_request = { - "id": 1, - "method": "Runtime.evaluate", - "params": { - "expression": f'process.mainModule.require("{os.path.join(module_path, "linux-perf.js")}").stop()', - "replMode": True, - }, - } - _send_socket_request(sock, cdp_request) - - -def validate_ns(nspid: int, expected_ns_link_name: str) -> None: +def _validate_ns(nspid: int, expected_ns_link_name: str) -> None: actual_nspid_name = os.readlink(os.path.join("/proc", str(nspid), "ns", "pid")) assert ( actual_nspid_name == expected_ns_link_name ), f"Wrong namespace, expected {expected_ns_link_name}, got {actual_nspid_name}" -def validate_pid(expected_pid: int, sock: WebSocket) -> None: - actual_pid = _get_node_pid(sock) - assert expected_pid == actual_pid, f"Wrong pid, expected {expected_pid}, actual {actual_pid}" +def _validate_ns_node(sock: WebSocket, nspid: str, expected_ns_link_name: str) -> None: + command = f'const fs = process.mainModule.require("fs"); fs.readlinkSync("/proc/{nspid}/ns/pid")' + actual_ns_link_name = cast(str,_execute_js_command(sock, command)) + logger.debug(f"Found link by node: {actual_ns_link_name}") + assert ( + actual_ns_link_name == expected_ns_link_name + ), f"Wrong namespace, expected {expected_ns_link_name}, got {actual_ns_link_name}" + + +def _validate_pid(expected_pid: int, sock: WebSocket) -> None: + actual_pid = cast(int, _execute_js_command(sock, "process.pid")) + assert expected_pid == actual_pid, f"Wrong pid, expected {expected_pid}, actual {actual_pid}" def create_debugger_socket(nspid: int, ns_link_name: str) -> WebSocket: - validate_ns(nspid, ns_link_name) + _validate_ns(nspid, ns_link_name) debugger_url = _get_debugger_url() sock = create_connection(debugger_url) sock.settimeout(10) - validate_pid(nspid, sock) + _validate_ns_node(sock, nspid, ns_link_name) + _validate_pid(nspid, sock) return sock @@ -188,13 +190,13 @@ def _copy_module_into_process_ns(process: psutil.Process, musl: bool, version: s def _generate_perf_map(module_path: str, nspid: int, ns_link_name: str) -> None: sock = create_debugger_socket(nspid, ns_link_name) - _load_dso(sock, module_path) + _change_dso_state(sock, module_path, "start") def _clean_up(module_path: str, nspid: int, ns_link_name: str) -> None: sock = create_debugger_socket(nspid, ns_link_name) try: - _stop_dso(sock, module_path) + _change_dso_state(sock, module_path, "stop") finally: os.remove(os.path.join("/tmp", f"perf-{nspid}.map")) diff --git a/tests/test_node.py b/tests/test_node.py index 00c799d6e..0ed952f5d 100644 --- a/tests/test_node.py +++ b/tests/test_node.py @@ -1,3 +1,6 @@ +# +# Copyright (c) Granulate. All rights reserved. +# Licensed under the AGPL3 License. See LICENSE.md in the project root for license information. from pathlib import Path from threading import Event from typing import List From ba0b5b01fddd1d53b55a89549652f1edcb9f6339 Mon Sep 17 00:00:00 2001 From: "Wisniewski, Krzysztof2" Date: Mon, 3 Oct 2022 13:03:03 +0200 Subject: [PATCH 8/9] Review changes v5 --- gprofiler/profilers/node.py | 19 +++++-------------- gprofiler/profilers/perf.py | 2 ++ 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/gprofiler/profilers/node.py b/gprofiler/profilers/node.py index 4192d58ed..b36ab3a40 100644 --- a/gprofiler/profilers/node.py +++ b/gprofiler/profilers/node.py @@ -4,11 +4,11 @@ # import json import os -from pathlib import Path import shutil import signal import stat from functools import lru_cache +from pathlib import Path from threading import Event from typing import Any, Dict, List, cast @@ -144,17 +144,9 @@ def _change_dso_state(sock: WebSocket, module_path: str, action: str) -> None: _send_socket_request(sock, cdp_request) -def _validate_ns(nspid: int, expected_ns_link_name: str) -> None: - actual_nspid_name = os.readlink(os.path.join("/proc", str(nspid), "ns", "pid")) - assert ( - actual_nspid_name == expected_ns_link_name - ), f"Wrong namespace, expected {expected_ns_link_name}, got {actual_nspid_name}" - - -def _validate_ns_node(sock: WebSocket, nspid: str, expected_ns_link_name: str) -> None: - command = f'const fs = process.mainModule.require("fs"); fs.readlinkSync("/proc/{nspid}/ns/pid")' - actual_ns_link_name = cast(str,_execute_js_command(sock, command)) - logger.debug(f"Found link by node: {actual_ns_link_name}") +def _validate_ns_node(sock: WebSocket, expected_ns_link_name: str) -> None: + command = 'const fs = process.mainModule.require("fs"); fs.readlinkSync("/proc/self/ns/pid")' + actual_ns_link_name = cast(str, _execute_js_command(sock, command)) assert ( actual_ns_link_name == expected_ns_link_name ), f"Wrong namespace, expected {expected_ns_link_name}, got {actual_ns_link_name}" @@ -166,11 +158,10 @@ def _validate_pid(expected_pid: int, sock: WebSocket) -> None: def create_debugger_socket(nspid: int, ns_link_name: str) -> WebSocket: - _validate_ns(nspid, ns_link_name) debugger_url = _get_debugger_url() sock = create_connection(debugger_url) sock.settimeout(10) - _validate_ns_node(sock, nspid, ns_link_name) + _validate_ns_node(sock, ns_link_name) _validate_pid(nspid, sock) return sock diff --git a/gprofiler/profilers/perf.py b/gprofiler/profilers/perf.py index c0e307c92..66831f1fa 100644 --- a/gprofiler/profilers/perf.py +++ b/gprofiler/profilers/perf.py @@ -212,6 +212,8 @@ def __init__( assert self._perf_fp is not None or self._perf_dwarf is not None def start(self) -> None: + # we have to also generate maps here, + # it might be too late for first round to generate it in snapshot() if self.perf_node_attach: self._node_processes = get_node_processes() generate_map_for_node_processes(self._node_processes) From a5c1056106f66a49fd5f937cfdbdd47d026ab162 Mon Sep 17 00:00:00 2001 From: "Wisniewski, Krzysztof2" Date: Fri, 7 Oct 2022 11:45:42 +0200 Subject: [PATCH 9/9] Ignore error from dockerfile lint --- pyi.Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyi.Dockerfile b/pyi.Dockerfile index cdb2b3722..f976e7fe2 100644 --- a/pyi.Dockerfile +++ b/pyi.Dockerfile @@ -315,7 +315,7 @@ COPY ./scripts/list_needed_libs.sh ./scripts/list_needed_libs.sh # we use list_needed_libs.sh to list the dynamic dependencies of *all* of our resources, # and make staticx pack them as well. # using scl here to get the proper LD_LIBRARY_PATH set -# hadolint ignore=SC2046 +# hadolint ignore=SC2046,SC2086 RUN set -e; \ if [ $(uname -m) != "aarch64" ]; then \ source scl_source enable devtoolset-8 llvm-toolset-7 ; \