Skip to content
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

feature: add use_local_time_encoder and show_caller config #59

Draft
wants to merge 20 commits into
base: master
Choose a base branch
from
Draft

feature: add use_local_time_encoder and show_caller config #59

Changes from 1 commit
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
ecfe574
add use_local_time_encoder and show_caller config
shellphy Nov 9, 2024
6d265f2
modify module name
shellphy Nov 9, 2024
583814d
增加插件名称固定长度
shellphy Nov 9, 2024
2583ed9
增加插件名称固定长度
shellphy Nov 9, 2024
07267aa
add test
shellphy Nov 10, 2024
bb8c385
merge
shellphy Nov 10, 2024
29bc270
add caller color
shellphy Nov 10, 2024
0e06e7a
Merge branch 'master' into show_locatltime_and_caller
shellphy Nov 10, 2024
c1430a7
increment logger name width
shellphy Nov 10, 2024
1305a3f
Do not modify nameencoder
shellphy Nov 12, 2024
5c738a0
update
shellphy Nov 12, 2024
cf91fbf
test
shellphy Nov 12, 2024
9b65581
caller长度调整
shellphy Nov 12, 2024
25d29ba
更新mod
shellphy Nov 12, 2024
48d6db1
update
shellphy Nov 12, 2024
d8a4101
update module name
shellphy Nov 12, 2024
e322c8c
update caller length
shellphy Nov 12, 2024
5b64a6a
Merge branch 'master' into show_locatltime_and_caller
shellphy Nov 12, 2024
b952530
update module name
shellphy Nov 12, 2024
571f8b6
update module name
shellphy Nov 12, 2024
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
38 changes: 32 additions & 6 deletions config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,15 +95,29 @@ type Config struct {

// File logger options
FileLogger *FileLoggerConfig `mapstructure:"file_logger_options"`

// UseLocalTimeEncoder is used to set the encoder to use local time instead of UTC.
UseLocalTimeEncoder bool `mapstructure:"use_local_time_encoder"`
Copy link
Member

Choose a reason for hiding this comment

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

I'll think about this, but atm, this name is too long, I suggest to use smt like: use_local_time.


// ShowCaller is used to set the encoder to show the caller.
ShowCaller bool `mapstructure:"show_caller"`
Copy link
Member

Choose a reason for hiding this comment

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

When developing custom plugins, it was difficult to find the corresponding location in the log, so caller was added
In the production environment, the log time format is not readable enough, so the local time display function is added

I'm not sure if I understand this. The caller is the line where your log is. How is that difficult to find where you send this log?

Copy link
Author

Choose a reason for hiding this comment

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

When developing custom plugins, although logs show what happened, pinpointing the exact source of a log entry in complex plugin code can be challenging without caller information. By enabling show_caller, we can immediately see which file and line generated the log, significantly improving debugging efficiency.

Copy link
Member

Choose a reason for hiding this comment

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

How can it be complex? It is just ctrl+f (or / for vim) -> enter log message -> here you are.

}

// BuildLogger converts config into Zap configuration.
func (cfg *Config) BuildLogger() (*zap.Logger, error) {
var zCfg zap.Config
callerKey := zapcore.OmitKey
if cfg.ShowCaller {
callerKey = "caller"
}
switch Mode(strings.ToLower(string(cfg.Mode))) {
case off, none:
return zap.NewNop(), nil
case production:
encodeTime := utcEpochTimeEncoder
if cfg.UseLocalTimeEncoder {
encodeTime = localTimeEncoder
}
Copy link
Member

Choose a reason for hiding this comment

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

Local time is often the source of the errors when using different time zones.

Copy link
Author

Choose a reason for hiding this comment

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

For projects that operate within a single timezone (which is common for many applications), local time provides more intuitive logging without the complexity of timezone management

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, this is fair. I agree with that change, but tests should be added + docs + shorter name in the configuration.

zCfg = zap.Config{
Level: zap.NewAtomicLevelAt(zap.InfoLevel),
Development: false,
Expand All @@ -112,20 +126,24 @@ func (cfg *Config) BuildLogger() (*zap.Logger, error) {
TimeKey: "ts",
LevelKey: "level",
NameKey: "logger",
CallerKey: zapcore.OmitKey,
CallerKey: callerKey,
FunctionKey: zapcore.OmitKey,
MessageKey: "msg",
StacktraceKey: zapcore.OmitKey,
LineEnding: cfg.LineEnding,
EncodeLevel: zapcore.LowercaseLevelEncoder,
EncodeTime: utcEpochTimeEncoder,
EncodeTime: encodeTime,
EncodeDuration: zapcore.SecondsDurationEncoder,
EncodeCaller: zapcore.ShortCallerEncoder,
},
OutputPaths: []string{"stderr"},
ErrorOutputPaths: []string{"stderr"},
}
case development:
encodeTime := utcISO8601TimeEncoder
if cfg.UseLocalTimeEncoder {
encodeTime = localTimeEncoder
}
zCfg = zap.Config{
Level: zap.NewAtomicLevelAt(zap.DebugLevel),
Development: true,
Expand All @@ -134,14 +152,14 @@ func (cfg *Config) BuildLogger() (*zap.Logger, error) {
TimeKey: "ts",
LevelKey: "level",
NameKey: "logger",
CallerKey: zapcore.OmitKey,
CallerKey: callerKey,
FunctionKey: zapcore.OmitKey,
MessageKey: "msg",
StacktraceKey: zapcore.OmitKey,
LineEnding: cfg.LineEnding,
EncodeLevel: ColoredLevelEncoder,
EncodeName: ColoredNameEncoder,
EncodeTime: utcISO8601TimeEncoder,
EncodeTime: encodeTime,
EncodeDuration: zapcore.StringDurationEncoder,
EncodeCaller: zapcore.ShortCallerEncoder,
},
Expand All @@ -160,21 +178,25 @@ func (cfg *Config) BuildLogger() (*zap.Logger, error) {
ErrorOutputPaths: []string{"stderr"},
}
default:
encodeTime := utcISO8601TimeEncoder
if cfg.UseLocalTimeEncoder {
encodeTime = localTimeEncoder
}
zCfg = zap.Config{
Level: zap.NewAtomicLevelAt(zap.DebugLevel),
Encoding: "console",
EncoderConfig: zapcore.EncoderConfig{
TimeKey: "T",
LevelKey: "L",
NameKey: "N",
CallerKey: zapcore.OmitKey,
CallerKey: callerKey,
FunctionKey: zapcore.OmitKey,
MessageKey: "M",
StacktraceKey: zapcore.OmitKey,
LineEnding: cfg.LineEnding,
EncodeLevel: ColoredLevelEncoder,
EncodeName: ColoredNameEncoder,
EncodeTime: utcISO8601TimeEncoder,
EncodeTime: encodeTime,
EncodeDuration: zapcore.StringDurationEncoder,
EncodeCaller: zapcore.ShortCallerEncoder,
},
Expand Down Expand Up @@ -251,3 +273,7 @@ func utcISO8601TimeEncoder(t time.Time, enc zapcore.PrimitiveArrayEncoder) {
func utcEpochTimeEncoder(t time.Time, enc zapcore.PrimitiveArrayEncoder) {
enc.AppendInt64(t.UTC().UnixNano())
}

func localTimeEncoder(t time.Time, enc zapcore.PrimitiveArrayEncoder) {
enc.AppendString(t.Local().Format("2006-01-02 15:04:05.000"))
Copy link
Member

Choose a reason for hiding this comment

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

Also, ISO8601 encode format should be used here to be in sync with the current encoder.

}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider maintaining consistent time format across encoders.

The localTimeEncoder uses a different time format compared to utcISO8601TimeEncoder. This inconsistency might cause issues with log parsing tools that expect a consistent format.

Consider using the same ISO8601 format for consistency:

 func localTimeEncoder(t time.Time, enc zapcore.PrimitiveArrayEncoder) {
-	enc.AppendString(t.Local().Format("2006-01-02 15:04:05.000"))
+	enc.AppendString(t.Local().Format("2006-01-02T15:04:05-0700"))
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func localTimeEncoder(t time.Time, enc zapcore.PrimitiveArrayEncoder) {
enc.AppendString(t.Local().Format("2006-01-02 15:04:05.000"))
}
func localTimeEncoder(t time.Time, enc zapcore.PrimitiveArrayEncoder) {
enc.AppendString(t.Local().Format("2006-01-02T15:04:05-0700"))
}

Loading