I would like bitcoind to put its log file in /var/log/bitcoin. This PR adds a new -logdir option that controls where the debug.log file is put.
Note: This flag affects bitcoind and bitcoin-qt. There appears to be a db.log file that is used for wallets, but I didn't update that code path. It would be a minor change to update that as well.
Concept ACK If only one file moves, I'd prefer just -log rather than logdir though
193 | +static fs::path GetLogDir() 194 | +{ 195 | + if (gArgs.IsArgSet("-logdir")) { 196 | + fs::path logdirPath = fs::system_complete(gArgs.GetArg("-logdir", "")); 197 | + if (!logdirPath.empty()) { 198 | + fs::create_directories(logdirPath);
Not sure if it should create directories,.. though if it does, in case the creation failed, it should not return an empty path, should rather return GetDataDir() in any failed case.
I don't think the code (as written) can return an empty path. If -logdir is unset the outer if statement is skipped completely, the inner if statement is to handle the case where -logdir is set to the empty string. As I understand the boost::filesystem docs, if the mkdir fails then fs::create_directories will actually throw an exception.
I'm OK with removing the fs::create_directories call, the precedent I was following is that GetDataDir() creates its directory, although that's a bit different.
Hmm.. not sure. I just found this in the docs: Returns: true if a new directory was created, otherwise false.
As I understand the documentation: fs::create_directories returns true if it actually had to issue a mkdir(2) call, and returns false if it did not (because the directory already existed). If the directory needs to be created and the underlying mkdir(2) call failed, then an exception is raised.
Oh.. yes.. your completely right. Nevermind then.
@MeshCollider Do you think it's possible there would be other log files in the future? I'm interested in the future on working on a multi-process architecture for bitcoind, so that parts of the process could be sandboxed on Linux using seccomp(). In that world I'd want a log directory so there could be a log file per process, but maybe I'm overthinking things since there's just one log file today.
Running regtest and testnet will use the same log file?
@eklitzke. I think what @promag means is, that in the default configuration (without the PR), the debug.log file is placed in <datadir>/testnet/ or <datadir>/regtest/ in case of testnet / regtest. Your logdir command would required one to switch the logdir when switching to -regtest, etc. Not sure if it's a real problem...
If only one file moves, I'd prefer just -log rather than logdir though
Would prefer just -logfile or such too. The log is just a text file, it needs to claim no environment, and a directory is not a good abstraction for it. If someone wants bitcoin to log to an existing directory with other log files e.g. /var/log/bitcoinlog that should be possible. The only requirement is write access.
so that parts of the process could be sandboxed on Linux using seccomp().
FWIW in my experiments with running bitcoin with CloudABI I've used -printtoconsole to log to a stream instead. This is much easier in restricted environments, and the stream can be processed and stored anywhere desired by an external process. I usually recommend that for more advanced logging setups.
Do you think it's possible there would be other log files in the future?
Possible - maybe an auditing / RPC log instead of a debug log. However there is no reason it'd need to be in the same directory, this could have a new option to place it.
There appears to be a db.log file that is used for wallets, but I didn't update that code path. It would be a minor change to update that as well.
db.log belongs with the wallet directory, no need to touch it here.
@laanwj what do you mean? Currently there is debug.log, testnet3/debug.log and regtest/debug.log.
Yes, the regtest/testnet edge case again. It really sucks that we have these configuration options shared between networks. It's not clear that users even want that layout when they manually specify the logfile! A similar issue to #11726. If we can get per-network configuration in (e.g. #10996) then most of this nonsense will go away.
I'm not sure per-network configuration gets the nonsense to go away :( There's three ways that it could work, I think:
I think the last one seems like it might be the least surprising behaviour? I've got some draft patches for both the latter options though.
@ajtowns I definitely prefer the first or second of those 3 options, I don't want to see a -regtest-logdir and -testnet-logdir argument as well, I think that's much too messy. Alternatively if we stick with -logdir instead of -logfile, we just call the log chain.log e.g. regtest.log, testnet3.log
logs always go in -logdir, if you start a regtest/testnet instance having specified it in bitcoin.conf, your logs get messy, you get told to RTFM, and cry bitter tears
For the log it'd be remotely acceptable, but you'd never want a similar thing to happen for wallets.
This is nothing new, by the way!
-port -bind -rpcport -rpcbind already suffer from this. They are really options that one would want to specify per network.
-logdir only has an effect on mainnet, you have to use -regtest-logdir or -testnet-logdir or similar to have debug.log go elsewhere
I'm starting to like this as this is explicit, they don't depend on context, e.g., what file something was specified it. It works as well with -conf=<file> as with the automatic way of finding configuration files. But it's a major change to argument handling nevertheless...