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(glog): add log rotation support for short-running process #2658

Merged
merged 2 commits into from
Jun 29, 2023

Conversation

xuyue86
Copy link
Contributor

@xuyue86 xuyue86 commented May 19, 2023

1.add file pattern matching detection for log rotation and backup file limitation:
before this commit:
log rotation is not working on short-running process

2.backup file limitation will working on all files matching pattern:

before this commit:
access-{y-m-d}.log will generate log like:
access-2023-05-19.log
access-2023-05-18.log

the limitation only working on rotation files like "access-2023-05-19.xxxxxxxxxxxx.log" , not counting the "access-2023-05-18.xxxxxxxxxxxxx.log"

@gqcn
Copy link
Member

gqcn commented May 23, 2023

@xuyue86 The CI failed: please gofmt -s your codes, using make lint to check your codes.

@gqcn
Copy link
Member

gqcn commented May 29, 2023

@xuyue86 你这样改动确实能解决日志文件滚动的问题,但会额外引发性能问题,因为每一次写日志都会触发滚动逻辑。我建议你不要调整异步逻辑,保留其他可能引发日志文件滚动逻辑失败的代码。

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


@xuyue86 Your changes can indeed solve the problem of log file scrolling, but it will cause additional performance problems, because every time a log is written, the scrolling logic will be triggered.

@xuyue86
Copy link
Contributor Author

xuyue86 commented May 31, 2023

@gqcn We have this condition "l.config.rotatedHandlerInitialized.Cas(false, true) ",I think it will only triggered the first time when calling print.
if !l.config.rotatedHandlerInitialized.Val() && l.config.rotatedHandlerInitialized.Cas(false, true) { l.rotateChecksTimely(ctx) intlog.Printf(ctx, "logger rotation initialized: every %s", l.config.RotateCheckInterval.String()) }

add file pattern matching detection for log rotation and backup file limitation.
backupfile limitation will working on all files matching pattern.
@codecov-commenter
Copy link

codecov-commenter commented May 31, 2023

Codecov Report

Patch coverage: 70.68% and project coverage change: -0.04 ⚠️

Comparison is base (aa8eabd) 78.88% compared to head (18bd3fa) 78.84%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2658      +/-   ##
==========================================
- Coverage   78.88%   78.84%   -0.04%     
==========================================
  Files         631      631              
  Lines       51784    51790       +6     
==========================================
- Hits        40848    40833      -15     
- Misses       8913     8934      +21     
  Partials     2023     2023              
Flag Coverage Δ
go-1.15-386 78.85% <70.68%> (?)
go-1.15-amd64 78.84% <70.68%> (-0.07%) ⬇️
go-1.16-386 78.84% <70.68%> (?)
go-1.16-amd64 78.84% <70.68%> (-0.05%) ⬇️
go-1.17-386 78.85% <70.68%> (?)
go-1.17-amd64 78.84% <70.68%> (-0.07%) ⬇️
go-1.18-386 78.78% <70.68%> (-0.06%) ⬇️
go-1.18-amd64 78.77% <70.68%> (-0.07%) ⬇️
go-1.19-386 78.84% <70.68%> (-0.06%) ⬇️
go-1.19-amd64 78.86% <70.68%> (-0.05%) ⬇️
go-1.20-386 78.83% <70.68%> (+<0.01%) ⬆️
go-1.20-amd64 78.84% <70.68%> (-0.04%) ⬇️

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

Impacted Files Coverage Δ
os/glog/glog_logger_rotate.go 61.23% <70.17%> (-2.12%) ⬇️
os/glog/glog_logger.go 73.33% <100.00%> (-2.97%) ⬇️

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

os/glog/glog_logger_rotate.go Show resolved Hide resolved
@gqcn gqcn merged commit 3fab7a3 into gogf:master Jun 29, 2023
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.

4 participants