Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove play-json dependency #55

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Remove play-json dependency #55

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions build.sbt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,6 @@ libraryDependencies ++= Seq(
"com.google.inject.extensions" % "guice-assistedinject" % "4.2.2",
"net.codingwell" %% "scala-guice" % "4.2.1",
"net.logstash.logback" % "logstash-logback-encoder" % "5.2", // structured logging to sumo
// The following will need to be provided by users of this lib,
// meaning they can supply their own version (as long as compatible).
"com.typesafe.play" %% "play-json" % "2.6.10" % Provided
)

resolvers += "Artifactory" at "https://flow.jfrog.io/flow/libs-release/"
Expand Down
51 changes: 2 additions & 49 deletions src/main/scala/io/flow/log/Rollbar.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,22 +1,15 @@
package io.flow.log

import com.fasterxml.jackson.core.JsonGenerator
import com.fasterxml.jackson.databind.{ObjectMapper, SerializerProvider}
import com.fasterxml.jackson.databind.module.SimpleModule
import com.google.inject.assistedinject.{AssistedInject, FactoryModuleBuilder}
import com.google.inject.{AbstractModule, Provider}
import com.rollbar.api.payload.Payload
import com.rollbar.api.payload.data.Data
import com.rollbar.notifier.Rollbar
import com.rollbar.notifier.config.ConfigBuilder
import com.rollbar.notifier.fingerprint.FingerprintGenerator
import com.rollbar.notifier.sender.result.Result
import com.rollbar.notifier.sender.{BufferedSender, SyncSender}
import io.flow.util.{Config, FlowEnvironment}
import javax.inject.{Inject, Singleton}
import net.codingwell.scalaguice.ScalaModule
import play.api.libs.json.jackson.PlayJsonModule
import play.api.libs.json._


class RollbarModule extends AbstractModule with ScalaModule {
Expand Down Expand Up @@ -57,7 +50,7 @@ class RollbarFactory @Inject()(
) extends RollbarLogger.Factory {
@AssistedInject
def rollbar(
attributes: Map[String, JsValue] = Map.empty[String, JsValue],
attributes: Map[String, AnyRef] = Map.empty[String, AnyRef],
legacyMessage: Option[String] = None
): RollbarLogger = RollbarLogger(
rollbarProvider.get(),
Expand All @@ -70,7 +63,7 @@ class RollbarFactory @Inject()(
object RollbarProvider {
def logger(
token: String,
attributes: Map[String, JsValue] = Map.empty[String, JsValue],
attributes: Map[String, AnyRef] = Map.empty[String, AnyRef],
legacyMessage: Option[String] = None
): RollbarLogger = {
val rb = Some(rollbar(token))
Expand All @@ -92,45 +85,6 @@ object RollbarProvider {
}
}

// give a jackson serializer to rollbar, instead of using rollbar's hand-rolled serializer
val jacksonSerializer = new com.rollbar.notifier.sender.json.JsonSerializer {
// plain old jackson serializer
val mapper = new ObjectMapper()

// de/serialize play-json types
mapper.registerModule(PlayJsonModule)
Copy link
Contributor

@benwaffle benwaffle Dec 21, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing PlayJsonModule this means that we can no longer serialize Js* types (e.g. JsObject). I feel like being able to log objects is important for richer structured log messages. It would be good to determine whether any flow code logs any Js* values


// serialize Rollbar JsonSerializable types
mapper.registerModule(new SimpleModule() {
addSerializer(
classOf[com.rollbar.api.json.JsonSerializable],
(value: com.rollbar.api.json.JsonSerializable, gen: JsonGenerator, serializers: SerializerProvider) => {
serializers.defaultSerializeValue(value.asJson(), gen)
}
)
})

override def toJson(payload: Payload): String = {
mapper.writeValueAsString(payload)
}

case class ResultObj(code: Int = -1, message: Option[String], uuid: Option[String])

override def resultFrom(response: String): Result = {
Json.parse(response).validate(Json.reads[ResultObj]) match {
case JsSuccess(obj, _) =>
val builder = new Result.Builder().code(obj.code)
if (obj.code == 0)
builder.body(obj.uuid.get)
else
builder.body(obj.message.get)
builder.build()
case _ =>
new Result.Builder().code(-1).body("Didn't get an object").build()
}
}
}

ConfigBuilder.withAccessToken(token)
.handleUncaughtErrors(true)
.language("scala")
Expand All @@ -140,7 +94,6 @@ object RollbarProvider {
.sender(
new SyncSender.Builder()
.accessToken(token)
.jsonSerializer(jacksonSerializer)
.build()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check to see if we can use the default sender here (i.e. not call it because the defaults are the same)

)
.build()
Expand Down
32 changes: 14 additions & 18 deletions src/main/scala/io/flow/log/RollbarLogger.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import com.google.inject.assistedinject.{Assisted, AssistedInject}
import com.rollbar.notifier.Rollbar
import net.logstash.logback.marker.Markers.appendEntries
import org.slf4j.LoggerFactory
import play.api.libs.json.{JsValue, Json, Writes}

import scala.collection.JavaConverters._

object RollbarLogger {
Expand All @@ -19,7 +19,7 @@ object RollbarLogger {
trait Factory {

@AssistedInject
def rollbar(attributes: Map[String, JsValue], legacyMessage: Option[String]): RollbarLogger
def rollbar(attributes: Map[String, AnyRef], legacyMessage: Option[String]): RollbarLogger
}

object Keys {
Expand All @@ -31,24 +31,20 @@ object RollbarLogger {
val ExperienceKey = "experience_key"
val SuppressRollbar = "suppress_rollbar"
}

def convert(attributes: Map[String, JsValue]): java.util.Map[String, Object] =
attributes.asJava.asInstanceOf[java.util.Map[String, Object]]
}

case class RollbarLogger @AssistedInject() (
rollbar: Option[Rollbar],
@Assisted attributes: Map[String, JsValue],
@Assisted attributes: Map[String, AnyRef],
@Assisted legacyMessage: Option[String],
shouldSendToRollbar: Boolean = true
) {

import RollbarLogger._
import RollbarLogger.Keys

private val logger = LoggerFactory.getLogger("application")

def withKeyValue[T: Writes](keyValue: (String, T)): RollbarLogger = withKeyValue(keyValue._1, keyValue._2)
def withKeyValue[T: Writes](key: String, value: T): RollbarLogger = this.copy(attributes = attributes + (key -> Json.toJson(value)))
def withKeyValue(keyValue: (String, AnyRef)): RollbarLogger = withKeyValue(keyValue._1, keyValue._2)
def withKeyValue(key: String, value: AnyRef): RollbarLogger = this.copy(attributes = attributes + (key -> value))
def fingerprint(value: String): RollbarLogger = withKeyValue(Keys.Fingerprint, value)
def organization(value: String): RollbarLogger = withKeyValue(Keys.Organization, value)
def orderNumber(value: String): RollbarLogger = withKeyValue(Keys.OrderNumber, value)
Expand All @@ -64,7 +60,7 @@ case class RollbarLogger @AssistedInject() (
*/
def withSendToRollbar(sendToRollbar:Boolean): RollbarLogger = this.copy(shouldSendToRollbar = sendToRollbar)

def withKeyValues[T: Writes](keyValue: (String, Seq[T])): RollbarLogger = withKeyValues(keyValue._1, keyValue._2)
def withKeyValues(keyValue: (String, Seq[AnyRef])): RollbarLogger = withKeyValues(keyValue._1, keyValue._2)

/**
* Accepts a list of values and writes them as individual attributes.
Expand All @@ -75,7 +71,7 @@ case class RollbarLogger @AssistedInject() (
* - error_1: foo
* - error_2: bar
*/
def withKeyValues[T: Writes](key: String, values: Seq[T]): RollbarLogger = {
def withKeyValues(key: String, values: Seq[AnyRef]): RollbarLogger = {
values.zipWithIndex.foldLeft(this) { case (l, pair) =>
val value = pair._1
val index = pair._2
Expand All @@ -89,27 +85,27 @@ case class RollbarLogger @AssistedInject() (
def error(message: => String): Unit = error(message, null)

def debug(message: => String, error: => Throwable): Unit = {
logger.debug(appendEntries(convert(attributes)), legacyMessage.getOrElse(message), error)
logger.debug(appendEntries(attributes.asJava), legacyMessage.getOrElse(message), error)
//not sending to rollbar to save quota
}

def info(message: => String, error: => Throwable): Unit = {
logger.info(appendEntries(convert(attributes)), legacyMessage.getOrElse(message), error)
logger.info(appendEntries(attributes.asJava), legacyMessage.getOrElse(message), error)
//not sending to rollbar to save quota
}

def warn(message: => String, error: => Throwable): Unit = {
logger.warn(appendEntries(convert(attributes)), legacyMessage.getOrElse(message), error)
logger.warn(appendEntries(attributes.asJava), legacyMessage.getOrElse(message), error)
if (shouldSendToRollbar) {
rollbar.foreach(_.warning(error, convert(attributes), message))
rollbar.foreach(_.warning(error, attributes.asJava, message))
}

}

def error(message: => String, error: => Throwable): Unit = {
logger.error(appendEntries(convert(attributes)), legacyMessage.getOrElse(message), error)
logger.error(appendEntries(attributes.asJava), legacyMessage.getOrElse(message), error)
if (shouldSendToRollbar) {
rollbar.foreach(_.error(error, convert(attributes), message))
rollbar.foreach(_.error(error, attributes.asJava, message))
}
}

Expand Down
13 changes: 0 additions & 13 deletions src/main/scala/io/flow/log/SerializePlayJson.scala
View file
Open in desktop

This file was deleted.