Merge pull request #148 from hmrc/improve_mdc

Improve mdc

build.sbt

@@ -8,18 +8,12 @@ Global / concurrentRestrictions += Tags.limitSum(1, Tags.Test, Tags.Untagged)
 val scala2_12 = "2.12.15"
 val scala2_13 = "2.13.8"
 
-val silencerVersion = "1.7.8"
-
 lazy val commonSettings = Seq(
   organization := "uk.gov.hmrc",
   majorVersion := 14,
   scalaVersion := scala2_12,
   isPublicArtefact := true,
-  scalacOptions ++= Seq("-feature"),
-  libraryDependencies ++= Seq(
-    compilerPlugin("com.github.ghik" % "silencer-plugin" % silencerVersion cross CrossVersion.full),
-    "com.github.ghik" % "silencer-lib" % silencerVersion % Provided cross CrossVersion.full
-  )
+  scalacOptions ++= Seq("-feature")
 )
 
 lazy val library = (project in file("."))

http-verbs-common/src/main/scala/uk/gov/hmrc/http/HttpExceptions.scala

@@ -16,7 +16,6 @@
 
 package uk.gov.hmrc.http
 
-import com.github.ghik.silencer.silent
 import uk.gov.hmrc.http.HttpExceptions._
 
 private object HttpExceptions {
@@ -131,7 +130,7 @@ sealed trait UpstreamErrorResponse extends Exception {
   def upstreamResponseCode: Int
 
   // final to help migrate away from upstreamResponseCode (i.e. read only - set via UpstreamErrorResponse.apply)
-  @silent("deprecated")
+  @annotation.nowarn("msg=deprecated")
   final def statusCode: Int =
     upstreamResponseCode
 
@@ -176,7 +175,7 @@ object UpstreamErrorResponse {
       headers    = Map.empty
     )
 
-  @silent("deprecated")
+  @annotation.nowarn("msg=deprecated")
   def apply(message: String, statusCode: Int, reportAs: Int, headers: Map[String, Seq[String]]): UpstreamErrorResponse =
     if (HttpErrorFunctions.is4xx(statusCode))
       uk.gov.hmrc.http.Upstream4xxResponse(

http-verbs-common/src/main/scala/uk/gov/hmrc/http/HttpReads.scala

@@ -16,8 +16,6 @@
 
 package uk.gov.hmrc.http
 
-import com.github.ghik.silencer.silent
-
 object HttpReads extends HttpReadsLegacyInstances {
   def apply[A : HttpReads] =
     implicitly[HttpReads[A]]
@@ -40,7 +38,6 @@ object HttpReads extends HttpReadsLegacyInstances {
   // compilation priority during implicit resolution. This means, unless
   // specified otherwise a verb call will return a plain HttpResponse
   @deprecated("Use uk.gov.hmrc.http.HttpReads.Implicits instead. See README for differences.", "11.0.0")
-  @silent("deprecated")
   implicit val readRaw: HttpReads[HttpResponse] = HttpReadsLegacyRawReads.readRaw
 
   object Implicits extends HttpReadsInstances

http-verbs-common/src/main/scala/uk/gov/hmrc/http/HttpResponse.scala

@@ -18,7 +18,6 @@ package uk.gov.hmrc.http
 
 import akka.stream.scaladsl.Source
 import akka.util.ByteString
-import com.github.ghik.silencer.silent
 import play.api.libs.json.{JsValue, Json}
 
 /**
@@ -39,7 +38,7 @@ trait HttpResponse {
   def allHeaders: Map[String, Seq[String]]
 
   // final to help migrate away from allHeaders (i.e. read only - set via HttpResponse.apply)
-  @silent("deprecated")
+  @annotation.nowarn("msg=deprecated")
   final def headers: Map[String, Seq[String]]
     = allHeaders
 

http-verbs-common/src/main/scala/uk/gov/hmrc/http/client/HttpClientV2Impl.scala

@@ -25,6 +25,7 @@ import play.api.libs.ws.{BodyWritable, EmptyBody, InMemoryBody, SourceBody, WSCl
 import play.core.parsers.FormUrlEncodedParser
 import uk.gov.hmrc.http.{BadGatewayException, BuildInfo, GatewayTimeoutException, HeaderCarrier, HttpReads, HttpResponse, Retries}
 import uk.gov.hmrc.play.http.BodyCaptor
+import uk.gov.hmrc.play.http.logging.Mdc
 import uk.gov.hmrc.play.http.ws.WSProxyConfiguration
 import uk.gov.hmrc.http.hooks.{Data, HookData, HttpHook, RequestData, ResponseData}
 import uk.gov.hmrc.http.logging.ConnectionTracing
@@ -175,10 +176,14 @@ final class RequestBuilderImpl(
   // -- Execution --
 
   override def execute[A](implicit r: HttpReads[A], ec: ExecutionContext): Future[A] =
-    executor.execute(request, hookDataF, isStream = false, r)
+    Mdc.preservingMdc(
+      executor.execute(request, hookDataF, isStream = false, r)
+    )
 
   override def stream[A](implicit r: StreamHttpReads[A], ec: ExecutionContext): Future[A] =
-    executor.execute(request, hookDataF, isStream = true, r)
+    Mdc.preservingMdc(
+      executor.execute(request, hookDataF, isStream = true, r)
+    )
 }
 
 class ExecutorImpl(

http-verbs-common/src/main/scala/uk/gov/hmrc/play/http/logging/MdcLoggingExecutionContext.scala

@@ -29,13 +29,18 @@ class MdcLoggingExecutionContext(wrapped: ExecutionContext, mdcData: Map[String,
 
   private class RunWithMDC(runnable: Runnable, mdcData: Map[String, String]) extends Runnable {
     def run(): Unit = {
+      val oldMdcData = MDC.getCopyOfContextMap
+      MDC.clear()
       mdcData.foreach {
         case (k, v) => MDC.put(k, v)
       }
       try {
         runnable.run()
       } finally {
-        MDC.clear()
+        if (oldMdcData == null)
+          MDC.clear()
+        else
+          MDC.setContextMap(oldMdcData)
       }
     }
   }

http-verbs-common/src/test/scala/uk/gov/hmrc/http/HeadersSpec.scala

@@ -17,7 +17,6 @@
 package uk.gov.hmrc.http
 
 import akka.actor.ActorSystem
-import com.github.ghik.silencer.silent
 import com.github.tomakehurst.wiremock.client.WireMock._
 import com.typesafe.config.Config
 import org.scalatest.concurrent.{IntegrationPatience, ScalaFutures}
@@ -44,7 +43,6 @@ class HeadersSpec
 
   private lazy val app: Application = new GuiceApplicationBuilder().build()
 
-  @silent("deprecated")
   private implicit val hc: HeaderCarrier = HeaderCarrier(
     authorization = Some(Authorization("authorization")),
     forwarded     = Some(ForwardedFor("forwarded-for")),

http-verbs-common/src/test/scala/uk/gov/hmrc/http/HttpErrorFunctionsSpec.scala

@@ -16,7 +16,6 @@
 
 package uk.gov.hmrc.http
 
-import com.github.ghik.silencer.silent
 import org.scalacheck.Gen
 import org.scalatest.prop.TableDrivenPropertyChecks
 import org.scalatest.TryValues
@@ -26,7 +25,7 @@ import org.scalatestplus.scalacheck.ScalaCheckDrivenPropertyChecks
 
 import scala.util.Try
 
-@silent("deprecated")
+@annotation.nowarn("msg=deprecated")
 class HttpErrorFunctionsSpec
     extends AnyWordSpec
     with Matchers

http-verbs-common/src/test/scala/uk/gov/hmrc/http/HttpReadsLegacyInstancesSpec.scala

@@ -16,14 +16,13 @@
 
 package uk.gov.hmrc.http
 
-import com.github.ghik.silencer.silent
 import org.scalacheck.Gen
 import org.scalatestplus.scalacheck.ScalaCheckDrivenPropertyChecks
 import org.scalatest.wordspec.AnyWordSpec
 import org.scalatest.matchers.should.Matchers
 import play.api.libs.json.Json
 
-@silent("deprecated")
+@annotation.nowarn("msg=deprecated")
 class HttpReadsLegacyInstancesSpec extends AnyWordSpec with ScalaCheckDrivenPropertyChecks with Matchers {
   "RawReads" should {
     val reads = HttpReads.readRaw

http-verbs-common/src/test/scala/uk/gov/hmrc/http/RetriesSpec.scala

@@ -27,13 +27,11 @@ import org.mockito.scalatest.MockitoSugar
 import org.scalatest.concurrent.{IntegrationPatience, ScalaFutures}
 import org.scalatest.matchers.should.Matchers
 import org.scalatest.wordspec.AnyWordSpecLike
-import org.slf4j.MDC
 import play.api.libs.json.{JsValue, Json, Writes}
 import uk.gov.hmrc.http.HttpReads.Implicits._
 import uk.gov.hmrc.http.hooks.{HttpHook, HttpHooks}
 import uk.gov.hmrc.play.http.logging.Mdc
 
-import scala.collection.JavaConverters._
 import scala.concurrent.duration._
 import scala.concurrent.{ExecutionContext, Future}
 import scala.util.{Random, Try}
@@ -195,12 +193,14 @@ class RetriesSpec
 
       val expectedResponse = HttpResponse(404, "")
 
+      org.slf4j.MDC.clear()
+
       val resultF =
         for {
           _   <- Future.successful(Mdc.putMdc(mdcData))
           res <- retries.retryOnSslEngineClosed("GET", "url") {
                   // assert mdc available to block execution
-                  Option(MDC.getCopyOfContextMap).map(_.asScala.toMap).getOrElse(Map.empty) shouldBe mdcData
+                  Mdc.mdcData shouldBe mdcData
 
                   retries.failFewTimesAndThenSucceed(
                     success   = Future.successful(expectedResponse),
@@ -209,7 +209,7 @@ class RetriesSpec
                 }
         } yield {
           // assert mdc available to continuation
-          Option(MDC.getCopyOfContextMap).map(_.asScala.toMap).getOrElse(Map.empty) shouldBe mdcData
+          Mdc.mdcData shouldBe mdcData
           res
         }
 

http-verbs-common/src/test/scala/uk/gov/hmrc/http/client/HttpClientV2Spec.scala

@@ -33,8 +33,10 @@ import play.api.libs.json.{Json, Reads, Writes}
 import play.api.libs.ws.ahc.{AhcWSClient, AhcWSClientConfigFactory}
 import uk.gov.hmrc.http.{HeaderCarrier, HttpReads, HttpReadsInstances, HttpResponse, Retries, StringContextOps, UpstreamErrorResponse}
 import uk.gov.hmrc.http.hooks.{Data, HookData, HttpHook, RequestData, ResponseData}
+import uk.gov.hmrc.play.http.logging.Mdc
 import uk.gov.hmrc.http.test.WireMockSupport
 
+import java.util.concurrent.Executors
 import java.util.concurrent.atomic.AtomicInteger
 import scala.concurrent.{ExecutionContext, Future}
 import scala.concurrent.ExecutionContext.Implicits.global
@@ -588,6 +590,39 @@ class HttpClientV2Spec
       res.failed.futureValue shouldBe a[UpstreamErrorResponse]
       count.get shouldBe 4
     }
+
+    "preserve Mdc" in new Setup {
+      implicit val hc = HeaderCarrier()
+
+      wireMockServer.stubFor(
+        WireMock.put(urlEqualTo("/"))
+          .willReturn(aResponse().withBody("\"res\"").withStatus(200))
+      )
+
+      val mdcData = Map("key1" -> "value1")
+
+      implicit val mdcEc = ExecutionContext.fromExecutor(new uk.gov.hmrc.play.http.logging.MDCPropagatingExecutorService(Executors.newFixedThreadPool(2)))
+
+      org.slf4j.MDC.clear()
+
+      val res: Future[Map[String, String]] =
+        for {
+          _ <- Future.successful(Mdc.putMdc(mdcData))
+          _ <- httpClientV2
+                .put(url"$wireMockUrl/")
+                .withBody(Json.toJson(ReqDomain("req")))
+                .setHeader("User-Agent" -> "ua2")
+                .execute[ResDomain]
+        } yield Mdc.mdcData
+
+      res.futureValue shouldBe mdcData
+
+      wireMockServer.verify(
+        putRequestedFor(urlEqualTo("/"))
+          .withRequestBody(equalTo("\"req\""))
+          .withHeader("User-Agent", equalTo("ua2"))
+      )
+    }
   }
 
   trait Setup {

http-verbs-common/src/test/scala/uk/gov/hmrc/play/connectors/ConnectorSpec.scala

@@ -16,14 +16,13 @@
 
 package uk.gov.hmrc.play.connectors
 
-import com.github.ghik.silencer.silent
 import org.mockito.scalatest.MockitoSugar
 import org.scalatest.wordspec.AnyWordSpecLike
 import org.scalatest.matchers.should.Matchers
 import play.api.test.WsTestClient
 import uk.gov.hmrc.http._
 
-@silent("deprecated")
+@annotation.nowarn("msg=deprecated")
 class ConnectorSpec extends AnyWordSpecLike with Matchers with MockitoSugar {
   WsTestClient.withClient { wsClient =>
 

http-verbs-common/src/test/scala/uk/gov/hmrc/play/http/HeaderCarrierConverterSpec.scala

@@ -16,7 +16,6 @@
 
 package uk.gov.hmrc.play.http
 
-import com.github.ghik.silencer.silent
 import org.scalatest.BeforeAndAfterAll
 import org.scalatest.wordspec.AnyWordSpecLike
 import org.scalatest.matchers.should.Matchers
@@ -31,7 +30,7 @@ import uk.gov.hmrc.http._
 
 import scala.concurrent.duration._
 
-@silent("deprecated")
+@annotation.nowarn("msg=deprecated")
 class HeaderCarrierConverterSpec extends AnyWordSpecLike with Matchers with BeforeAndAfterAll {
 
   "Extracting the request timestamp from the session and headers" should {