Skip to content

fixed:Log level does not work on V2 logs #4300

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 5 commits into from
Closed

fixed:Log level does not work on V2 logs #4300

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
dfd771a
fixed:Log level does not work on V2 logs
xengine-qyt Mar 13, 2025
84d5372
fix log level and improve compatible
xengine-qyt Mar 14, 2025
1ca51c1
Update srs_kernel_log.hpp
xengine-qyt Mar 14, 2025
821f39c
style modify
xengine-qyt Mar 17, 2025
b46fec7
Merge branch 'log' of github.com:xengine-qyt/srs into log
xengine-qyt Mar 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions trunk/src/app/srs_app_utility.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,17 @@ SrsLogLevel srs_get_log_level(string level)

SrsLogLevel srs_get_log_level_v2(string level)
{
if ("trace" == level) {
if ("VERB" == level) {
return SrsLogLevelVerbose;
} else if ("debug" == level) {
return SrsLogLevelInfo;
} else if ("info" == level) {
} else if ("TRACE" == level) {
return SrsLogLevelTrace;
} else if ("warn" == level) {
} else if ("DEBUG" == level) {
return SrsLogLevelDebug;
} else if ("INFO" == level) {
return SrsLogLevelInfo;
} else if ("WARN" == level) {
return SrsLogLevelWarn;
} else if ("error" == level) {
} else if ("ERROR" == level) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this PR don't make sense.
The SRS_LOG_LEVEL_V2 is enabled by default.

srs/trunk/auto/options.sh

Line 71 in 93cba24

SRS_LOG_LEVEL_V2=YES

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.o,it different from the documentation
image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

upper cases terms, TRACE, DEBUG, INFO, WARN, ERROR, are print to logs, not the configurable claims, use lower-cases in conf file.

return SrsLogLevelError;
} else {
return SrsLogLevelDisabled;
Expand Down
2 changes: 1 addition & 1 deletion trunk/src/kernel/srs_kernel_log.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
const char* srs_log_level_strings[] = {
#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,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

"WARN", NULL, NULL, NULL, NULL, NULL, NULL, NULL,
"ERROR", NULL, NULL, NULL, NULL, NULL, NULL, NULL,
NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
Expand Down
30 changes: 23 additions & 7 deletions trunk/src/kernel/srs_kernel_log.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,20 +20,36 @@

// 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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

enum SrsLogLevel
{
SrsLogLevelForbidden = 0x00,

// Only used for very verbose debug, generally,
// we compile without this level for high performance.
SrsLogLevelForbidden = 0x00,
//v2 type
SrsLogLevelVerbose = 0x01,
SrsLogLevelInfo = 0x02,
SrsLogLevelTrace = 0x04,
SrsLogLevelTrace = 0x02,
SrsLogLevelDebug = 0x03,
SrsLogLevelInfo = 0x04,
SrsLogLevelWarn = 0x08,
SrsLogLevelError = 0x10,

SrsLogLevelDisabled = 0x20,
SrsLogLevelDisabled = 0x20,
};
#else
enum SrsLogLevel
{
SrsLogLevelForbidden = 0x00,

// Only used for very verbose debug, generally,
// we compile without this level for high performance.
SrsLogLevelVerbose = 0x01,
SrsLogLevelInfo = 0x02,
SrsLogLevelTrace = 0x04,
SrsLogLevelWarn = 0x08,
SrsLogLevelError = 0x10,

SrsLogLevelDisabled = 0x20,
};
#endif

// Get the level in string.
extern const char* srs_log_level_strings[];
Expand Down
Loading