No-SilverBullet commented on code in PR #3023:
URL: https://github.com/apache/dubbo-go/pull/3023#discussion_r2343737692
##########
filter/accesslog/filter.go:
##########
@@ -182,6 +189,68 @@ func (f *Filter) OnResponse(_ context.Context, result
result.Result, _ base.Invo
return result
}
+// start initializes and starts the background goroutine for processing logs
+func (f *Filter) start() {
+ go f.processLogs()
+}
+
+// processLogs runs in a background goroutine to process log data
+func (f *Filter) processLogs() {
+ defer func() {
+ if r := recover(); r != nil {
+ logger.Errorf("AccessLog processLogs panic: %v", r)
+ }
+ f.drainLogs()
+ }()
+
+ for {
+ select {
+ case accessLogData, ok := <-f.logChan:
+ if !ok {
+ return
+ }
+ f.writeLogToFileWithTimeout(accessLogData,
5*time.Second)
+ case <-f.ctx.Done():
+ return
+ }
+ }
+}
+
+// drainLogs drains remaining log data with timeout protection
+func (f *Filter) drainLogs() {
+ timeout := time.After(5 * time.Second)
+ for {
+ select {
+ case accessLogData, ok := <-f.logChan:
+ if !ok {
+ return
+ }
+ f.writeLogToFileWithTimeout(accessLogData,
1*time.Second)
+ case <-timeout:
+ logger.Warnf("AccessLog drain timeout, some logs may be
lost")
+ return
+ default:
+ return
+ }
+ }
+}
+
+// writeLogToFileWithTimeout writes log with timeout protection
+func (f *Filter) writeLogToFileWithTimeout(data Data, timeout time.Duration) {
+ done := make(chan struct{})
+ go func() {
+ defer close(done)
+ f.writeLogToFile(data)
+ }()
+
+ select {
+ case <-done:
+ // Success
Review Comment:
> Hi, @No-SilverBullet. Good suggestion! However, I have concerns about
adding an Info-level success log here due to potential log volume.
>
> Analysis:
>
> * This method is called for every RPC request that has access logging
enabled
> * In high-traffic scenarios, this would generate massive amounts of
success logs
> * Could lead to log file bloat and I/O pressure
>
> Would you prefer adding a Debug-level log instead?
Your idea is very thoughtful, but it also reminds me that it seems that it
would be a good feature to introduce the configuration of log sampling rate in
the future.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@dubbo.apache.org
For additional commands, e-mail: notifications-h...@dubbo.apache.org
