From 0a639af5173419c13c116272fd7e61d88cfd30be Mon Sep 17 00:00:00 2001 From: Cuichen Li Date: Fri, 15 Apr 2022 22:07:04 +0800 Subject: [PATCH 1/4] add support for jboss-logmanager-mdc Signed-off-by: Cuichen Li --- .../javaagent/build.gradle.kts | 21 ++++ .../JbossLoggingEventInstrumentation.java | 114 ++++++++++++++++++ .../v1_1/JbossLogmanagerInstrumentation.java | 50 ++++++++ .../JbossLogmanagerInstrumentationModule.java | 26 ++++ .../test/groovy/JbossLogmanagerMdcTest.groovy | 93 ++++++++++++++ settings.gradle.kts | 1 + 6 files changed, 305 insertions(+) create mode 100644 instrumentation/jboss-logmanager-mdc-1.1/javaagent/build.gradle.kts create mode 100644 instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jbosslogmanager/v1_1/JbossLoggingEventInstrumentation.java create mode 100644 instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jbosslogmanager/v1_1/JbossLogmanagerInstrumentation.java create mode 100644 instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jbosslogmanager/v1_1/JbossLogmanagerInstrumentationModule.java create mode 100644 instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/test/groovy/JbossLogmanagerMdcTest.groovy diff --git a/instrumentation/jboss-logmanager-mdc-1.1/javaagent/build.gradle.kts b/instrumentation/jboss-logmanager-mdc-1.1/javaagent/build.gradle.kts new file mode 100644 index 000000000000..ae1066d7f159 --- /dev/null +++ b/instrumentation/jboss-logmanager-mdc-1.1/javaagent/build.gradle.kts @@ -0,0 +1,21 @@ +plugins { + id("otel.javaagent-instrumentation") +} + +muzzle { + pass { + group.set("org.jboss.logmanager") + module.set("jboss-logmanager") + versions.set("[1.1.0.GA,)") + assertInverse.set(true) + } +} + +dependencies { + library("org.jboss.logmanager:jboss-logmanager:1.1.0.GA") + + compileOnly(project(":instrumentation-appender-api-internal")) + + // ensure no cross interference + testInstrumentation(project(":instrumentation:java-util-logging:javaagent")) +} diff --git a/instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jbosslogmanager/v1_1/JbossLoggingEventInstrumentation.java b/instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jbosslogmanager/v1_1/JbossLoggingEventInstrumentation.java new file mode 100644 index 000000000000..fb77524267b0 --- /dev/null +++ b/instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jbosslogmanager/v1_1/JbossLoggingEventInstrumentation.java @@ -0,0 +1,114 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.jbosslogmanager.v1_1; + +import static io.opentelemetry.instrumentation.api.log.LoggingContextConstants.SPAN_ID; +import static io.opentelemetry.instrumentation.api.log.LoggingContextConstants.TRACE_FLAGS; +import static io.opentelemetry.instrumentation.api.log.LoggingContextConstants.TRACE_ID; +import static net.bytebuddy.matcher.ElementMatchers.isMethod; +import static net.bytebuddy.matcher.ElementMatchers.isPublic; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; +import static net.bytebuddy.matcher.ElementMatchers.takesArguments; + +import io.opentelemetry.api.trace.Span; +import io.opentelemetry.api.trace.SpanContext; +import io.opentelemetry.instrumentation.api.field.VirtualField; +import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; +import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; +import java.util.Hashtable; +import java.util.Map; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; +import org.jboss.logmanager.ExtLogRecord; +import org.jboss.logmanager.MDC; + +public class JbossLoggingEventInstrumentation implements TypeInstrumentation { + @Override + public ElementMatcher typeMatcher() { + return named("org.jboss.logmanager.ExtLogRecord"); + } + + @Override + public void transform(TypeTransformer transformer) { + transformer.applyAdviceToMethod( + isMethod() + .and(isPublic()) + .and(named("getMdc")) + .and(takesArguments(1)) + .and(takesArgument(0, String.class)), + JbossLoggingEventInstrumentation.class.getName() + "$GetMdcAdvice"); + + transformer.applyAdviceToMethod( + isMethod().and(isPublic()).and(named("getMdcCopy")).and(takesArguments(0)), + JbossLoggingEventInstrumentation.class.getName() + "$GetMdcCopyAdvice"); + } + + @SuppressWarnings("unused") + public static class GetMdcAdvice { + + @Advice.OnMethodExit(suppress = Throwable.class) + public static void onExit( + @Advice.This ExtLogRecord record, + @Advice.Argument(0) String key, + @Advice.Return(readOnly = false) String value) { + if (TRACE_ID.equals(key) || SPAN_ID.equals(key) || TRACE_FLAGS.equals(key)) { + if (value != null) { + // Assume already instrumented event if traceId/spanId/sampled is present. + return; + } + + Span span = VirtualField.find(ExtLogRecord.class, Span.class).get(record); + if (span == null || !span.getSpanContext().isValid()) { + return; + } + + SpanContext spanContext = span.getSpanContext(); + switch (key) { + case TRACE_ID: + value = spanContext.getTraceId(); + break; + case SPAN_ID: + value = spanContext.getSpanId(); + break; + case TRACE_FLAGS: + value = spanContext.getTraceFlags().asHex(); + break; + default: + // do nothing + } + } + } + } + + @SuppressWarnings("unused") + public static class GetMdcCopyAdvice { + + @SuppressWarnings({"unchecked", "rawtypes"}) + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void onEnter( + @Advice.This ExtLogRecord record, + @Advice.FieldValue(value = "mdcCopy", readOnly = false) Map mdcCopy) { + // this advice basically replaces the original method + + Hashtable mdc = new Hashtable(MDC.copy()); + + // Assume already instrumented event if traceId is present. + if (!mdc.containsKey(TRACE_ID)) { + Span span = VirtualField.find(ExtLogRecord.class, Span.class).get(record); + if (span != null && span.getSpanContext().isValid()) { + SpanContext spanContext = span.getSpanContext(); + mdc.put(TRACE_ID, spanContext.getTraceId()); + mdc.put(SPAN_ID, spanContext.getSpanId()); + mdc.put(TRACE_FLAGS, spanContext.getTraceFlags().asHex()); + } + } + + mdcCopy = mdc; + } + } +} diff --git a/instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jbosslogmanager/v1_1/JbossLogmanagerInstrumentation.java b/instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jbosslogmanager/v1_1/JbossLogmanagerInstrumentation.java new file mode 100644 index 000000000000..337406493ec4 --- /dev/null +++ b/instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jbosslogmanager/v1_1/JbossLogmanagerInstrumentation.java @@ -0,0 +1,50 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.jbosslogmanager.v1_1; + +import static net.bytebuddy.matcher.ElementMatchers.isMethod; +import static net.bytebuddy.matcher.ElementMatchers.isPublic; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; +import static net.bytebuddy.matcher.ElementMatchers.takesArguments; + +import io.opentelemetry.api.trace.Span; +import io.opentelemetry.instrumentation.api.field.VirtualField; +import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; +import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; +import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; +import org.jboss.logmanager.ExtLogRecord; + +public class JbossLogmanagerInstrumentation implements TypeInstrumentation { + @Override + public ElementMatcher typeMatcher() { + return named("org.jboss.logmanager.Logger"); + } + + @Override + public void transform(TypeTransformer transformer) { + transformer.applyAdviceToMethod( + isMethod() + .and(isPublic()) + .and(named("logRaw")) + .and(takesArguments(1)) + .and(takesArgument(0, named("org.jboss.logmanager.ExtLogRecord"))), + JbossLogmanagerInstrumentation.class.getName() + "$CallAppendersAdvice"); + } + + @SuppressWarnings("unused") + public static class CallAppendersAdvice { + + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void onEnter(@Advice.Argument(0) ExtLogRecord record) { + VirtualField.find(ExtLogRecord.class, Span.class) + .set(record, Java8BytecodeBridge.currentSpan()); + } + } +} diff --git a/instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jbosslogmanager/v1_1/JbossLogmanagerInstrumentationModule.java b/instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jbosslogmanager/v1_1/JbossLogmanagerInstrumentationModule.java new file mode 100644 index 000000000000..da3d2038edcb --- /dev/null +++ b/instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jbosslogmanager/v1_1/JbossLogmanagerInstrumentationModule.java @@ -0,0 +1,26 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.jbosslogmanager.v1_1; + +import static java.util.Arrays.asList; + +import com.google.auto.service.AutoService; +import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule; +import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; +import java.util.List; + +@AutoService(InstrumentationModule.class) +public class JbossLogmanagerInstrumentationModule extends InstrumentationModule { + + public JbossLogmanagerInstrumentationModule() { + super("jboss-logmanager-mdc", "jboss-logmanager-mdc-1.1"); + } + + @Override + public List typeInstrumentations() { + return asList(new JbossLogmanagerInstrumentation(), new JbossLoggingEventInstrumentation()); + } +} diff --git a/instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/test/groovy/JbossLogmanagerMdcTest.groovy b/instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/test/groovy/JbossLogmanagerMdcTest.groovy new file mode 100644 index 000000000000..297de7d4e426 --- /dev/null +++ b/instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/test/groovy/JbossLogmanagerMdcTest.groovy @@ -0,0 +1,93 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +import io.opentelemetry.api.trace.Span +import io.opentelemetry.instrumentation.test.AgentInstrumentationSpecification +import org.jboss.logmanager.ExtLogRecord +import org.jboss.logmanager.Level +import org.jboss.logmanager.LogContext + +import java.util.logging.Handler +import java.util.logging.LogRecord + +class JbossLogmanagerMdcTest extends AgentInstrumentationSpecification { + class LogHandler extends Handler { + public LinkedList logRecords + + LogHandler(LinkedList logRecords) { + this.logRecords = logRecords + } + @Override + void publish(LogRecord record) { + logRecords.push(record as ExtLogRecord) + } + + @Override + void flush() { + + } + + @Override + void close() throws SecurityException { + } + } + def "no ids when no span"() { + given: + LinkedList logRecords = [] + def logger = LogContext.getLogContext().getLogger('TestLogger') + logger.setLevel(Level.INFO) + logger.addHandler(new LogHandler(logRecords)) + + when: + logger.info("log message 1") + + then: + logRecords.size() == 1 + logRecords.get(0).message == "log message 1" + logRecords.get(0).getMdc("trace_id") == null + logRecords.get(0).getMdc("span_id") == null + logRecords.get(0).getMdc("trace_flags") == null + } + + def "ids when span"() { + given: + def logger = LogContext.getLogContext().getLogger('TestLogger') + logger.setLevel(Level.DEBUG) + LinkedList logRecords = [] + logger.addHandler(new LogHandler(logRecords)) + when: + def span1 = runWithSpan("test") { + logger.info("log message 1") + Span.current() + } + + logger.info("log message 2") + + def span2 = runWithSpan("test 2") { + logger.info("log message 3") + Span.current() + } + + then: + + logRecords.size() == 3 + logRecords.get(2).message == "log message 1" + logRecords.get(2).getMdc("trace_id") == span1.spanContext.traceId + logRecords.get(2).getMdc("span_id") == span1.spanContext.spanId + logRecords.get(2).getMdc("trace_flags") == "01" + + logRecords.get(1).message == "log message 2" + logRecords.get(1).getMdc("trace_id") == null + logRecords.get(1).getMdc("span_id") == null + logRecords.get(1).getMdc("trace_flags") == null + + logRecords.get(0).message == "log message 3" + // this explicit getMdcCopy() call here is to make sure that whole instrumentation is tested + logRecords.get(0).copyMdc() + logRecords.get(0).getMdc("trace_id") == span2.spanContext.traceId + logRecords.get(0).getMdc("span_id") == span2.spanContext.spanId + logRecords.get(0).getMdc("trace_flags") == "01" + } +} diff --git a/settings.gradle.kts b/settings.gradle.kts index 8f83ff87daf6..1c81c7804242 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -253,6 +253,7 @@ include(":instrumentation:jaxws:jaxws-2.0-wildfly-testing") include(":instrumentation:jaxws:jaxws-common:library") include(":instrumentation:jaxws:jaxws-jws-api-1.1:javaagent") include(":instrumentation:jboss-logmanager-1.1:javaagent") +include(":instrumentation:jboss-logmanager-mdc-1.1:javaagent") include(":instrumentation:jdbc:javaagent") include(":instrumentation:jdbc:library") include(":instrumentation:jdbc:testing") From 8840108cae1e5b2a08f4fdb246988f458ded0753 Mon Sep 17 00:00:00 2001 From: Cuichen Li Date: Mon, 18 Apr 2022 11:13:44 +0800 Subject: [PATCH 2/4] update based on feedback Signed-off-by: Cuichen Li --- .../javaagent/build.gradle.kts | 3 --- ... => JbossExtLogRecordInstrumentation.java} | 17 ++++++++-------- ...n.java => JbossLoggerInstrumentation.java} | 10 +++++----- .../JbossLogmanagerInstrumentationModule.java | 2 +- .../test/groovy/JbossLogmanagerMdcTest.groovy | 20 ++++++++++++------- 5 files changed, 27 insertions(+), 25 deletions(-) rename instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jbosslogmanager/v1_1/{JbossLoggingEventInstrumentation.java => JbossExtLogRecordInstrumentation.java} (86%) rename instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jbosslogmanager/v1_1/{JbossLogmanagerInstrumentation.java => JbossLoggerInstrumentation.java} (83%) diff --git a/instrumentation/jboss-logmanager-mdc-1.1/javaagent/build.gradle.kts b/instrumentation/jboss-logmanager-mdc-1.1/javaagent/build.gradle.kts index ae1066d7f159..07524d3a1579 100644 --- a/instrumentation/jboss-logmanager-mdc-1.1/javaagent/build.gradle.kts +++ b/instrumentation/jboss-logmanager-mdc-1.1/javaagent/build.gradle.kts @@ -15,7 +15,4 @@ dependencies { library("org.jboss.logmanager:jboss-logmanager:1.1.0.GA") compileOnly(project(":instrumentation-appender-api-internal")) - - // ensure no cross interference - testInstrumentation(project(":instrumentation:java-util-logging:javaagent")) } diff --git a/instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jbosslogmanager/v1_1/JbossLoggingEventInstrumentation.java b/instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jbosslogmanager/v1_1/JbossExtLogRecordInstrumentation.java similarity index 86% rename from instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jbosslogmanager/v1_1/JbossLoggingEventInstrumentation.java rename to instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jbosslogmanager/v1_1/JbossExtLogRecordInstrumentation.java index fb77524267b0..2cd4d9286b77 100644 --- a/instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jbosslogmanager/v1_1/JbossLoggingEventInstrumentation.java +++ b/instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jbosslogmanager/v1_1/JbossExtLogRecordInstrumentation.java @@ -16,9 +16,11 @@ import io.opentelemetry.api.trace.Span; import io.opentelemetry.api.trace.SpanContext; +import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.field.VirtualField; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; +import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge; import java.util.Hashtable; import java.util.Map; import net.bytebuddy.asm.Advice; @@ -27,7 +29,7 @@ import org.jboss.logmanager.ExtLogRecord; import org.jboss.logmanager.MDC; -public class JbossLoggingEventInstrumentation implements TypeInstrumentation { +public class JbossExtLogRecordInstrumentation implements TypeInstrumentation { @Override public ElementMatcher typeMatcher() { return named("org.jboss.logmanager.ExtLogRecord"); @@ -41,11 +43,11 @@ public void transform(TypeTransformer transformer) { .and(named("getMdc")) .and(takesArguments(1)) .and(takesArgument(0, String.class)), - JbossLoggingEventInstrumentation.class.getName() + "$GetMdcAdvice"); + JbossExtLogRecordInstrumentation.class.getName() + "$GetMdcAdvice"); transformer.applyAdviceToMethod( - isMethod().and(isPublic()).and(named("getMdcCopy")).and(takesArguments(0)), - JbossLoggingEventInstrumentation.class.getName() + "$GetMdcCopyAdvice"); + isMethod().and(isPublic()).and(named("copyMdc")).and(takesArguments(0)), + JbossExtLogRecordInstrumentation.class.getName() + "$GetMdcCopyAdvice"); } @SuppressWarnings("unused") @@ -62,12 +64,9 @@ public static void onExit( return; } - Span span = VirtualField.find(ExtLogRecord.class, Span.class).get(record); - if (span == null || !span.getSpanContext().isValid()) { - return; - } + Context context = VirtualField.find(ExtLogRecord.class, Context.class).get(record); + SpanContext spanContext = Java8BytecodeBridge.spanFromContext(context).getSpanContext(); - SpanContext spanContext = span.getSpanContext(); switch (key) { case TRACE_ID: value = spanContext.getTraceId(); diff --git a/instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jbosslogmanager/v1_1/JbossLogmanagerInstrumentation.java b/instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jbosslogmanager/v1_1/JbossLoggerInstrumentation.java similarity index 83% rename from instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jbosslogmanager/v1_1/JbossLogmanagerInstrumentation.java rename to instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jbosslogmanager/v1_1/JbossLoggerInstrumentation.java index 337406493ec4..5b5f1e6f4a27 100644 --- a/instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jbosslogmanager/v1_1/JbossLogmanagerInstrumentation.java +++ b/instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jbosslogmanager/v1_1/JbossLoggerInstrumentation.java @@ -11,7 +11,7 @@ import static net.bytebuddy.matcher.ElementMatchers.takesArgument; import static net.bytebuddy.matcher.ElementMatchers.takesArguments; -import io.opentelemetry.api.trace.Span; +import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.field.VirtualField; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; @@ -21,7 +21,7 @@ import net.bytebuddy.matcher.ElementMatcher; import org.jboss.logmanager.ExtLogRecord; -public class JbossLogmanagerInstrumentation implements TypeInstrumentation { +public class JbossLoggerInstrumentation implements TypeInstrumentation { @Override public ElementMatcher typeMatcher() { return named("org.jboss.logmanager.Logger"); @@ -35,7 +35,7 @@ public void transform(TypeTransformer transformer) { .and(named("logRaw")) .and(takesArguments(1)) .and(takesArgument(0, named("org.jboss.logmanager.ExtLogRecord"))), - JbossLogmanagerInstrumentation.class.getName() + "$CallAppendersAdvice"); + JbossLoggerInstrumentation.class.getName() + "$CallAppendersAdvice"); } @SuppressWarnings("unused") @@ -43,8 +43,8 @@ public static class CallAppendersAdvice { @Advice.OnMethodEnter(suppress = Throwable.class) public static void onEnter(@Advice.Argument(0) ExtLogRecord record) { - VirtualField.find(ExtLogRecord.class, Span.class) - .set(record, Java8BytecodeBridge.currentSpan()); + VirtualField.find(ExtLogRecord.class, Context.class) + .set(record, Java8BytecodeBridge.currentContext()); } } } diff --git a/instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jbosslogmanager/v1_1/JbossLogmanagerInstrumentationModule.java b/instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jbosslogmanager/v1_1/JbossLogmanagerInstrumentationModule.java index da3d2038edcb..f4215aee1e78 100644 --- a/instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jbosslogmanager/v1_1/JbossLogmanagerInstrumentationModule.java +++ b/instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jbosslogmanager/v1_1/JbossLogmanagerInstrumentationModule.java @@ -21,6 +21,6 @@ public JbossLogmanagerInstrumentationModule() { @Override public List typeInstrumentations() { - return asList(new JbossLogmanagerInstrumentation(), new JbossLoggingEventInstrumentation()); + return asList(new JbossLoggerInstrumentation(), new JbossExtLogRecordInstrumentation()); } } diff --git a/instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/test/groovy/JbossLogmanagerMdcTest.groovy b/instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/test/groovy/JbossLogmanagerMdcTest.groovy index 297de7d4e426..9b13935233f2 100644 --- a/instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/test/groovy/JbossLogmanagerMdcTest.groovy +++ b/instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/test/groovy/JbossLogmanagerMdcTest.groovy @@ -14,7 +14,7 @@ import java.util.logging.LogRecord class JbossLogmanagerMdcTest extends AgentInstrumentationSpecification { class LogHandler extends Handler { - public LinkedList logRecords + public List logRecords LogHandler(LinkedList logRecords) { this.logRecords = logRecords @@ -46,9 +46,12 @@ class JbossLogmanagerMdcTest extends AgentInstrumentationSpecification { then: logRecords.size() == 1 logRecords.get(0).message == "log message 1" - logRecords.get(0).getMdc("trace_id") == null - logRecords.get(0).getMdc("span_id") == null - logRecords.get(0).getMdc("trace_flags") == null + logRecords.get(0).getMdc("trace_id") == "00000000000000000000000000000000" + logRecords.get(0).getMdc("span_id") == "0000000000000000" + logRecords.get(0).getMdc("trace_flags") == "00" + + cleanup: + logRecords.clear() } def "ids when span"() { @@ -79,9 +82,9 @@ class JbossLogmanagerMdcTest extends AgentInstrumentationSpecification { logRecords.get(2).getMdc("trace_flags") == "01" logRecords.get(1).message == "log message 2" - logRecords.get(1).getMdc("trace_id") == null - logRecords.get(1).getMdc("span_id") == null - logRecords.get(1).getMdc("trace_flags") == null + logRecords.get(1).getMdc("trace_id") == "00000000000000000000000000000000" + logRecords.get(1).getMdc("span_id") == "0000000000000000" + logRecords.get(1).getMdc("trace_flags") == "00" logRecords.get(0).message == "log message 3" // this explicit getMdcCopy() call here is to make sure that whole instrumentation is tested @@ -89,5 +92,8 @@ class JbossLogmanagerMdcTest extends AgentInstrumentationSpecification { logRecords.get(0).getMdc("trace_id") == span2.spanContext.traceId logRecords.get(0).getMdc("span_id") == span2.spanContext.spanId logRecords.get(0).getMdc("trace_flags") == "01" + + cleanup: + logRecords.clear() } } From 73f3e64ab7bcaebf1cc6326e2434de344b321b67 Mon Sep 17 00:00:00 2001 From: Cuichen Li Date: Tue, 19 Apr 2022 14:59:29 +0800 Subject: [PATCH 3/4] update --- .../javaagent/build.gradle.kts | 2 - .../JbossExtLogRecordInstrumentation.java | 38 ++----------------- .../test/groovy/JbossLogmanagerMdcTest.groovy | 2 - 3 files changed, 3 insertions(+), 39 deletions(-) diff --git a/instrumentation/jboss-logmanager-mdc-1.1/javaagent/build.gradle.kts b/instrumentation/jboss-logmanager-mdc-1.1/javaagent/build.gradle.kts index 07524d3a1579..41716f6e7dae 100644 --- a/instrumentation/jboss-logmanager-mdc-1.1/javaagent/build.gradle.kts +++ b/instrumentation/jboss-logmanager-mdc-1.1/javaagent/build.gradle.kts @@ -13,6 +13,4 @@ muzzle { dependencies { library("org.jboss.logmanager:jboss-logmanager:1.1.0.GA") - - compileOnly(project(":instrumentation-appender-api-internal")) } diff --git a/instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jbosslogmanager/v1_1/JbossExtLogRecordInstrumentation.java b/instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jbosslogmanager/v1_1/JbossExtLogRecordInstrumentation.java index 2cd4d9286b77..94e430e59fdd 100644 --- a/instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jbosslogmanager/v1_1/JbossExtLogRecordInstrumentation.java +++ b/instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jbosslogmanager/v1_1/JbossExtLogRecordInstrumentation.java @@ -14,20 +14,16 @@ import static net.bytebuddy.matcher.ElementMatchers.takesArgument; import static net.bytebuddy.matcher.ElementMatchers.takesArguments; -import io.opentelemetry.api.trace.Span; import io.opentelemetry.api.trace.SpanContext; import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.field.VirtualField; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge; -import java.util.Hashtable; -import java.util.Map; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; import org.jboss.logmanager.ExtLogRecord; -import org.jboss.logmanager.MDC; public class JbossExtLogRecordInstrumentation implements TypeInstrumentation { @Override @@ -44,10 +40,6 @@ public void transform(TypeTransformer transformer) { .and(takesArguments(1)) .and(takesArgument(0, String.class)), JbossExtLogRecordInstrumentation.class.getName() + "$GetMdcAdvice"); - - transformer.applyAdviceToMethod( - isMethod().and(isPublic()).and(named("copyMdc")).and(takesArguments(0)), - JbossExtLogRecordInstrumentation.class.getName() + "$GetMdcCopyAdvice"); } @SuppressWarnings("unused") @@ -65,6 +57,9 @@ public static void onExit( } Context context = VirtualField.find(ExtLogRecord.class, Context.class).get(record); + if (context == null) { + return; + } SpanContext spanContext = Java8BytecodeBridge.spanFromContext(context).getSpanContext(); switch (key) { @@ -83,31 +78,4 @@ public static void onExit( } } } - - @SuppressWarnings("unused") - public static class GetMdcCopyAdvice { - - @SuppressWarnings({"unchecked", "rawtypes"}) - @Advice.OnMethodEnter(suppress = Throwable.class) - public static void onEnter( - @Advice.This ExtLogRecord record, - @Advice.FieldValue(value = "mdcCopy", readOnly = false) Map mdcCopy) { - // this advice basically replaces the original method - - Hashtable mdc = new Hashtable(MDC.copy()); - - // Assume already instrumented event if traceId is present. - if (!mdc.containsKey(TRACE_ID)) { - Span span = VirtualField.find(ExtLogRecord.class, Span.class).get(record); - if (span != null && span.getSpanContext().isValid()) { - SpanContext spanContext = span.getSpanContext(); - mdc.put(TRACE_ID, spanContext.getTraceId()); - mdc.put(SPAN_ID, spanContext.getSpanId()); - mdc.put(TRACE_FLAGS, spanContext.getTraceFlags().asHex()); - } - } - - mdcCopy = mdc; - } - } } diff --git a/instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/test/groovy/JbossLogmanagerMdcTest.groovy b/instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/test/groovy/JbossLogmanagerMdcTest.groovy index 9b13935233f2..2e60cb8be03b 100644 --- a/instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/test/groovy/JbossLogmanagerMdcTest.groovy +++ b/instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/test/groovy/JbossLogmanagerMdcTest.groovy @@ -87,8 +87,6 @@ class JbossLogmanagerMdcTest extends AgentInstrumentationSpecification { logRecords.get(1).getMdc("trace_flags") == "00" logRecords.get(0).message == "log message 3" - // this explicit getMdcCopy() call here is to make sure that whole instrumentation is tested - logRecords.get(0).copyMdc() logRecords.get(0).getMdc("trace_id") == span2.spanContext.traceId logRecords.get(0).getMdc("span_id") == span2.spanContext.spanId logRecords.get(0).getMdc("trace_flags") == "01" From 98b61190d0b87b7d25fc12db10399fc65c755d2d Mon Sep 17 00:00:00 2001 From: Cuichen Li Date: Tue, 19 Apr 2022 21:09:20 +0800 Subject: [PATCH 4/4] check spanContext is valid --- .../v1_1/JbossExtLogRecordInstrumentation.java | 3 +++ .../src/test/groovy/JbossLogmanagerMdcTest.groovy | 12 ++++++------ 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jbosslogmanager/v1_1/JbossExtLogRecordInstrumentation.java b/instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jbosslogmanager/v1_1/JbossExtLogRecordInstrumentation.java index 94e430e59fdd..2d306b6ae90f 100644 --- a/instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jbosslogmanager/v1_1/JbossExtLogRecordInstrumentation.java +++ b/instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jbosslogmanager/v1_1/JbossExtLogRecordInstrumentation.java @@ -61,6 +61,9 @@ public static void onExit( return; } SpanContext spanContext = Java8BytecodeBridge.spanFromContext(context).getSpanContext(); + if (!spanContext.isValid()) { + return; + } switch (key) { case TRACE_ID: diff --git a/instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/test/groovy/JbossLogmanagerMdcTest.groovy b/instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/test/groovy/JbossLogmanagerMdcTest.groovy index 2e60cb8be03b..00591292beba 100644 --- a/instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/test/groovy/JbossLogmanagerMdcTest.groovy +++ b/instrumentation/jboss-logmanager-mdc-1.1/javaagent/src/test/groovy/JbossLogmanagerMdcTest.groovy @@ -46,9 +46,9 @@ class JbossLogmanagerMdcTest extends AgentInstrumentationSpecification { then: logRecords.size() == 1 logRecords.get(0).message == "log message 1" - logRecords.get(0).getMdc("trace_id") == "00000000000000000000000000000000" - logRecords.get(0).getMdc("span_id") == "0000000000000000" - logRecords.get(0).getMdc("trace_flags") == "00" + logRecords.get(0).getMdc("trace_id") == null + logRecords.get(0).getMdc("span_id") == null + logRecords.get(0).getMdc("trace_flags") == null cleanup: logRecords.clear() @@ -82,9 +82,9 @@ class JbossLogmanagerMdcTest extends AgentInstrumentationSpecification { logRecords.get(2).getMdc("trace_flags") == "01" logRecords.get(1).message == "log message 2" - logRecords.get(1).getMdc("trace_id") == "00000000000000000000000000000000" - logRecords.get(1).getMdc("span_id") == "0000000000000000" - logRecords.get(1).getMdc("trace_flags") == "00" + logRecords.get(1).getMdc("trace_id") == null + logRecords.get(1).getMdc("span_id") == null + logRecords.get(1).getMdc("trace_flags") == null logRecords.get(0).message == "log message 3" logRecords.get(0).getMdc("trace_id") == span2.spanContext.traceId