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.

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

optimize: add Logger for zap #33

Merged
merged 2 commits into from
Jun 11, 2023
Merged

Conversation

Skyenought
Copy link
Member

@Skyenought Skyenought commented Jun 9, 2023

What type of PR is this?

optimize

What this PR does / why we need it (English/Chinese):

zh: 暴露 zap.Logger 实例以用于自定义 fields 等

logger := hertzap.NewLogger()
hlog.SetLogger(logger)
	
h.GET("/hello", func(ctx context.Context, c *app.RequestContext) {
	logger.GetLogger().Info("Hello, World!", zap.Int("status", c.Response().StatusCode()))
	c.String(consts.StatusOK, "Hello hertz!")
})

Which issue(s) this PR fixes:

#32
cloudwego/hertz#809

@Skyenought
Copy link
Member Author

这个 ci 有问题吧, 我没有修改过 _test 文件

@Skyenought Skyenought changed the title optimize: add GetLogger for zap optimize: add Logger for zap Jun 9, 2023
@li-jin-gou
Copy link
Contributor

这种场景不如让用户直接使用 zap 进行日志打印,hlog 的目的是为了给用户提供一个注入自定义日志的能力保证框架的日志能被用户收集到以及简单场景可以使用,在复杂场景下我们更推荐用户直接使用 zap 或 zerolog.

@Skyenought
Copy link
Member Author

Skyenought commented Jun 9, 2023

NewLogger 时已经创建了 zap.Logger, 暴露出来不就可以直接使用了吗?
logrus 扩展就暴露出来了
https://github.com/hertz-contrib/logger/blob/main/logrus/logger.go#LL78C1-L80C2
这不就是你上面的说法吗

如果类似这种设计, 不就不用搞这些了吗

logger, _ := zap.NewProduction()
app := fiber.New()
fiberlog.SetLogger(fiberzap.NewLogger(fiberzap.LoggerConfig{
	SetLogger: logger,
}))

@li-jin-gou

@li-jin-gou
Copy link
Contributor

li-jin-gou commented Jun 9, 2023

NewLogger 时已经创建了 zap.Logger, 暴露出来不就可以直接使用了吗?
logrus 扩展就暴露出来了
https://github.com/hertz-contrib/logger/blob/main/logrus/logger.go#LL78C1-L80C2
这不就是你上面的说法吗 @li-jin-gou

cc @rogerogers

Copy link
Collaborator

@rogerogers rogerogers left a comment

Choose a reason for hiding this comment

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

zerolog里这个方法好像叫unwrap,不过影响不大。

@li-jin-gou li-jin-gou merged commit d6d12b3 into hertz-contrib:main Jun 11, 2023
7 of 8 checks passed
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.

None yet

3 participants