Support date/time in log file name

[TuEmb]

Hi @estk,

As #242. I want to create a PR to apply date/time in log file name.
The path in file appender will be:

    path: "log/file-{%Y-%m-%d_%H-%M-%S}.log"

It has the same format with chrono::datetime

[Creative-Difficulty]

@estk

[bconn98]

FYI @Creative-Difficulty @TuEmb I'll help you guys prep this PR for merging.

However, it looks like in working your change you've removed functionality from the library. Can you please rework this maintain the original functionality?

[TuEmb]

@bconn98,

Thanks for your reply. Could you tell me which part do I need to rework ? I really want to do that features, because I used log4rs in my project and want to record date/time in log file.

Is this examples/sample_config.yml ?

[bconn98]

@TuEmb The previous solution also handles environment variables as part of the path, unless I'm missing it, I don't see that in the new solution.

[bconn98]

Please also update the documentation. Most is generated by rustdoc but we also have some manual markdown as well in the docs directory.

[TuEmb]

@bconn98, I have updated new commits to keep environment variable as part of the path and updated documentation for date & time format path. Please help me review those changes

[9-FS]

Is this feature still being worked on?

[bconn98]

@9-FS It doesnt look like @TuEmb is interested in taking this to closure. Feel empowered to step in and finish it

[9-FS]

@bconn98 Thank you for your quick response. Unfortunately, setting up proper logging is literally the very first thing I do when I start a new language, so I think immediately contributing to an established project without ever having written anything in Rust is above my ability... For the time being, I will probably set up a custom appender, but I will come back to this in a couple of months should I feel confident enough to meaningfully contribute.

[TuEmb]

@bconn98 sorry, I assumed that you accepted with my implementation using chrono pattern. Could you help me review again with my latest commits ?

[bconn98]

My previous review comment has not been completed. We need a pattern around the date '{}' similar to how the environment variables works. This will allow for you to find it regardless of where it falls in the pattern string.

[TuEmb]

I added a pattern with $TIME{chrono_format} for the log file name. It will be checked after $ENV (which is using function expend_env_vars). how do you think about that @bconn98 ?

[9-FS]

@TuEmb Sorry for the question, but is there a reason for $TIME{}? Looking at the existing encoder pattern implementation, as a user I would've expected the {d()} syntax again.

[bconn98]

@TuEmb I'll take a look this evening.

@9-FS The reason for the requested change is that we support different patterns when generating log file names. Historically, we support either environment variables using ENV{} or iteration via {}. So the new addition of date/time needs a similarly unique key for us to search for that would not negatively impact the existing options and allows for the pattern to exist anywhere in the string without concern for ordering.

[bconn98]

Looks much better! Just a few small comments @TuEmb

[estk]

can you add some unit tests for this function? It would be great to just verify some edgecases and verify this is working as intended.

[TuEmb]

I added unit test for this function:

• valid $TIME{} format
• invalid $TIME{} format
• no $TIME{} format
• multiple $TIME{} in the path

[TuEmb]

@bconn98 @estk can we go close this PR ?

[bconn98]

It’s blocked by #367

[samsungapore]

reading this thread made me sad

[bconn98]

This is blocked by the pr to resolve chrono deprecations.