[#396] Read and set LogLevel from environment variable

[zmostafa]

Notes for Reviewer

Adds functionality to set the log level from the environment.

I went for @elBoberido proposal solution, where I read an environment variable called IOX2_LOG_LEVEL and set the LogLevel, with its value at runtime.

Pre-Review Checklist for the PR Author

1. Add sensible notes for the reviewer
2. PR title is short, expressive and meaningful
3. Relevant issues are linked in the References section
4. Every source code file has a copyright header with SPDX-License-Identifier: Apache-2.0 OR MIT
5. Branch follows the naming format (iox2-123-introduce-posix-ipc-example)
6. Commits messages are according to this guideline
   • Commit messages have the issue ID ([#123] Add posix ipc example)
   • Commit author matches Eclipse Contributor Agreement (and ECA is signed)
7. Tests follow the best practice for testing
8. Changelog updated in the unreleased section including API breaking changes
9. Assign PR to reviewer
10. All checks have passed (except task-list-completed)

Checklist for the PR Reviewer

• Commits are properly organized and messages are according to the guideline
• Unit tests have been written for new behavior
• Public API is documented
• PR title describes the changes

Post-review Checklist for the PR Author

1. All open points are addressed and tracked via issues

References

Closes #396

[xieyuschen]

Moreover, please follow the convention here:

• title: [#396]: Read and set LogLevel from environment variable

[elfenpiff]

@zmostafa hey hey ... you are back. I am happy to see your PR plopping up here.

[zmostafa]

@elfenpiff you have to thank Jeff for the motivation, and thanks for the nice words as expected from you.

You guys are the best ❤️

[elBoberido]

@zmostafa great to have you back :)

I guess you need to sign the ECA. It can be done as individual contributor if you do the contributions in your spare time.

[elBoberido]

This is great. I had something else in mind when I created the issue but this idea is better :)

My only concern is the allocation with String, which is a hurdle for ASIL-D. But that can be solved with a feature flag later on.

I would also suggest to use .map(LogLevel::from_str).unwrap_or(DEFAULT_LOG_LEVEL). This is one less allocation.

The lazy initialization is problematic in this case. Let's assume the following scenario

• set env variable to trace
• start process A
• set env variable to fatal
• start process B
• process A reads the env variable and sets it to fatal instead of trace

Since the initialization is dependent on an external state, the evaluation should be done when the application starts.

[zmostafa]

1. Can you explain more about string allocation? how do you track/bench them?
2. about the scenario you mentioned, is not it what should be in the first place? since we might run different instance of application from or within the same shell/env? so each application will utilize its logger?

[elBoberido]

1. There would be heaptrack as a nice gui to track all heap allocation. But in this case I just know that String will allocate on the heap from its documentation.
2. Each application will utilize its own logger instance but if the reading of the env variable is delayed, application A might read the value which was set after it was started and meant to be the value for application B.

[elBoberido]

@zmostafa can you please revert the lazy initialization for the logger

[zmostafa]

@elBoberido I reverted back the DEFAULT_LOGGER lazy initialization

[elBoberido]

Can you do the same for ENV_LOG_LEVEL

[codecov]

Codecov Report

Attention: Patch coverage is 64.00000% with 9 lines in your changes missing coverage. Please review.

Project coverage is 79.64%. Comparing base (810fb87) to head (e2b323c).
Report is 9 commits behind head on main.

Files with missing lines	Patch %	Lines
iceoryx2-bb/log/src/lib.rs	64.00%	9 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #474      +/-   ##
==========================================
+ Coverage   79.22%   79.64%   +0.42%     
==========================================
  Files         200      200              
  Lines       23765    23779      +14     
==========================================
+ Hits        18828    18939     +111     
+ Misses       4937     4840      -97     

Files with missing lines	Coverage Δ
iceoryx2-bb/log/src/lib.rs	73.77% <64.00%> (-3.16%)	⬇️

... and 21 files with indirect coverage changes

[elBoberido]

@zmostafa sorry for the slow response. I was quite busy the last days, but I try to improve on my response time :)

[elBoberido]

Oh no, does the script have some autocorrection?
This breaks the github alerts syntax. > [!IMPORTANT] must be on a separate line than the content

[elBoberido]

@zmostafa can you please revert those changes. It breaks the alert syntax. Please also have a look at the other open point.

[zmostafa]

@elBoberido hmmm, I think i did, are you sure you are looking at the latest commit?
I will check the other point about reverting the lazy init, did not see that

[elBoberido]

@zmostafa I hope to look at the latest commit 😅

I did a Ctrl+F5, which should reload everything and dismiss caches but I still see > [!IMPORTANT] on the same line as Be aware that ...

[orecham]

Note I believe this happens because the auto-corrector does not recognise GitHub-style admonitions, so you need to manually ensure these are on their own line otherwise the auto corrector will try and do line length correction with it on the same line.

Not ideal, I know, but the idea is for the corrector to fix around 90% of the issues which should save more time than manually fixing these small edge cases (e.g. manually fixing all line lengths).

[elBoberido]

Can we skip line breaks in the auto-corrector? It's just a styling issue, not a rendering issue and it seems to break the github admonitions.

[orecham]

I believe this is the line length corrector causing this. If disabled, then users would need to ensure line length limits were maintained, either manually or with an IDE. I deduced that this would be a larger annoyance for contributors than fixing these more limited edge cases.

Of course, we could check if the auto corrector could be updated to recognise the GitHub admonitions.

[elBoberido]

Well, I would argue that one is not only an annoyance but breaks the rendering of the readme if the reviewer does not pay attention and the other one is a choice by ourselves and does not break the rendering if the line is too long. I would rather deactivate the line length detection than having to keep in mind to check for broken admonitions in the PR.