sociomantic-tsunami / ocean

General purpose, platform-dependent, high-performance library for D
Other
61 stars 56 forks source link

Logger configuration should auto-create directory #273

Open mathias-lang-sociomantic opened 7 years ago

mathias-lang-sociomantic commented 7 years ago

Currently, if a config file has a logger configuration setup to write to a file which doesn't exists, the file is created. However, if the file is in a directory that doesn't exists, the directory is not created.

This impose an unnecessary constraint on deployment: while installing a package, all subfolders of the log directory have to be created by the package, effectively creating a coupling between the package definition and the application configuration.

I think the best course of action is to make the configuration phase auto create those directories.

leandro-lucarella-sociomantic commented 7 years ago

I don't agree there is an equivalence between files and directories. Directory permissions are handled differently than file permissions, as depending of what you do a logrotate might not have permissions to write new files (rotate) for example.

I think it is reasonable to require for a directory to exist, it should be part of the installation process.

leandro-lucarella-sociomantic commented 7 years ago

(it's quite some effort to fight the temptation to close as WONTFIX and to give some time to further discussion)

mathias-lang-sociomantic commented 7 years ago

Let me provide an example. Let's say we have a large application, which logs to tons of file. At the top, the application receive 3 different kind of streams, and the maintainer only ever have to debug one stream at a time - making it for an obvious point to separate the log files. So he'll write a config file like:

[LOG.root]
level = info
console = false
propagate = true
file = log/root.log

[LOG.App.streamA]
file=log/stream_a/root.log
[LOG.App.streamB]
file=log/stream_b/root.log
[LOG.App.streamC]
file=log/stream_c/root.log

[LOG.App.streamA.SomeMoreSpecializedLogger]
file=log/stream_a/specialized.log
# Etc...

The issue is, now the package definition has to create those directories. If the maintainer suddenly want to add a new stream, he has to:

I am not arguing that the application should create the root log folder. Well in fact it could, but since /xxx/zzz/app/ will be created by root, the app wouldn't be able to start without the root logging directory, however it should auto-create any subdirectory.

We can discuss that IRL, as it's tied to our deployment structure.

leandro-lucarella-sociomantic commented 7 years ago

Many objections to this:

  1. Normally applications use only one log directoy, just log to: log/stream_a.log, log/stream_b.log, log/stream_b_specialized.log, etc. instead of creating subdirectories.
  2. If you insist in implementing this using subdirectories, it looks awfully like a very special case of this application, so just make the application deal with the extra directory creation.

But I'm still quite convinced that making the logger silently create directories is a mistake.

mihails-strasuns-sociomantic commented 7 years ago

To me creating directories within own installation dirs looks reasonable and sufficiently time-saving to warrant library support. But I am more or less neutral on the matter.

nemanja-boric-sociomantic commented 6 years ago

Related: https://github.com/dlang/phobos/commit/e85381ee42652029a8b1c8d8397aee78c2ae7139#diff-41bb8731b16e43f1b48e0d529c498fa9

leandro-lucarella-sociomantic commented 6 years ago

I think at the bare minimum it should be configurable if you want the app to create directories. For example? What kind of permissions you give to the new directory? Usually you want to give admins read access but not to everyone, and stuff like that, this is definitely something that the "sysadmin" should take care of.

mathias-lang-sociomantic commented 6 years ago

Same as files currently being created ?

leandro-lucarella-sociomantic commented 6 years ago

Might not be a good guess, as some external program might need to rotate logs. Maybe put permissions as an option too. I'm fine if things are configurable and the default is to not create directories, which IMHO is the safest option, the user should know if the app starts creating directories unless he configured the app to do that.