Revert Logback instrumentation (#1252)

* Revert "append MDC keys, even if the MDC is empty. Fixes #1225 (#1227)"

This reverts commit e10cba2.

* Revert "reorganize the logback instrumentation to avoid stack overflows, fixes #1212 (#1220)"

This reverts commit 5cf9653.

* Fixes instrumentation for Logback 1.4.x

instrumentation/kamon-logback/src/main/scala/kamon/instrumentation/logback/LogbackInstrumentation.scala

@@ -25,8 +25,10 @@ import kamon.tag.Tag
 import kamon.trace.{Identifier, Span}
 import kanela.agent.api.instrumentation.InstrumentationBuilder
 import kanela.agent.libs.net.bytebuddy.asm.Advice
+import kanela.agent.libs.net.bytebuddy.implementation.bind.annotation.{RuntimeType, SuperCall}
+import org.slf4j.MDC
 
-import java.util
+import java.util.concurrent.Callable
 import scala.collection.JavaConverters._
 
 
@@ -38,8 +40,8 @@ class LogbackInstrumentation extends InstrumentationBuilder {
   onType("ch.qos.logback.core.spi.AppenderAttachableImpl")
     .advise(method("appendLoopOnAppenders"), AppendLoopOnAppendersAdvice)
 
-  onType("ch.qos.logback.classic.util.LogbackMDCAdapter")
-    .advise(method("getPropertyMap"), classOf[MdcPropertyMapAdvice])
+  onType("ch.qos.logback.classic.spi.LoggingEvent")
+    .intercept(method("getMDCPropertyMap"), GetPropertyMapMethodInterceptor)
 }
 
 object LogbackInstrumentation {
@@ -86,46 +88,48 @@ object AppendLoopOnAppendersAdvice {
     scope.close()
 }
 
-object ContextToMdcPropertyMapAppender {
+object GetPropertyMapMethodInterceptor {
 
-  def appendContext(mdc: java.util.Map[String, String]): java.util.Map[String, String] = {
+  @RuntimeType
+  def aroundGetMDCPropertyMap(@SuperCall callable: Callable[_]): Any = {
     val settings = LogbackInstrumentation.settings()
 
     if (settings.propagateContextToMDC) {
+      val mdcContextMapBeforePropagation = MDC.getCopyOfContextMap
       val currentContext = Kamon.currentContext()
       val span = currentContext.get(Span.Key)
-      val mdcWithKamonContext = {
-        if(mdc == null)
-          new util.HashMap[String, String]()
-        else
-          new util.HashMap[String, String](mdc)
-      }
 
       if (span.trace.id != Identifier.Empty) {
-        mdcWithKamonContext.put(settings.mdcTraceIdKey, span.trace.id.string)
-        mdcWithKamonContext.put(settings.mdcSpanIdKey, span.id.string)
-        mdcWithKamonContext.put(settings.mdcSpanOperationNameKey, span.operationName())
+        MDC.put(settings.mdcTraceIdKey, span.trace.id.string)
+        MDC.put(settings.mdcSpanIdKey, span.id.string)
+        MDC.put(settings.mdcSpanOperationNameKey, span.operationName())
       }
 
       mdcWithKamonContext.put(settings.mdcSourceThreadKey, Thread.currentThread().getName)
 
       if (settings.mdcCopyTags) {
         currentContext.tags.iterator().foreach(t => {
-          mdcWithKamonContext.put(t.key, Tag.unwrapValue(t).toString)
+          MDC.put(t.key, Tag.unwrapValue(t).toString)
         })
       }
 
       settings.mdcCopyKeys.foreach { key =>
         currentContext.get(Context.key[Any](key, "")) match {
-          case Some(value) if value.toString.nonEmpty => mdcWithKamonContext.put(key, value.toString)
-          case keyValue if keyValue != null && keyValue.toString.nonEmpty => mdcWithKamonContext.put(key, keyValue.toString)
+          case Some(value) if value.toString.nonEmpty => MDC.put(key, value.toString)
+          case keyValue if keyValue != null && keyValue.toString.nonEmpty => MDC.put(key, keyValue.toString)
           case _ => // Just ignore the nulls and empty strings
         }
       }
 
-      mdcWithKamonContext
+      try callable.call() finally {
+        if (mdcContextMapBeforePropagation != null) {
+          MDC.setContextMap(mdcContextMapBeforePropagation)
+        } else { // a null contextMap is possible and means 'empty'
+          MDC.clear()
+        }
+      }
     } else {
-      mdc
+      callable.call()
     }
   }
 }