feat: tag-driven struct field logging with omit/redact modifiers#11
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates golog’s struct field logging to be entirely tag-driven, unifying Message.StructFields and Message.TaggedStructFields behind a single parser that supports omit/redact modifiers and encoding/json-compatible tag grammar where applicable.
Changes:
- Refactored struct-field reflection logging to resolve tags by priority (
golog→log→json) and parse tag modifiers (omitempty,omitzero,omitnull,redact/redacted). - Expanded test coverage to pin the new grammar, precedence rules, and confirm
encoding/jsonignores golog-specific modifiers. - Updated README docs to describe the new tag-driven behavior, modifiers, and breaking changes.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| message.go | Implements unified tag lookup + directive parsing and omit/redact behavior; updates public godoc accordingly. |
| message_test.go | Updates existing struct field tests for tag-driven behavior and adds extensive new cases for grammar/modifiers/stdlib assumptions. |
| README.md | Replaces the redaction section with a full “tags and modifiers” spec + examples and breaking-change notes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Unify Message.StructFields and Message.TaggedStructFields onto a
single variadic structFields(v, keyTags ...string) core. StructFields
now resolves tags in the order golog, log, json (first match wins for
both naming and modifiers). TaggedStructFields is a single-tag
shortcut. Public method signatures are unchanged.
Recognized modifiers in any tag value (parsed once by the new
parseStructFieldDirectives helper in struct.go):
- omitempty (encoding/json.Marshal parity: false, 0, nil pointer
or interface, empty array, slice, map, string)
- omitzero (strict superset of encoding/json Go 1.24 omitzero:
union of an IsZero bool method on the type and the Go zero
value via reflect.Value.IsZero; both value-receiver and
pointer-receiver IsZero methods are honored)
- omitnull (calls IsNull bool when the type implements it,
falls back to omitzero semantics otherwise; both value- and
pointer-receiver IsNull methods are honored; covers
golog.Timestamp and any sql.NullString-shaped wrapper)
- redact (also spelled redacted; replaces the value with
***REDACTED*** before it reaches the writer)
Suppression modifiers win over redact: a tag like
json:",redact,omitempty"
on an empty string emits nothing, not the marker. Matches
encoding/json.Marshal omitempty precedence.
Tag value grammar:
- bare "-" skips the field
-,... literal field name "-" with modifiers
"" empty value -> fall back to Go field name
",mod" empty name -> Go field name + modifiers
"name,mod" explicit name + modifiers
Whitespace around the name and each modifier is trimmed. Unknown
modifier tokens are ignored silently so that json:"name,omitnull"
is a valid tag for both golog (honors omitnull) and
encoding/json.Marshal (ignores it). A new test pins this stdlib
assumption against future Go releases.
Wildcard escape hatch:
TaggedStructFields(s, "")
logs every exported field by its Go name with no rename and no
modifier pickup. This restores the pre-rewrite "log every exported
field" behavior as an explicit opt-in. StructFields(s) deliberately
stays strict tag-driven.
Pointer-receiver detection:
The invokeIsNull and invokeIsZero helpers try the value-receiver
fast path first (zero allocation), then fall back to a
pointer-receiver path. If the field is reached via a pointer to
the enclosing struct it is already addressable and we use fv.Addr
directly. Otherwise we allocate one addressable copy via
reflect.New + Set so a value-typed field with a pointer-receiver
method does not silently fall through.
Nil-pointer safety:
shouldOmitStructField and isZeroValueForOmitzero short-circuit on
fv.IsNil for Ptr / Interface kinds before any interface assertion.
Without this, a nil *time.Time with ,omitzero or a nil
*golog.Timestamp with ,omitnull panicked at log time when the
value-receiver IsZero / IsNull method auto-dereferenced the nil
pointer. A nil pointer or interface is now treated as definitively
null and zero, which is both safe and semantically correct.
Breaking changes:
1. golog:"redact" (bare, single token) no longer triggers
redaction. Under the unified parser it names the field
"redact" in the log. Migrate to golog:",redact" (or combine
with other modifiers: golog:",redact,omitempty").
2. StructFields(s) on a struct where no exported field carries
any of the default keyTags (golog, log, json) now logs
nothing. Previously every exported field was logged by its
Go name. The cleanest replacement is the new wildcard
TaggedStructFields(s, ""). Per-field, you can also opt in by
adding an empty tag like json:"" or golog:"".
3. TaggedStructFields(s, "json") with json:"" on a field now
logs the field under its Go name instead of skipping it.
This is encoding/json.Marshal parity.
File layout:
- struct.go (new): the structFieldDirectives type plus
parseStructFieldDirectives, shouldOmitStructField,
invokeIsNull, invokeIsZero, asAddressableValue,
isZeroValueForOmitzero, isEmptyValueForOmitempty. Pure
reflect-based helpers with no Message dependency.
- struct_test.go (new): TestParseStructFieldDirectives (21-case
table), TestEncodingJSONIgnoresCustomModifiers (stdlib
assumption pin), TestShouldOmitStructField_NilPointer (5-case
panic regression guard), TestShouldOmitStructField_Pointer
Receiver (8-case value- and pointer-receiver coverage on
addressable and non-addressable fields).
- message.go: rewritten StructFields, TaggedStructFields, and
the private structFields walker. Public signatures unchanged.
Doc comments expanded with the full grammar, modifier
semantics, the wildcard opt-in, and pointers to the breaking
changes.
- message_test.go: existing redact tests migrated from
golog:"redact" to golog:",redact" and the json+redact tests
rewritten to use a new buildTestStruct helper that builds the
struct type at runtime via reflect.StructOf so static
analyzers (go vet structtag, staticcheck SA5008) never see
the custom modifiers in a tag literal. New integration
subtests cover multi-keyTag fallback, winning-tag controls
modifiers, untagged struct logs nothing, all four modifiers
across every primitive kind, omitnull fallback, redact plus
omitempty precedence, json.Marshal parity for omitempty and
omitzero, the wildcard escape hatch, and nil pointer fields
through the full Message.StructFields pipeline.
- README.md: the old "Redacting Sensitive Data" subsection is
rewritten as "Struct Field Logging: Tags and Modifiers" with
the full grammar, modifier table (including pointer-receiver
coverage notes), complete example struct, breaking-changes
callout, the wildcard escape hatch paragraph, and an
omitnull vs omitzero note that uses sql.NullString as the
canonical "null is not zero" example. Feature bullet list
near the top updated to mention tag-driven struct logging.
Verified clean: go test ./..., go vet ./..., go build ./..., and
staticcheck -checks SA5008 ./... all pass silently. ~60 new
subtests across the four new test functions plus the migrated
existing ones.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ungerik
force-pushed
the
struct-field-modifiers
branch
from
c402c50 to
bbd8edd
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Message.StructFieldsandMessage.TaggedStructFieldsonto a single variadicstructFields(v, keyTags ...string)core.StructFieldsnow resolves tags in the ordergolog,log,json(first match wins).TaggedStructFieldsis a single-tag shortcut.omitempty,omitzero,omitnull, andredact(also spelledredacted). Suppression modifiers win overredact(matchesencoding/json.Marshal'somitemptyprecedence).omitemptymatchesencoding/json.Marshalexactly;omitzerois a strict superset ofencoding/json's Go 1.24omitzero(unions theIsZero() boolmethod result withreflect.Value.IsZero);omitnullcallsIsNull() booland falls back toomitzerosemantics when the type has no such method. Both value-receiver and pointer-receiverIsNull/IsZeromethods are honored, even on non-addressable struct fields (the helper allocates a one-time addressable copy when needed).TaggedStructFields(s, "")now logs every exported field by its Go name with no rename and no modifier pickup. This restores the pre-rewrite "log every exported field" behavior as an explicit opt-in.StructFields(s)deliberately stays strict tag-driven.json:"name,omitnull"is a valid tag for bothencoding/json.Marshal(ignores it) and golog (honors it). A new test,TestEncodingJSONIgnoresCustomModifiers, pins that assumption against the stdlib — if a future Go release starts validating these, CI will flag it.Tag value grammar
"field_name"field_name""",omitempty""-"(bare)"-,omitempty"--+ modifiersWhitespace around the name and each modifier is trimmed. Modifiers from other tags on the same field are ignored — only the first-matching tag contributes both the name and the modifiers.
Breaking changes
golog:"redact"(bare, single token) no longer triggers redaction. Under the unified parser it names the field"redact". Migrate togolog:",redact"(or combine:golog:",redact,omitempty").StructFields(s)on an untagged struct now logs nothing. Previously every exported field was logged by its Go name. The cleanest replacement is the newTaggedStructFields(s, "")wildcard. Per-field, you can also opt in by adding an empty tag likejson:""orgolog:"".TaggedStructFields(s, "json")withjson:""now logs the field (under its Go name) instead of skipping it. This isencoding/json.Marshalparity.Test plan
go test ./...passes — full coverage adds ~60 subtests acrossTestMessage_StructFields,TestMessage_TaggedStructFields,TestParseStructFieldDirectives(21-case table-driven parser test),TestShouldOmitStructField_NilPointer(5 subtests, regression guard for the panic fix),TestShouldOmitStructField_PointerReceiver(8 subtests, value-receiver + pointer-receiver methods on addressable + non-addressable fields), andTestEncodingJSONIgnoresCustomModifiers.go vet ./...clean.go build ./...clean.staticcheck -checks SA5008 ./...clean (custom modifiers in test json tags live inreflect.StructOf-built types via thebuildTestStructhelper, so no AST-level struct tag literals trigger the check).golog:"redact"tests migrated togolog:",redact"+json:"name,redact"and still assert the"***REDACTED***"substitution.omitnullverified ongolog.Timestamp(usesIsNull) andtime.Time(falls back toomitzero).omitzeroverified ontime.Time{}and on zero structs without anIsZeromethod.,redact,omitemptyon an empty string suppresses the field rather than emitting the redaction marker.*time.Timewith,omitzeroand*golog.Timestampwith,omitnull(both nil) are suppressed instead of panicking on the value-receiver method auto-dereference. End-to-end test throughMessage.StructFieldsplus direct predicate tests.func (p *T) IsNull() boolandfunc (p *T) IsZero() boolare detected as value fields in non-addressable structs (allocates an addressable copy) and via the fastAddr()path on addressable structs.TaggedStructFields(s, "")logs every exported field by Go name, ignores struct tags entirely, skips unexported fields.encoding/json.Marshalstdlib assumption pinned by a test.Documentation
Latest doc-release sweep on this branch:
### Struct Field Logging: Tags and Modifiers— full grammar, modifier table, complete example struct, breaking-changes callout, and anomitnullvsomitzeronote usingsql.NullStringas the canonical "null is not zero" example.TaggedStructFields(s, "").omitzeroandomitnullrows now note that both value-receiver and pointer-receiver methods are honored, matching theaddressable-copy fallbackininvokeIsNull/invokeIsZero.StructFields(s)migration tip now points atTaggedStructFields(s, "")as the cleanest restore path.Message.StructFieldsandMessage.TaggedStructFieldsgodoc — expanded with the new grammar, modifier semantics, the wildcardTaggedStructFields(s, "")opt-in, and a pointer to the breaking change.benchmarks/,goslog/,logfile/,logsentry/) — audited, no changes required (none reference the renamed/repurposed methods).