vdk-structlog: incorporate vdk-core built-in logging plugin into vdk-structlog #2939
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
Signed-off-by: Yoan Salambashev <[email protected]>
Signed-off-by: Yoan Salambashev <[email protected]>
projects/vdk-core/src/vdk/internal/builtin_plugins/config/log_config.py
Outdated
Show resolved
Hide resolved
| @hookimpl | ||
| def vdk_initialize(self, context: CoreContext): | ||
| # Use this plugin for logging config and warn the user about the vdk-core logging deprecation | ||
| os.environ["VDK_USE_STRUCTLOG"] = "1" | ||
| configure_initial_logging() |
There was a problem hiding this comment.
Can you please post log outputs of jobs using all formats, e.g. one for json, one fore console, one for ltsv?
I have a hunch that this might mess up parts of the initial log. But I also see we still eject the formatter later on, so not sure. It would be nice to see the full output.
There was a problem hiding this comment.
Added 1 for JSON and 1 for console to the PR description.
There was a problem hiding this comment.
Can you show the whole output in something like pastebin? My concern is that it won't be formatted as JSON top to bottom, or it will have metadata that we didn't want.
There was a problem hiding this comment.
Here it is: https://pastebin.com/8NnfF0En
projects/vdk-core/src/vdk/internal/builtin_plugins/config/log_config.py
Outdated
Show resolved
Hide resolved
projects/vdk-core/src/vdk/internal/builtin_plugins/config/log_config.py
Outdated
Show resolved
Hide resolved
Signed-off-by: Yoan Salambashev <[email protected]>
Signed-off-by: Yoan Salambashev <[email protected]>
Signed-off-by: Yoan Salambashev <[email protected]>
Signed-off-by: Yoan Salambashev <[email protected]>
Signed-off-by: Yoan Salambashev <[email protected]>
DeltaMichael
suggested changes
Nov 29, 2023
yonitoo
added a commit
that referenced
this pull request
Nov 30, 2023
…lugin is used (#2944) What: This is part of #2813 which is addressed by #2939. Add logging plugin warning and check if the vdk-structlog plugin is used by checking the dedicated env var VDK_USE_STRUCTLOG. Why: To notify the user about the logging configuration changes and to guide them to use vdk-structlog instead. Signed-off-by: Yoan Salambashev <[email protected]>
|
Issue was resolved here. #2985 |
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.
What:
Incorporate vdk-core built-in logging plugin into vdk-structlog by moving the logging configuration and relevant tests.
Introduced the vdk-core changes (warning and keeping the vdk-core functionality behind a flag which is activated upon the vdk-structlog installation) as part of the PR #2944.
Why: To avoid reconfiguring the logging config, thus losing information, by vdk-structlog but instead just configure it once.
Testing Done: Existing tests pass and the introduced tests from vdk-core logging are passing.
Logging format screenshots:
Signed-off-by: Yoan Salambashev [email protected]