phuslu / log

Fastest structured logging
MIT License
672 stars 44 forks source link

log With() #31

Closed bobmcallan closed 3 years ago

bobmcallan commented 3 years ago

Great logger, simple and easy to use. Are you looking to add a with() function, as this is most important when logging web transactions and correlation of single web requests, through the core application.

Thanks, Bob

phuslu commented 3 years ago

Considered it many times but still not wanna support it currently.

Please let me explain,

  1. support .With() need to add a "Context" field to log.Logger
  2. .With() only benefits the global scope context cases, for request scope contexts, it helps little(maybe we create a new Logger with request scope context, but it hurts performance)
  3. so, I prefer expose a explicit context method in https://github.com/phuslu/log#contextual-fields , below picture is its usage and the performance is good. image

Thanks for reading.

phuslu commented 3 years ago

If you want to just output a map[string]interface{} struct, please use .Fields() method, https://pkg.go.dev/github.com/phuslu/log?utm_source=godoc#Entry.Fields

ogimenezb commented 3 years ago

I have a project that works with different types of hardware and we use .with() a lot because it add "context" to our logs without having to build a context.

We also use https://github.com/grpc-ecosystem/go-grpc-middleware/blob/v2/providers/zerolog/logger.go

I think in my case and also GRPC the .with() has a reasonable usage.

There are many more cases where you need a "base" or "template" log and then add more information to it.

What would you suggest?

phuslu commented 3 years ago

OKay, I added a commit https://github.com/phuslu/log/commit/2d582de24a260957daa54a25b1cba91b956974e7 for this.

ogimenezb commented 3 years ago

Maybe I'm wrong but this add context globally to logger.

The idea would be to add "context"/with to they log entry.

If you are working on a developer board and have several sensors or readers connected you usually want to generate 1 log file globally, not per sensor and add the With. Something like:

sensorLog := log.With().
Str("sensor", "humidity").
Str("model", "xxx").
Int32("pin", "12").
Logger()

sensorLog.Error().Msg("could not connect")

Maybe I'm missing something?

phuslu commented 3 years ago

Can see https://github.com/phuslu/log#contextual-fields if solve your requirement?

ogimenezb commented 3 years ago

When using:

logger := log.Logger{
    Level:   log.InfoLevel,
    Context: log.NewContext(nil).Str("ctx", "some_ctx").Value(),
}

It's not using the same config as DefaultLogger you are creating a new config for logging.

This works:

logger := log.DefaultLogger
logger.Context = plog.NewContext(nil).Str("ctx", "some_ctx").Value()
logger.Info().Msg("hello world from context fields")
log.Info().Msg("hello world")

Maybe add this example in Readme

phuslu commented 3 years ago

Yes, I agree/admit that

It's not using the same config as DefaultLogger you are creating a new config for logging.

will supervise people.

But I'd like to say it's a design philosophy, I want to keep logger/writers be a "Value Semantics", as a struct like stdlib "http.Transport"

ogimenezb commented 3 years ago

I agree.

I have several interface{} for hardware and ideally want to add this extra logging information per interface/defaultLog instead of copy paste each time I log. Also when having several compress methods and others implementations where you need to add this extra information on an implementation basis.

Maybe it's because when coming from other logger you have With() or WithFields() and use those methods to add base information to the log.

ogimenezb commented 3 years ago

I'm trying to integrate phuslog into grpc-gateway, go-grpc-middleware but I need this interface. https://github.com/grpc-ecosystem/go-grpc-middleware/blob/f6fb287988a0db3ab9700aab33cb440e6d99064a/interceptors/logging/logging.go#L85 Any ideas how to proceed?

phuslu commented 3 years ago

I afraid that you must write a wrapper of log.Logger like https://github.com/phuslu/log/blob/master/logr.go

it implements the logr.Logger interface, and the example is https://github.com/phuslu/log#stdlog--logr--grpc-interceptor

ogimenezb commented 3 years ago

Happy to report that it's finally working!!! That change you made on context and your links helped a lot.

Going for a walk, then final testing then pull request to you and grpc-middleware.

{"time":"2021-06-13T18:52:36.060+02:00","level":"info","message":"[core]Subchannel Connectivity change to READY"}
{"time":"2021-06-13T18:52:36.062+02:00","level":"info","message":"[core]Subchannel Connectivity change to READY"}
{"time":"2021-06-13T18:52:36.062+02:00","level":"info","message":"[core]Subchannel Connectivity change to READY"}
{"time":"2021-06-13T18:52:36.062+02:00","level":"info","message":"[core]pickfirstBalancer: UpdateSubConnState: 0x20ca81a0, {READY \u003cnil>}"}
{"time":"2021-06-13T18:52:36.063+02:00","level":"info","message":"[core]Channel Connectivity change to READY"}
{"time":"2021-06-13T18:52:36.063+02:00","level":"info","message":"[core]pickfirstBalancer: UpdateSubConnState: 0x2088ad50, {READY \u003cnil>}"}
{"time":"2021-06-13T18:52:36.064+02:00","level":"info","message":"[core]Channel Connectivity change to READY"}
{"time":"2021-06-13T18:52:36.063+02:00","level":"info","message":"[core]pickfirstBalancer: UpdateSubConnState: 0x20a60200, {READY \u003cnil>}"}
{"time":"2021-06-13T18:52:36.064+02:00","level":"info","message":"[core]Channel Connectivity change to READY"}
{"time":"2021-06-13T18:52:36.063+02:00","level":"info","message":"[core]pickfirstBalancer: UpdateSubConnState: 0x2088aa70, {READY \u003cnil>}"}
{"time":"2021-06-13T18:52:36.065+02:00","level":"info","message":"[core]Channel Connectivity change to READY"}
{"time":"2021-06-13T18:52:36.064+02:00","level":"info","message":"[core]Subchannel Connectivity change to READY"}
{"time":"2021-06-13T18:52:36.061+02:00","level":"info","message":"[core]pickfirstBalancer: UpdateSubConnState: 0x2088a9c0, {READY \u003cnil>}"}
{"time":"2021-06-13T18:52:36.065+02:00","level":"info","message":"[core]Channel Connectivity change to READY"}
{"time":"2021-06-13T18:52:36.065+02:00","level":"info","message":"[core]pickfirstBalancer: UpdateSubConnState: 0x2088ac60, {READY \u003cnil>}"}
{"time":"2021-06-13T18:52:36.066+02:00","level":"info","message":"[core]Channel Connectivity change to READY"}
{"time":"2021-06-13T18:52:36.068+02:00","level":"info","message":"[core]Subchannel Connectivity change to READY"}
{"time":"2021-06-13T18:52:36.068+02:00","level":"info","message":"[core]Subchannel Connectivity change to READY"}
{"time":"2021-06-13T18:52:36.068+02:00","level":"info","message":"[core]pickfirstBalancer: UpdateSubConnState: 0x20a60040, {READY \u003cnil>}"}
{"time":"2021-06-13T18:52:36.068+02:00","level":"info","message":"[core]Channel Connectivity change to READY"}
{"time":"2021-06-13T18:52:36.068+02:00","level":"info","message":"[core]pickfirstBalancer: UpdateSubConnState: 0x20a60570, {READY \u003cnil>}"}
{"time":"2021-06-13T18:52:36.069+02:00","level":"info","message":"[core]Channel Connectivity change to READY"}
{"time":"2021-06-13T18:52:53.782+02:00","level":"info","protocol":"grpc","grpc.component":"server","grpc.service":"idManager.api.Devices","grpc.method":"GetCapture","grpc.method_type":"unary","grpc.start_time":"2021-06-13T18:52:53+02:00","grpc.code":"OK","peer.address":"127.0.0.1:64034","grpc.time_ms":"0","message":"started call"}
{"time":"2021-06-13T18:52:57.694+02:00","level":"info","protocol":"grpc","grpc.component":"server","grpc.service":"idManager.api.Devices","grpc.method":"GetCapture","grpc.method_type":"unary","grpc.start_time":"2021-06-13T18:52:53+02:00","grpc.code":"OK","peer.address":"127.0.0.1:64034","grpc.time_ms":"3911.902","message":"finished call"}
{"time":"2021-06-13T18:53:53.492+02:00","level":"info","protocol":"grpc","grpc.component":"server","grpc.service":"idManager.api.Devices","grpc.method":"GetCapabilities","grpc.method_type":"unary","grpc.start_time":"2021-06-13T18:53:53+02:00","grpc.code":"OK","peer.address":"127.0.0.1:64034","grpc.time_ms":"0","message":"started call"}
{"time":"2021-06-13T18:53:53.497+02:00","level":"info","protocol":"grpc","grpc.component":"server","grpc.service":"idManager.api.Devices","grpc.method":"GetCapabilities","grpc.method_type":"unary","grpc.start_time":"2021-06-13T18:53:53+02:00","grpc.code":"OK","peer.address":"127.0.0.1:64034","grpc.time_ms":"4.997","message":"finished call"}
{"time":"2021-06-13T18:53:59.831+02:00","level":"info","protocol":"grpc","grpc.component":"server","grpc.service":"idManager.api.Devices","grpc.method":"GetDescriptions","grpc.method_type":"unary","grpc.start_time":"2021-06-13T18:53:59+02:00","grpc.code":"OK","peer.address":"127.0.0.1:64034","grpc.time_ms":"0","message":"started call"}
{"time":"2021-06-13T18:53:59.837+02:00","level":"info","protocol":"grpc","grpc.component":"server","grpc.service":"idManager.api.Devices","grpc.method":"GetDescriptions","grpc.method_type":"unary","grpc.start_time":"2021-06-13T18:53:59+02:00","grpc.code":"OK","peer.address":"127.0.0.1:64034","grpc.time_ms":"5.499","message":"finished call"}
{"time":"2021-06-13T18:54:13.313+02:00","level":"info","protocol":"grpc","grpc.component":"server","grpc.service":"idManager.api.Devices","grpc.method":"GetDescriptions","grpc.method_type":"unary","grpc.start_time":"2021-06-13T18:54:13+02:00","grpc.code":"OK","peer.address":"127.0.0.1:64034","grpc.time_ms":"0","message":"started call"}
{"time":"2021-06-13T18:56:48.665+02:00","level":"error","protocol":"grpc","grpc.component":"server","grpc.service":"idManager.api.Devices","grpc.method":"GetDescriptions","grpc.method_type":"unary","grpc.start_time":"2021-06-13T18:56:48+02:00","grpc.code":"Internal","grpc.error":"rpc error: code = Internal desc = device 57720555-9e2b-3457-949b not found","peer.address":"127.0.0.1:54542","grpc.time_ms":"35.001","message":"finished call"}