Made Handle accessible on logger enabling multilog environments

[izolyomi]

Based on the discussion in issue https://github.com/estk/log4rs/issues/390 this is a proof-of-concept experiment that enables using log4rs in a multilog environment.

Probably naming, documentation and others can still be improved a lot, what do you think?

[izolyomi]

Can I please have an opinion whether the direction taken by these few lines of changes looks reasonable to any main contributor? Would a detailed rationale help the review?

[estk]

@izolyomi For context we have been slowly exposing more internals, this is not a use case that was considered.

[estk]

My only issue with this impl is the env mode, there are a number of other places that would also set the max log level and therefore it would be inconsistent. I think the only way we can get away with this without a major version or inconsistent api is either add a set_max_level param to the config or peer methods where set_max_level is called in log4rs, but without that functionality. (eg init_config_without_max_level(config))

As for pub fn handle I think there is some reason log does not recommend this. In lieu of this I would suggest just saving the handle somewhere accessible for later use.

[izolyomi]

My only issue with this impl is the env mode, there are a number of other places that would also set the max log level and therefore it would be inconsistent. I think the only way we can get away with this without a major version or inconsistent api is either add a set_max_level param to the config or peer methods where set_max_level is called in log4rs, but without that functionality. (eg init_config_without_max_level(config))

As for pub fn handle I think there is some reason log does not recommend this. In lieu of this I would suggest just saving the handle somewhere accessible for later use.

Thank you for the review and the reply, I agree that the environment mode was not fully consistent. Considering other possibilities some more, I've realized that whenever the logger is externally reconfigured then it can be the caller's responsibility to keep the global log level consistent by calling log::set_max_level after set_config(). Consequently, I've removed the environment mode from my suggested changes.

I think the PR became trivial now, is it acceptable in the current form? Frankly I couldn't fully understand your train of thoughts at pub fn handle, do you see a better way to make it accessible?

[izolyomi]

Is there any planned timeline to review and hopefully merge the dozen lines of the updated PR?

[izolyomi]

I guess it's a NO then.

[estk]

@izolyomi i am sorry for the delay. Bryan had agreed to send me an email when he saw PRs as ready. My last email from him was Feb last year.

i am adding this review to my queue.

[codecov-commenter]

:warning: Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 0% with 5 lines in your changes missing coverage. Please review.

Project coverage is 61.50%. Comparing base (8ab1b34) to head (0e7979e). Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
src/lib.rs	0.00%	4 Missing :warning:
src/config/mod.rs	0.00%	1 Missing :warning:

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #393      +/-   ##
==========================================
- Coverage   63.39%   61.50%   -1.89%     
==========================================
  Files          24       25       +1     
  Lines        1557     1569      +12     
==========================================
- Hits          987      965      -22     
- Misses        570      604      +34     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.