Repository: go-kratos/kratos
Pull request #3443 (2024-10-21 07:20:52)

feat(log): add Logger() method for Helper
### Description

In the real world, some log fields have significant contextual relevance. A reasonable approach is to use `log.With` to attach fields and create a new `Logger`, then convert it to `*log.Helper` held by an object.

For statically created Loggers, this works well; but for dynamically created Loggers, such as when an API request contains a user ID and we perform many operations for it, in order to easily query logs afterwards, we need to include his user ID at each logging point.

Because Service objects generally only contain `*log.Helper`, there is currently no public method to obtain `log.Logger` from `*log.Helper` and add fields, and we have to pass all those fields to every logging point, which is quiet boring.

### Show case

```go
package kratos

import (
	"fmt"
	"github.com/go-kratos/kratos/v2/middleware/tracing"
	"os"

	"github.com/go-kratos/kratos/v2/log"
)

type Worker struct {
	log *log.Helper
}

func main() {
	id, _ := os.Hostname()
	logger := log.With(log.NewStdLogger(os.Stdout),
		"ts", log.DefaultTimestamp,
		"caller", log.DefaultCaller,
		"service.id", id,
		"service.name", Name,
		"service.version", Version,
		"trace.id", tracing.TraceID(),
		"span.id", tracing.SpanID(),
	)
	var workerId string
	// some logic to get workerId
	NewWorker(logger, workerId).Work()
}

func NewWorker(logger log.Logger, workerId string) *Worker {
	logger = log.With(logger, "module", "worker", "worker_id", workerId)
	return &Worker{
		log: log.NewHelper(logger),
	}
}

// Work method is an example of how to use the new method `(*log.Helper).Logger()` to get the original logger and
// attach some fields to it. This is useful when you want to identify some concurrent goroutines.
func (w *Worker) Work() {
	// ... some preparation work that may emit logs

	// in the real world, tasks are created dynamically, and the number of tasks is not fixed
	for id := 0; id < 10; id++ {
		workerId := fmt.Sprintf("sub_worker_%d", id)
		if id%2 == 0 {
			go NewSubWorker(w.log.Logger(), workerId).Work()
		} else {
			go NewSubWorkerWithoutSubLogger(w.log, workerId).Work()
		}

	}
}

type SubWorker1 struct {
	log *log.Helper
}

func NewSubWorker(logger log.Logger, subWorkerId string) *SubWorker1 {
	logger = log.With(logger, "module", "sub_worker", "sub_worker_id", subWorkerId)
	return &SubWorker1{
		log: log.NewHelper(logger),
	}
}

func (w *SubWorker1) Work() {
	var err error
	// do something
	if err != nil {
		w.log.Errorw(
			"event", "error_event_1",
			"error", err,
		)
	}
	// do something
	if err != nil {
		w.log.Errorw("event", "error_event_2", "error", err)
	}

	// and so on, you can add more log points here

	return
}

type SubWorkerWithoutSubLogger struct {
	log  *log.Helper
	name string
}

func NewSubWorkerWithoutSubLogger(l *log.Helper, name string) *SubWorkerWithoutSubLogger {
	return &SubWorkerWithoutSubLogger{
		log:  l,
		name: name,
	}
}

func (w *SubWorkerWithoutSubLogger) Work() {
	var err error
	// do something
	if err != nil {
		// without the new method, you have to attach the fields every time
		w.log.Errorw(
			"module", "sub_worker",
			"sub_worker_id", w.name,
			"event", "error_event_1",
			"error", err,
		)
	}
	// do something
	if err != nil {
		w.log.Errorw(
			"module", "sub_worker",
			"sub_worker_id", w.name,
			"event", "error_event_2",
			"error", err,
		)
	}

	// and so on, you can add more log points here

	return
}

```

DCjanus: @shenqidebaozi feel free to comment something if you want
DCjanus: I have communicated with @shenqidebaozi about this PR through other channels, and I'd like to sync the discussion content here:

@shenqidebaozi believes that `With` method should enough to be exposed to LogHelper. 

But I think the `With` method is not a lightweight operation (it requires copying an entire list), and providing an easily callable `With` method might lead users to misuse it; exposing only the `Logger` method can encourage users to reuse Logger instance. And  @shenqidebaozi agreement with that.

DCjanus: @shenqidebaozi Hi, would you mind to take a look on this PR, maybe it's time to merge this

Patch:
diff --git a/log/helper.go b/log/helper.go
index 19fd613b..7eb5b25d 100644
--- a/log/helper.go
+++ b/log/helper.go
@@ -75,6 +75,11 @@ func (h *Helper) Enabled(level Level) bool {
 	return true
 }
 
+// Logger returns logger in the helper.
+func (h *Helper) Logger() Logger {
+	return h.logger
+}
+
 // Log Print log by level and keyvals.
 func (h *Helper) Log(level Level, keyvals ...interface{}) {
 	_ = h.logger.Log(level, keyvals...)


Test patch:
diff --git a/log/helper_test.go b/log/helper_test.go
index 750710c2..c056fd88 100644
--- a/log/helper_test.go
+++ b/log/helper_test.go
@@ -8,7 +8,12 @@ import (
 )
 
 func TestHelper(_ *testing.T) {
-	logger := With(DefaultLogger, "ts", DefaultTimestamp, "caller", DefaultCaller)
+	logger := With(
+		DefaultLogger,
+		"ts", DefaultTimestamp,
+		"caller", DefaultCaller,
+		"module", "test",
+	)
 	log := NewHelper(logger)
 
 	log.Log(LevelDebug, "msg", "test debug")
@@ -19,6 +24,12 @@ func TestHelper(_ *testing.T) {
 	log.Warn("test warn")
 	log.Warnf("test %s", "warn")
 	log.Warnw("log", "test warn")
+
+	subLogger := With(log.Logger(),
+		"module", "sub",
+	)
+	subLog := NewHelper(subLogger)
+	subLog.Infof("sub logger test with level %s", "info")
 }
 
 func TestHelperWithMsgKey(_ *testing.T) {