From b2ce969f43697f7fb53fac59003ee57f97360a62 Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Mon, 3 Apr 2023 20:42:35 +0300 Subject: [PATCH] address review comment --- .../logback/appender/v1_0/LogbackTest.java | 3 ++- .../appender/v1_0/internal/LoggingEventMapper.java | 13 +++++++------ .../logback/appender/v1_0/Slf4j2Test.java | 11 ++++------- .../v1_0/OpenTelemetryAppenderConfigTest.java | 6 ++++-- 4 files changed, 17 insertions(+), 16 deletions(-) diff --git a/instrumentation/logback/logback-appender-1.0/javaagent/src/test/java/io/opentelemetry/instrumentation/logback/appender/v1_0/LogbackTest.java b/instrumentation/logback/logback-appender-1.0/javaagent/src/test/java/io/opentelemetry/instrumentation/logback/appender/v1_0/LogbackTest.java index ff1dc7b9e1e6..fe077bd43fc6 100644 --- a/instrumentation/logback/logback-appender-1.0/javaagent/src/test/java/io/opentelemetry/instrumentation/logback/appender/v1_0/LogbackTest.java +++ b/instrumentation/logback/logback-appender-1.0/javaagent/src/test/java/io/opentelemetry/instrumentation/logback/appender/v1_0/LogbackTest.java @@ -16,6 +16,7 @@ import io.opentelemetry.sdk.common.InstrumentationScopeInfo; import io.opentelemetry.sdk.logs.data.LogRecordData; import io.opentelemetry.semconv.trace.attributes.SemanticAttributes; +import java.util.Arrays; import java.util.stream.Stream; import org.assertj.core.api.AbstractLongAssert; import org.junit.jupiter.api.Test; @@ -215,7 +216,7 @@ public void testMarker() { .hasAttributesSatisfyingExactly( equalTo(SemanticAttributes.THREAD_NAME, Thread.currentThread().getName()), equalTo(SemanticAttributes.THREAD_ID, Thread.currentThread().getId()), - equalTo(AttributeKey.stringKey("logback.marker"), markerName), + equalTo(AttributeKey.stringArrayKey("logback.marker"), Arrays.asList(markerName)), equalTo(SemanticAttributes.CODE_NAMESPACE, LogbackTest.class.getName()), equalTo(SemanticAttributes.CODE_FUNCTION, "testMarker"), satisfies(SemanticAttributes.CODE_LINENO, AbstractLongAssert::isPositive), 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 d6cb5cc21a52..cbeedec735bc 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 @@ -20,6 +20,7 @@ import io.opentelemetry.semconv.trace.attributes.SemanticAttributes; import java.io.PrintWriter; import java.io.StringWriter; +import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.concurrent.TimeUnit; @@ -36,7 +37,8 @@ public final class LoggingEventMapper { private static final boolean supportsMultipleMarkers = supportsMultipleMarkers(); private static final Cache> mdcAttributeKeys = Cache.bounded(100); - private static final AttributeKey LOG_MARKER = AttributeKey.stringKey("logback.marker"); + private static final AttributeKey> LOG_MARKER = + AttributeKey.stringArrayKey("logback.marker"); private final boolean captureExperimentalAttributes; private final List captureMdcAttributes; @@ -245,19 +247,18 @@ private static void captureSingleMarkerAttribute( AttributesBuilder attributes, ILoggingEvent loggingEvent) { Marker marker = loggingEvent.getMarker(); if (marker != null) { - String markerName = marker.getName(); - attributes.put(LOG_MARKER, markerName); + attributes.put(LOG_MARKER, marker.getName()); } } @NoMuzzle private static void captureMultipleMarkerAttributes( AttributesBuilder attributes, ILoggingEvent loggingEvent) { - int i = 1; + List markerNames = new ArrayList<>(loggingEvent.getMarkerList().size()); for (Marker marker : loggingEvent.getMarkerList()) { - String markerName = marker.getName(); - attributes.put(AttributeKey.stringKey("logback.marker." + i++), markerName); + markerNames.add(marker.getName()); } + attributes.put(LOG_MARKER, markerNames.toArray(new String[0])); } @NoMuzzle diff --git a/instrumentation/logback/logback-appender-1.0/library/src/slf4j2ApiTest/java/io/opentelemetry/instrumentation/logback/appender/v1_0/Slf4j2Test.java b/instrumentation/logback/logback-appender-1.0/library/src/slf4j2ApiTest/java/io/opentelemetry/instrumentation/logback/appender/v1_0/Slf4j2Test.java index a9e98a03e595..ca8f7d0d113e 100644 --- a/instrumentation/logback/logback-appender-1.0/library/src/slf4j2ApiTest/java/io/opentelemetry/instrumentation/logback/appender/v1_0/Slf4j2Test.java +++ b/instrumentation/logback/logback-appender-1.0/library/src/slf4j2ApiTest/java/io/opentelemetry/instrumentation/logback/appender/v1_0/Slf4j2Test.java @@ -15,6 +15,7 @@ import io.opentelemetry.sdk.logs.export.InMemoryLogRecordExporter; import io.opentelemetry.sdk.logs.export.SimpleLogRecordProcessor; import io.opentelemetry.sdk.resources.Resource; +import java.util.Arrays; import java.util.List; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; @@ -84,14 +85,10 @@ void multipleMarkers() { assertThat(logData.getResource()).isEqualTo(resource); assertThat(logData.getInstrumentationScopeInfo()).isEqualTo(instrumentationScopeInfo); assertThat(logData.getBody().asString()).isEqualTo("log message 1"); - assertThat(logData.getAttributes().size()).isEqualTo(6); // 4 code attributes + 2 markers + assertThat(logData.getAttributes().size()).isEqualTo(5); // 4 code attributes + 1 marker assertThat(logData.getAttributes()) .hasEntrySatisfying( - AttributeKey.stringKey("logback.marker.1"), - value -> assertThat(value).isEqualTo(markerName1)); - assertThat(logData.getAttributes()) - .hasEntrySatisfying( - AttributeKey.stringKey("logback.marker.2"), - value -> assertThat(value).isEqualTo(markerName2)); + AttributeKey.stringArrayKey("logback.marker"), + value -> assertThat(value).isEqualTo(Arrays.asList(markerName1, markerName2))); } } 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 f67b8f8b9d74..5d6566f73df4 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 @@ -22,6 +22,7 @@ import io.opentelemetry.sdk.trace.SdkTracerProvider; import io.opentelemetry.semconv.trace.attributes.SemanticAttributes; import java.time.Instant; +import java.util.Arrays; import java.util.List; import java.util.concurrent.TimeUnit; import org.junit.jupiter.api.BeforeAll; @@ -141,8 +142,9 @@ void logWithExtras() { Long lineNumber = logData.getAttributes().get(SemanticAttributes.CODE_LINENO); assertThat(lineNumber).isGreaterThan(1); - String logMarker = logData.getAttributes().get(AttributeKey.stringKey("logback.marker")); - assertThat(logMarker).isEqualTo(markerName); + List logMarker = + logData.getAttributes().get(AttributeKey.stringArrayKey("logback.marker")); + assertThat(logMarker).isEqualTo(Arrays.asList(markerName)); } @Test