feat!: add ContextPair to StructuredLogger#903
feat!: add ContextPair to StructuredLogger#903jhenahan wants to merge 1 commit intotypelevel:mainfrom
Conversation
Motivation
==========
`addContext` has one sharp edge on it that seems to come up a lot during
onboarding: if you want to add a single context element, the syntax is
unintuitive.
Say we want to add the context `"foo" -> "bar"` to a logger. Naturally,
one might reach for `addContext(pairs: (String, Shown)*)`, but alas,
after erasure we have
```scala
logger.addContext(
"foo" -> "bar" // Tuple2
)
```
which doesn't conform to either of `Map` or `Seq`. This leaves us with
these options:
```scala
logger.addContext(
Map(
"foo" -> bar.show
)
)
logger.addContext(
"foo" -> (bar: Shown)
)
```
Neither feels _great_, and it would be really pleasant to have
consistent syntax for single-element and multi-element context data
without having to use `Map` literals and explicit `.show`/`.toString`.
Result
======
This patch hijacks `StructuredLogger`'s implicit scope to do a bit of
juggling and resolve the erasure issue, and in the process cleans up a
chunk of duplicative code in its descendants.
I've tested this in the work code, and it appears to work as expected
for any subtype of `StructuredLogger`. The signature ain't exactly
beautiful, but the syntactic consistency is gratifying.
b0afda5 to
45232f2
Compare
|
Thanks! This breaks binary compatibility. It might be possible to regain it by making the old signature private. I have never much liked |
|
I had noticed that. I'll see if I can recover binary compatibility as you say. I have a few other ideas around structured logging, mainly around avoiding all the |
Motivation
addContexthas one sharp edge on it that seems to come up a lot during onboarding: if you want to add a single context element, the syntax is unintuitive.Say we want to add the context
"foo" -> "bar"to a logger. Naturally, one might reach foraddContext(pairs: (String, Shown)*), but alas, after erasure we havewhich doesn't conform to either of
MaporSeq. This leaves us with these options:Neither feels great, and it would be really pleasant to have consistent syntax for single-element and multi-element context data without having to use
Mapliterals and explicit.show/.toString.Result
This patch hijacks
StructuredLogger's implicit scope to do a bit of juggling and resolve the erasure issue, and in the process cleans up a chunk of duplicative code in its descendants.I've tested this in the work code, and it appears to work as expected for any subtype of
StructuredLogger. The signature ain't exactly beautiful, but the syntactic consistency is gratifying.I'll add tests if this looks tolerable, just wanted to get the idea out of my head.