Repository: go-kratos/kratos
Pull request #3455 (2024-10-31 09:17:18)

fix: return inner Logger from GetLogger for log.WithContext reuse
fix: Ensure thread safety by eliminating lock copy in GetLogger

<!--
🎉 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
-->


#### 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:
<!--
* Some things that need extra attention for the reviewers
* Some additional notes, TODO list, etc.
-->


shenqidebaozi: This pull request includes changes to improve the thread safety of the global logger and adds a new test to ensure the logger works correctly with context values.

Improvements to thread safety:

* [`log/global.go`](diffhunk://#diff-4c6fd012b19506d3ff44d0b0ad6032365a16d894061760e0469cf4988f8c85baL16-R16): Changed the `lock` field in `loggerAppliance` from `sync.Mutex` to `sync.RWMutex` to allow for concurrent read access.
* [`log/global.go`](diffhunk://#diff-4c6fd012b19506d3ff44d0b0ad6032365a16d894061760e0469cf4988f8c85baL38-R40): Updated the `GetLogger` function to use a read lock (`RLock`) when accessing the global logger, ensuring thread-safe read operations.

Enhancements to testing:

* [`log/global_test.go`](diffhunk://#diff-d37c1b9730a4f68e52fad1223525fdb0921769e8e6c652e755eaf06f211fea54R120-R139): Added a new test function `TestContextWithGlobalLog` to verify that the global logger correctly logs context values, ensuring the logger's functionality with context-based trace IDs.

Patch:
diff --git a/log/global.go b/log/global.go
index f1984fc3..ae9244df 100644
--- a/log/global.go
+++ b/log/global.go
@@ -13,7 +13,7 @@ var global = &loggerAppliance{}
 // loggerAppliance is the proxy of `Logger` to
 // make logger change will affect all sub-logger.
 type loggerAppliance struct {
-	lock sync.Mutex
+	lock sync.RWMutex
 	Logger
 }
 
@@ -35,7 +35,9 @@ func SetLogger(logger Logger) {
 
 // GetLogger returns global logger appliance as logger in current process.
 func GetLogger() Logger {
-	return global
+	global.lock.RLock()
+	defer global.lock.RUnlock()
+	return global.Logger
 }
 
 // Log Print log by level and keyvals.


Test patch:
diff --git a/log/global_test.go b/log/global_test.go
index 8ee15705..469fae31 100644
--- a/log/global_test.go
+++ b/log/global_test.go
@@ -117,3 +117,23 @@ func TestGlobalContext(t *testing.T) {
 		t.Errorf("Expected:%s, got:%s", "INFO msg=111", buffer.String())
 	}
 }
+
+func TestContextWithGlobalLog(t *testing.T) {
+	buffer := &bytes.Buffer{}
+
+	type traceKey struct{}
+	// set "trace-id" Valuer
+	newLogger := With(NewStdLogger(buffer), "trace-id", Valuer(func(ctx context.Context) interface{} {
+		return ctx.Value(traceKey{})
+	}))
+
+	SetLogger(newLogger)
+
+	// add value to ctx
+	ctx := context.WithValue(context.Background(), traceKey{}, "test-trace-id")
+
+	_ = WithContext(ctx, GetLogger()).Log(LevelInfo)
+	if buffer.String() != "INFO trace-id=test-trace-id\n" {
+		t.Errorf("Expected:%s, got:%s", "INFO trace-id=test-trace-id", buffer.String())
+	}
+}