zylatis / turnstiles Goto Github PK

I imagine most people don't need to rotate their logs under 1mb in size but potentially certain environments require that, it makes testing easier, and there's no good reason why we shouldn't support it. The question is, do we change the argument of the enum to float or make a separate SizeBytes enum option?

This flag is unlikely to change during runtime and so to avoid runtime costs this should be made a compile time check.

While there seems to be no issue using the normal slog-term to write to file, using the json drain seems to have issues with writing a single log line over multiple files if rotation occurs during that write, presumably because it's not a single buffer being written.

Making the io::Write trait write using write_all didn't fix this, and also feels naughty as that changes what we mean by write but maybe it's okay. Need to probe deeper into how the slog-json drain works.

As noted in current docs:

Not all internal errors are handled the same way. For example, if during the process of checking if rotation is required an error occurs, the default is to print a warning to stdout and not rotate. In contrast to this, if an error occurs during the actual rotation procedure, this error is bubbled up through error handling eventually returning as a std::io::Error to the caller. However probable future state will outsource all error handling logic to the caller of this library rather than making assumptions.

The question is: do we pass all errors up to the caller of write, which might be annoying for transient ones when checking if we should rotate, or do we add some config options to let people decide at compile time. Personally I think the former is safest, at least the user knows where they stand.

Currently here it links to a different tempdir which is not the correct one. In some sense this is a problem with crates.io as it is not noticing this is a subcrate in this crates folder, but maybe it can't know that given the name.

Anyway, a name change will fix this.

Rather than relying on external code to come in and purge old files we can add a step to the rotation which will clear away 'old' logs, under a certain definition (age/index/etc)

If we're always writing to the highest integer label then this will continue forever even if we remove old logs. Similar to the issue noted in the readme, it's also not really expected behaviour to have the active file change name constantly. One option is to rename all files and +1 their indices, another (shamelessly pinched from https://docs.rs/flexi_logger/latest/flexi_logger/) is to just have an active file fixed name based on the root file name which then gets renamed to the highest index on rotation, which is a nice middleground between no renames and renaming everything.

Currently we can prune old logs if either:

• there are >N of them, or
• some are older than a certain timestamp

However, there are situations where we want both to apply. For example we might want to say we want to keep 2 weeks of logs, unless this is going to fill up a hdd in which case prune them so we'd want to prune if either of these conditions are met.