Add a column with a message pattern to text_log#44543
Conversation
robot-ch-test-poll4
added
pr-improvement
|
This is really great! I'd also like it if we add an array of arguments (strings ok). |
src/Common/logger_useful.h
Outdated
| #define LOG_FATAL(logger, ...) LOG_IMPL(logger, DB::LogsLevel::error, Poco::Message::PRIO_FATAL, __VA_ARGS__) | ||
|
|
||
|
|
||
| #define LOG_TEST_PREFORMATTED(logger, FORMAT_STRING, MESSAGE) LOG_IMPL_PREFORMATTED(logger, DB::LogsLevel::test, Poco::Message::PRIO_TEST, FORMAT_STRING, MESSAGE) |
There was a problem hiding this comment.
Please let's not add another set of logging macros.
- To the reader, the difference between the existing and the new set of macros isn't immediately clear (this could be resolved with code comments),
- If someone requires another variant of the log macro in future, he would have to duplicate not 7 but 14 macros, leading to 28 macros in total. These things spread like cancer.
- There are just two files (out of hundreds) which use the new macros. I am sure that these few places can be converted to the existing macros (maybe at the cost of a negligible performance hit).
Yes, it would be a great enhancement, but it requires formatting and storing all arguments twice (current |
src/Common/logger_useful.h
Outdated
| #include <Poco/Message.h> | ||
| #include <Common/CurrentThread.h> | ||
|
|
||
| /// This wrapper is usefult to save formatted message into a String before sending it to a logger |
| #include <Common/CurrentThread.h> | ||
|
|
||
| /// This wrapper is useful to save formatted message into a String before sending it to a logger | ||
| class LogToStrImpl |
There was a problem hiding this comment.
Don't have a useful suggestion just wanted to say that this is a really nice idea to avoid double formatting.
|
Integration tests (release) [2/4] - test_system_merges is flaky |
|
I installed 23.1 and was surprised to see that my text_log did not change. |
6 participants
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Added a
message_format_stringcolumn tosystem.text_log. The column contains a pattern that was used to format the message.