-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Leveled fields: Fields present only at certain log levels #1078
Comments
Summarizing discussion with @mway here: Matt proposed the following options:
My thoughts on the options:
So if we were going to solve this problem, based on current options, I lean (3). |
I think this is a useful feature, and more generally have found cases where the fields to log depend on some condition, and I find myself writing code like: fieldsToLog := []zap.Field{...}
if someCondition {
fieldsToLog = append(fieldsToLog, zap.String(...), zap.Int(...))
} Looking at the options, agree on (1), (2), and (4) with @abhinav. Re: 3: This is an interesting idea, and could be used for more generic helpers, like Another variant (more similar to the We could take that idea a little further, and have field constructors per-level, e.g., logger.Info("my message",
zap.String("foo", "bar"),
zap.DebugField(zap.String("debugOnly", "baz")),
) |
Oh, I like the idea of the top-level functions. Although I suspect that accepting multiple fields there will make that a one-alloc-each operation because we'll need to be able to turn |
True, may want to separate the single-field and multi-field functions. |
I think we should start with single-field filtering and extend to logger.Info(
"handled request",
zap.String("caller", "..."),
zap.String("method", "..."),
zap.DebugField(zap.String("debug1", "...")),
zap.DebugField(zap.String("debug2", "...")),
zap.DebugField(zap.String("debug3", "...")),
) is arguably not worse than logger.Info(
"handled request",
zap.String("caller", "..."),
zap.String("method", "..."),
zap.DebugFields(
zap.String("debug1", "..."),
zap.String("debug2", "..."),
zap.String("debug3", "..."),
),
) IMO, if we want to deal with We should probably also be clear about whether all levels add value, or only some. I can't think of a useful case in which a It may also be useful (if possible) to prototype the API in an experimental package (e.g. |
If you do decide to add support for field groups in the future would you consider adding group-level filtering as well? Group level filtering would be useful in situations where you have multiple related fields that you only want to log at a certain log level. For example, let's say you have a set of fields related to HTTP requests and you only want to log them at the Debug level. You would define a "request" group that includes all of these fields and then filter the group based on log level. So instead of: logger.Debug("processing request",
zap.String("url", req.URL.String()),
zap.String("method", req.Method),
zap.String("user-agent", req.UserAgent()),
zap.Int("content-length", int(req.ContentLength)),
zap.String("remote-addr", req.RemoteAddr),
zap.String("proto", req.Proto),
) You would do: reqGroup := zap.Group("request", zap.String("url", req.URL.String()), zap.String("method", req.Method), zap.String("user-agent", req.UserAgent()), zap.Int("content-length", int(req.ContentLength)), zap.String("remote-addr", req.RemoteAddr), zap.String("proto", req.Proto))
logger.Debug("processing request",
zap.Fields(reqGroup.Filter(zap.DebugLevel)),
) This way, if you decide to add more fields to the "request" group later on, you can filter them all together without having to add individual filters for each field. Some other ideas that I had around group-level filtering just now are
g := zap.Group("user_info")
g.String("user_id", "123")
g.String("username", "jdoe")
g.Level(zap.DebugLevel) // Set debug level for this group
logger.Info("processing request", zap.Group("user_info")) The last line would log only the fields in the "user_info" group, since it has a debug level set. If the logger was at info level, it would skip the group and only log the main message.
{"level":"info","msg":"processing request","user_info":{"user_id":"123","username":"jdoe"}} Hierarchical groups: Allow groups to be nested inside other groups, creating a hierarchical structure. This would allow for more complex filtering, where a parent group could be enabled or disabled based on its level and all child groups would inherit that level. For example: g1 := zap.Group("foo")
g1.String("a", "1")
g2 := g1.Group("bar")
g2.String("b", "2")
g3 := g2.Group("baz")
g3.String("c", "3")
g1.Level(zap.DebugLevel)
logger.Info("processing request", zap.Group("foo")) logger.Info("processing request", zap.Group("foo")) would only log the fields in the "foo" group, since it has a debug level set. Since "bar" and "baz" are child groups of "foo", they would also be logged even though they don't have a level set. The key point would be to allow users to group related fields together and then selectively log those groups based on their level, without having to manually construct each log statement. The introduction of "log contexts" for group-level filtering would be another concept. A log context would be a grouping of fields that are related to a particular context within the application. For example, you might have a "database" context with fields like "table_name", "query", "result_count", and so on. Another context might be "user" with fields like "user_id", "ip_address", "username", and so on With log contexts, you could filter out entire contexts based on log level. For example, you might only want to log the "database" context at the debug level, but log everything else at the info level. This would allow you to easily see what's happening at the database level without cluttering up the logs with unnecessary information. You would also have the ability to add fields to a context dynamically at runtime. For example, you might have a context for a specific request and you could add fields like "request_id" or "responsive_code" to that context as needed. Another idea would be to allow for filtering based on the value of a field. For example you might only want to log requests that take longer than a certain amount of time, or only log errors that are related to a specific subsystem. This would require some way to define rules for which fields to match on and what values to match against. |
took a stab at this #1251 - let me know if this is what you guys had in mind! |
An issue that users sometimes have is this:
They have a log statement like the following,
They would like to include additional information in that log statement, but only if the debug level is enabled.
Currently, the options are:
I think it might be valuable for Zap to natively provide some concept of "leveled fields"—fields present only at certain log levels—based on something like the levelField helper above.
I don't have an exact design in mind, but Zap-native support for something like this would likely be able to drop that extra
log
argument.The text was updated successfully, but these errors were encountered: