refactor: logrotate setup + rspamd log path + tests log helper fallback path

[georglauterbach]

Description

In order to allow users to more easily go through log files, I added most to the set of packages we install. This adds +122kB in size, which I think is negligible.

Moreover, I added a section to the Rspamd documentation about where to find the corresponding log files.

I adjusted this PR to only include changes to logrotate and to the Rspamd log. In detail, I improved _setup_logrotate by simplifying it and making it more readable. Moreover, I adjusted the Rspamd log file location and added a logrotate config for the Rspamd log file.

I will provide a follow-up PR that includes changes about adding and removing packages, as discussed here.

Associated: #3570

Type of change

• Improvement (non-breaking change that does improve existing functionality)
• This change is a documentation update

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation (README.md or the documentation under docs/)
• If necessary I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes

[casperklein]

Instead of less, we could use most. It's the newest and feature rich pager tool:

more < less < most.

[georglauterbach]

Instead of less, we could use most. It's the newest and feature rich pager tool:

more < less < most.

Done :D

[reneploetz]

Just a general thought:

Is there a specific reason to put the rspamd.log inside the container instead of the central (usually volumed) logging directory in /var/log/mail ?

I'm currently using the following patches to add logrotate + change the path:

/etc/rspamd/local.d/logging.inc
filename = "/var/log/mail/rspamd.log";

/etc/logrotate.d/rspamd

/var/log/mail/rspamd.log
{
  compress
  copytruncate
  delaycompress
  rotate 4
  weekly
}

[polarathene]

Is there a specific reason to put the rspamd.log inside the container instead of the central (usually volumed) logging directory in /var/log/mail ?

supervisord manages the services by running them in the foreground with stdout + stderr logs written to files in /var/log/supervisor/${service}.log. Often these logs are empty / minimal or repeated in the main mail log.

I imagine it's just been that way since supervisor was introduced. There has been talk about refactoring how we manage logging, where Vector may be suitable. Ideally each service will keep it's logs separate and Vector would support configuration to output a log that combines those streams at whatever log level desired. It wouldn't be fixed to /var/log/mail then either.

[reneploetz]

Yeah, that sounds good.

My reasoning for the comment was somewhat along the lines of: If you write to /var/log/supervisor you will likely write inside the container as well as not performing any log rotation. This does not matter for small files, but might matter for larger one - especially on systems with many users/mail and/or very infrequent restarts.

If I look at the file sizes of the files in /var/log/supervisor on my system I can see that they are either empty or really small (with the exception of saslauthd_ldap.log for me). The rspamd.log also has a significant size (tough I'm using the Abuseix RBL - mileage may vary here).

For me it's just a bit odd that we use a somewhat centralized logging directory + logrotate for many things mail-related, but not here. But this is also the wrong ticket to discuss this anyway, so this is just a +1 for centralized logging.

[georglauterbach]

I'll come up with an update tomorrow

[georglauterbach]

I adjusted the PR so that the changes proposed by @reneploetz are incorporated. I will provide another PR that introduces updates to debug-packages later to have this PR only care about Rspamd and its logs.

The Rspamd logs are now managed by logrotate as well. Until Vector is used, I think this is the best way to move forward.

[reneploetz]

Thanks for the updated pull request!

[polarathene]

No action required

This is much more readable than the previous approach with the split { + } that looked almost like a typo 🚀

Regarding rotate 4, I guess that makes sense for weekly, while a bit odd for daily and monthly. I'm assuming git blame would show this was introduced with weekly in mind.

Given you've dropped the switch statement and no one seems to be bothered I guess leaving it at 4 here is fine, I was just a little curious what that setting did when I saw it here.

[casperklein]

I stumbled upon the same while reviewing. We could introduce $LOGROTATE_COUNT or (not so readable) $LOGROTATE_ROTATE to make the rotation count configurable.

[polarathene]

It should be ok, no need to add new ENV to support this until we get actual requests to adjust the default.

[github-actions]

Documentation preview for this PR is ready! 🎉

Built with commit: 011b298