Support logging to file

[emizzle]

Closes #554.

As in #554, this supports logging to a file, however in this PR, it is accomplished via a dynamic Chronicle's sink. The reason for a dynamic sink (textlines[dynamic]) is so that file logging can be disabled, because Chronicles' default behavior for textlines[file] is to log to a default file.

The main drawback, as discussed in #554 is that the file is not closed, which is no better than Chronicles. I did attempt to return the opened file from setupLogging and subsequently close it (on SIGINT and SIGTERM), however Chronicles continues to log past these events, so handling of file close would need to come after Chronicles is done with it (I'm not entirely sure if this is doable).

[gmega]

I think the main drawback I see with this is that if you want to have the output match the other switches (e.g. write JSON instead of text lines when log-format is JSON) you'll need to add another sink, and then some extra logic to activate the right sinks and mute the other ones depending on the switch. But if we don't care about JSON, then I guess this is just as good, with the upside that the sink is explicitly stated in the config.

But now @dryajov has two options to choose from. 😁

[emizzle]

If we swapped the newly added (index 2) textlines[dynamic] in config.nims with json[dynamic], then I assume json output would be written to the file instead of text lines, with no new logic needed.

[gmega]

This would work, but it would also mean we'd now have two different ways to get JSON output depending on whether it's to stdout or to a file. It could also violate the principle of least surprise as people seeing the log-format switch might assume that it affects file output as well, but it won't.

And I'm also somewhat unclear on this (not sure how these nims files work), but you'd also need to recompile to make the change go into effect?

[emizzle]

Yes, it would need a recompile for the changes to take effect, however they wouldn't compile because the properties of a json Chronicles output are different than a dynamic textlines output, and as such, outputs[2].writer wouldn't exist.

In my opinion, end users shouldn't (or wouldn't) be swapping these Chronicles options out in config.nims (which simply serves as cli flags during Codex compilation) as end users will use a Codex binary and its own cli flags, eg --log-file=.

[arnetheduck]

fwiw, we're looking to remove this option from nimbus-eth2, instead guiding users to log-rotating redirection tools (ie tee/logrotate etc - logging to file is fraught with issues such as rotation, file ownership etc which chronicles handles poorly at best leading to a sub-par user experience in general.

[gmega]

I did bring this up, @arnetheduck, but the concern that got raised was that we need to support environments where such tools might not be available (e.g. Windows). Though in all honesty I don't think anyone of us bothered to check if Windows had a tee equivalent, which it apparently does (https://superuser.com/questions/74127/tee-for-windows).

[dryajov]

is this printing the entire config object?

[emizzle]

Yes, perhaps we can add a separate trace/debug that prints it. It's very useful for debugging.

[emizzle]

On second thought, this could be useful info for the user. We can always downgrade the log level later if we dislike it.

[dryajov]

I think it can get somewhat large tho, maybe trace is more appropriate?

[emizzle]

It's not too bad, and shows default values that were not specified by the user. But i can change it to trace.

[dryajov]

LGTM.

This seems to make our life easier right now, tho I agree that we're probably better off using tee/logrotate in prod, this seems useful for debugging in some scenarios.