GNOME Bugzilla – Bug 757571
log messages to syslog
Last modified: 2017-01-05 15:23:29 UTC
Created attachment 314797 [details] [review] log messages to syslog all messages are being log under gnome-session, which makes ABRT crash reports lack any journal output. This makes debugging bugs much harder
wont this result in all messages going to the journal twice?
I feel like we should have something like this as an API in glib, so you can easily set it up. Also, why not the sd_journal API? I didn't think syslog was per-user.
(In reply to Jasper St. Pierre from comment #2) > I feel like we should have something like this as an API in glib, so you can > easily set it up. We want to get this into the 3.18.x packages for Fedora - so we'll at least need a mutter-only patch - though that could just be carried in Fedora packaging. > Also, why not the sd_journal API? I didn't think syslog was per-user. From the systemd docs, the difference is pretty small - the main thing to be aware of is that mixed syslog() and sd_journal_* calls don't order correctly. Using the sd_journal_* calls allows you to add extra fields, but I don't know what fields we'd want to add. I don't think it matters much. The gdm-log.c code in gnome-session and gdm uses syslog(). (syslog() avoids a dependency if we eventually move it into GLib as well.) It's worth comparing to the gdm-log code, or maybe even using that - I think this code is double-logging as Ray says, and also losing the log-domain, so GTK+, JS, etc, errors get mixed together without distinction. I think it's probably good to conditionalize this with isatty(stderr_fd) - at least as long as we're supporting gnome-shell as an X compositor, it's handy to be able to replace gnome-shell and see your error messages on the terminal.
so I mean this is probably fine, but I think we could solve this more generally in gnome-session by having it set stdio up to use a custom identifier before execing the child.
though there's also dbus activation to think about.
Created attachment 314921 [details] [review] autostart-app: give ever app its own journal id Right now all session output gets attributed to gnome-session which isn't very useful. This commit makes sure launched apps each get their own journal identifier.
so the above is untested, but I just realized it has a big problem (other than the dbus activation complication)... Things launched from the shell will get misattributed to the shell, so it's just spreading the problem around a little.
(In reply to Ray Strode [halfline] from comment #7) > Things launched from the shell will get misattributed to the shell, > so it's just spreading the problem around a little. Wasn't that supposed to be fixed in bug 711626?
(In reply to Florian Müllner from comment #8) > (In reply to Ray Strode [halfline] from comment #7) > > Things launched from the shell will get misattributed to the shell, > > so it's just spreading the problem around a little. > > Wasn't that supposed to be fixed in bug 711626? heh. yea, I guess so. I didn't know about that. And the shell is the main thing we start that can start other independent things, so indeed that might be good enough.
Review of attachment 314921 [details] [review]: ::: gnome-session/gsm-autostart-app.c @@ +1037,3 @@ + + if (standard_output >= 0) { + dup2 (standard_output, STDOUT_FILENO); i'm forgetting a close() here and below
an argument could be made that GDesktopAppInfo should be doing this stream fd stuff for us I guess.
and that bug you pointed to points to another bug that says the dbus activation problem was already fixed too. I think this really is a better way to go than doing it in mutter. Marek, Owen, sound good? Assume it tests out fine, i'll push it and do a 3.19 release.
So advantages of the gnome-session approach: - Simple - Fixes gnome-settings-daemon, etc, not just gnome-shell Advantages of direct logging: - Messages are logged with the right priority - Multiple line messages are logged as a single message - No stdout/stderr ordering issues I think we should go ahead with fixing gnome-session, unless it goes into GDesktopAppInfo. I could see a gnome-shell patch as well, but it's pretty low priority.
So the other advantage of doing it in gnome-session is you get stdout/stderr not just g_logv(). most stuff of interest goes through g_logv but it would nice if a stray g_printerr or whatever didn't still get misattributed to gnome-session. But just to follow up, there was a bit of discussion about this on IRC with Jasper, Owen and Matthias, and it seems everyone in the conversation was on board with this going in GDesktopAppInfo. Matthias gives his "blessing". We, of course, still need to get desrt's take.
My input: I'm not really keen on adding more complication to the fork()/exec() case when we're trying pretty hard to convince everyone to be using d-bus activation instead. There are just too many things that we could try to handle if we want to keep adding patches here and we'll never cover all of the cases, because no matter what we do, the thing will never end up in the correct cgroup. We could talk then about using the journal for improving log output that was sent via g_log() and I think this might be interesting, but only if we can find a way to keep the log output on stderr when apps are started in a terminal. Put another way: if there is some nice way to detect when stderr is a journal log stream and produce better output for it, I'm totally happy to do that (ideally without systemd library dependencies), but I don't like these other approaches very much.
A thought: I would be really happy to see some session service that I can send a D-Bus message to for "launch this desktop file" for apps that don't do D-Bus activation. This service would be responsible for setting up all of this logging stuff, making sure the app ends up with the right environment, right cgroup, etc.
chatted with desrt a bit about this on irc... we came to the conclusion we could write a systemd generator to make sure non dbus-activatable desktop files get a systemd --user service file written in /run. Then we could change GDesktopAppInfo to start those programs using the systemd dbus api and they'll get the right logging for free as well as get us out of the fork()/exec() game.
(comment 16 and comment 17 were a midair collision)
since you want to push an update into 3.18 I believe, and the scope of the glib changes balooned a bit, I think we should move this bug to gnome-session for the near term work around. for 3.19 we may also want to land the gnome-shell patches in bug 741666 so gnome-shell starts being dbus activatable which would attack this problem from the other direction.
Comment on attachment 314921 [details] [review] autostart-app: give ever app its own journal id I added the close calls to attachment 314921 [details] [review] and tested it. seems to work fine. Attachment 314921 [details] pushed as 5449174 - autostart-app: give ever app its own journal id
i started to sketch out a desktop file <-> systemd generator this morning here: https://git.gnome.org/browse/gnome-session/commit/?h=wip/desktop-generator&id=680cd621c0b526c2ceab760a7cf52831bb518b6f not even compile tested yet, because as I realized this should probably go in systemd actually alongside the dbus generator, so will need to be done differently.
(attachment 314921 [details] [review] landed a while ago, closing)