From f91dede5163e2c50f0281c663a614b9966d44a85 Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Tue, 31 Oct 2023 13:46:11 +0200 Subject: [PATCH 1/2] Add tests for resource attributes in mdc --- .../javaagent/build.gradle.kts | 1 + .../src/test/groovy/AutoLog4j2Test.groovy | 20 +++++++++++++ .../OpenTelemetryContextDataProvider.java | 17 +++++++---- .../javaagent/build.gradle.kts | 1 + .../SpanDecoratingContextDataInjector.java | 28 +++++++++++++------ .../src/test/groovy/Log4j27Test.groovy | 20 +++++++++++++ .../log4j-mdc-1.2/javaagent/build.gradle.kts | 6 ++++ .../mdc/v1_2/LoggingEventInstrumentation.java | 6 ++-- .../log4j/mdc/v1_2/Log4j1MdcTest.java | 15 ++++++++++ .../javaagent/build.gradle.kts | 4 +++ .../mdc/v1_0/LoggingEventInstrumentation.java | 3 +- .../logback/v1_0/LogbackTest.java | 21 ++++++++++++++ 12 files changed, 123 insertions(+), 19 deletions(-) diff --git a/instrumentation/log4j/log4j-context-data/log4j-context-data-2.17/javaagent/build.gradle.kts b/instrumentation/log4j/log4j-context-data/log4j-context-data-2.17/javaagent/build.gradle.kts index 754b33b63db2..28aa63c81196 100644 --- a/instrumentation/log4j/log4j-context-data/log4j-context-data-2.17/javaagent/build.gradle.kts +++ b/instrumentation/log4j/log4j-context-data/log4j-context-data-2.17/javaagent/build.gradle.kts @@ -74,6 +74,7 @@ tasks { test { jvmArgs("-Dlog4j2.is.webapp=false") jvmArgs("-Dlog4j2.enable.threadlocals=true") + jvmArgs("-Dotel.instrumentation.mdc.resource-attributes=service.name,telemetry.sdk.language") } named("check") { diff --git a/instrumentation/log4j/log4j-context-data/log4j-context-data-2.17/javaagent/src/test/groovy/AutoLog4j2Test.groovy b/instrumentation/log4j/log4j-context-data/log4j-context-data-2.17/javaagent/src/test/groovy/AutoLog4j2Test.groovy index 260ff5915f07..b75ff250e91e 100644 --- a/instrumentation/log4j/log4j-context-data/log4j-context-data-2.17/javaagent/src/test/groovy/AutoLog4j2Test.groovy +++ b/instrumentation/log4j/log4j-context-data/log4j-context-data-2.17/javaagent/src/test/groovy/AutoLog4j2Test.groovy @@ -3,7 +3,27 @@ * SPDX-License-Identifier: Apache-2.0 */ +import io.opentelemetry.instrumentation.log4j.contextdata.ListAppender import io.opentelemetry.instrumentation.test.AgentTestTrait +import org.apache.logging.log4j.LogManager class AutoLog4j2Test extends Log4j2Test implements AgentTestTrait { + + def "resource attributes"() { + given: + def logger = LogManager.getLogger("TestLogger") + + when: + logger.info("log message 1") + + def events = ListAppender.get().getEvents() + + then: + events.size() == 1 + events[0].message == "log message 1" + events[0].contextData["trace_id"] == null + events[0].contextData["span_id"] == null + events[0].contextData["service.name"] == "unknown_service:java" + events[0].contextData["telemetry.sdk.language"] == "java" + } } diff --git a/instrumentation/log4j/log4j-context-data/log4j-context-data-2.17/library-autoconfigure/src/main/java/io/opentelemetry/instrumentation/log4j/contextdata/v2_17/OpenTelemetryContextDataProvider.java b/instrumentation/log4j/log4j-context-data/log4j-context-data-2.17/library-autoconfigure/src/main/java/io/opentelemetry/instrumentation/log4j/contextdata/v2_17/OpenTelemetryContextDataProvider.java index e28eab7fe608..8941bfbe84b0 100644 --- a/instrumentation/log4j/log4j-context-data/log4j-context-data-2.17/library-autoconfigure/src/main/java/io/opentelemetry/instrumentation/log4j/contextdata/v2_17/OpenTelemetryContextDataProvider.java +++ b/instrumentation/log4j/log4j-context-data/log4j-context-data-2.17/library-autoconfigure/src/main/java/io/opentelemetry/instrumentation/log4j/contextdata/v2_17/OpenTelemetryContextDataProvider.java @@ -33,6 +33,15 @@ public class OpenTelemetryContextDataProvider implements ContextDataProvider { private static final boolean configuredResourceAttributeAccessible = isConfiguredResourceAttributeAccessible(); + private static final Map staticContextData = getStaticContextData(); + + private static Map getStaticContextData() { + if (configuredResourceAttributeAccessible) { + return ConfiguredResourceAttributesHolder.getResourceAttributes(); + } + return Collections.emptyMap(); + } + /** * Checks whether {@link ConfiguredResourceAttributesHolder} is available in classpath. The result * is true if {@link ConfiguredResourceAttributesHolder} can be loaded, false otherwise. @@ -61,19 +70,17 @@ public Map supplyContextData() { Context context = Context.current(); Span currentSpan = Span.fromContext(context); if (!currentSpan.getSpanContext().isValid()) { - return Collections.emptyMap(); + return staticContextData; } Map contextData = new HashMap<>(); + contextData.putAll(staticContextData); + SpanContext spanContext = currentSpan.getSpanContext(); contextData.put(TRACE_ID, spanContext.getTraceId()); contextData.put(SPAN_ID, spanContext.getSpanId()); contextData.put(TRACE_FLAGS, spanContext.getTraceFlags().asHex()); - if (configuredResourceAttributeAccessible) { - contextData.putAll(ConfiguredResourceAttributesHolder.getResourceAttributes()); - } - if (BAGGAGE_ENABLED) { Baggage baggage = Baggage.fromContext(context); for (Map.Entry entry : baggage.asMap().entrySet()) { diff --git a/instrumentation/log4j/log4j-context-data/log4j-context-data-2.7/javaagent/build.gradle.kts b/instrumentation/log4j/log4j-context-data/log4j-context-data-2.7/javaagent/build.gradle.kts index 05587ee92418..d29a28a596dd 100644 --- a/instrumentation/log4j/log4j-context-data/log4j-context-data-2.7/javaagent/build.gradle.kts +++ b/instrumentation/log4j/log4j-context-data/log4j-context-data-2.7/javaagent/build.gradle.kts @@ -26,6 +26,7 @@ tasks { filter { excludeTestsMatching("Log4j27BaggageTest") } + jvmArgs("-Dotel.instrumentation.mdc.resource-attributes=service.name,telemetry.sdk.language") } val testAddBaggage by registering(Test::class) { diff --git a/instrumentation/log4j/log4j-context-data/log4j-context-data-2.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/log4j/contextdata/v2_7/SpanDecoratingContextDataInjector.java b/instrumentation/log4j/log4j-context-data/log4j-context-data-2.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/log4j/contextdata/v2_7/SpanDecoratingContextDataInjector.java index 1cd131257e69..7e660c557241 100644 --- a/instrumentation/log4j/log4j-context-data/log4j-context-data-2.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/log4j/contextdata/v2_7/SpanDecoratingContextDataInjector.java +++ b/instrumentation/log4j/log4j-context-data/log4j-context-data-2.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/log4j/contextdata/v2_7/SpanDecoratingContextDataInjector.java @@ -29,6 +29,8 @@ public final class SpanDecoratingContextDataInjector implements ContextDataInjec InstrumentationConfig.get() .getBoolean("otel.instrumentation.log4j-context-data.add-baggage", false); + private static final StringMap staticContextData = getStaticContextData(); + private final ContextDataInjector delegate; public SpanDecoratingContextDataInjector(ContextDataInjector delegate) { @@ -41,26 +43,21 @@ public StringMap injectContextData(List list, StringMap stringMap) { if (contextData.containsKey(TRACE_ID)) { // Assume already instrumented event if traceId is present. - return contextData; + return staticContextData.isEmpty() ? contextData : newContextData(contextData); } Context context = Context.current(); Span span = Span.fromContext(context); SpanContext currentContext = span.getSpanContext(); if (!currentContext.isValid()) { - return contextData; + return staticContextData.isEmpty() ? contextData : newContextData(contextData); } - StringMap newContextData = new SortedArrayStringMap(contextData); + StringMap newContextData = newContextData(contextData); newContextData.putValue(TRACE_ID, currentContext.getTraceId()); newContextData.putValue(SPAN_ID, currentContext.getSpanId()); newContextData.putValue(TRACE_FLAGS, currentContext.getTraceFlags().asHex()); - for (Map.Entry entry : - ConfiguredResourceAttributesHolder.getResourceAttributes().entrySet()) { - newContextData.putValue(entry.getKey(), entry.getValue()); - } - if (BAGGAGE_ENABLED) { Baggage baggage = Baggage.fromContext(context); for (Map.Entry entry : baggage.asMap().entrySet()) { @@ -75,4 +72,19 @@ public StringMap injectContextData(List list, StringMap stringMap) { public ReadOnlyStringMap rawContextData() { return delegate.rawContextData(); } + + private static StringMap newContextData(StringMap contextData) { + StringMap newContextData = new SortedArrayStringMap(contextData); + newContextData.putAll(staticContextData); + return newContextData; + } + + private static StringMap getStaticContextData() { + StringMap map = new SortedArrayStringMap(); + for (Map.Entry entry : + ConfiguredResourceAttributesHolder.getResourceAttributes().entrySet()) { + map.putValue(entry.getKey(), entry.getValue()); + } + return map; + } } diff --git a/instrumentation/log4j/log4j-context-data/log4j-context-data-2.7/javaagent/src/test/groovy/Log4j27Test.groovy b/instrumentation/log4j/log4j-context-data/log4j-context-data-2.7/javaagent/src/test/groovy/Log4j27Test.groovy index 24e5b98f4cfe..b58519657017 100644 --- a/instrumentation/log4j/log4j-context-data/log4j-context-data-2.7/javaagent/src/test/groovy/Log4j27Test.groovy +++ b/instrumentation/log4j/log4j-context-data/log4j-context-data-2.7/javaagent/src/test/groovy/Log4j27Test.groovy @@ -3,7 +3,27 @@ * SPDX-License-Identifier: Apache-2.0 */ +import io.opentelemetry.instrumentation.log4j.contextdata.ListAppender import io.opentelemetry.instrumentation.test.AgentTestTrait +import org.apache.logging.log4j.LogManager class Log4j27Test extends Log4j2Test implements AgentTestTrait { + + def "resource attributes"() { + given: + def logger = LogManager.getLogger("TestLogger") + + when: + logger.info("log message 1") + + def events = ListAppender.get().getEvents() + + then: + events.size() == 1 + events[0].message == "log message 1" + events[0].contextData["trace_id"] == null + events[0].contextData["span_id"] == null + events[0].contextData["service.name"] == "unknown_service:java" + events[0].contextData["telemetry.sdk.language"] == "java" + } } diff --git a/instrumentation/log4j/log4j-mdc-1.2/javaagent/build.gradle.kts b/instrumentation/log4j/log4j-mdc-1.2/javaagent/build.gradle.kts index 9ede7f4fc5b1..856bd835dfd9 100644 --- a/instrumentation/log4j/log4j-mdc-1.2/javaagent/build.gradle.kts +++ b/instrumentation/log4j/log4j-mdc-1.2/javaagent/build.gradle.kts @@ -30,3 +30,9 @@ configurations { exclude("javax.jms", "jms") } } + +tasks { + test { + jvmArgs("-Dotel.instrumentation.mdc.resource-attributes=service.name,telemetry.sdk.language") + } +} diff --git a/instrumentation/log4j/log4j-mdc-1.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/log4j/mdc/v1_2/LoggingEventInstrumentation.java b/instrumentation/log4j/log4j-mdc-1.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/log4j/mdc/v1_2/LoggingEventInstrumentation.java index 31efdac1ec20..f3d87dec222b 100644 --- a/instrumentation/log4j/log4j-mdc-1.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/log4j/mdc/v1_2/LoggingEventInstrumentation.java +++ b/instrumentation/log4j/log4j-mdc-1.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/log4j/mdc/v1_2/LoggingEventInstrumentation.java @@ -80,10 +80,8 @@ public static void onExit( default: // do nothing } - } else if (ConfiguredResourceAttributesHolder.getAttributeValue(key) != null) { - if (value == null) { - value = ConfiguredResourceAttributesHolder.getAttributeValue(key); - } + } else if (value == null) { + value = ConfiguredResourceAttributesHolder.getAttributeValue(key); } } } diff --git a/instrumentation/log4j/log4j-mdc-1.2/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/log4j/mdc/v1_2/Log4j1MdcTest.java b/instrumentation/log4j/log4j-mdc-1.2/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/log4j/mdc/v1_2/Log4j1MdcTest.java index 5a3bcc190628..3661ac8ecbab 100644 --- a/instrumentation/log4j/log4j-mdc-1.2/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/log4j/mdc/v1_2/Log4j1MdcTest.java +++ b/instrumentation/log4j/log4j-mdc-1.2/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/log4j/mdc/v1_2/Log4j1MdcTest.java @@ -90,4 +90,19 @@ void idsWhenSpan() { assertEquals(events.get(2).getMDC("span_id"), span2.getSpanContext().getSpanId()); assertEquals(events.get(2).getMDC("trace_flags"), "01"); } + + @Test + void resourceAttributes() { + logger.info("log message 1"); + + List events = ListAppender.getEvents(); + + assertEquals(1, events.size()); + assertEquals("log message 1", events.get(0).getMessage()); + assertNull(events.get(0).getMDC("trace_id")); + assertNull(events.get(0).getMDC("span_id")); + assertNull(events.get(0).getMDC("trace_flags")); + assertEquals("unknown_service:java", events.get(0).getMDC("service.name")); + assertEquals("java", events.get(0).getMDC("telemetry.sdk.language")); + } } diff --git a/instrumentation/logback/logback-mdc-1.0/javaagent/build.gradle.kts b/instrumentation/logback/logback-mdc-1.0/javaagent/build.gradle.kts index 9d616d079f9a..8ebb5d8d9c8a 100644 --- a/instrumentation/logback/logback-mdc-1.0/javaagent/build.gradle.kts +++ b/instrumentation/logback/logback-mdc-1.0/javaagent/build.gradle.kts @@ -63,6 +63,10 @@ dependencies { } tasks { + test { + jvmArgs("-Dotel.instrumentation.mdc.resource-attributes=service.name,telemetry.sdk.language") + } + named("check") { dependsOn(testing.suites) } diff --git a/instrumentation/logback/logback-mdc-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/logback/mdc/v1_0/LoggingEventInstrumentation.java b/instrumentation/logback/logback-mdc-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/logback/mdc/v1_0/LoggingEventInstrumentation.java index 5cb3bd5b1771..4f706cc062b0 100644 --- a/instrumentation/logback/logback-mdc-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/logback/mdc/v1_0/LoggingEventInstrumentation.java +++ b/instrumentation/logback/logback-mdc-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/logback/mdc/v1_0/LoggingEventInstrumentation.java @@ -80,9 +80,8 @@ public static void onExit( spanContextData.put(TRACE_ID, spanContext.getTraceId()); spanContextData.put(SPAN_ID, spanContext.getSpanId()); spanContextData.put(TRACE_FLAGS, spanContext.getTraceFlags().asHex()); - - spanContextData.putAll(ConfiguredResourceAttributesHolder.getResourceAttributes()); } + spanContextData.putAll(ConfiguredResourceAttributesHolder.getResourceAttributes()); if (LogbackSingletons.addBaggage()) { Baggage baggage = Java8BytecodeBridge.baggageFromContext(context); diff --git a/instrumentation/logback/logback-mdc-1.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/logback/v1_0/LogbackTest.java b/instrumentation/logback/logback-mdc-1.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/logback/v1_0/LogbackTest.java index f59a2bcf9ad0..a3a062bf50ca 100644 --- a/instrumentation/logback/logback-mdc-1.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/logback/v1_0/LogbackTest.java +++ b/instrumentation/logback/logback-mdc-1.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/logback/v1_0/LogbackTest.java @@ -5,9 +5,14 @@ package io.opentelemetry.javaagent.instrumentation.logback.v1_0; +import static org.assertj.core.api.Assertions.assertThat; + +import ch.qos.logback.classic.spi.ILoggingEvent; import io.opentelemetry.instrumentation.logback.mdc.v1_0.AbstractLogbackTest; import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension; import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; +import java.util.List; +import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; class LogbackTest extends AbstractLogbackTest { @@ -19,4 +24,20 @@ class LogbackTest extends AbstractLogbackTest { public InstrumentationExtension getInstrumentationExtension() { return agentTesting; } + + @Test + void resourceAttributes() { + logger.info("log message 1"); + + List events = listAppender.list; + + assertThat(events.size()).isEqualTo(1); + assertThat(events.get(0).getMessage()).isEqualTo("log message 1"); + assertThat(events.get(0).getMDCPropertyMap().get("trace_id")).isNull(); + assertThat(events.get(0).getMDCPropertyMap().get("span_id")).isNull(); + assertThat(events.get(0).getMDCPropertyMap().get("trace_flags")).isNull(); + assertThat(events.get(0).getMDCPropertyMap().get("service.name")) + .isEqualTo("unknown_service:java"); + assertThat(events.get(0).getMDCPropertyMap().get("telemetry.sdk.language")).isEqualTo("java"); + } } From 4a868f22f50d58842f6fd2464cdd4b3bf2cb9333 Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Thu, 2 Nov 2023 21:27:25 +0200 Subject: [PATCH 2/2] fix test --- .../log4j-context-data-2.17/javaagent/build.gradle.kts | 1 + 1 file changed, 1 insertion(+) diff --git a/instrumentation/log4j/log4j-context-data/log4j-context-data-2.17/javaagent/build.gradle.kts b/instrumentation/log4j/log4j-context-data/log4j-context-data-2.17/javaagent/build.gradle.kts index 28aa63c81196..e36b98e6cf70 100644 --- a/instrumentation/log4j/log4j-context-data/log4j-context-data-2.17/javaagent/build.gradle.kts +++ b/instrumentation/log4j/log4j-context-data/log4j-context-data-2.17/javaagent/build.gradle.kts @@ -40,6 +40,7 @@ testing { testTask.configure { jvmArgs("-Dlog4j2.is.webapp=false") jvmArgs("-Dlog4j2.enable.threadlocals=false") + jvmArgs("-Dotel.instrumentation.mdc.resource-attributes=service.name,telemetry.sdk.language") } } }