= kamon-core: add context name and token to MDC #363

Merged
merged 1 commit into from Jul 3, 2016

Projects

None yet

2 participants

@dspasojevic

This change pushes the trace name and token into the MDC. This means that they can be accessed in async appenders using the normal mechanism - without using the LogbackTraceTokenConverter.

@dpsoft dpsoft commented on the diff Jun 22, 2016
...c/main/scala/kamon/trace/logging/MdcKeysSupport.scala
@@ -34,7 +39,12 @@ trait MdcKeysSupport {
private[this] def copyToMdc(traceContext: TraceContext): Iterable[String] = traceContext match {
case ctx: MetricsOnlyContext
- ctx.traceLocalStorage.underlyingStorage.collect {
+
+ // Add the default key value pairs for the trace token and trace name.
@dpsoft
dpsoft Jun 22, 2016 edited Contributor

@dspasojevic @ivantopo I think that instead of using:

val traceTokenKey = "traceToken"
val traceNameKey = "traceName"

private val defaultKeys = Seq(traceTokenKey, traceNameKey)

MDC.put(traceTokenKey, ctx.token)
MDC.put(traceNameKey, ctx.name)

we can use the kamon api like this:

val TraceTokenKey = AvailableToMdc.fromKey("traceToken")
val TraceNameKey = AvailableToMdc.fromKey("traceName")

TraceLocal.store(TraceTokenKey)(ctx.token)
TraceLocal.store(TraceNameKey)(ctx.name)

this is only a idea, please guys WDYT?

@dspasojevic
dspasojevic Jun 23, 2016

That is fine with me.

It does mean that calling withMdc has the side-effect of modifying TraceLocal's storage though. At the moment, it should return the MDC and TraceLocal storage to the initial state.

@dpsoft
dpsoft Jun 23, 2016 Contributor

I see your point, and I think that is a good point :). @ivantopo?

@dspasojevic

@dpsoft are there any changes that you would like to this PR?

@dpsoft dpsoft merged commit 0a106c8 into kamon-io:master Jul 3, 2016
@dpsoft
Contributor
dpsoft commented Jul 3, 2016

@dspasojevic thanks for proposing this PR!!!! Done.

@dspasojevic dspasojevic deleted the dspasojevic:mdc branch Jul 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment