= newrelic: Associate logged errors with correct transaction

Previously, errors were associated with transactions named
"OtherTransaction/<trace-name>", with this change they are associated with
"WebTransaction/Uri/<trace-name" which is the name expected by NewRelic.

The additional complexity of wrapping messages in LoggedError instances
ensures that the logged message is not repeated in the New Relic UI.

4 files changed, 36 insertions, 21 deletions

diff --git a/kamon-newrelic/src/main/scala/kamon/newrelic/NewRelicErrorLogger.scala b/kamon-newrelic/src/main/scala/kamon/newrelic/NewRelicErrorLogger.scala
index c1601fa8..0076b668 100644
--- a/kamon-newrelic/src/main/scala/kamon/newrelic/NewRelicErrorLogger.scala
+++ b/kamon-newrelic/src/main/scala/kamon/newrelic/NewRelicErrorLogger.scala
@@ -17,11 +17,12 @@
 package kamon.newrelic
 
 import java.util
-
 import akka.actor.{ Actor, ActorLogging }
 import akka.event.Logging.{ Error, InitializeLogger, LoggerInitialized }
-import com.newrelic.api.agent.{ NewRelic ⇒ NR }
+import com.newrelic.agent.errors.{ ThrowableError }
+import com.newrelic.agent.service.ServiceFactory
 import kamon.trace.{ Tracer, TraceContextAware }
+import scala.util.control.NoStackTrace
 
 trait CustomParamsSupport {
   this: NewRelicErrorLogger ⇒
@@ -38,24 +39,38 @@ class NewRelicErrorLogger extends Actor with ActorLogging with CustomParamsSuppo
     case anythingElse                                       ⇒
   }
 
-  def notifyError(error: Error): Unit = runInFakeTransaction {
+  def notifyError(error: Error): Unit = {
     val params = new util.HashMap[String, String]()
+    customParams foreach { case (k, v) ⇒ params.put(k, v) }
 
-    if (error.isInstanceOf[TraceContextAware]) {
-      val ctx = error.asInstanceOf[TraceContextAware].traceContext
-      params put ("TraceToken", ctx.token)
+    params.put("LogSource", error.logSource)
+    params.put("LogClass", error.logClass.toString)
+    error match {
+      case e: TraceContextAware ⇒ params.put("TraceToken", e.traceContext.token)
+      case _                    ⇒
     }
 
-    customParams foreach { case (k, v) ⇒ params.put(k, v) }
-
-    if (error.cause == Error.NoCause) NR.noticeError(error.message.toString, params)
-    else NR.noticeError(error.cause, params)
+    if (error.cause == Error.NoCause)
+      reportError(loggedMessage(error.message.toString, params))
+    else
+      reportError(loggedException(error.message.toString, error.cause, params))
   }
 
-  //Really ugly, but temporal hack until next release...
-  def runInFakeTransaction[T](thunk: ⇒ T): T = {
-    val oldName = Thread.currentThread.getName
-    Thread.currentThread.setName(Tracer.currentContext.name)
-    try thunk finally Thread.currentThread.setName(oldName)
+  def loggedMessage(message: String, params: util.HashMap[String, String]) =
+    loggedException("", LoggedMessage(message), params)
+
+  def loggedException(message: String, cause: Throwable, params: util.HashMap[String, String]) = {
+    if (null != message && message.length > 0) params.put("ErrorMessage", message)
+    val uri = s"/${Tracer.currentContext.name}"
+    val transaction = s"WebTransaction/Uri$uri"
+    LoggedException(cause, params, transaction, uri);
   }
-}
\ No newline at end of file
+
+  def reportError(error: ThrowableError) = ServiceFactory.getRPMService().getErrorService().reportError(error)
+
+}
+
+case class LoggedMessage(message: String) extends Throwable(message) with NoStackTrace
+
+case class LoggedException(cause: Throwable, params: util.HashMap[String, String], transaction: String, uri: String)
+  extends ThrowableError(null, transaction, cause, uri, System.currentTimeMillis(), null, null, null, params, null)
diff --git a/kamon-newrelic/src/main/scala/kamon/newrelic/WebTransactionMetricExtractor.scala b/kamon-newrelic/src/main/scala/kamon/newrelic/WebTransactionMetricExtractor.scala
index 1a31ac58..604969ab 100644
--- a/kamon-newrelic/src/main/scala/kamon/newrelic/WebTransactionMetricExtractor.scala
+++ b/kamon-newrelic/src/main/scala/kamon/newrelic/WebTransactionMetricExtractor.scala
@@ -43,7 +43,7 @@ object WebTransactionMetricExtractor extends MetricExtractor {
           apdexBuilder.record(Time.Nanoseconds.scale(Time.Seconds)(record.level), record.count)
         }
 
-        Metric(elapsedTime, Time.Nanoseconds, "WebTransaction/Custom/" + entity.name, None)
+        Metric(elapsedTime, Time.Nanoseconds, "WebTransaction/Uri/" + entity.name, None)
     }
 
     // Accumulate all segment metrics
@@ -89,7 +89,7 @@ object WebTransactionMetricExtractor extends MetricExtractor {
     val externalScopedByHostAndLibrary = externalScopedByHostAndLibrarySnapshots.map {
       case ((host, library, traceName), snapshots) ⇒
         val mergedSnapshots = snapshots.foldLeft(Histogram.Snapshot.empty)(_.merge(_, collectionContext))
-        Metric(mergedSnapshots, Time.Nanoseconds, s"External/$host/$library", Some("WebTransaction/Custom/" + traceName))
+        Metric(mergedSnapshots, Time.Nanoseconds, s"External/$host/$library", Some("WebTransaction/Uri/" + traceName))
     }
 
     Map(httpDispatcher, webTransaction, webTransactionTotal, externalAllWeb, externalAll, apdexBuilder.build) ++
diff --git a/project/Dependencies.scala b/project/Dependencies.scala
index a10b3bb2..3b9b7980 100644
--- a/project/Dependencies.scala
+++ b/project/Dependencies.scala
@@ -35,7 +35,7 @@ object Dependencies {
   val scalatest         = "org.scalatest"             %%  "scalatest"             % "2.2.1"
   val logback           = "ch.qos.logback"            %   "logback-classic"       % "1.0.13"
   val aspectJ           = "org.aspectj"               %   "aspectjweaver"         % aspectjVersion
-  val newrelic          = "com.newrelic.agent.java"   %   "newrelic-api"          % "3.11.0"
+  val newrelic          = "com.newrelic.agent.java"   %   "newrelic-agent"        % "3.11.0"
   val hdrHistogram      = "org.hdrhistogram"          %   "HdrHistogram"          % "2.1.7"
   val sprayCan          = "io.spray"                  %%  "spray-can"             % sprayVersion
   val sprayRouting      = "io.spray"                  %%  "spray-routing"         % sprayVersion
diff --git a/project/Projects.scala b/project/Projects.scala
index a68677ef..f375acb8 100644
--- a/project/Projects.scala
+++ b/project/Projects.scala
@@ -99,7 +99,7 @@ object Projects extends Build {
     .settings(
       libraryDependencies ++=
         compile(sprayCan, sprayClient, sprayRouting, sprayJson, sprayJsonLenses, newrelic, akkaSlf4j) ++
-        provided(aspectJ) ++
+        provided(aspectJ, newrelic) ++
         test(scalatest, akkaTestKit, sprayTestkit, slf4Api, akkaSlf4j))
 
 
@@ -212,7 +212,7 @@ object Projects extends Build {
     .settings(formatSettings: _*)
     .settings(
       libraryDependencies ++=
-        compile(sprayCan, sprayClient, sprayRouting, sprayJson, sprayJsonLenses, newrelic, akkaSlf4j) ++
+        compile(sprayCan, sprayClient, sprayRouting, sprayJson, sprayJsonLenses, akkaSlf4j) ++
         test(scalatest, akkaTestKit, slf4Api, slf4nop))
 
   val noPublishing = Seq(publish := (), publishLocal := (), publishArtifact := false)