Repository: go-kratos/kratos
Pull request #3121 (2023-12-15 03:49:53)

feat: use writer directly in stdLogger.
<!--
🎉 Thanks for sending a pull request to Kratos! Here are some tips for you:

1. If this is your first time contributing to Kratos, please read our contribution guide: https://go-kratos.dev/en/docs/community/contribution/
2. Ensure you have added or ran the appropriate tests and lint for your PR, please use `make lint` and `make test` before filing your PR, use `make clean` to tidy your go mod.
3. If the PR is unfinished, you may need to mark it as a WIP(Work In Progress) PR or Draft PR
4. Please use a semantic commits format title, such as `<type>[optional scope]: <description>`, see: https://go-kratos.dev/docs/community/contribution#type
5. at the same time, please note that similar work should be submitted in one PR as far as possible to reduce the workload of reviewers. Do not split a work into multiple PR unless it should.
-->

<!--
🎉 感谢您向 Kratos 发送 PR！以下是一些提示：
如果这是你第一次为 Kratos 贡献，请阅读我们的贡献指南：https://go-kratos.dev/en/docs/community/contribution/
2、确保您已经为您的 PR 添加或运行了适当的测试和lint，请在提交PR之前使用“make lint”和“make test”，使用“make clean”整理您的 go.mod。
3、如果 PR 未完成，您可能需要将其标记为 WIP（Work In Progress）PR 或 Draft PR
4、请使用语义提交格式标题，如“<类型>[可选范围]：<说明>`，请参阅：https://go-kratos.dev/docs/community/contribution#type
5. 同时请注意，同类的工作请尽量在一个PR中提交，以减轻 review 者的工作负担，不要把一项工作拆分成很多个PR，除非它应该这样做。
-->


#### Description (what this PR does / why we need it):
<!--
* The description should include the motivation for this PR or contrast this with previous behavior
-->

log.stdLogger use log.Logger internally, and disable all options such as prefix or log flags. log.stdLogger use l.log like a io.Writer. So I removed l.log, use io.Writer directly.


Code "stdLogger use log.Logger with no options":
https://github.com/go-kratos/kratos/blob/08300d8a4178b4e0ff7b226887d66c0764f0d172/log/std.go#L20-L21

Code "stdLogger not use any format features of log.Logger":
https://github.com/go-kratos/kratos/blob/08300d8a4178b4e0ff7b226887d66c0764f0d172/log/std.go#L39-L43

#### Which issue(s) this PR fixes (resolves / be part of):
<!--
* Automatically closes linked issue when PR is merged.
* If your PR is not fully resolved the issue, please use `part of #<issue number>` instead.

Usage: `fixes/resolves #<issue number>`, or `fixes/resolves (paste link of issue)`.
-->


#### Other special notes for the reviewers:
<!--
* Somethings that need extra attention for the reviewers
* Some additional notes, TODO list, etc.
-->

I did not add test for stdLogger.Close, because there is no way to call Close outside.

kvii: A benchmark for it.

* new code

```sh
goos: darwin
goarch: arm64
pkg: github.com/go-kratos/kratos/v2/log
BenchmarkNew-10     16720666            72.12 ns/op        0 B/op          0 allocs/op
PASS
```

* old code

```sh
goos: darwin
goarch: arm64
pkg: github.com/go-kratos/kratos/v2/log
BenchmarkOld-10      6824382           158.4 ns/op        32 B/op          1 allocs/op
PASS
```
codecov-commenter: ## [Codecov](https://app.codecov.io/gh/go-kratos/kratos/pull/3121?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Comparison is base [(`f753a9f`)](https://app.codecov.io/gh/go-kratos/kratos/commit/f753a9f01c1c1999e4d97eb35825be5f53f681b7?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos) 84.43% compared to head [(`9110633`)](https://app.codecov.io/gh/go-kratos/kratos/pull/3121?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos) 84.39%.
> Report is 4 commits behind head on main.

> :exclamation: Current head 9110633 differs from pull request most recent head ca2e9df. Consider uploading reports for the commit ca2e9df to get more accurate results


:exclamation: Your organization needs to install the [Codecov GitHub app](https://github.com/apps/codecov/installations/select_target) to enable full functionality.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##             main    #3121      +/-   ##
==========================================
- Coverage   84.43%   84.39%   -0.04%     
==========================================
  Files          88       88              
  Lines        3983     3987       +4     
==========================================
+ Hits         3363     3365       +2     
- Misses        443      445       +2     
  Partials      177      177              
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/go-kratos/kratos/pull/3121?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos).

kvii: @hawkingrei And could you help me to review this code too? This code removed unnecessary "log.Output"  so it doubles the performance of stdLogger.

Patch:
diff --git a/log/std.go b/log/std.go
index 6f253981..24ee6acb 100644
--- a/log/std.go
+++ b/log/std.go
@@ -4,21 +4,25 @@ import (
 	"bytes"
 	"fmt"
 	"io"
-	"log"
 	"sync"
 )
 
 var _ Logger = (*stdLogger)(nil)
 
+// stdLogger corresponds to the standard library's [log.Logger] and provides
+// similar capabilities. It also can be used concurrently by multiple goroutines.
 type stdLogger struct {
-	log  *log.Logger
-	pool *sync.Pool
+	w         io.Writer
+	isDiscard bool
+	mu        sync.Mutex
+	pool      *sync.Pool
 }
 
 // NewStdLogger new a logger with writer.
 func NewStdLogger(w io.Writer) Logger {
 	return &stdLogger{
-		log: log.New(w, "", 0),
+		w:         w,
+		isDiscard: w == io.Discard,
 		pool: &sync.Pool{
 			New: func() interface{} {
 				return new(bytes.Buffer)
@@ -29,21 +33,27 @@ func NewStdLogger(w io.Writer) Logger {
 
 // Log print the kv pairs log.
 func (l *stdLogger) Log(level Level, keyvals ...interface{}) error {
-	if len(keyvals) == 0 {
+	if l.isDiscard || len(keyvals) == 0 {
 		return nil
 	}
 	if (len(keyvals) & 1) == 1 {
 		keyvals = append(keyvals, "KEYVALS UNPAIRED")
 	}
+
 	buf := l.pool.Get().(*bytes.Buffer)
+	defer l.pool.Put(buf)
+
 	buf.WriteString(level.String())
 	for i := 0; i < len(keyvals); i += 2 {
 		_, _ = fmt.Fprintf(buf, " %s=%v", keyvals[i], keyvals[i+1])
 	}
-	_ = l.log.Output(4, buf.String()) //nolint:gomnd
-	buf.Reset()
-	l.pool.Put(buf)
-	return nil
+	buf.WriteByte('\n')
+	defer buf.Reset()
+
+	l.mu.Lock()
+	defer l.mu.Unlock()
+	_, err := l.w.Write(buf.Bytes())
+	return err
 }
 
 func (l *stdLogger) Close() error {


Test patch:
diff --git a/log/std_test.go b/log/std_test.go
index 2518a046..971b169f 100644
--- a/log/std_test.go
+++ b/log/std_test.go
@@ -1,6 +1,11 @@
 package log
 
-import "testing"
+import (
+	"bytes"
+	"testing"
+
+	"golang.org/x/sync/errgroup"
+)
 
 func TestStdLogger(_ *testing.T) {
 	logger := DefaultLogger
@@ -15,3 +20,21 @@ func TestStdLogger(_ *testing.T) {
 	logger2 := DefaultLogger
 	_ = logger2.Log(LevelDebug)
 }
+
+func TestStdLogger_Log(t *testing.T) {
+	var b bytes.Buffer
+	logger := NewStdLogger(&b)
+
+	var eg errgroup.Group
+	eg.Go(func() error { return logger.Log(LevelInfo, "msg", "a", "k", "v") })
+	eg.Go(func() error { return logger.Log(LevelInfo, "msg", "a", "k", "v") })
+
+	err := eg.Wait()
+	if err != nil {
+		t.Fatalf("log error: %v", err)
+	}
+
+	if s := b.String(); s != "INFO msg=a k=v\nINFO msg=a k=v\n" {
+		t.Fatalf("log not match: %q", s)
+	}
+}