Add code attributes for log4j1 #13947
Conversation
|
/easycla |
There was a problem hiding this comment.
hi @oliver-zhang! can you add tests for this similar to those in log4j2?
Unit tests have been added |
...main/java/io/opentelemetry/javaagent/instrumentation/log4j/appender/v1_2/LogEventMapper.java
Show resolved
Hide resolved
|
hi @oliver-zhang, thanks for your patience, hopefully we should get #13860 merged in the next few days and then you can rebase and update your PR |
# Conflicts: # instrumentation/log4j/log4j-appender-1.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/log4j/appender/v1_2/LogEventMapper.java
| } | ||
|
|
||
| String lineNumber = locationInfo.getLineNumber(); | ||
| int codeLineNo = 0; |
There was a problem hiding this comment.
Do the line numbers always start at 1 ? Just in case it could be safer to initialize it to -1 by default and then test it with >=0 afterwards.
| equalTo(THREAD_NAME, Thread.currentThread().getName()), | ||
| equalTo(THREAD_ID, Thread.currentThread().getId()), | ||
| equalTo(CODE_NAMESPACE, Log4j1Test.class.getName()), | ||
| equalTo(CODE_FUNCTION, "testCodeAttributes"), | ||
| satisfies(CODE_LINENO, AbstractLongAssert::isPositive), | ||
| equalTo(CODE_FILEPATH, "Log4j1Test.java"))); |
There was a problem hiding this comment.
you can use io.opentelemetry.instrumentation.testing.junit.code.SemconvCodeStabilityUtil for assertions, that was added in #13860 and allows to remove the @SuppressWarnings annotation by providing assertions for all the code attributes. This also allows to test both old and new conventions, but here only the old ones are tested which is fine.
Add the ability to get code attributes for log4j1