Skip to content

integrate Slog into logx#327

Open
JiChenSSG wants to merge 6 commits into
zeromicro:mainfrom
JiChenSSG:slog
Open

integrate Slog into logx#327
JiChenSSG wants to merge 6 commits into
zeromicro:mainfrom
JiChenSSG:slog

Conversation

@JiChenSSG

@JiChenSSG JiChenSSG commented Sep 28, 2024

Copy link
Copy Markdown

#325

Greptile Summary

This pull request integrates the new log/slog package from Go 1.21 into the logx module of the zero-contrib project, implementing a SlogWriter struct in logx/slogx/slog.go to adapt the logx.Writer interface for slog usage.

  • Replaced fmt.Sprint with fmt.Sprintf in logx/slogx/slog.go for consistent formatting across all types in log messages
  • Optimized toSlogFields function to create a single slice for all fields, improving efficiency
  • Implemented SlogWriter struct with methods for various log levels (Debug, Error, Info, etc.) using slog
  • Added NewSlogWriter function to create a new logx.Writer instance using slog.Handler

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

5 file(s) reviewed, 5 comment(s)
Edit PR Review Bot Settings

Comment thread logx/slogx/README.md
Comment thread logx/slogx/go.mod
Comment thread logx/slogx/go.mod Outdated
Comment thread logx/slogx/go.mod
Comment thread logx/slogx/slog.go
Comment on lines +58 to +65
func toSlogFields(fields ...logx.LogField) []interface{} {
slogFields := make([]interface{}, 0, len(fields) * 2)
for _, field := range fields {
slogFields = append(slogFields, field.Key, field.Value)
}

return slogFields
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

style: toSlogFields creates a new slice for each call. Consider using a pool for better performance in high-volume logging scenarios.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

2 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings

Comment thread logx/slogx/go.mod

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

1 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings

Comment thread logx/slogx/slog.go Outdated

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM

1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant