fix new writer in logx

[marsmay]

NewWriter 方法用于自定义 logx 的 Writer，例如： NewWriter method is used to customize logx's Writer, e.g.:

logx.SetWriter(logx.NewWriter(w))

但当前的 logx.NewWriter 使用 log.New 和 logx.newLogWriter 对传入的自定义 writer 进行了包装； 实际调用时，调用路径为，log.Print -> log.output -> writer.Write； 这会导致一系列问题，因为 log 库中使用了 sync.Pool 产生 *[]byte 用于拼接字符串； 如果自定义 writer 是同步写入，则可能不会出现问题； 如果自定义 writer 是异步写入，如使用 chan 来实现并发安全，这时实际传入 writer.Write 的 []byte 可能具有同一地址，造成写入重复内容； 并且 logx.newLogWriter 函数还忽略了自定义 writer 的 Close 方法； 通过修改 logx.NewWriter 方法，直接传入 io.WriteCloser，可以避免这些问题； 在实际测试中也是符合预期的；

When it is actually called, the path is log.Print -> log.output -> writer.Write; This will lead to some problems, the std log library uses sync.Pool to generate *[]byte for string splicing; If the custom writer writes synchronously, there may not be a problem; But if the custom writer writes asynchronously, e.g., using chan for concurrency safety, the []byte that is actually passed to writer.Write may have the same address, resulting in duplicate content being written; And logx.newLogWriter function also ignores the Close method of the custom writer; These problems can be avoided by modifying the logx.NewWriter method and passing in io.WriteCloser directly; It also meets the expectation in the actual test;

[kevwan]

Thanks for your contribution!

But this is a breaking change. And please add unit tests.

[marsmay]

Add NewAsyncWriter function to avoid affecting previous code. Add unit test for NewAsyncWriter.

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 95.04%. Comparing base (8690859) to head (09e7c39). Report is 129 commits behind head on master.

Additional details and impacted files

| [Files](https://app.codecov.io/gh/zeromicro/go-zero/pull/4311?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zeromicro) | Coverage Δ | | |---|---|---| | [core/logx/writer.go](https://app.codecov.io/gh/zeromicro/go-zero/pull/4311?src=pr&el=tree&filepath=core%2Flogx%2Fwriter.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zeromicro#diff-Y29yZS9sb2d4L3dyaXRlci5nbw==) | `97.06% <100.00%> (+1.53%)` | :arrow_up: | ... and [254 files with indirect coverage changes](https://app.codecov.io/gh/zeromicro/go-zero/pull/4311/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zeromicro)

[kevwan]

Merge this PR later. Let's figure out if it's possible to fix the problem with the same method NewWriter.

Would you please give a simple example to reproduce the problem?

[marsmay]

NewWriter has two main issues.

1. NewWriter uses log.New to wrap the original writer, and use log.Print to write data, log package use a sync.Pool to get *[]byte when write data. If the original writer does not write the data immediately, but processes it asynchronously, this can result in duplicate ([]byte reuse) or no write ([]byte reset) of the data actually written.
2. Ignores the Close() method of raw writer, If raw writer has asynchronous processing, the cleanup logic in the Close() method will not be executed properly, which may result in data loss.

Simply replacing NewAyncWriter with NewWriter for the unit test in this commit will reproduce the problem and the test will fail.

2024年8月27日 22:19，Kevin Wan @.***> 写道：

Merge this PR later. Let's figure out if it's possible to fix the problem with the same method NewWriter.

Would you please give a simple example to reproduce the problem?

— Reply to this email directly, view it on GitHub https://github.com/zeromicro/go-zero/pull/4311#issuecomment-2312707818, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJTM3GB677YQ62CZRWJSHLZTSDHHAVCNFSM6AAAAABMH5YMHKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMJSG4YDOOBRHA. You are receiving this because you authored the thread.