go-kratos / kratos

Your ultimate Go microservices framework for the cloud-native era.
https://go-kratos.dev
MIT License
23.41k stars 4.01k forks source link

feat(log): add Logger() method for Helper #3443

Closed DCjanus closed 3 weeks ago

DCjanus commented 1 month ago

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

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 commented 1 month ago

@shenqidebaozi feel free to comment something if you want

DCjanus commented 4 weeks ago

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 commented 3 weeks ago

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