Fix TRACE, ALL, OFF and NONE log levels #3922
Conversation
mihaitodor
force-pushed
the
mihaitodor-fix-trace-log
branch
from
85cb23f to
378556e
Compare
mihaitodor
force-pushed
the
mihaitodor-fix-trace-log
branch
2 times, most recently
from
5d69134 to
7af9039
Compare
mihaitodor
changed the title
| case "off", "none": | ||
| // Logging disabled | ||
| default: | ||
| fbLogger.Errorf("Log level '%s' not recognized, using to default level %s", logsLevelStr, logsLevel) |
There was a problem hiding this comment.
Thought(non-blocker): Is it worth changing this error whilst we're at it? I presume it's meant to read "using the default level".
There was a problem hiding this comment.
Good call-out, thank you! I looked through the code again and I think it's better now.
I also realised that topicLogger.Enabled() returned true when level == nil, but I don't think that ever happened in the original code because it was passed by value. I could be wrong, but I think this should return false when users set the log level to OFF / NONE, so I updated it. WDYT?
|
Thanks @mihaitodor! |
mihaitodor
force-pushed
the
mihaitodor-fix-trace-log
branch
from
7af9039 to
6b37110
Compare
| // Logging disabled | ||
| default: | ||
| logsLevel = levelPtr(slog.LevelInfo) | ||
| fbLogger.Errorf("Log level '%s' not recognized, using to default level %s", logsLevelStr, logsLevel) |
There was a problem hiding this comment.
| fbLogger.Errorf("Log level '%s' not recognized, using to default level %s", logsLevelStr, logsLevel) | |
| fbLogger.Errorf("Log level '%s' not recognized, using the default level %s", logsLevelStr, logsLevel) |
There was a problem hiding this comment.
I also realised that topicLogger.Enabled() returned true when level == nil, but I don't think that ever happened in the original code because it was passed by value. I could be wrong, but I think this should return false when users set the log level to OFF / NONE, so I updated it. WDYT?
Sounds reasonable, so the behaviour remains the same?
Sending trace logs on the topic logger probably doesn't make much sense, so we fall back to DEBUG for it if TRACE is requested. Fixes redpanda-data#3872. Signed-off-by: Mihai Todor <todormihai@gmail.com>
mihaitodor
force-pushed
the
mihaitodor-fix-trace-log
branch
from
6b37110 to
74fb70a
Compare
3 participants
Sending trace logs on the topic logger probably doesn't make much sense, so we fall back to DEBUG for it if TRACE is requested.
Fixes #3872.
PS: I noticed that
benthos run --log.level=tracedoesn't error out. Guess the topic logger is only configured via theloggerconfig section, which might be a bit confusing...