Document traceId/spanId injection into logs #1339
Conversation
mateuszrzeszutek
requested review from
anuraaga,
iNikem,
jkwatson,
trask and
tylerbenson
as code owners
docs/logger-mdc-instrumentation.md
Outdated
| (same as `TracingContextUtils.getCurrentSpan().getContext().getSpanIdAsHexString()`); | ||
| - `sampled` - a boolean flag marking whether the current span is sampled or not | ||
| (same as `TracingContextUtils.getCurrentSpan().getContext().isSampled()`). | ||
| Only added when `sampled` is `true`. |
There was a problem hiding this comment.
If it is always true, why include it?
There was a problem hiding this comment.
I've added an explanation to the doc. Basically if it's there the span is sampled, if not it is not - I guess the reasoning behind it is that most of spans are not sampled and keeping sampled=false in every log line would be just a waste of space.
There was a problem hiding this comment.
Aren't most span sampled? With AlwaysOn sampler, which is default
There was a problem hiding this comment.
🤦 That's true...
I somehow thought that it worked completely the other way 😅
Now I'm starting to wonder why it works that way... @anuraaga I think you've added it first in the logback instrumentation -- why do we only log true value?
|
Yeah it's just a premature optimization, I don't think I taught too deeply when making that change. Always adding sampled seems nice to me too. |
Does this mean we have to make some code change in our MDC handling? |
|
@iNikem Yeah - so I'm good with merging this first and updating code / README in a followup. But noticed the build failure |
|
Hmm, looks like random errors:
|
mateuszrzeszutek
force-pushed
the
document-mdc
branch
from
9ce2ae1
to
da7948f
Compare
|
Rebased to force CircleCI run |
|
@mateuszrzeszutek will you do a follow-up fix right away or will you create an issue for that? |
|
I'll do it right away. |
Resolves #1249