Repository: go-kratos/kratos
Pull request #2869 (2023-06-15 08:37:02)

fix logging middle don't get trace_id value when logger is filterLogger
<!--
🎉 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):
fix logging middle don't get trace_id value when logger is filterLogger


#### Which issue(s) this PR fixes (resolves / be part of):
#2570 
#2571 


codecov-commenter: ## [Codecov](https://app.codecov.io/gh/go-kratos/kratos/pull/2869?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos) Report
> Merging [#2869](https://app.codecov.io/gh/go-kratos/kratos/pull/2869?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos) (af2fcd7) into [main](https://app.codecov.io/gh/go-kratos/kratos/commit/e86ad248c3c1e5da366dd6d79847aabd840ad4bb?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos) (e86ad24) will **increase** coverage by `0.04%`.
> The diff coverage is `100.00%`.

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

:exclamation: 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](https://github.com/apps/codecov) for your organization. [Read more](https://about.codecov.io/blog/codecov-is-updating-its-github-integration/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos).

```diff
@@            Coverage Diff             @@
##             main    #2869      +/-   ##
==========================================
+ Coverage   84.42%   84.47%   +0.04%     
==========================================
  Files          88       88              
  Lines        3936     3948      +12     
==========================================
+ Hits         3323     3335      +12     
  Misses        437      437              
  Partials      176      176              
```


| [Impacted Files](https://app.codecov.io/gh/go-kratos/kratos/pull/2869?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos) | Coverage Δ | |
|---|---|---|
| [log/filter.go](https://app.codecov.io/gh/go-kratos/kratos/pull/2869?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos#diff-bG9nL2ZpbHRlci5nbw==) | `95.45% <100.00%> (+0.21%)` | :arrow_up: |
| [log/log.go](https://app.codecov.io/gh/go-kratos/kratos/pull/2869?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos#diff-bG9nL2xvZy5nbw==) | `100.00% <100.00%> (ø)` | |



Patch:
diff --git a/cmd/protoc-gen-go-http/http.go b/cmd/protoc-gen-go-http/http.go
index 8d799a3b..f52a2077 100644
--- a/cmd/protoc-gen-go-http/http.go
+++ b/cmd/protoc-gen-go-http/http.go
@@ -24,7 +24,7 @@ const (
 var methodSets = make(map[string]int)
 
 // generateFile generates a _http.pb.go file containing kratos errors definitions.
-func generateFile(gen *protogen.Plugin, file *protogen.File, omitempty bool) *protogen.GeneratedFile {
+func generateFile(gen *protogen.Plugin, file *protogen.File, omitempty bool, omitemptyPrefix string) *protogen.GeneratedFile {
 	if len(file.Services) == 0 || (omitempty && !hasHTTPRule(file.Services)) {
 		return nil
 	}
@@ -42,12 +42,12 @@ func generateFile(gen *protogen.Plugin, file *protogen.File, omitempty bool) *pr
 	g.P()
 	g.P("package ", file.GoPackageName)
 	g.P()
-	generateFileContent(gen, file, g, omitempty)
+	generateFileContent(gen, file, g, omitempty, omitemptyPrefix)
 	return g
 }
 
 // generateFileContent generates the kratos errors definitions, excluding the package statement.
-func generateFileContent(gen *protogen.Plugin, file *protogen.File, g *protogen.GeneratedFile, omitempty bool) {
+func generateFileContent(gen *protogen.Plugin, file *protogen.File, g *protogen.GeneratedFile, omitempty bool, omitemptyPrefix string) {
 	if len(file.Services) == 0 {
 		return
 	}
@@ -59,11 +59,11 @@ func generateFileContent(gen *protogen.Plugin, file *protogen.File, g *protogen.
 	g.P()
 
 	for _, service := range file.Services {
-		genService(gen, file, g, service, omitempty)
+		genService(gen, file, g, service, omitempty, omitemptyPrefix)
 	}
 }
 
-func genService(_ *protogen.Plugin, file *protogen.File, g *protogen.GeneratedFile, service *protogen.Service, omitempty bool) {
+func genService(_ *protogen.Plugin, file *protogen.File, g *protogen.GeneratedFile, service *protogen.Service, omitempty bool, omitemptyPrefix string) {
 	if service.Desc.Options().(*descriptorpb.ServiceOptions).GetDeprecated() {
 		g.P("//")
 		g.P(deprecationComment)
@@ -81,11 +81,11 @@ func genService(_ *protogen.Plugin, file *protogen.File, g *protogen.GeneratedFi
 		rule, ok := proto.GetExtension(method.Desc.Options(), annotations.E_Http).(*annotations.HttpRule)
 		if rule != nil && ok {
 			for _, bind := range rule.AdditionalBindings {
-				sd.Methods = append(sd.Methods, buildHTTPRule(g, method, bind))
+				sd.Methods = append(sd.Methods, buildHTTPRule(g, service, method, bind, omitemptyPrefix))
 			}
-			sd.Methods = append(sd.Methods, buildHTTPRule(g, method, rule))
+			sd.Methods = append(sd.Methods, buildHTTPRule(g, service, method, rule, omitemptyPrefix))
 		} else if !omitempty {
-			path := fmt.Sprintf("/%s/%s", service.Desc.FullName(), method.Desc.Name())
+			path := fmt.Sprintf("%s/%s/%s", omitemptyPrefix, service.Desc.FullName(), method.Desc.Name())
 			sd.Methods = append(sd.Methods, buildMethodDesc(g, method, http.MethodPost, path))
 		}
 	}
@@ -109,7 +109,7 @@ func hasHTTPRule(services []*protogen.Service) bool {
 	return false
 }
 
-func buildHTTPRule(g *protogen.GeneratedFile, m *protogen.Method, rule *annotations.HttpRule) *methodDesc {
+func buildHTTPRule(g *protogen.GeneratedFile, service *protogen.Service, m *protogen.Method, rule *annotations.HttpRule, omitemptyPrefix string) *methodDesc {
 	var (
 		path         string
 		method       string
@@ -137,6 +137,12 @@ func buildHTTPRule(g *protogen.GeneratedFile, m *protogen.Method, rule *annotati
 		path = pattern.Custom.Path
 		method = pattern.Custom.Kind
 	}
+	if method == "" {
+		method = http.MethodPost
+	}
+	if path == "" {
+		path = fmt.Sprintf("%s/%s/%s", omitemptyPrefix, service.Desc.FullName(), m.Desc.Name())
+	}
 	body = rule.Body
 	responseBody = rule.ResponseBody
 	md := buildMethodDesc(g, m, method, path)
diff --git a/cmd/protoc-gen-go-http/main.go b/cmd/protoc-gen-go-http/main.go
index a18fbddc..3752019c 100644
--- a/cmd/protoc-gen-go-http/main.go
+++ b/cmd/protoc-gen-go-http/main.go
@@ -9,8 +9,9 @@ import (
 )
 
 var (
-	showVersion = flag.Bool("version", false, "print the version and exit")
-	omitempty   = flag.Bool("omitempty", true, "omit if google.api is empty")
+	showVersion     = flag.Bool("version", false, "print the version and exit")
+	omitempty       = flag.Bool("omitempty", true, "omit if google.api is empty")
+	omitemptyPrefix = flag.String("omitempty_prefix", "", "omit if google.api is empty")
 )
 
 func main() {
@@ -27,7 +28,7 @@ func main() {
 			if !f.Generate {
 				continue
 			}
-			generateFile(gen, f, *omitempty)
+			generateFile(gen, f, *omitempty, *omitemptyPrefix)
 		}
 		return nil
 	})
diff --git a/log/filter.go b/log/filter.go
index 993fa926..7ef96655 100644
--- a/log/filter.go
+++ b/log/filter.go
@@ -1,5 +1,7 @@
 package log
 
+import "context"
+
 // FilterOption is filter option.
 type FilterOption func(*Filter)
 
@@ -39,6 +41,7 @@ func FilterFunc(f func(level Level, keyvals ...interface{}) bool) FilterOption {
 
 // Filter is a logger filter.
 type Filter struct {
+	ctx    context.Context
 	logger Logger
 	level  Level
 	key    map[interface{}]struct{}
@@ -67,6 +70,9 @@ func (f *Filter) Log(level Level, keyvals ...interface{}) error {
 	// prefixkv contains the slice of arguments defined as prefixes during the log initialization
 	var prefixkv []interface{}
 	l, ok := f.logger.(*logger)
+	if ok {
+		l.ctx = f.ctx
+	}
 	if ok && len(l.prefix) > 0 {
 		prefixkv = make([]interface{}, 0, len(l.prefix))
 		prefixkv = append(prefixkv, l.prefix...)
diff --git a/log/log.go b/log/log.go
index 5022ba4b..38cef805 100644
--- a/log/log.go
+++ b/log/log.go
@@ -51,13 +51,27 @@ func With(l Logger, kv ...interface{}) Logger {
 // to ctx. The provided ctx must be non-nil.
 func WithContext(ctx context.Context, l Logger) Logger {
 	c, ok := l.(*logger)
-	if !ok {
-		return &logger{logger: l, ctx: ctx}
+	if ok {
+		return &logger{
+			logger:    c.logger,
+			prefix:    c.prefix,
+			hasValuer: c.hasValuer,
+			ctx:       ctx,
+		}
 	}
-	return &logger{
-		logger:    c.logger,
-		prefix:    c.prefix,
-		hasValuer: c.hasValuer,
-		ctx:       ctx,
+
+	f, ok := l.(*Filter)
+	if ok {
+		f.ctx = ctx
+		return &Filter{
+			ctx:    ctx,
+			logger: f.logger,
+			level:  f.level,
+			key:    f.key,
+			value:  f.value,
+			filter: f.filter,
+		}
 	}
+
+	return &logger{logger: l, ctx: ctx}
 }


Test patch:
diff --git a/log/filter_test.go b/log/filter_test.go
index 9258400c..679fbe8c 100644
--- a/log/filter_test.go
+++ b/log/filter_test.go
@@ -2,7 +2,9 @@ package log
 
 import (
 	"bytes"
+	"context"
 	"io"
+	"strings"
 	"testing"
 )
 
@@ -140,3 +142,33 @@ func testFilterFuncWithLoggerPrefix(level Level, keyvals ...interface{}) bool {
 	}
 	return false
 }
+
+func TestFilterWithContext(t *testing.T) {
+	ctxKey := struct{}{}
+	ctxValue := "filter test value"
+
+	v1 := func() Valuer {
+		return func(ctx context.Context) interface{} {
+			return ctx.Value(ctxKey)
+		}
+	}
+
+	info := &bytes.Buffer{}
+
+	logger := With(NewStdLogger(info), "request_id", v1())
+	filter := NewFilter(logger, FilterLevel(LevelError))
+
+	ctx := context.WithValue(context.Background(), ctxKey, ctxValue)
+
+	_ = WithContext(ctx, filter).Log(LevelInfo, "kind", "test")
+
+	if info.String() != "" {
+		t.Error("filter is not woring")
+		return
+	}
+
+	_ = WithContext(ctx, filter).Log(LevelError, "kind", "test")
+	if !strings.Contains(info.String(), ctxValue) {
+		t.Error("don't read ctx value")
+	}
+}