-
Notifications
You must be signed in to change notification settings - Fork 867
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
Add logback appender javaagent instrumentation #4939
Add logback appender javaagent instrumentation #4939
Conversation
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.
Can we go ahead and add library instrumentation for logback and log4j? I guess we expect jul to be redirected to one in most cases and it's not as important there.
We already seem to be getting asked for non-javaagent logs instrumentation
We already have library instrumentation for log4j. @jack-berg mentioned last week he was working on (or thinking about writing?) a logback appender. |
created #4947 to track adding a logback appender instrumentation module |
…-javaagent-instrumentation
...ain/java/io/opentelemetry/javaagent/instrumentation/logback/appender/v1_0/LogbackHelper.java
Outdated
Show resolved
Hide resolved
...ain/java/io/opentelemetry/javaagent/instrumentation/logback/appender/v1_0/LogbackHelper.java
Outdated
Show resolved
Hide resolved
…n/java/io/opentelemetry/javaagent/instrumentation/logback/appender/v1_0/LogbackHelper.java Co-authored-by: Anuraag Agrawal <[email protected]>
…n/java/io/opentelemetry/javaagent/instrumentation/logback/appender/v1_0/LogbackHelper.java Co-authored-by: Anuraag Agrawal <[email protected]>
instrumentation/logback/logback-appender-1.0/javaagent/src/test/groovy/LogbackTest.groovy
Show resolved
Hide resolved
logger = args[0] | ||
loggerName = args[1] | ||
testMethod = args[2] | ||
severity = args[3] | ||
severityText = args[4] |
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.
Would
logger = args[0] | |
loggerName = args[1] | |
testMethod = args[2] | |
severity = args[3] | |
severityText = args[4] | |
[logger, loggerName, testMethod, severity, severityText] = args |
work?
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.
almost! it looks like def (logger, loggerName, testMethod, severity, severityText) = args
should work, but not in where blocks:
where-blocks may only contain parameterizations (e.g. 'salary << [1000, 5000, 9000]; salaryk = salary / 1000')
* logback * Use assertInverse * sync * sync * Update instrumentation/logback/logback-appender-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/logback/appender/v1_0/LogbackHelper.java Co-authored-by: Anuraag Agrawal <[email protected]> * Update instrumentation/logback/logback-appender-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/logback/appender/v1_0/LogbackHelper.java Co-authored-by: Anuraag Agrawal <[email protected]> * Unroll Co-authored-by: Anuraag Agrawal <[email protected]>
Renames
logback-1.0
instrumentation module tologback-mdc-1.0
, and introduceslogback-appender-1.0
module.There's no autoconfiguration of log exporter (yet), so this instrumentation doesn't benefit end users unless they explicitly configure a logging exporter via javaagent extension (this is how the tests work).