-
Notifications
You must be signed in to change notification settings - Fork 156
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.
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
feat(log4j):add rollbar support #359
base: master
Are you sure you want to change the base?
Conversation
d8ccbf5
to
64582e5
Compare
e926142
to
9d764e0
Compare
9d764e0
to
19db0a3
Compare
log4j.appender.ROLLBAR=com.nextdoor.rollbar.RollbarLog4jAppender | ||
log4j.appender.ROLLBAR.AccessToken=${ROLLBAR_ACCESS_TOKEN} | ||
log4j.appender.ROLLBAR.Environment=${ROLLBAR_ENV:-production} | ||
log4j.appender.ROLLBAR.layout=org.apache.log4j.PatternLayout |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe let's start with error?
@@ -16,7 +16,7 @@ | |||
# | |||
|
|||
# Set everything to be logged to the console | |||
log4j.rootLogger=INFO,console | |||
log4j.rootLogger=INFO,console,ROLLBAR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this work if rollbar is not in this file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes
@@ -69,7 +69,8 @@ libraryDependencies ++= Seq( | |||
"org.apache.avro" % "avro" % "1.8.2" % "provided", | |||
"org.apache.hive" % "hive-jdbc" % "2.3.3" % "provided" excludeAll(excludeNetty, excludeNettyAll, excludeLog4j, excludeParquet), | |||
"org.apache.hadoop" % "hadoop-aws" % "2.7.3" % "provided", | |||
"com.amazon.deequ" % "deequ" % "1.0.4" excludeAll(excludeSpark) | |||
"com.amazon.deequ" % "deequ" % "1.0.4" excludeAll(excludeSpark), | |||
"com.nextdoor.rollbar" % "rollbar-log4j" % "1.0.2" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think in order to use this feature
https://docs.rollbar.com/docs/java#synchronous-option
We need to create a custom appender, copy paste from here (https://github.com/zendesk/rollbar-log4j/blob/master/src/main/java/com/zendesk/rollbar/RollbarLog4jAppender.java)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or try fixing with a shutdown hook?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the problem is they haven't implemented the close function there...
log4j.appender.ROLLBAR.AccessToken=${ROLLBAR_ACCESS_TOKEN} | ||
log4j.appender.ROLLBAR.Environment=${ROLLBAR_ENV:-production} | ||
log4j.appender.ROLLBAR.layout=org.apache.log4j.PatternLayout | ||
log4j.appender.ROLLBAR.Threshold=${ROLLBAR_ENV:-WARN} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ROLLBAR_ENV? Warn should be error
No description provided.