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

feat: output log to file and update them periodically #145

Merged
merged 1 commit into from
Dec 20, 2024

Conversation

mutezebra
Copy link
Member

自查 PR 结构

  • PR 标题符合这个格式: <type>(optional scope): <description>

  • 此 PR 标题的描述以用户为导向,足够清晰,其他人可以理解。

  • 我已经对所有 commit 提供了签名(GPG 密钥签名、SSH 密钥签名)

  • 这个 PR 属于强制变更/破坏性更改

如果是,请在 PR 标题中添加 BREAKING CHANGE 前缀,并在 PR 描述中详细说明。

这个 PR 的类型是什么?

feat: 一个新的特性

这个 PR 做了什么 / 我们为什么需要这个 PR?

将原有的日志模块进行重构, 将日志输出到文件当中, 并且定期更新输出位置

(可选)这个 PR 解决了哪个/些 issue?

对 Reviewer 预留的一些提醒

我把 klog 改成自定义 logger 是要控制 loggerObj 的所有权, 这样在更新日志输出文件的时候原来的 logger 可以被释放

包括其他 redis 或 mysql 甚至 es 的模块的与 logger 相关功能都被我改了, 现在我们拥有的是一个完全可控的, 清晰的了解他整个生命周期的 loggerObj

如果我们不更改output.go中的klog的话,最终输出也是一样的,毕竟他们引用一个底层 logger, 但这会增加未知度. 而且很难保证不发生数据竞争的问题.

至于为什么一直在强调数据竞争: 我让现在的日志重定向输出到文件, 并且会以天为单位更新输出的目标, 也就是 logger 的 output 是在变化的, 我要确保的就是这个变化的途中没有新的调用来让 logger 输出日志, 否则就会出现数据竞争, 因为一边在读一边在写

Copy link

codecov bot commented Dec 19, 2024

Codecov Report

Attention: Patch coverage is 46.39640% with 119 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
pkg/logger/klog.go 4.34% 44 Missing ⚠️
pkg/logger/logger.go 61.73% 43 Missing and 1 partial ⚠️
pkg/logger/output.go 31.03% 17 Missing and 3 partials ⚠️
pkg/logger/mysql.go 0.00% 4 Missing ⚠️
config/config.go 0.00% 3 Missing ⚠️
pkg/logger/redis.go 0.00% 2 Missing ⚠️
pkg/base/client/db.go 0.00% 1 Missing ⚠️
pkg/eshook/logger.go 0.00% 1 Missing ⚠️
@@           Coverage Diff            @@
##            main    #145      +/-   ##
========================================
+ Coverage   2.73%   3.12%   +0.38%     
========================================
  Files        180     182       +2     
  Lines      36680   36799     +119     
========================================
+ Hits        1004    1149     +145     
+ Misses     35619   35588      -31     
- Partials      57      62       +5     
Flag Coverage Δ
unittest 3.12% <46.39%> (+0.38%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
pkg/logger/config.go 100.00% <100.00%> (+100.00%) ⬆️
pkg/base/client/db.go 0.00% <0.00%> (ø)
pkg/eshook/logger.go 0.00% <0.00%> (ø)
pkg/logger/redis.go 0.00% <0.00%> (ø)
config/config.go 0.00% <0.00%> (ø)
pkg/logger/mysql.go 0.00% <0.00%> (ø)
pkg/logger/output.go 30.76% <31.03%> (+30.76%) ⬆️
pkg/logger/klog.go 4.34% <4.34%> (ø)
pkg/logger/logger.go 62.40% <61.73%> (+62.39%) ⬆️

... and 3 files with indirect coverage changes

@mutezebra mutezebra requested a review from FantasyRL December 19, 2024 07:55
Copy link
Member

@FantasyRL FantasyRL left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

Choose a reason for hiding this comment

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

这里为什么改成goconvey了,到时候会上可以说一下

Copy link
Member Author

Choose a reason for hiding this comment

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

额. 只是个人习惯问题. 而且我认为 convey 的输出更直观. 全对就全是绿叉叉, 有错误就是红叉叉. 而且可以有层级的测试, 总的来说还是个人习惯

@jiuxia211 jiuxia211 merged commit f4c4864 into west2-online:main Dec 20, 2024
7 checks passed
@mutezebra mutezebra deleted the log branch December 20, 2024 11:24
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.

3 participants