Skip to content

fix: propagate Tag and Context fields through Logger.With - #57

Open
costela wants to merge 1 commit into
TheZeroSlave:masterfrom
exaring:fix-tag-context-via-with
Open

costela wants to merge 1 commit into
TheZeroSlave:masterfrom
exaring:fix-tag-context-via-with

Conversation

@costela

@costela costela commented Sep 9, 2026 •

Copy link
Copy Markdown
Contributor

zapsentry.Tag and zapsentry.Context were only honored when passed directly to the log call. Fields attached earlier via zap's Logger.With were silently dropped, because core.with() did not carry them into the child core and Write() only scanned the call-site fields.

Store tags and the context on the core in with(), and have Write() read them from the already-computed clone. Also added regression tests for both fields.

Fixes #56

zapsentry.Tag and zapsentry.Context were only honoured when passed
directly to the log call. Fields attached earlier via zap's
Logger.With were silently dropped, because core.with() did not
carry them into the child core and Write() only scanned the
call-site fields.

Store tags and the context on the core in with(), and have Write()
read them from the already-computed clone. Add regression tests
for both fields.
@costela

costela commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

FYI, the benchmarks look slightly better in some cases. The ones that look worse are actually the feature working as it should 🙈

pkg: github.com/TheZeroSlave/zapsentry
cpu: AMD Ryzen AI 9 HX 370 w/ Radeon 890M
                           │   old.txt   │               new.txt               │
                           │   sec/op    │   sec/op     vs base                │
Write_PlainFields            5.123µ ± 4%   4.903µ ± 4%   -4.29% (p=0.006 n=10)
Write_CallSiteTag            5.401µ ± 4%   5.561µ ± 4%   +2.97% (p=0.029 n=10)
Write_ErrorField             5.995µ ± 3%   5.821µ ± 5%        ~ (p=0.143 n=10)
Write_DerivedLoggerWithTag   5.593µ ± 3%   5.487µ ± 1%   -1.90% (p=0.011 n=10)
With_Tag                     475.4n ± 2%   867.4n ± 3%  +82.43% (p=0.000 n=10)
With_PlainField              905.7n ± 2%   940.3n ± 5%   +3.82% (p=0.000 n=10)
geomean                      2.714µ        2.987µ       +10.07%

                           │   old.txt    │               new.txt                │
                           │     B/op     │     B/op      vs base                │
Write_PlainFields            4.422Ki ± 0%   4.406Ki ± 0%   -0.35% (p=0.000 n=10)
Write_CallSiteTag            4.445Ki ± 0%   4.727Ki ± 0%   +6.33% (p=0.000 n=10)
Write_ErrorField             4.523Ki ± 0%   4.508Ki ± 0%   -0.35% (p=0.000 n=10)
Write_DerivedLoggerWithTag   4.352Ki ± 0%   4.336Ki ± 0%   -0.36% (p=0.000 n=10)
With_Tag                       416.0 ± 0%     752.0 ± 0%  +80.77% (p=0.000 n=10)
With_PlainField                976.0 ± 0%    1008.0 ± 0%   +3.28% (p=0.000 n=10)
geomean                      2.305Ki        2.579Ki       +11.91%

                           │  old.txt   │               new.txt                │
                           │ allocs/op  │ allocs/op   vs base                  │
Write_PlainFields            35.00 ± 0%   34.00 ± 0%   -2.86% (p=0.000 n=10)
Write_CallSiteTag            35.00 ± 0%   35.00 ± 0%        ~ (p=1.000 n=10) ¹
Write_ErrorField             38.00 ± 0%   37.00 ± 0%   -2.63% (p=0.000 n=10)
Write_DerivedLoggerWithTag   34.00 ± 0%   33.00 ± 0%   -2.94% (p=0.000 n=10)
With_Tag                     7.000 ± 0%   8.000 ± 0%  +14.29% (p=0.000 n=10)
With_PlainField              9.000 ± 0%   9.000 ± 0%        ~ (p=1.000 n=10) ¹
geomean                      21.53        21.71        +0.80%
¹ all samples are equal

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Using zap.Logger.With drops context and tags

1 participant