lloesche / valheim-server-docker

Valheim dedicated gameserver with automatic update, World backup, BepInEx and ValheimPlus mod support
https://hub.docker.com/r/lloesche/valheim-server
Apache License 2.0
1.94k stars 271 forks source link

supervisor can't fetch child process logs (because it doesn't keep them) #243

Closed greyltc closed 3 years ago

greyltc commented 3 years ago

If I visit the supervisor web interface and try to view a process' logs by clicking on its name, I get:

ERROR: unexpected rpc fault [30] FAILED

The tail links also don't seem to work. It would be cool to be able to use that web interface to actually view and monitor the log files, but that seems impossible today.

I've had a look at the supervisord config file used here. https://github.com/lloesche/valheim-server-docker/blob/7e9ac2961b29c7c3ea4370a37050579b5e4cc884/supervisord.conf#L26 https://github.com/lloesche/valheim-server-docker/blob/7e9ac2961b29c7c3ea4370a37050579b5e4cc884/supervisord.conf#L28 I think the way the _logfile parameters are configured means the child process output is stolen from supervisor's management and dumped to the terminal. This means the web interface can't produce the logs on demand because supervisor doesn't have them on disk. If I look into, childlogdir (/var/log/supervisor) in the container, there are no child logs there. I think this also breaks a bunch of supervisor's XML-RPC goodness too because it has no logs.

A fix for this is to change all the _logfile parameters to AUTO (or just delete them since they default to that). But I guess you're redirecting the output to /dev/stdout//dev/stderr for some reason. One thing you could do is add loglevel=debug to the [supervisord] section to get the child processes to again print to the terminal maybe?

lloesche commented 3 years ago

See the README.

lloesche commented 3 years ago

The logs are written to stdout/stderr because this is a container. You want the logs on your host not fill up the container space.

greyltc commented 3 years ago

I bet there's a way to do this without breaking supervisor's ability to fetch historical logs. If you'd like to keep the log files out of the container's file system, then why not tell supervisor to store its the child logs in a docker mapped folder?

By the way, https://github.com/lloesche/valheim-server-docker/blob/7e9ac2961b29c7c3ea4370a37050579b5e4cc884/supervisord.conf#L5 logs to the container's file system right now.

lloesche commented 3 years ago

I bet there's a way to do this without breaking supervisor's ability to fetch historical logs

Sure, I can think of several workarounds. You could capture STDOUT_FILENO/STDERR_FILENO of each process you're interested in in /proc/<pid>/fd/ and write to stdout. Or tee the processes outputs to log files, or let supervisor write the logs and have another service tail those logs back to stdout/stderr. The only clean way is to patch supervisord and add an option to write logs to multiple locations so they can be written to /dev/stdout as well as a seekable file on disk.

I think the easiest way to cover your use case would be to just volume mount /usr/local/etc/supervisord.conf with your modifications. Or I can add you a PRE_SUPERVISOR_HOOK that will allow you to modify the supervisor config any which way you like without breaking existing user's setups.

supervisord.log logs to the container's file system right now

That's why I wrote you don't want it to fill up the container space. The supervisor log only logs service startup/restart events. I.e. it grows by few log line per day so hardly qualifying as "filling up the container". While all the other logs are growing much more frequently depending on your server's activity. On our server supervisor.log grows around 2kb/day and stdout/stderr go into an elastic cluster at 80-100 MB/day. ValheimPlus used to be the biggest offender here as it logged its entire config every time a player connects. They fixed that in the latest release.

You can of course limit supervisor to truncate those logs at certain sizes but then you're losing those old logs. So I guess the ideal situation would be if there was in-container logs of few MB but also written to stdout. Like I mentioned the best place to do this cleanly is to submit a supervisord patch that allows to write multiple output files.

lloesche commented 3 years ago

While looking over the supervisor sources I found another possible option here: https://github.com/Supervisor/supervisor/blob/9f030973ec283d3d392bb2614733ecfae2dc4dcb/supervisor/rpcinterface.py#L584

It seems you can redirect all output to syslog. Which we've already got running inside of our container. I'm just not sure if it'll then still honor the file options. If that was the case we could write stdout/stderr to syslog as well as the logfiles and have supervisor truncate the logs. Then you would have the logs in the supervisor web UI and nobody else's setup would break.

lloesche commented 3 years ago

Ok, it's looking promising, let's reopen this issue.

lloesche commented 3 years ago

@greyltc see if lloesche/valheim-server:dev fixes your use case.

greyltc commented 3 years ago

I'll test it shortly! Thanks!

lloesche commented 3 years ago

fyi its merged to main. There's also a couple of new syslog and log filter hook options.

greyltc commented 3 years ago

Seems pretty great to me! Supervisor is pretty cool