tracebox: Explicit daemon management #3726

sashwinbalaji · 2025-11-19T00:41:39Z

Introduce tracebox ctl for explicit daemon lifecycle management
Default behavior now required daemons to be running
--autodaemonize flag preserves old auto-spawn behavior
--system-sockets deprecated with a warning printed on usage

Fixes: b/458318005
Fixes: #3540

LalitMaganti

Biggest request: please ensure you treat this CL with the gravity and importance it deserves. We try very hard in Perfetto to not make big breaking changes and when we do, it's really important that we get ahead of questions and make sure people proactively are todl exactly what they need to do. For example, when I broken people with the track migration last year, I sent out https://perfetto.dev/docs/analysis/perfetto-sql-backcompat

For example I think people should be told if they are doing tracebox by default, they should really try to switch to using ctl or if they just want to unblock as fast as possible, to use --autodaemonize for old beheaviour.

I also think this might require us to change some of our docs pages: please do an audit and make necessary changes.

docs/reference/tracebox.md

include/perfetto/base/build_configs/bazel/perfetto_build_flags.h

gn/perfetto.gni

src/tracebox/tracebox.cc

CHANGELOG

sashwinbalaji · 2025-11-19T01:21:43Z

Thanks for the review:

for communication plan was to send out mail(+announcement) will look into https://perfetto.dev/docs/analysis/perfetto-sql-backcompat
regarding doc it's in pipeline as a separate cl as there are multiple files using it: https://source.chromium.org/search?q=tracebox%20&sq=&ss=chromium%2Fchromium%2Fsrc:third_party%2Fperfetto%2Fdocs%2F
Switching to draft until windows mode is aligned, tracebiox.md rereviewed and try out --background option instead of reinventing the wheel

github-actions · 2025-11-21T11:04:59Z

🎨 Perfetto UI Build

✅ UI build is ready: https://storage.googleapis.com/perfetto-ci-artifacts/gh-19646592328-ui/ui/index.html

LalitMaganti · 2025-11-21T21:16:45Z

Overall lgtm but I don't quite understand the systemd changes. Can we maybe pull that out into a separate cl? The rest looks really good!

- Introdcue `tracebox ct` for explicit daemon lifecycle management - Default behavior now required daemons to be running (linux/macos) - `--autodaemonize` flag preserves old auto-spawn behavior - `--system-sockets` deprecated with a warning printed on usage - Windows maintains backward compatibility by auto-spawning with system sockets - Systemd checks to nudge user to use them Fixes: b/458318005 Fixes: #3540

primiano

Awesome work. In hindsight, no this was not a fixit item :D live and learn

BUt we are here now, so let's go for this.

I have a bunch of comments.
Also as a meta comment: do we need to support windows? can we just error out on OS_WIN for now?
I doubt anybody would ever use tracebox on windows. what for? traced_probes/perf don't exist. the SDK cross-process is "weird" (it uses TCP, nobody will want that in production) . Windows cross-process support exists really because I didn't want to #ifdef out code all over the places, and was easier to add support over a TCP socket.
but it's really a 3rd class citizen

primiano · 2025-11-27T15:27:34Z

src/tracebox/tracebox.cc

+  > tracebox -t 10s -o trace.pftrace sched
+  > tracebox ctl stop
+
+MODE 2: Autodaemonize mode


I'd call this

MODE 2: --autodaemonize mode (legacy behavior, discouraged)

primiano · 2025-11-27T15:49:02Z

src/tracebox/tracebox.cc

+      memmove(static_cast<void*>(&argv[i]), static_cast<void*>(&argv[i + 1]),
+              sizeof(char*) * static_cast<size_t>(argc - i - 1));


I think you can do just:

std::move(argv + i + 1, argv + argc, argv + i);

primiano · 2025-11-27T15:50:27Z

src/tracebox/tracebox.cc

+  if (autodaemonize) {
+    if (use_system_sockets) {
+      PERFETTO_ELOG(
+          "Warning: --system-sockets with --autodaemonize is supported but "


i'd say "supported for legacy reasons"

primiano · 2025-11-27T15:52:54Z

src/tracebox/tracebox.cc

+  }
+
+  if (use_system_sockets) {
+    PERFETTO_FATAL(


Fatal seems a bit aggressive given that this is just the new default?
I would

Move this message in the for loop above. there i sno need to set a variable, just to emit a lot here:

SOften this into a PERFETTO_ELOG ?

primiano · 2025-11-27T15:53:49Z

src/tracebox/tracebox.cc

-and tearing down the services for you.
+cmdline client in one binary. It can be used in two modes:
+
+MODE 1: Daemon mode (Recommended)


Add a (Default behavior, reccomended)

primiano · 2025-11-27T16:15:55Z

src/tracebox/tracebox_ctl.cc

+    if (pid && kill(pid, SIGTERM) == 0) {
+      daemons_stopped++;
+      printf("%s terminated (PID: %d)\n", daemon, pid);
+      if (remove(pid_path.c_str()) != 0) {


shouldn't the remove be done by the process?
Otherwise you can you tell when a process has actually ended?
There is an expectation that "ctl stop" waits for the termination, so you can call start > stop > start and it should work.

I think the way this could work is: the daemon removes the pid when it quits, and here you poll for the deletion of the pid

primiano · 2025-11-27T16:16:57Z

src/tracebox/tracebox_ctl.cc

+  ServiceSockets sockets = GetRunningSockets();
+  if (sockets.IsValid()) {
+    printf("Status: Daemons are already running with %s\n",
+           sockets.ToString().c_str());


I'd add something more helpful like ". Maybe the daemons have been started via systemd/android init or other means?"

primiano · 2025-11-27T16:17:10Z

src/tracebox/tracebox_ctl.cc

+  constexpr uint32_t kDaemonStartWaitUs = 1000 * 1000;  // 1s
+  ServiceSockets sockets = GetRunningSockets();
+  if (sockets.IsValid()) {
+    printf("Status: Daemons are already running with %s\n",


primiano · 2025-11-27T16:19:12Z

src/tracebox/tracebox_ctl.cc

+    printf("%s started (PID: %d)\n", daemon, pid);
+  }
+
+  base::SleepMicroseconds(kDaemonStartWaitUs);


imho this needs to be a poll of some sort. 1 s is too low on an emulator, and if you go higher, it's annoying to wait that long.
Poll every 250ms for at most 5s?

primiano · 2025-11-27T16:21:10Z

src/tracebox/tracebox_ctl.cc

+}
+
+int CtlStatus() {
+  ServiceSockets sockets = GetRunningSockets();


I'd be a bit more pedantic here. I'd check both for (1) GetRunningSockets(); (2) the existance of pid files (at least just for traced)

If 1 && 2: you are happy
if 1 && not(2) you say "The daemon seems running but the pid file doesn't exist. looks like something else started the daemon (systemd? initrc?). Use lsof ... (figure out the command) for details"

sashwinbalaji requested a review from a team as a code owner November 19, 2025 00:41

LalitMaganti reviewed Nov 19, 2025

View reviewed changes

sashwinbalaji force-pushed the dev/sashwinbalaji/tracebox_applet branch from 1ffd9b3 to daf629a Compare November 19, 2025 01:19

sashwinbalaji marked this pull request as draft November 19, 2025 01:21

sashwinbalaji force-pushed the dev/sashwinbalaji/tracebox_applet branch 5 times, most recently from cdcd105 to 455bcb1 Compare November 20, 2025 16:56

LalitMaganti mentioned this pull request Nov 20, 2025

Improve UX for combined in-application and system event trace on Linux #3437

Open

sashwinbalaji force-pushed the dev/sashwinbalaji/tracebox_applet branch from 4d449d9 to 1ea1253 Compare November 21, 2025 10:49

sashwinbalaji force-pushed the dev/sashwinbalaji/tracebox_applet branch 5 times, most recently from fafffcf to ee510ae Compare November 21, 2025 19:25

sashwinbalaji marked this pull request as ready for review November 21, 2025 19:26

sashwinbalaji added 10 commits November 25, 2025 03:13

fix#1

e6cf8fc

fixx

c7aaabd

f

f41f5cc

f

d55a7c4

build on windows

42b3ce4

fix ctl for windows

eb2ea36

add todo

3cad59f

fix

18130d5

f

ecf6b72

remove sudo

a8cde29

sashwinbalaji force-pushed the dev/sashwinbalaji/tracebox_applet branch from ee510ae to 9ba1b54 Compare November 24, 2025 19:23

remove systemd

d3730a8

sashwinbalaji force-pushed the dev/sashwinbalaji/tracebox_applet branch from 9ba1b54 to d3730a8 Compare November 24, 2025 19:25

primiano reviewed Nov 27, 2025

View reviewed changes

		memmove(static_cast<void>(&argv[i]), static_cast<void>(&argv[i + 1]),
		sizeof(char) static_cast<size_t>(argc - i - 1));

tracebox: Explicit daemon management #3726

Are you sure you want to change the base?

tracebox: Explicit daemon management #3726

Uh oh!

Conversation

sashwinbalaji commented Nov 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

LalitMaganti left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sashwinbalaji commented Nov 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🎨 Perfetto UI Build

Uh oh!

LalitMaganti commented Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

primiano left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sashwinbalaji commented Nov 19, 2025 •

edited

Loading

sashwinbalaji commented Nov 19, 2025 •

edited

Loading

github-actions bot commented Nov 21, 2025 •

edited

Loading

LalitMaganti commented Nov 21, 2025 •

edited

Loading