-
-
Notifications
You must be signed in to change notification settings - Fork 443
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
adds support for logback encoders (the message field is encoded). #794
Conversation
eaa118f
to
cafb55b
Compare
adds support for global tags and extras. tags and extras are distinguished by the existing mdcTags property. basic usage: encoders ``` <appender name="jsonSentryAppender" class="io.sentry.logback.SentryAppender"> <filter class="ch.qos.logback.classic.filter.ThresholdFilter"> <level>WARN</level> </filter> <encoder class="net.logstash.logback.encoder.LoggingEventCompositeJsonEncoder"> <providers> <timestamp /> <version /> <logLevel /> ... ... ``` basic usage: global entries as tags or extras ``` LoggerContext context = (LoggerContext)LoggerFactory.getILoggerFactory(); context.putProperty("global", "value"); ```
cafb55b
to
0732e4a
Compare
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.
LGTM, apart from the one nitpick.
/** | ||
* Appender encoder. | ||
*/ | ||
protected Encoder<ILoggingEvent> encoder; |
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.
Shouldn't this be at least AtomicReference
and CAS'd on the use sites so that the unsynchronized access is somewhat handled? Or is it only ever supposed to be mutated by logback during instantiation and before any logging can occur?
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.
as far as I understand it this isn't an issue. And yes, I think you'd typically create and replace with a new appender than dynamically replace an encoder.
fyi fwiw logstashencoder is doing the same thing:
https://github.com/logstash/logstash-logback-encoder/blob/bbd8333120f274f4d9ef12e797eae2331fed1a52/src/main/java/net/logstash/logback/appender/AbstractLogstashTcpSocketAppender.java#L199
Thank you @jeacott1 |
should probably add something to the documentation too at some point :) |
@jeacott1 would be willing to contribute to: |
sure! |
Thank you @jeacott1 |
please check getsentry/sentry-docs#1379 |
adds support for logback encoders (the message field is encoded)
adds support for global tags and extras (not threadbound). tags and extras are
distinguished by the existing mdcTags property.
basic usage:
encoders
basic usage:
global entries as tags or extras