Always log to console and file #7825
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hubertp
requested review from
4e6,
JaroslavTulach and
Akirathan
as code owners
hubertp
force-pushed
the
wip/hubert/7752-verbose-file-output
branch
2 times, most recently
from
9adf63e to
5b0f20a
Compare
|
This PR now logs to console and log file with INFO and TRACE log levels, respectively, by default. |
hubertp
changed the title
hubertp
changed the title
Akirathan
approved these changes
Sep 20, 2023
hubertp
requested review from
jdunkerley,
radeusgd and
GregoryTravis
as code owners
GregoryTravis
approved these changes
Sep 20, 2023
hubertp
added
the
CI: Clean build required
hubertp
force-pushed
the
wip/hubert/7752-verbose-file-output
branch
from
5582b7f to
a30c2e9
Compare
radeusgd
approved these changes
Sep 22, 2023
|
Can we measure what is the cost of always tracing? i.e. on some example run in the CLI or on the startup time in the GUI (probably hard to measure) I hope it is negligible but it would be nice to know the cost. |
The change adds an option by default to always log to a file with verbose log level. The implementation is a bit tricky because in the most common use-case we have to always log in verbose mode to a socket and only later apply the desired log levels. Previously socket appender would respect the desired log level already before forwarding the log. If by default we log to a file, verbose mode is simply ignored and does not override user settings. To test run `project-manager` with `ENSO_LOGSERVER_APPENDER=console` env variable. That will output to the console with the default `INFO` level and `TRACE` log level for the file.
1. Log INFO level to CONSOLE by default 2. Change runner's default log level from ERROR to WARN Took a while to figure out why the correct log level wasn't being passed to the language server, therefore ignoring the (desired) verbose logs from the log file.
Getting rid of the warning by adding a log4j over slf4j bridge: ``` ERROR StatusLogger Log4j2 could not find a logging implementation. Please add log4j-core to the classpath. Using SimpleLogger to log to the console... ```
Having `application.conf` in `src/main/resources` and `test/resources` does not guarantee that in Tests we will pick up the latter. Instead, by default it seems to do some kind of merge of different configurations, which is far from desired.
Logging to console and (temporary) files is problematic for Windows. The CI also revealed a problem with the native configuration because it was not possible to modify the launcher via env variables as everything was initialized during build time.
hubertp
force-pushed
the
wip/hubert/7752-verbose-file-output
branch
from
bf8f4d1 to
df40ac7
Compare
4e6
approved these changes
Sep 26, 2023
The difference is negligible in both CLI and server mode, but I will be happy to get some feedback on Windows where it used to be the main pain point. |
hubertp
added a commit
that referenced
this pull request
Sep 28, 2023
PR #7825 enabled parallel logging to a file with a much more fine-grained log level by default. However, logging at `TRACE` level on Windows appears to be still problematic. This PR reduced the default log level to file from `DEBUG` to `TRACE` and allows to control it via an environment variable if one wishes to change the verbosity without making code changes.
This was referenced Sep 28, 2023
hubertp
added a commit
that referenced
this pull request
Oct 2, 2023
* Enable log-to-file configuration PR #7825 enabled parallel logging to a file with a much more fine-grained log level by default. However, logging at `TRACE` level on Windows appears to be still problematic. This PR reduced the default log level to file from `DEBUG` to `TRACE` and allows to control it via an environment variable if one wishes to change the verbosity without making code changes. * PR comments
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull Request Description
The change adds an option by default to always log to a file with verbose log level.
The implementation is a bit tricky because in the most common use-case we have to always log in verbose mode to a socket and only later apply the desired log levels. Previously socket appender would respect the desired log level already before forwarding the log.
If by default we log to a file, verbose mode is simply ignored and does not override user settings.
Important Notes
To test run
project-manager. That will output to the console with the defaultINFOlevel andTRACElog level for the file.Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
Java,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.