Bug 461289 - ApplicationLaunchJob should give each launched app its own labelled systemd-journald stream
Summary: ApplicationLaunchJob should give each launched app its own labelled systemd-j...
Status: CONFIRMED
Alias: None
Product: frameworks-kio
Classification: Frameworks and Libraries
Component: general (show other bugs)
Version: unspecified
Platform: Other Linux
: NOR wishlist
Target Milestone: ---
Assignee: KIO Bugs
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-11-01 13:12 UTC by Simon McVittie
Modified: 2023-10-04 09:09 UTC (History)
4 users (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Simon McVittie 2022-11-01 13:12:35 UTC
SUMMARY

On systems that use systemd, whenever Plasma starts an app that is not DBusActivatable, log messages in the systemd journal will "blame" `plasmashell` for its output. They should show the .desktop file name instead, so users can attribute messages to apps.

STEPS TO REPRODUCE

1. journalctl -f
2. Have an app that is *not* DBusActivatable=true, and logs distinctive messages to stdout/stderr. I used Steam.
3. Launch the app from an icon on the desktop (from the application menu or an applet would probably have similar behaviour)

OBSERVED RESULT

Messages with plasmashell as their "syslog identifier", like this:

$date $HOSTNAME plasmashell[12345]: steam.sh[12345]: running Steam on arch rolling 64-bit
$date $HOSTNAME plasmashell[12345]: Installing breakpad exception handler blah blah blah

EXPECTED RESULT

Messages with the .desktop filename or app ID as their "syslog identifier", similar to what GNOME Shell does:

$date $HOSTNAME steam.desktop[12345]: steam.sh[12345]: running Steam on ubuntu 22.04 64-bit
$date $HOSTNAME steam.desktop[12345]: Installing breakpad exception handler blah blah blah

SOFTWARE/OS VERSIONS

Linux: Arch Linux rolling release as of 2022-11-01
KDE Plasma Version: 5.26.2
KDE Frameworks Version: 5.99.0
Qt Version: 5.15.7

ADDITIONAL INFORMATION

The easiest way to implement this is to link to libsystemd, call sd_journal_stream_fd() to open new streams, and use dup2() to make them overwrite launched apps' stdout and stderr, like dbus does: https://gitlab.freedesktop.org/dbus/dbus/-/blob/dbus-1.14.4/dbus/dbus-spawn-unix.c#L1392

If linking to libsystemd is undesirable, it is fairly straightforward to reimplement sd_journal_stream_fd(), like GLib does: https://github.com/GNOME/glib/blob/2.74.1/gio/gio-launch-desktop.c

Plasma could either do this unconditionally (like dbus does), or only when it detects that stdout/stderr are already pointing to the Journal (like GLib does, sample implementation: https://github.com/GNOME/glib/blob/2.74.1/glib/gjournal-private.c)
Comment 1 Simon McVittie 2022-11-01 13:29:46 UTC
Note that steam.sh explicitly logs its own name and pid in each message, to make its messages easier to identify/attribute, which is where the `steam.sh[12345]` in my examples comes from.

If an app is writing random unattributed debug messages to stdout/stderr with no particular markers, then they'll look more like the `Installing breakpad exception handler blah blah blah` lines in my examples. The behaviour I'm asking for becomes particularly useful in this case.
Comment 2 David Edmundson 2022-11-01 14:52:25 UTC
FWIW, there is a mechanism that exists now:

exporting KDE_APPLICATIONS_AS_SERVICE=1

It's an alternative launching process in which we tell systemd to run our application, rather than us forking+exec'ing. Journald handling would then be resolved completely implicitly.

It's currently not default due to upstream changes being needed which we couldn't rely on at a KDE level. It might be worth revisiting that.
Comment 3 Bug Janitor Service 2022-11-01 16:57:24 UTC
A possibly relevant merge request was started @ https://invent.kde.org/frameworks/kio/-/merge_requests/1019
Comment 4 Simon McVittie 2022-11-01 21:06:14 UTC
If plasmashell (and presumably other ApplicationLaunchJob users) ends up exclusively using the transient service code path, then yes that would address this request.

GLib/GNOME has been trying to do something similar, but the people implementing it seem to have run into a lot of practical problems (perhaps the relevant GLib APIs expose more implementation details to the app than their KDE equivalents, making a change of implementation more likely to break things?), so we weren't able to switch over to transient services everywhere, and the stream-fd-based approach that I suggested here gave existing apps a small incremental improvement.
Comment 5 David Edmundson 2023-03-31 14:19:57 UTC
Git commit 5c0ce0f5882d732578be22ac6bb7e1d250e2046b by David Edmundson.
Committed on 31/03/2023 at 14:19.
Pushed by davidedmundson into branch 'master'.

Set ExitType when running applications as transient systemd services

KIO internally has 3 paths to launch applications.
 - fork and exec
- fork and exec then put in a "scope" so that we have our own cgroup
(default)
 - asking systemd to launch the application

The latter was explicitly disabled by default because we had some issues
with applications being cleaned up when the first process exited. We
made those upstream changes a year ago, but haven't adopted the changes
here. This brings it in line with the setup used by xdg-autostart-generator.

Nothing will change when we land this as this operation mode is not
enabled.
Related: bug 423756

M  +13   -11   src/gui/systemd/systemdprocessrunner.cpp

https://invent.kde.org/frameworks/kio/commit/5c0ce0f5882d732578be22ac6bb7e1d250e2046b
Comment 6 Bug Janitor Service 2023-04-03 08:44:53 UTC
A possibly relevant merge request was started @ https://invent.kde.org/frameworks/kio/-/merge_requests/1244
Comment 7 David Edmundson 2023-04-03 09:18:49 UTC
Git commit 0c540119248c1eb373eaf2ee323094798897967a by David Edmundson.
Committed on 03/04/2023 at 08:44.
Pushed by davidedmundson into branch 'kf5'.

Set ExitType when running applications as transient systemd services

KIO internally has 3 paths to launch applications.
 - fork and exec
- fork and exec then put in a "scope" so that we have our own cgroup
(default)
 - asking systemd to launch the application

The latter was explicitly disabled by default because we had some issues
with applications being cleaned up when the first process exited. We
made those upstream changes a year ago, but haven't adopted the changes
here. This brings it in line with the setup used by xdg-autostart-generator.

Nothing will change when we land this as this operation mode is not
enabled.
Related: bug 423756


(cherry picked from commit 5c0ce0f5882d732578be22ac6bb7e1d250e2046b)

M  +13   -11   src/gui/systemd/systemdprocessrunner.cpp

https://invent.kde.org/frameworks/kio/commit/0c540119248c1eb373eaf2ee323094798897967a