Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Log runtime configuration on SIGUSR2 #36

Open
realsimix opened this issue Aug 10, 2021 · 8 comments
Open

Log runtime configuration on SIGUSR2 #36

realsimix opened this issue Aug 10, 2021 · 8 comments

Comments

@realsimix
Copy link

Hi Maxim,

I'm coming back to you with an idea to improve configuration management.

While we can use spampd --show config to show the current config, it shows only the config options defined in config files.

Would it be possible to add a signal handler so that one can send a SIGUSR2 to the master spampd process to it will send the current runtime config to the log? (The idea is shamelessly taken from OpenVPN which logs statistics on SIGUSR2)

That way it would be possible to see what spampd really runs, even with a combination of command options and config files.

Unfortunately I don't know perl enough to do this myself. I saw that Net::Server handles HUP and provides a hook for it but is it possible to handle USR2 with Net::Server?

Thanks,
Simon

@mpaperno
Copy link
Owner

Hi Simon,

I should certainly be possible to add a custom SIGUSR2 handler (on any/all processes).

What format should the logged output be in? Same as show --config with one option per line? All options on one line? The format could be anything.

Also to clarify, you mean a dump of all the current config options (like show --config), not just the options passed to spampd on startup (like show --start and as logged on main process startup)... correct?

show --config should take into account any arguments on the actual command line as well as in config files (if it doesn't then it's a bug), but of course one would have to exactly re-create the full command line for this to be effective (which I think is what you meant, but just making sure).

Cheers,
-Max

@realsimix
Copy link
Author

Hi Maxim,

I'm not sure about the format in syslog, so maybe something without lots of spaces or tabs, like so:

SpamPD v2.61 [Perl 5.10.1, Net::Server::PreForkSimple 2.007, SA 3.4.6, rules v1892106]
[running config start]
host=127.0.0.1
local-only=0
log-rules-hit=0
logfacility=mail
logfile=syslog
logident=spampd
logsock=(undefined)
max-servers=5
max-spare=4
maxrequests=20
maxsize=64
[running config end]

What do you think?

The show --config works correctly if you reproduce exactly the same command line and arguments.

The problem which could be solved with the SIGUSR2 is that you can lose track on what your currently running instance is running exactly. Maybe you modified several config files and you don't know if your running instance is up to date. Maybe you forgot to send SIGHUP or you don't remember whether you did it or not. Just send a SIGUSR2 and you can read the running config in the logs.

Regards,
Simon

@realsimix
Copy link
Author

Hi Maxim,

Wow that's cool and it works very well, thanks!

Sorry to be a pain but the ideas are just coming while testing:

  1. Maybe you could change this to make to out look less terse (does it still work that way if used in a config file with the spaces)?
@@ -1546,7 +1546,7 @@
   $self->inf("[running config start]\n");
   my $result = resolve_options({$self->options_map()});
   for my $k (sort keys %$result) {
-    $self->inf(sprintf("%s=%s\n", $k, $result->{$k}));
+    $self->inf(sprintf("%s = %s\n", $k, $result->{$k}));
   }
   $self->inf("[running config end]\n");
 }
  1. What would be really nice in the output of --show config and SIGUSR2 is to have default config options show the leading # and modified (non default) options without # . That way one could even better use the config out as a real config file:
auto-whitelist = (undefined)
# childtimeout = 360
# debug = (undefined)
detach = 0
# dose = 0
group = 985 985
homedir = /var/spool/spampd
# host = 127.0.0.1
port = 10028
relayhost = 127.0.0.1:10029

What do you think?

@mpaperno
Copy link
Owner

mpaperno commented Aug 12, 2021

Hi Simon,

  1. Maybe you could change this to make to out look less terse (does it still work that way if used in a config file with the spaces)?

Done. And yes the extra spaces are ignored in config files.

As for the 2nd point, the "problem" is that the defaults are only known before any config options are processed. IOW the default values aren't stored separately anywhere, so once config options are processed, the defaults are effectively unknown. We could of course store the defaults as a copy somewhere and keep them loaded, and then we could present them as you suggest. But I'm not really sure the extra complexity or storage space is warranted...

Ultimately a --show defaults,config (with a full config setup) already shows all the necessary parts, albeit not in a format quite as nice/simple as you suggest. I'm also not quite seeing how this would be useful in an already-running SpamPD... after all, the options must have come from somewhere in the first place, right? Copy/pasting from a log file doesn't seem very helpful at that point (but there's probably a use case I'm not thinking of).

So, I'm undecided on that 2nd part but open to further arguments for it... :)

-Max

@realsimix
Copy link
Author

Hi Maxim,

From trying to understand the code I thought it's how you describe it.

My idea was to store the initial config into a defaults variable/array/object at startup and keep it. Then, when doing the --show config or SIGUSR2, while producing the output, every parameter could be checked against the defaults parameter and then decided to print it with or withpout # .

Would this be possible without too much headaches?

Thanks,
Simon

@realsimix
Copy link
Author

Hi Maxim,

Ping, are you still looking into this?

If not, it's also fine as the current version works very well.

Thanks,
Simon

@mpaperno
Copy link
Owner

Hi Simon,

I was looking for some justification for implementing all this, but haven't really heard it. Even my current version (from the related PR) has an issue that it cannot properly display some config settings because the variables have already been overwritten (eg. everything stored in the self.assassin class member). As I stated previously, I'm just not sure this feature is worth the hassle or, especially, the extra code.

Best,
-Max

@realsimix
Copy link
Author

No problem, I like the way it is now but I can also live without the feature.

Regards,
Simon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants