Add Severity and VerboseLevel to grpclog. #922
Conversation
grpclog/logger.go
Outdated
| stringerPrintln | ||
| ) | ||
|
|
||
| type grpclogStringer struct { |
There was a problem hiding this comment.
call this "Stringer" since the package name is grpclog already?
|
This approach adds an allocation to every logged line 🙍 |
|
Reverted the commit to use stringer, including the memory allocations. |
menghanl
force-pushed
the
log
branch
2 times, most recently
from
258324a to
96ddbb9
Compare
|
Hey, what's missing here aside the |
|
ping |
|
As discussed, please maintain compatibility with older clients by creating a new interface and keeping the old one, and wrapping the old Logger with a new Logger. |
grpclog/glogger/glogger.go
Outdated
| "github.com/golang/glog" | ||
| "google.golang.org/grpc/grpclog" | ||
| ) | ||
|
|
||
| func init() { | ||
| grpclog.SetLogger(&glogger{}) | ||
| grpclog.SetLoggerv2(&glogger{}) |
| @@ -37,38 +37,64 @@ Package glogger defines glog-based logging for grpc. | |||
| package glogger | |||
There was a problem hiding this comment.
How about a usage comment for this package? "Import this package anonymously to activate it as the enabled grpc logger" or something?
grpclog/loggerv2.go
Outdated
| @@ -0,0 +1,185 @@ | |||
| /* | |||
| * | |||
| * Copyright 2015, Google Inc. | |||
grpclog/loggerv2.go
Outdated
|
|
||
| // Loggerv2 does underlying logging work for grpclog. | ||
| type Loggerv2 interface { | ||
| // Info logs args. Arguments are handled in the manner of fmt.Print. |
There was a problem hiding this comment.
Info logs args informationally? Or something?
grpclog/loggerv2.go
Outdated
| // Error logs will be written to errorW, warningW and infoW. | ||
| // Warning logs will be written to warningW and infoW. | ||
| // Info logs will be written to infoW. | ||
| func NewLoggerv2(infoW io.Writer, warningW io.Writer, errorW io.Writer) Loggerv2 { |
There was a problem hiding this comment.
infoW, warningW, errorW io.Writer
grpclog/loggerv2_test.go
Outdated
| @@ -0,0 +1,77 @@ | |||
| /* | |||
| * | |||
| * Copyright 2016, Google Inc. | |||
grpclog/grpclog.go
Outdated
| os.Exit(1) | ||
| } | ||
|
|
||
| // Fatalf is equivalent to Infof() followed by a call to os.Exit() with a non-zero exit code. |
There was a problem hiding this comment.
Please fix this comment to refer to FATAL log.
grpclog/loggerv2.go
Outdated
| ) | ||
|
|
||
| // VerboseLevel identifies the verbose level used in grpclog.V() function. | ||
| type VerboseLevel int32 |
There was a problem hiding this comment.
Remove this type and use int directly in V.
|
Can we make this more "standard" with regards to popular logging packages, ie github.com/Sirupsen/logrus and github.com/uber-go-zap? Things in particular:
I don't think https://github.com/golang/glog was ever meant to be this widely used, and it would be nicer for this to be more in line with existing loggers. Also to note: do we have to support both |
|
@peter-edge And also, we want to use |
menghanl
added
1.5
Type: API Change
updates #120
updates #1044
#1052
#954
#1059