fix: propagate log-config changes #12566
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
sdirix
force-pushed
the
propagate-log-config-changes
branch
2 times, most recently
from
f8c598f
to
4879567
Compare
sdirix
force-pushed
the
propagate-log-config-changes
branch
3 times, most recently
from
0abdec7
to
d469f92
Compare
|
I noticed that the frontend loggers were correctly updated with the PR, however the backend loggers were not. I reworked the PR a bit for a cleaner architecture including getting rid of a pre-existing |
|
@msujew Ping? :-) |
sdirix
force-pushed
the
propagate-log-config-changes
branch
from
d469f92
to
8ed7854
Compare
|
Rebased the PR on the latest state of |
msujew
approved these changes
Sep 26, 2023
If Theia is started with the "--log-config" option the 'LogLevelCliContribution' watches for file changes and fires a 'logConfigChangedEvent'. This event was not listened to. Therefore already existing 'ILogger' levels were not updated. This is now fixed by notifying loggers about log config changes to update their log level. Also refactors the backend architecture to also use the existing client mechanism for notifications. Signed-off-by: Stefan Dirix <sdirix@eclipsesource.com> Contributed on behalf of STMicroelectronics
sdirix
force-pushed
the
propagate-log-config-changes
branch
from
8ed7854
to
8f1ae28
Compare
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.
If Theia is started with the "--log-config" option the 'LogLevelCliContribution' watches for file changes and fires a 'logConfigChangedEvent'. This event was not listened to. Therefore already existing 'ILogger' levels were not updated. This is now fixed.
Contributed on behalf of STMicroelectronics
What it does
Installs a listener in the
console-logger-serverto update log levels when the log config file changesHow to test
Create a log config file and start Theia with the
--log-configoption, e.g.theia start --log-config=logConfig.json.Example log config:
{ "defaultLevel": "info", "levels": { "task": "warn" } }While Theia is running modify the handed over log config file (e.g. change the
defaultLeveltowarnorfatal) and observe that from then on the log messages are filtered accordingly.Review checklist
Reminder for reviewers