fixed:Log level does not work on V2 logs #4300
Conversation
suzp1984
left a comment
suzp1984
left a comment
There was a problem hiding this comment.
I don't think this PR make sense.
trunk/src/app/srs_app_utility.cpp
Outdated
| } else if ("WARN" == level) { | ||
| return SrsLogLevelWarn; | ||
| } else if ("error" == level) { | ||
| } else if ("ERROR" == level) { |
There was a problem hiding this comment.
I think this PR don't make sense.
The SRS_LOG_LEVEL_V2 is enabled by default.
Line 71 in 93cba24
The SRS_LOG_LEVEL_V2, which means the log level names defined in https://github.com/apache/logging-log4j2/blob/2.x/log4j-api/src/main/java/org/apache/logging/log4j/Level.java
or
https://stackoverflow.com/questions/2031163/when-to-use-the-different-log-levels/2031209#2031209
This func is just a bridge between log level enum, defined by SRS, and srs.conf. So it's not necessary to redefine another SrsLogLevelXXX for log level 2.
The only fault of this func is log level fatal is missing, which can be mapping to SrsLogLevelError anyway.
If the level string are upper cases, the srs.conf and Unit test cases need to change to match this change, so I don't think its a good idea also.
There was a problem hiding this comment.
.o,it different from the documentation
There was a problem hiding this comment.
upper cases terms, TRACE, DEBUG, INFO, WARN, ERROR, are print to logs, not the configurable claims, use lower-cases in conf file.
trunk/src/kernel/srs_kernel_log.cpp
Outdated
| #ifdef SRS_LOG_LEVEL_V2 | ||
| // The v2 log level specs by log4j. | ||
| "FORB", "TRACE", "DEBUG", NULL, "INFO", NULL, NULL, NULL, | ||
| "FORB", "VERB", "TRACE", "DEBUG", "INFO", NULL, NULL, NULL, |
There was a problem hiding this comment.
https://stackoverflow.com/questions/2031163/when-to-use-the-different-log-levels/2031209#2031209
https://github.com/apache/logging-log4j2/blob/2.x/log4j-api/src/main/java/org/apache/logging/log4j/Level.java
VERB is not the term defined by log level v2.
And the log level sequence is incorrect.
srs_log_level_strings[1] = TRACE;
srs_log_level_strings[2] = DEBUG;
srs_log_level_strings[4] = INFO;
srs_log_level_strings[8] = WARN;
srs_log_level_strings[16] = ERROR;
srs_log_level_strings[32] = FATAL;
FATAL is not there, instead a [32] = OFF, maybe it is the only arguable point.
trunk/src/kernel/srs_kernel_log.hpp
Outdated
|
|
||
| // The log level, see https://github.com/apache/logging-log4j2/blob/release-2.x/log4j-api/src/main/java/org/apache/logging/log4j/Level.java | ||
| // Please note that the enum name might not be the string, to keep compatible with previous definition. | ||
| #ifdef SRS_LOG_LEVEL_V2 |
There was a problem hiding this comment.
SrsLogLevel is SRS's internal log level enum.
SRS_LOG_LEVEL_V2 controls the log print with the new standard, ref https://stackoverflow.com/questions/2031163/when-to-use-the-different-log-levels/2031209#2031209
then the log analysis tools can compatible with those new log level keywords.
So, redefine SrsLogLevel don't make sense here.
|
Do not replace spaces with tabs; it is uncomfortable to look at.
|
|
It seems that this pull request is not necessary. |
4 participants
No description provided.