go-grpc-middleware icon indicating copy to clipboard operation
go-grpc-middleware copied to clipboard

Configure logrus level for proto messages

Open coderjz opened this issue 7 years ago • 5 comments

We would like to be able to use the logrus logging interceptor to:

  • Log all messages at info level
  • Log the full json dump of the payload at debug level

I was able to configure the log level for the server messages (e.g. grpc_logrus.UnaryServerInterceptor) with grpc_logrus.WithLevels. However, for payload messages it looks to be hardcoded code link.

Is there interest in adding something similar for the payload levels? I'm open to submitting a pull request, but am not sure the best approach for where to set the payload level; some ideas: Add an option such as grpc_logrus.WithPayloadLevels? Pass it directly into the Payload*Interceptor function? Have it returned from the decider function? Other?

coderjz avatar Oct 03 '17 19:10 coderjz

Glad you're finding the payloads logger useful :)

The idea here was that the Entry you pass into the Payload logger already contains the level you want to log as.

Would that be enough?

mwitkow avatar Oct 20 '17 06:10 mwitkow

We're using both the validation and logging packages and they're quite useful, thank you! :)

Note that I first did the change by using the log level of the entry and the tests were throwing panics. Looking into it, it seems that logrus only sets the log level of the entry when you log at least once, and the entry defaults to loglevel.Panic (link1, link2).

Because of the panics, I instead use the entry's logger level.

To test logrus default levels:

	fmt.Println(logrus.NewEntry(logrus.StandardLogger()).Level)  //Prints panic
	fmt.Println(logrus.NewEntry(logrus.StandardLogger()).Logger.Level)  // Prints info

coderjz avatar Oct 20 '17 17:10 coderjz

Just a question though, the logger's level usually refers to the minimum level that we allow to print logs, whereas the payload logger level is the level we are printing the logs. Could this cause some conceptual confusion to users of the package?

coderjz avatar Oct 20 '17 17:10 coderjz

I'm just wondering whether this could be solved with one solution like #89, and make that extend to the payload logging.

mwitkow avatar Nov 04 '17 08:11 mwitkow

I suspect that we may want to use a PayloadOptions type (feel it would be poor to overload the Options to include modifications to the Payload interceptors) -- let me know if you agree.

Solution #89 is interesting but in our case we just want a default log level, similar to what is provided by grpc_logrus.WithLevels.

I won't have time in the near future to do a PR, but if we agree on the approach I should be able to get to it eventually.

coderjz avatar Nov 06 '17 04:11 coderjz

This is pretty old and we know have v2 code with different structure. Let us know if this is still important, we can reopen.

bwplotka avatar Mar 19 '23 01:03 bwplotka