application_logs: add bootstrap option to set log format#27816
application_logs: add bootstrap option to set log format#27816mattklein123 merged 1 commit intoenvoyproxy:mainfrom
Conversation
Signed-off-by: ohadvano <ohadvano@gmail.com>
|
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to |
mattklein123
left a comment
mattklein123
left a comment
There was a problem hiding this comment.
Thanks for adding this. One question.
/wait-any
| if (application_log_config.log_format().has_text_format()) { | ||
| Logger::Registry::setLogFormat(application_log_config.log_format().text_format()); |
There was a problem hiding this comment.
What happens if someone sets the CLI option? I think this is similar to the other convo we had? Should we have an error if someone sets the CLI option and then sets the bootstrap option? I admit that perhaps there is a use case where someone could set the log format before the bootstrap is read, but that seems like kind of an edge case and we could just see if anyone complains about that? wdyt?
There was a problem hiding this comment.
I agree - if we decided to follow some pattern then I think it should be applied to all cases.
The exclusive format method assertion function is called before stepping into this function, in:
Line 426 in 72c2b96
So this case should be covered, and any other log format that would be added in the future.
…27816) add bootstrap option to set log format Signed-off-by: ohadvano <ohadvano@gmail.com> Signed-off-by: asheryer <asheryer@amazon.com>
…27816) add bootstrap option to set log format Signed-off-by: ohadvano <ohadvano@gmail.com> Signed-off-by: Ryan Eskin <ryan.eskin89@protonmail.com>
2 participants
Commit Message: application_logs: add bootstrap option to set log format
Additional Description: Related to #27278 which adds a bootstrap option to set JSON log format. Resolves #27761
Risk Level: Low
Testing: Unit tests
Docs Changes: Changelog
Release Notes: None
Platform Specific Features: None