Added Logger that takes an implicit parameter (e.g. CorrelationId)

[pwliwanow]

Including request contextual data in the logs (e.g. CorrelationId, UserId, etc.) is a common scenario, however currently there is no good way to do it.

With Java common approach is to use MDC, however I think that in Scala it's a bad idea for two reasons:

• in case of async computations custom execution context is needed, but more importantly
• correlationId value is not present in the function signature and then it's taken out of thin air which in Scala should be a serious red flag

Here is relevant part of the code that expresses idea in this pull request:

trait CanLog[A] {
  def logMessage(originalMsg: String, a: A): String
  def afterLog(a: A): Unit = ()
}

class LoggerTakingImplicit[A] (val underlying: Underlying)(implicit val canLogEv: CanLog[A]) { 
  def error(message: String)(implicit a: A): Unit
}

It should also solve #80

[analytically]

What's the pattern for two implicits?

[pwliwanow]

If you want to log multiple values I think it's natural to put them inside one class:

case class TracingDetails(messageId: String, causationId: String, correlationId: String)

I don't see a very good reason for having more than one implicit in the signature.

[Kreinoee]

I like you approach for handling this. How about adding a default MDCMap class, that would basically just be a wrapper arround an immutable map, and then a default implementation of the CanLog trait that takes an MDCMap and add them to the MDC context. Something like:

final case class MDCMap(map: scala.collection.immutable.HashMap[String, String])

implicit object CanLogMDCMap extends CanLog[MDCMap] {
  override def logMessage(originalMsg: String, a: MDCMap): String = {
    a.map.foreach(t ⇒ MDC.put(t._1, t._2))
    originalMsg
  }

  override def afterLog(a: MDCMap): Unit = {
    a.map.keys.foreach(MDC.remove)
  }
}

That would provide a default bridge to the java world. The reason that is important, is that a lot of tools are build arround the concept of MDC values.

[pwliwanow]

@Kreinoee thanks, I could add that class as well or you could create pull request after this one is merged :)

@analytically if you like this idea and you would like to merge it, please let me know, so I can update the pull request. And if you don't like it, I think it should be closed

[analytically]

@pwliwanow yeah I like this idea, please update your PR

[analytically]

why a SerialVersionUID and why Serializable?

[analytically]

let's remove this

[pwliwanow]

Just SerialVersionUID or also Serializable?

Responding to an earlier question, it just mirrors whats in "regular" Logger + to allow Spark users to use it

[analytically]

Ah ok makes sense

[Kreinoee]

Hi @pwliwanow. Will you update the pull request, or should I create a new pull request based on this one, with the CanLogMDCMap objct and MDCMap class?

[pwliwanow]

@Kreinoee sorry, I forgot about this one. I will update the pull request this week

[Kreinoee]

Super sounds great.

And like analytically I am a bit curious about why CanLog should be serializable?

[Kreinoee]

We ended up not including the CanLogMDCMap and MDCMap classes. Is it something I should create a new pull request for, or do you think its best without them?

[analytically]

You're welcome to open a new PR.

[Atry]

I think CanLogMDCMap can be implemented from MDC.setContextMap(map.asJava), which is more efficient than Map.foreach