-
Notifications
You must be signed in to change notification settings - Fork 10
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
refactor: use zap instead of logrus #31
Conversation
cmd/template/main.go
Outdated
@@ -67,7 +70,7 @@ func Init() { | |||
// log | |||
EsInit() | |||
klog.SetLevel(klog.LevelDebug) | |||
klog.SetLogger(kitexlogrus.NewLogger(kitexlogrus.WithHook(EsHookLog()))) | |||
klog.SetLogger(kitexzap.NewLogger(EsHookLog()...)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这边 setlogger 后,我们后续是否直接用 klog 就可以记录日志并尝试推送到 es 服务器了?
pkg 里还有一个 logger 的封装,这个 logger 封装是否是多余的?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
并不多余,因为直接使用zap的logger是无法实现kitex中Logger接口的。
kitexzap.NewLogger()
中对zap.Logger
进行了封装,增加了一些方法来实现klog定义的Logger接口。
那我觉得我们专注于添加Hook和自定义输出格式以及路径即可,没有必要再专门去实现他缺少的方法。因为还蛮复杂的。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
那后续就直接使用 klog 就可以了是这样吗?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
是这样的,等明天考完试我会构建一遍项目验证一下。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
任何使用klog输出的日志都会触发ES的hook
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
我觉得可以把logger替换成klog了
嗯,现在已经重构了Logger,并且把kitexzap也集成进了logger包里,暴露了两个函数出来。 另外,还对以前的实现保留了兼容性,现在你既可以使用 |
可以考虑保持和以前一样的开箱即用,不需要在其他地方显式初始化👍 |
我们可以使用init()吗? |
这个init似乎会在这个包第一次用到的时候初始化,但我不确定是否会导致其他地方引用的时候出现没有初始化的问题 init肯定没有问题,就应该做到这样的无感,然后在服务端入口直接调用就好 |
直接改用klog吧,这个替换很方便,一行命令的事。私有不暴露loggerObj,后续统一用klog |
我可以实现正常kitexzap的无感,也就是直接调用logger包或者使用klog(推荐logger)。 |
好的 |
直接使用klog的话,假如我们只有一个main.go ,他直接调用了 我们的Logger包里已经没有loggerObj了,Logger包内就是调用的klog |
我觉得现在有两种前提:
|
逃不开初始化,所以我的理解是还是调用logger包吧。或者logger暴露一个将klog集成zap的init,之后应该都可以直接使用klog了吧 |
是的,这就是我说的两种前提,一种直接使用logger包,我们用zap也可以,用集成了zap的klog也可以,在不需要额外参数(hook)的条件下都是开箱即用的。 一种就是你说的第二种方法,暴露一个显式的接口出来用于初始化klog。 然后在所有的地方调用klog 总的比起来我觉得还是前者更优,因为第一种方法实际上也是在隐式初始化之后封装klog然后暴露那些函数。 |
我也建议第一种,高可拓展性优先 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
作为对日志的优化我觉得没啥问题,但是作为福uu的后端,我们是否真的需要把日志上传到es?或者说这些日志信息有什么价值让我们必须要使用es吗?
因为我之前试过将日志信息上传到es,要让es中的日志信息有价值,必须要给日志做非常完善的index,我们才能从众多日志中筛选出一些信息,我现在不认为上es有什么很高的价值
作为拓展使用,实际部署上去肯定用不了es |
日志肯定需要一个集中存放的地方,本地提供 1-3M的临时缓存即可。这在持久运行后是一个很严重的问题,活动室的Clash容器日志半年不到有整整 27G的日志,单独记录肯定不行。 但是我非常同意不用es,这个java项目过于臃肿了,也许有更轻量化的解决办法,但是tracer和log不是一个东西,一个提供追踪支持,一个提供日志记录 实际上,拥有日志也可以让AI那边参与进行数据分析,统计高频问题和用户习惯 |
在程序入口不是有统一的初始化吗?比如 klog.SetLogger ,在这里完成钩子初始化就可以了吧 实际上可以考虑把这个初始化抽象到pkg里,因为基本所有模块初始化逻辑是一样的,做到keep once kafka的日志不是应该在kafka容器中做的吗?为什么看起来是在程序中的? |
我现在所实现的是无感的直接Logger,以及需要初始化的带有Hook的Logger。 已经把带有Hook的Logger的初始化抽象了,但现在存在包与包之间的引用,我不确定这是好是坏(无环)。 关于kafka的日志是我理解错了,他暴露的不是kafka内部的日志,而是框架返回error。而我们会处理这些error的,所以不必在意这个 :) |
给出两个使用Demo func main() {
logger.Info("Hello World!")
} 第二种显式初始化的,带有hook的Logger func main() {
logger.InitLoggerWithHook(serviceName)
logger.Info("Hello World!")
} 两种方法均已实现 |
正常来说是先init logger再 init config,但是要使用hook的话要先init config.es? |
是这样的。不然的话hook的es.client哪里来呢? |
可是在init config的过程中已经使用了logger来记录.... |
稍等,现在有环,我解决一下 |
但是这是无可避免的吧?你不init config的话,程序甚至不知道你的配置,那怎么给你推送到相应的位置呢? func Init() {
config.Init(*path, serviceName)
utils.InitLoggerWithHook(serviceName)
dal.Init()
tracer.Init()
...
} 这样的话除了init config期间的日志只输出不同步,其他日志都会同步。 |
后续记录日志是直接logger.Info还是klog.Info |
直接调用logger.Info 调用logger包的时候会触发一个初始化,此时的logger只是zap,不包含hook,所以不会同步到es,只会输出到指定的output。 总之,在config init阶段的日志不带有hook,只有在config init之后手动调用 具体代码可以看commit记录
直接调用logger.Info 调用logger包的时候会触发一个初始化,此时的logger只是zap,不包含hook,所以不会同步到es,只会输出到指定的output。 总之,在config init阶段的日志不带有hook,只有在config init之后手动调用 具体代码可以看commit记录 |
utils.InitLoggerWithHook这个函数放在logger包里面会更合适一点? |
两方面考虑
|
嗯,那我没啥问题了,写得很好 |
config 的日志很明显除了hot reload 外都不需要做钩子,没什么逻辑毛病 因为你程序一启动就出问题,那不是直接 panic 了吗? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
做一些简单修改就可以 merge 了
好像梓玄还没上班,你顺便帮忙把目前仓库现存的几个项目的初始化改了吧,当成一个大 pr 合了 |
done |
使用zap替代logrus,维护依赖的统一性,减少依赖的复杂性,增强性能(zap强于logrus4倍)
依赖关系
Refs #19