-
Notifications
You must be signed in to change notification settings - Fork 584
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
Implement the slog Handler functionality #5314
Conversation
3751e72
to
35a1789
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5314 +/- ##
=======================================
+ Coverage 61.3% 61.6% +0.2%
=======================================
Files 186 186
Lines 11253 11382 +129
=======================================
+ Hits 6907 7019 +112
- Misses 4147 4161 +14
- Partials 199 202 +3
|
Co-authored-by: Sam Xie <[email protected]>
Co-authored-by: Robert Pająk <[email protected]>
New: func() any { | ||
// Based on slog research (https://go.dev/blog/slog#performance), 95% | ||
// of use-cases will use 5 or less attributes. | ||
return newKVBuffer(5) |
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.
I used 5 for the array inside a Record
because there was a tradeoff: using too much space would hurt performance. There's no similar tradeoff here, so you could make this a bit larger. Maybe 10?
if b == nil { | ||
return nil | ||
} | ||
return &kvBuffer{data: slices.Clone(b.data)} |
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.
You lose any extra capacity when you clone b.data, so the subsequent AddAttrs will always allocate. (I think.)
In general though, it's probably best not to worry too much about these details. You can always tweak the performance later.
Part of #5195
This adds the types that hold groups and attributes within the
Handler
. ThekvBuffer
is just a wrapper around[]log.KeyValue
andgroup
is a linked list of embedded group structure (see type comment fornext
field for mapping information).This does not include complete coverage of the added functionality. That is left for a follow-on PR. See #5195 for planned tests.