eventpb: allow encoding of zero values #86242
Conversation
|
This change is |
|
With this patch, the following fields in the following proto messages would start to be emitted unconditionally, as they do not contain an |
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @knz and @nicktrav)
pkg/util/log/eventpb/eventpbgen/gen.go line 385 at r1 (raw file):
// Allow zero values if the field is NOT annotated with 'omitempty'. allowZeroValue := !strings.Contains(line, "omitempty")
Let's use a different boolean (something like includeempty?)
It's important that zero values get omitted by default. We are really anxious about the size of log entries, and I'd rather have folk think twice about including data with an includeempty than forgetting to exclude it with omitempty.
Currently, the JSON encoder unconditionally elides zero values, due the
presence of a `if m.{{.FieldName}} != 0 {` rule in the template. There
are situations where a zero value should still be emitted, despite it
being the default value.
Update the encoder to use the absence of the `includeempty` proto
annotation as an indication that a zero value should be emitted.
Boolean, string, and numerical native types take this annotation into
account.
Release note: None.
Release justification: Low risk, high benefit changes to existing
functionality.
nicktrav
force-pushed
the
nickt.zero-value
branch
from
a628df0 to
ff24cf3
Compare
There was a problem hiding this comment.
Reviewable status:
pkg/util/log/eventpb/eventpbgen/gen.go line 385 at r1 (raw file):
Previously, knz (kena) wrote…
Let's use a different boolean (something like
includeempty?)It's important that zero values get omitted by default. We are really anxious about the size of log entries, and I'd rather have folk think twice about including data with an
includeemptythan forgetting to exclude it withomitempty.
Sounds good. Done.
There was a problem hiding this comment.
Reviewed 3 of 3 files at r2, all commit messages.
Reviewable status:
|
TFTR! bors r=knz |
|
Build failed (retrying...): |
|
Build failed (retrying...): |
|
Build failed (retrying...): |
|
Build failed: |
|
bors r=knz |
|
Build succeeded: |
Currently, the JSON encoder unconditionally elides zero values, due the
presence of a
if m.{{.FieldName}} != 0 {rule in the template. Thereare situations where a zero value should still be emitted, despite it
being the default value.
Update the encoder to use the absence of the
omitemptyprotoannotation as an indication that a zero value should be emitted.
Boolean, string, and numerical native types take this annotation into
account.
Release note: None.
Release justification: Low risk, high benefit changes to existing functionality.