From 7528865e18d12c90ecf6235d91dd8c4f58f46b5a Mon Sep 17 00:00:00 2001 From: Jean Bisutti Date: Mon, 12 Sep 2022 08:23:25 -0700 Subject: [PATCH 01/12] Add code attributes for Logback --- .../javaagent/build.gradle.kts | 1 + .../appender/v1_0/LogbackSingletons.java | 7 ++++++- .../src/test/groovy/LogbackTest.groovy | 6 +++--- .../appender/v1_0/OpenTelemetryAppender.java | 16 +++++++++++++- .../v1_0/internal/LoggingEventMapper.java | 17 ++++++++++++++- .../v1_0/OpenTelemetryAppenderConfigTest.java | 21 +++++++++++++++---- .../v1_0/internal/LoggingEventMapperTest.java | 6 +++--- .../src/test/resources/logback-test.xml | 1 + 8 files changed, 62 insertions(+), 13 deletions(-) diff --git a/instrumentation/logback/logback-appender-1.0/javaagent/build.gradle.kts b/instrumentation/logback/logback-appender-1.0/javaagent/build.gradle.kts index 0fd4232118d5..7d8bfa3c39e9 100644 --- a/instrumentation/logback/logback-appender-1.0/javaagent/build.gradle.kts +++ b/instrumentation/logback/logback-appender-1.0/javaagent/build.gradle.kts @@ -54,4 +54,5 @@ tasks.withType().configureEach { // TODO run tests both with and without experimental log attributes jvmArgs("-Dotel.instrumentation.logback-appender.experimental.capture-mdc-attributes=*") jvmArgs("-Dotel.instrumentation.logback-appender.experimental-log-attributes=true") + jvmArgs("-Dotel.instrumentation.logback-appender.experimental.code-attributes=true") } diff --git a/instrumentation/logback/logback-appender-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/logback/appender/v1_0/LogbackSingletons.java b/instrumentation/logback/logback-appender-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/logback/appender/v1_0/LogbackSingletons.java index 93c6aedf8c91..396315e17b79 100644 --- a/instrumentation/logback/logback-appender-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/logback/appender/v1_0/LogbackSingletons.java +++ b/instrumentation/logback/logback-appender-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/logback/appender/v1_0/LogbackSingletons.java @@ -21,12 +21,17 @@ public final class LogbackSingletons { boolean captureExperimentalAttributes = config.getBoolean( "otel.instrumentation.logback-appender.experimental-log-attributes", false); + boolean captureCodeAttributes = + config.getBoolean( + "otel.instrumentation.logback-appender.experimental.code-attributes", false); List captureMdcAttributes = config.getList( "otel.instrumentation.logback-appender.experimental.capture-mdc-attributes", emptyList()); - mapper = new LoggingEventMapper(captureExperimentalAttributes, captureMdcAttributes); + mapper = + new LoggingEventMapper( + captureExperimentalAttributes, captureMdcAttributes, captureCodeAttributes); } public static LoggingEventMapper mapper() { diff --git a/instrumentation/logback/logback-appender-1.0/javaagent/src/test/groovy/LogbackTest.groovy b/instrumentation/logback/logback-appender-1.0/javaagent/src/test/groovy/LogbackTest.groovy index fc62df876ab4..e82cce996af1 100644 --- a/instrumentation/logback/logback-appender-1.0/javaagent/src/test/groovy/LogbackTest.groovy +++ b/instrumentation/logback/logback-appender-1.0/javaagent/src/test/groovy/LogbackTest.groovy @@ -56,12 +56,12 @@ class LogbackTest extends AgentInstrumentationSpecification { assertThat(log.getSeverity()).isEqualTo(severity) assertThat(log.getSeverityText()).isEqualTo(severityText) if (exception) { - assertThat(log.getAttributes().size()).isEqualTo(5) + assertThat(log.getAttributes().size()).isEqualTo(5 + 4) // 4 code attributes assertThat(log.getAttributes().get(SemanticAttributes.EXCEPTION_TYPE)).isEqualTo(IllegalStateException.getName()) assertThat(log.getAttributes().get(SemanticAttributes.EXCEPTION_MESSAGE)).isEqualTo("hello") assertThat(log.getAttributes().get(SemanticAttributes.EXCEPTION_STACKTRACE)).contains(LogbackTest.name) } else { - assertThat(log.getAttributes().size()).isEqualTo(2) + assertThat(log.getAttributes().size()).isEqualTo(2 + 4) // 4 code attributes assertThat(log.getAttributes().get(SemanticAttributes.EXCEPTION_TYPE)).isNull() assertThat(log.getAttributes().get(SemanticAttributes.EXCEPTION_MESSAGE)).isNull() assertThat(log.getAttributes().get(SemanticAttributes.EXCEPTION_STACKTRACE)).isNull() @@ -123,7 +123,7 @@ class LogbackTest extends AgentInstrumentationSpecification { assertThat(log.getInstrumentationScopeInfo().getName()).isEqualTo("abc") assertThat(log.getSeverity()).isEqualTo(Severity.INFO) assertThat(log.getSeverityText()).isEqualTo("INFO") - assertThat(log.getAttributes().size()).isEqualTo(4) + assertThat(log.getAttributes().size()).isEqualTo(3 + 4) // 4 code attributes assertThat(log.getAttributes().get(AttributeKey.stringKey("logback.mdc.key1"))).isEqualTo("val1") assertThat(log.getAttributes().get(AttributeKey.stringKey("logback.mdc.key2"))).isEqualTo("val2") assertThat(log.getAttributes().get(SemanticAttributes.THREAD_NAME)).isEqualTo(Thread.currentThread().getName()) diff --git a/instrumentation/logback/logback-appender-1.0/library/src/main/java/io/opentelemetry/instrumentation/logback/appender/v1_0/OpenTelemetryAppender.java b/instrumentation/logback/logback-appender-1.0/library/src/main/java/io/opentelemetry/instrumentation/logback/appender/v1_0/OpenTelemetryAppender.java index 24b7ebcb2f52..a46a548e52a7 100644 --- a/instrumentation/logback/logback-appender-1.0/library/src/main/java/io/opentelemetry/instrumentation/logback/appender/v1_0/OpenTelemetryAppender.java +++ b/instrumentation/logback/logback-appender-1.0/library/src/main/java/io/opentelemetry/instrumentation/logback/appender/v1_0/OpenTelemetryAppender.java @@ -25,6 +25,7 @@ public class OpenTelemetryAppender extends UnsynchronizedAppenderBase captureMdcAttributes = emptyList(); private volatile LoggingEventMapper mapper; @@ -33,7 +34,9 @@ public OpenTelemetryAppender() {} @Override public void start() { - mapper = new LoggingEventMapper(captureExperimentalAttributes, captureMdcAttributes); + mapper = + new LoggingEventMapper( + captureExperimentalAttributes, captureMdcAttributes, captureCodeAttributes); super.start(); } @@ -62,6 +65,17 @@ public void setCaptureExperimentalAttributes(boolean captureExperimentalAttribut this.captureExperimentalAttributes = captureExperimentalAttributes; } + /** + * Sets whether the code attributes (file name, class name, method name and line number) should be + * set to logs. + * + * @param captureCodeAttributes To enable or disable the code attributes (file name, class name, + * method name and line number) + */ + public void setCaptureCodeAttributes(boolean captureCodeAttributes) { + this.captureCodeAttributes = captureCodeAttributes; + } + /** Configures the {@link MDC} attributes that will be copied to logs. */ public void setCaptureMdcAttributes(String attributes) { if (attributes != null) { diff --git a/instrumentation/logback/logback-appender-1.0/library/src/main/java/io/opentelemetry/instrumentation/logback/appender/v1_0/internal/LoggingEventMapper.java b/instrumentation/logback/logback-appender-1.0/library/src/main/java/io/opentelemetry/instrumentation/logback/appender/v1_0/internal/LoggingEventMapper.java index e2469fe26340..90648f3b062f 100644 --- a/instrumentation/logback/logback-appender-1.0/library/src/main/java/io/opentelemetry/instrumentation/logback/appender/v1_0/internal/LoggingEventMapper.java +++ b/instrumentation/logback/logback-appender-1.0/library/src/main/java/io/opentelemetry/instrumentation/logback/appender/v1_0/internal/LoggingEventMapper.java @@ -34,10 +34,14 @@ public final class LoggingEventMapper { private final boolean captureExperimentalAttributes; private final List captureMdcAttributes; private final boolean captureAllMdcAttributes; + private final boolean captureCodeAttributes; public LoggingEventMapper( - boolean captureExperimentalAttributes, List captureMdcAttributes) { + boolean captureExperimentalAttributes, + List captureMdcAttributes, + boolean captureCodeAttributes) { this.captureExperimentalAttributes = captureExperimentalAttributes; + this.captureCodeAttributes = captureCodeAttributes; this.captureMdcAttributes = captureMdcAttributes; this.captureAllMdcAttributes = captureMdcAttributes.size() == 1 && captureMdcAttributes.get(0).equals("*"); @@ -104,6 +108,17 @@ private void mapLoggingEvent(LogRecordBuilder builder, ILoggingEvent loggingEven attributes.put(SemanticAttributes.THREAD_ID, currentThread.getId()); } + if (captureCodeAttributes) { + StackTraceElement[] callerData = loggingEvent.getCallerData(); + if (callerData != null && callerData.length > 0) { + StackTraceElement firstStackElement = callerData[0]; + attributes.put(SemanticAttributes.CODE_FILEPATH, firstStackElement.getFileName()); + attributes.put(SemanticAttributes.CODE_NAMESPACE, firstStackElement.getClassName()); + attributes.put(SemanticAttributes.CODE_FUNCTION, firstStackElement.getMethodName()); + attributes.put(SemanticAttributes.CODE_LINENO, firstStackElement.getLineNumber()); + } + } + builder.setAllAttributes(attributes.build()); // span context diff --git a/instrumentation/logback/logback-appender-1.0/library/src/test/java/io/opentelemetry/instrumentation/logback/appender/v1_0/OpenTelemetryAppenderConfigTest.java b/instrumentation/logback/logback-appender-1.0/library/src/test/java/io/opentelemetry/instrumentation/logback/appender/v1_0/OpenTelemetryAppenderConfigTest.java index c8be519d719a..06853ba75784 100644 --- a/instrumentation/logback/logback-appender-1.0/library/src/test/java/io/opentelemetry/instrumentation/logback/appender/v1_0/OpenTelemetryAppenderConfigTest.java +++ b/instrumentation/logback/logback-appender-1.0/library/src/test/java/io/opentelemetry/instrumentation/logback/appender/v1_0/OpenTelemetryAppenderConfigTest.java @@ -8,7 +8,6 @@ import static org.assertj.core.api.AssertionsForInterfaceTypes.assertThat; import io.opentelemetry.api.common.AttributeKey; -import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.trace.Span; import io.opentelemetry.api.trace.SpanContext; import io.opentelemetry.context.Scope; @@ -71,7 +70,7 @@ void logNoSpan() { assertThat(logData.getResource()).isEqualTo(resource); assertThat(logData.getInstrumentationScopeInfo()).isEqualTo(instrumentationScopeInfo); assertThat(logData.getBody().asString()).isEqualTo("log message 1"); - assertThat(logData.getAttributes()).isEqualTo(Attributes.empty()); + assertThat(logData.getAttributes().size()).isEqualTo(4); // 4 code attributes } @Test @@ -115,13 +114,27 @@ void logWithExtras() { .isLessThan(TimeUnit.MILLISECONDS.toNanos(Instant.now().toEpochMilli())); assertThat(logData.getSeverity()).isEqualTo(Severity.INFO); assertThat(logData.getSeverityText()).isEqualTo("INFO"); - assertThat(logData.getAttributes().size()).isEqualTo(3); + assertThat(logData.getAttributes().size()).isEqualTo(3 + 4); // 4 code attributes assertThat(logData.getAttributes().get(SemanticAttributes.EXCEPTION_TYPE)) .isEqualTo(IllegalStateException.class.getName()); assertThat(logData.getAttributes().get(SemanticAttributes.EXCEPTION_MESSAGE)) .isEqualTo("Error!"); assertThat(logData.getAttributes().get(SemanticAttributes.EXCEPTION_STACKTRACE)) .contains("logWithExtras"); + + String file = logData.getAttributes().get(SemanticAttributes.CODE_FILEPATH); + assertThat(file).isEqualTo("OpenTelemetryAppenderConfigTest.java"); + + String codeClass = logData.getAttributes().get(SemanticAttributes.CODE_NAMESPACE); + assertThat(codeClass) + .isEqualTo( + "io.opentelemetry.instrumentation.logback.appender.v1_0.OpenTelemetryAppenderConfigTest"); + + String method = logData.getAttributes().get(SemanticAttributes.CODE_FUNCTION); + assertThat(method).isEqualTo("logWithExtras"); + + Long lineNumber = logData.getAttributes().get(SemanticAttributes.CODE_LINENO); + assertThat(lineNumber).isGreaterThan(1); } @Test @@ -140,7 +153,7 @@ void logContextData() { assertThat(logData.getResource()).isEqualTo(resource); assertThat(logData.getInstrumentationScopeInfo()).isEqualTo(instrumentationScopeInfo); assertThat(logData.getBody().asString()).isEqualTo("log message 1"); - assertThat(logData.getAttributes().size()).isEqualTo(2); + assertThat(logData.getAttributes().size()).isEqualTo(2 + 4); // 4 code attributes AssertionsForClassTypes.assertThat( logData.getAttributes().get(AttributeKey.stringKey("logback.mdc.key1"))) .isEqualTo("val1"); diff --git a/instrumentation/logback/logback-appender-1.0/library/src/test/java/io/opentelemetry/instrumentation/logback/appender/v1_0/internal/LoggingEventMapperTest.java b/instrumentation/logback/logback-appender-1.0/library/src/test/java/io/opentelemetry/instrumentation/logback/appender/v1_0/internal/LoggingEventMapperTest.java index 915f62f05bf7..f4b6ac530412 100644 --- a/instrumentation/logback/logback-appender-1.0/library/src/test/java/io/opentelemetry/instrumentation/logback/appender/v1_0/internal/LoggingEventMapperTest.java +++ b/instrumentation/logback/logback-appender-1.0/library/src/test/java/io/opentelemetry/instrumentation/logback/appender/v1_0/internal/LoggingEventMapperTest.java @@ -22,7 +22,7 @@ class LoggingEventMapperTest { @Test void testDefault() { // given - LoggingEventMapper mapper = new LoggingEventMapper(false, emptyList()); + LoggingEventMapper mapper = new LoggingEventMapper(false, emptyList(), false); Map contextData = new HashMap<>(); contextData.put("key1", "value1"); contextData.put("key2", "value2"); @@ -38,7 +38,7 @@ void testDefault() { @Test void testSome() { // given - LoggingEventMapper mapper = new LoggingEventMapper(false, singletonList("key2")); + LoggingEventMapper mapper = new LoggingEventMapper(false, singletonList("key2"), false); Map contextData = new HashMap<>(); contextData.put("key1", "value1"); contextData.put("key2", "value2"); @@ -55,7 +55,7 @@ void testSome() { @Test void testAll() { // given - LoggingEventMapper mapper = new LoggingEventMapper(false, singletonList("*")); + LoggingEventMapper mapper = new LoggingEventMapper(false, singletonList("*"), false); Map contextData = new HashMap<>(); contextData.put("key1", "value1"); contextData.put("key2", "value2"); diff --git a/instrumentation/logback/logback-appender-1.0/library/src/test/resources/logback-test.xml b/instrumentation/logback/logback-appender-1.0/library/src/test/resources/logback-test.xml index ddefe47c3a4d..d213483122ee 100644 --- a/instrumentation/logback/logback-appender-1.0/library/src/test/resources/logback-test.xml +++ b/instrumentation/logback/logback-appender-1.0/library/src/test/resources/logback-test.xml @@ -11,6 +11,7 @@ false + true * From 1ebe9f451ae32b88ac37075b5eaddceaf2b15e34 Mon Sep 17 00:00:00 2001 From: Jean Bisutti Date: Mon, 12 Sep 2022 10:57:08 -0700 Subject: [PATCH 02/12] Rename property --- .../logback/logback-appender-1.0/javaagent/build.gradle.kts | 2 +- .../logback/appender/v1_0/LogbackSingletons.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/instrumentation/logback/logback-appender-1.0/javaagent/build.gradle.kts b/instrumentation/logback/logback-appender-1.0/javaagent/build.gradle.kts index 7d8bfa3c39e9..493bf9a34c12 100644 --- a/instrumentation/logback/logback-appender-1.0/javaagent/build.gradle.kts +++ b/instrumentation/logback/logback-appender-1.0/javaagent/build.gradle.kts @@ -54,5 +54,5 @@ tasks.withType().configureEach { // TODO run tests both with and without experimental log attributes jvmArgs("-Dotel.instrumentation.logback-appender.experimental.capture-mdc-attributes=*") jvmArgs("-Dotel.instrumentation.logback-appender.experimental-log-attributes=true") - jvmArgs("-Dotel.instrumentation.logback-appender.experimental.code-attributes=true") + jvmArgs("-Dotel.instrumentation.logback-appender.experimental.capture-code-attributes=true") } diff --git a/instrumentation/logback/logback-appender-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/logback/appender/v1_0/LogbackSingletons.java b/instrumentation/logback/logback-appender-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/logback/appender/v1_0/LogbackSingletons.java index 396315e17b79..6789d3567ab0 100644 --- a/instrumentation/logback/logback-appender-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/logback/appender/v1_0/LogbackSingletons.java +++ b/instrumentation/logback/logback-appender-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/logback/appender/v1_0/LogbackSingletons.java @@ -23,7 +23,7 @@ public final class LogbackSingletons { "otel.instrumentation.logback-appender.experimental-log-attributes", false); boolean captureCodeAttributes = config.getBoolean( - "otel.instrumentation.logback-appender.experimental.code-attributes", false); + "otel.instrumentation.logback-appender.experimental.capture-code-attributes", false); List captureMdcAttributes = config.getList( "otel.instrumentation.logback-appender.experimental.capture-mdc-attributes", From d9bbf85f260f4b63809a34f7d02e5311d811de06 Mon Sep 17 00:00:00 2001 From: Jean Bisutti Date: Mon, 12 Sep 2022 11:07:34 -0700 Subject: [PATCH 03/12] Add a note about performance --- .../logback/appender/v1_0/OpenTelemetryAppender.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/instrumentation/logback/logback-appender-1.0/library/src/main/java/io/opentelemetry/instrumentation/logback/appender/v1_0/OpenTelemetryAppender.java b/instrumentation/logback/logback-appender-1.0/library/src/main/java/io/opentelemetry/instrumentation/logback/appender/v1_0/OpenTelemetryAppender.java index a46a548e52a7..2f2c13760f6e 100644 --- a/instrumentation/logback/logback-appender-1.0/library/src/main/java/io/opentelemetry/instrumentation/logback/appender/v1_0/OpenTelemetryAppender.java +++ b/instrumentation/logback/logback-appender-1.0/library/src/main/java/io/opentelemetry/instrumentation/logback/appender/v1_0/OpenTelemetryAppender.java @@ -67,7 +67,8 @@ public void setCaptureExperimentalAttributes(boolean captureExperimentalAttribut /** * Sets whether the code attributes (file name, class name, method name and line number) should be - * set to logs. + * set to logs. Enabling these attributes can potentially impact performance (see + * https://logback.qos.ch/manual/layouts.html). * * @param captureCodeAttributes To enable or disable the code attributes (file name, class name, * method name and line number) From 8d1cf4d46d29748e7393d2f34deb6b590d18aa73 Mon Sep 17 00:00:00 2001 From: Jean Bisutti Date: Tue, 13 Sep 2022 03:31:41 -0700 Subject: [PATCH 04/12] Add null check on file name --- .../logback/appender/v1_0/internal/LoggingEventMapper.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/instrumentation/logback/logback-appender-1.0/library/src/main/java/io/opentelemetry/instrumentation/logback/appender/v1_0/internal/LoggingEventMapper.java b/instrumentation/logback/logback-appender-1.0/library/src/main/java/io/opentelemetry/instrumentation/logback/appender/v1_0/internal/LoggingEventMapper.java index 90648f3b062f..30b4e3533b2f 100644 --- a/instrumentation/logback/logback-appender-1.0/library/src/main/java/io/opentelemetry/instrumentation/logback/appender/v1_0/internal/LoggingEventMapper.java +++ b/instrumentation/logback/logback-appender-1.0/library/src/main/java/io/opentelemetry/instrumentation/logback/appender/v1_0/internal/LoggingEventMapper.java @@ -112,7 +112,10 @@ private void mapLoggingEvent(LogRecordBuilder builder, ILoggingEvent loggingEven StackTraceElement[] callerData = loggingEvent.getCallerData(); if (callerData != null && callerData.length > 0) { StackTraceElement firstStackElement = callerData[0]; - attributes.put(SemanticAttributes.CODE_FILEPATH, firstStackElement.getFileName()); + String fileName = firstStackElement.getFileName(); + if(fileName != null) { + attributes.put(SemanticAttributes.CODE_FILEPATH, fileName); + } attributes.put(SemanticAttributes.CODE_NAMESPACE, firstStackElement.getClassName()); attributes.put(SemanticAttributes.CODE_FUNCTION, firstStackElement.getMethodName()); attributes.put(SemanticAttributes.CODE_LINENO, firstStackElement.getLineNumber()); From df6d85ae91b2b299a06ecc5807c1394a3a7f176f Mon Sep 17 00:00:00 2001 From: Jean Bisutti Date: Tue, 13 Sep 2022 03:42:37 -0700 Subject: [PATCH 05/12] Add check on line number --- .../logback/appender/v1_0/internal/LoggingEventMapper.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/instrumentation/logback/logback-appender-1.0/library/src/main/java/io/opentelemetry/instrumentation/logback/appender/v1_0/internal/LoggingEventMapper.java b/instrumentation/logback/logback-appender-1.0/library/src/main/java/io/opentelemetry/instrumentation/logback/appender/v1_0/internal/LoggingEventMapper.java index 30b4e3533b2f..a6b5fd25a7bd 100644 --- a/instrumentation/logback/logback-appender-1.0/library/src/main/java/io/opentelemetry/instrumentation/logback/appender/v1_0/internal/LoggingEventMapper.java +++ b/instrumentation/logback/logback-appender-1.0/library/src/main/java/io/opentelemetry/instrumentation/logback/appender/v1_0/internal/LoggingEventMapper.java @@ -118,7 +118,10 @@ private void mapLoggingEvent(LogRecordBuilder builder, ILoggingEvent loggingEven } attributes.put(SemanticAttributes.CODE_NAMESPACE, firstStackElement.getClassName()); attributes.put(SemanticAttributes.CODE_FUNCTION, firstStackElement.getMethodName()); - attributes.put(SemanticAttributes.CODE_LINENO, firstStackElement.getLineNumber()); + int lineNumber = firstStackElement.getLineNumber(); + if(lineNumber != -1) { + attributes.put(SemanticAttributes.CODE_LINENO, lineNumber); + } } } From 6d9d7067504af2a293bd4d3e7fc884bde573f0df Mon Sep 17 00:00:00 2001 From: Jean Bisutti Date: Tue, 13 Sep 2022 03:52:27 -0700 Subject: [PATCH 06/12] Fix test following new behavior --- .../javaagent/src/test/groovy/LogbackTest.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/logback/logback-appender-1.0/javaagent/src/test/groovy/LogbackTest.groovy b/instrumentation/logback/logback-appender-1.0/javaagent/src/test/groovy/LogbackTest.groovy index e82cce996af1..96a65b34ac54 100644 --- a/instrumentation/logback/logback-appender-1.0/javaagent/src/test/groovy/LogbackTest.groovy +++ b/instrumentation/logback/logback-appender-1.0/javaagent/src/test/groovy/LogbackTest.groovy @@ -123,7 +123,7 @@ class LogbackTest extends AgentInstrumentationSpecification { assertThat(log.getInstrumentationScopeInfo().getName()).isEqualTo("abc") assertThat(log.getSeverity()).isEqualTo(Severity.INFO) assertThat(log.getSeverityText()).isEqualTo("INFO") - assertThat(log.getAttributes().size()).isEqualTo(3 + 4) // 4 code attributes + assertThat(log.getAttributes().size()).isEqualTo(3 + 3) // 3 code attributes assertThat(log.getAttributes().get(AttributeKey.stringKey("logback.mdc.key1"))).isEqualTo("val1") assertThat(log.getAttributes().get(AttributeKey.stringKey("logback.mdc.key2"))).isEqualTo("val2") assertThat(log.getAttributes().get(SemanticAttributes.THREAD_NAME)).isEqualTo(Thread.currentThread().getName()) From 5db9dedae25f99e7074774214d64e1d6aab5717b Mon Sep 17 00:00:00 2001 From: Jean Bisutti Date: Tue, 13 Sep 2022 03:54:22 -0700 Subject: [PATCH 07/12] spotless --- .../logback/appender/v1_0/internal/LoggingEventMapper.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/instrumentation/logback/logback-appender-1.0/library/src/main/java/io/opentelemetry/instrumentation/logback/appender/v1_0/internal/LoggingEventMapper.java b/instrumentation/logback/logback-appender-1.0/library/src/main/java/io/opentelemetry/instrumentation/logback/appender/v1_0/internal/LoggingEventMapper.java index a6b5fd25a7bd..c4b130d8e161 100644 --- a/instrumentation/logback/logback-appender-1.0/library/src/main/java/io/opentelemetry/instrumentation/logback/appender/v1_0/internal/LoggingEventMapper.java +++ b/instrumentation/logback/logback-appender-1.0/library/src/main/java/io/opentelemetry/instrumentation/logback/appender/v1_0/internal/LoggingEventMapper.java @@ -113,13 +113,13 @@ private void mapLoggingEvent(LogRecordBuilder builder, ILoggingEvent loggingEven if (callerData != null && callerData.length > 0) { StackTraceElement firstStackElement = callerData[0]; String fileName = firstStackElement.getFileName(); - if(fileName != null) { + if (fileName != null) { attributes.put(SemanticAttributes.CODE_FILEPATH, fileName); } attributes.put(SemanticAttributes.CODE_NAMESPACE, firstStackElement.getClassName()); attributes.put(SemanticAttributes.CODE_FUNCTION, firstStackElement.getMethodName()); int lineNumber = firstStackElement.getLineNumber(); - if(lineNumber != -1) { + if (lineNumber != -1) { attributes.put(SemanticAttributes.CODE_LINENO, lineNumber); } } From 887fe3ac76adb54ac13fdcb0204f14852328b0a8 Mon Sep 17 00:00:00 2001 From: Jean Bisutti Date: Tue, 13 Sep 2022 16:02:36 +0200 Subject: [PATCH 08/12] Update instrumentation/logback/logback-appender-1.0/library/src/main/java/io/opentelemetry/instrumentation/logback/appender/v1_0/internal/LoggingEventMapper.java Co-authored-by: Mateusz Rzeszutek --- .../logback/appender/v1_0/internal/LoggingEventMapper.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/logback/logback-appender-1.0/library/src/main/java/io/opentelemetry/instrumentation/logback/appender/v1_0/internal/LoggingEventMapper.java b/instrumentation/logback/logback-appender-1.0/library/src/main/java/io/opentelemetry/instrumentation/logback/appender/v1_0/internal/LoggingEventMapper.java index c4b130d8e161..576e32395237 100644 --- a/instrumentation/logback/logback-appender-1.0/library/src/main/java/io/opentelemetry/instrumentation/logback/appender/v1_0/internal/LoggingEventMapper.java +++ b/instrumentation/logback/logback-appender-1.0/library/src/main/java/io/opentelemetry/instrumentation/logback/appender/v1_0/internal/LoggingEventMapper.java @@ -119,7 +119,7 @@ private void mapLoggingEvent(LogRecordBuilder builder, ILoggingEvent loggingEven attributes.put(SemanticAttributes.CODE_NAMESPACE, firstStackElement.getClassName()); attributes.put(SemanticAttributes.CODE_FUNCTION, firstStackElement.getMethodName()); int lineNumber = firstStackElement.getLineNumber(); - if (lineNumber != -1) { + if (lineNumber > 0) { attributes.put(SemanticAttributes.CODE_LINENO, lineNumber); } } From e363a1fa42523a0572774a5f772b62c03f024585 Mon Sep 17 00:00:00 2001 From: Jean Bisutti Date: Tue, 13 Sep 2022 07:18:25 -0700 Subject: [PATCH 09/12] Fix test --- .../javaagent/src/test/groovy/LogbackTest.groovy | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/instrumentation/logback/logback-appender-1.0/javaagent/src/test/groovy/LogbackTest.groovy b/instrumentation/logback/logback-appender-1.0/javaagent/src/test/groovy/LogbackTest.groovy index 96a65b34ac54..de7aa8b0ecfd 100644 --- a/instrumentation/logback/logback-appender-1.0/javaagent/src/test/groovy/LogbackTest.groovy +++ b/instrumentation/logback/logback-appender-1.0/javaagent/src/test/groovy/LogbackTest.groovy @@ -56,12 +56,12 @@ class LogbackTest extends AgentInstrumentationSpecification { assertThat(log.getSeverity()).isEqualTo(severity) assertThat(log.getSeverityText()).isEqualTo(severityText) if (exception) { - assertThat(log.getAttributes().size()).isEqualTo(5 + 4) // 4 code attributes + assertThat(log.getAttributes().size()).isEqualTo(5 + 3) // 3 code attributes assertThat(log.getAttributes().get(SemanticAttributes.EXCEPTION_TYPE)).isEqualTo(IllegalStateException.getName()) assertThat(log.getAttributes().get(SemanticAttributes.EXCEPTION_MESSAGE)).isEqualTo("hello") assertThat(log.getAttributes().get(SemanticAttributes.EXCEPTION_STACKTRACE)).contains(LogbackTest.name) } else { - assertThat(log.getAttributes().size()).isEqualTo(2 + 4) // 4 code attributes + assertThat(log.getAttributes().size()).isEqualTo(2 + 3) // 3 code attributes assertThat(log.getAttributes().get(SemanticAttributes.EXCEPTION_TYPE)).isNull() assertThat(log.getAttributes().get(SemanticAttributes.EXCEPTION_MESSAGE)).isNull() assertThat(log.getAttributes().get(SemanticAttributes.EXCEPTION_STACKTRACE)).isNull() From dba1c75117dbe24cd7faed3453edeb0bc5d7f987 Mon Sep 17 00:00:00 2001 From: Jean Bisutti Date: Tue, 13 Sep 2022 08:04:51 -0700 Subject: [PATCH 10/12] Adapt test for Java 18 --- .../javaagent/src/test/groovy/LogbackTest.groovy | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/instrumentation/logback/logback-appender-1.0/javaagent/src/test/groovy/LogbackTest.groovy b/instrumentation/logback/logback-appender-1.0/javaagent/src/test/groovy/LogbackTest.groovy index de7aa8b0ecfd..583b373250f6 100644 --- a/instrumentation/logback/logback-appender-1.0/javaagent/src/test/groovy/LogbackTest.groovy +++ b/instrumentation/logback/logback-appender-1.0/javaagent/src/test/groovy/LogbackTest.groovy @@ -44,6 +44,13 @@ class LogbackTest extends AgentInstrumentationSpecification { waitForTraces(1) } + String jvmVersion = System.getProperty("java.vm.specification.version"); + int codeAttributes = 3; + boolean jvmVersionGreaterThanOrEqualTo18 = jvmVersion.startsWith("1.8") && Integer.parseInt(jvmVersion) >= 18 + if(jvmVersionGreaterThanOrEqualTo18) { + codeAttributes = 4; // Java 18 specificity on line number + } + if (severity != null) { await() .untilAsserted( @@ -56,12 +63,12 @@ class LogbackTest extends AgentInstrumentationSpecification { assertThat(log.getSeverity()).isEqualTo(severity) assertThat(log.getSeverityText()).isEqualTo(severityText) if (exception) { - assertThat(log.getAttributes().size()).isEqualTo(5 + 3) // 3 code attributes + assertThat(log.getAttributes().size()).isEqualTo(5 + codeAttributes) assertThat(log.getAttributes().get(SemanticAttributes.EXCEPTION_TYPE)).isEqualTo(IllegalStateException.getName()) assertThat(log.getAttributes().get(SemanticAttributes.EXCEPTION_MESSAGE)).isEqualTo("hello") assertThat(log.getAttributes().get(SemanticAttributes.EXCEPTION_STACKTRACE)).contains(LogbackTest.name) } else { - assertThat(log.getAttributes().size()).isEqualTo(2 + 3) // 3 code attributes + assertThat(log.getAttributes().size()).isEqualTo(2 + codeAttributes) assertThat(log.getAttributes().get(SemanticAttributes.EXCEPTION_TYPE)).isNull() assertThat(log.getAttributes().get(SemanticAttributes.EXCEPTION_MESSAGE)).isNull() assertThat(log.getAttributes().get(SemanticAttributes.EXCEPTION_STACKTRACE)).isNull() From 33b65c52854dd285258c7ec52290ca335837a7fe Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Tue, 13 Sep 2022 08:21:22 -0700 Subject: [PATCH 11/12] codenarc --- .../javaagent/src/test/groovy/LogbackTest.groovy | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/instrumentation/logback/logback-appender-1.0/javaagent/src/test/groovy/LogbackTest.groovy b/instrumentation/logback/logback-appender-1.0/javaagent/src/test/groovy/LogbackTest.groovy index 583b373250f6..52f6d6687edb 100644 --- a/instrumentation/logback/logback-appender-1.0/javaagent/src/test/groovy/LogbackTest.groovy +++ b/instrumentation/logback/logback-appender-1.0/javaagent/src/test/groovy/LogbackTest.groovy @@ -44,11 +44,11 @@ class LogbackTest extends AgentInstrumentationSpecification { waitForTraces(1) } - String jvmVersion = System.getProperty("java.vm.specification.version"); - int codeAttributes = 3; + String jvmVersion = System.getProperty("java.vm.specification.version") + int codeAttributes = 3 boolean jvmVersionGreaterThanOrEqualTo18 = jvmVersion.startsWith("1.8") && Integer.parseInt(jvmVersion) >= 18 - if(jvmVersionGreaterThanOrEqualTo18) { - codeAttributes = 4; // Java 18 specificity on line number + if (jvmVersionGreaterThanOrEqualTo18) { + codeAttributes = 4 // Java 18 specificity on line number } if (severity != null) { From 4dd5c6bbb9529b62d0f8e13479adcc5b324805a7 Mon Sep 17 00:00:00 2001 From: Jean Bisutti Date: Tue, 13 Sep 2022 08:41:23 -0700 Subject: [PATCH 12/12] Test fix --- .../javaagent/src/test/groovy/LogbackTest.groovy | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/instrumentation/logback/logback-appender-1.0/javaagent/src/test/groovy/LogbackTest.groovy b/instrumentation/logback/logback-appender-1.0/javaagent/src/test/groovy/LogbackTest.groovy index 52f6d6687edb..8e7af56cb922 100644 --- a/instrumentation/logback/logback-appender-1.0/javaagent/src/test/groovy/LogbackTest.groovy +++ b/instrumentation/logback/logback-appender-1.0/javaagent/src/test/groovy/LogbackTest.groovy @@ -46,9 +46,9 @@ class LogbackTest extends AgentInstrumentationSpecification { String jvmVersion = System.getProperty("java.vm.specification.version") int codeAttributes = 3 - boolean jvmVersionGreaterThanOrEqualTo18 = jvmVersion.startsWith("1.8") && Integer.parseInt(jvmVersion) >= 18 + boolean jvmVersionGreaterThanOrEqualTo18 = !jvmVersion.startsWith("1.8") && Integer.parseInt(jvmVersion) >= 18 if (jvmVersionGreaterThanOrEqualTo18) { - codeAttributes = 4 // Java 18 specificity on line number + codeAttributes = 4 // Java 18 specificity on line number (lineNumber > 0 check) } if (severity != null) {