-
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 #1251
base: master
Are you sure you want to change the base?
Conversation
@@ -106,6 +106,7 @@ type Field struct { | |||
Type FieldType | |||
Integer int64 | |||
String string | |||
Level *Level |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Making this attribute a pointer seemed the cleanest way to implement this - doing so would make all existing fields' Level
attribute be nil
.
field.go
Outdated
@@ -35,8 +35,17 @@ type Field = zapcore.Field | |||
var ( | |||
_minTimeInt64 = time.Unix(0, math.MinInt64) | |||
_maxTimeInt64 = time.Unix(0, math.MaxInt64) | |||
|
|||
DebugLevelField = zapcore.DebugLevel |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
needed to duplicate this in order to be able to pass the address of the DebugLevel
for assignment (https://github.com/uber-go/zap/pull/1251/files#diff-b494c03820a49f9efc491691ed31a2acc16d4cebf39d679ffd74bf97bbee968bR45)
DebugLevel
doesn't work because https://github.com/uber-go/zap/blob/master/level.go#L32 already uses that name in the same package.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1251 +/- ##
=======================================
Coverage 98.08% 98.09%
=======================================
Files 50 50
Lines 3240 3252 +12
=======================================
+ Hits 3178 3190 +12
Misses 53 53
Partials 9 9 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doing this in the ioCore means that this won't work with other cores. For example, this won't work with zaptest/observer
.
Edit: I think the implementation of this could live in CheckedEntry
to work with all cores.
ah darn - thanks for taking a look! will look into it when i get a chance :.) |
@prashantv moved the filtering over to where you pointed out in the comment - sorry about the incorrect change earlier. not super sure if https://github.com/uber-go/zap/pull/1251/files#diff-4a689f7bcb3f92ee8d87b3d9070215a7fcb37680843003657a60357c6a821d90R273 is also necessary since all invocations of
let me know what you think! |
#1078
Added just
DebugField
for now per the suggestions in the issue. Extending this should be very straight-forward if need be.