From 3d1e782fc5cff645adb2618a1fc31c3801b16d79 Mon Sep 17 00:00:00 2001 From: Mateusz Rzeszutek Date: Tue, 13 Jul 2021 09:34:02 +0200 Subject: [PATCH] Instrumentation-specific bootstrap classes (#3495) * Instrumentation-specific bootstrap classes * try to move bootstrap modules dependency to javaagent --- .../kotlin/otel.java-conventions.gradle.kts | 2 +- instrumentation/build.gradle.kts | 8 ++++ .../undertow-1.4/bootstrap/build.gradle.kts | 7 +++ .../bootstrap/undertow/KeyHolder.java | 37 ++++++++++++++ .../undertow/UndertowActiveHandlers.java | 48 +++++++++++++++++++ .../undertow-1.4/javaagent/build.gradle.kts | 2 + .../undertow/UndertowHttpServerTracer.java | 4 +- javaagent/build.gradle.kts | 7 +-- settings.gradle.kts | 1 + testing/agent-for-testing/build.gradle.kts | 1 + 10 files changed, 111 insertions(+), 6 deletions(-) create mode 100644 instrumentation/undertow-1.4/bootstrap/build.gradle.kts create mode 100644 instrumentation/undertow-1.4/bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/undertow/KeyHolder.java create mode 100644 instrumentation/undertow-1.4/bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/undertow/UndertowActiveHandlers.java diff --git a/buildSrc/src/main/kotlin/otel.java-conventions.gradle.kts b/buildSrc/src/main/kotlin/otel.java-conventions.gradle.kts index 7d016436b4..a9981f4bdf 100644 --- a/buildSrc/src/main/kotlin/otel.java-conventions.gradle.kts +++ b/buildSrc/src/main/kotlin/otel.java-conventions.gradle.kts @@ -261,7 +261,7 @@ idea { } when (projectDir.name) { - "javaagent", "library", "testing" -> { + "bootstrap", "javaagent", "library", "testing" -> { // We don't use this group anywhere in our config, but we need to make sure it is unique per // instrumentation so Gradle doesn't merge projects with same name due to a bug in Gradle. // https://github.com/gradle/gradle/issues/847 diff --git a/instrumentation/build.gradle.kts b/instrumentation/build.gradle.kts index e2887e7c48..c9bf1ea058 100644 --- a/instrumentation/build.gradle.kts +++ b/instrumentation/build.gradle.kts @@ -6,6 +6,8 @@ plugins { id("otel.java-conventions") } +val bootstrap by configurations.creating + val instrumentationProjectTest = tasks.named("test") val instrumentationProjectDependencies = dependencies @@ -15,6 +17,12 @@ subprojects { instrumentationProjectTest.configure { dependsOn(subProj.tasks.named("test")) } + + if (subProj.name == "bootstrap") { + instrumentationProjectDependencies.run { + add(bootstrap.name, project(subProj.path)) + } + } } plugins.withId("otel.javaagent-instrumentation") { diff --git a/instrumentation/undertow-1.4/bootstrap/build.gradle.kts b/instrumentation/undertow-1.4/bootstrap/build.gradle.kts new file mode 100644 index 0000000000..13256085fd --- /dev/null +++ b/instrumentation/undertow-1.4/bootstrap/build.gradle.kts @@ -0,0 +1,7 @@ +plugins { + id("otel.java-conventions") +} + +dependencies { + compileOnly("io.opentelemetry:opentelemetry-api") +} diff --git a/instrumentation/undertow-1.4/bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/undertow/KeyHolder.java b/instrumentation/undertow-1.4/bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/undertow/KeyHolder.java new file mode 100644 index 0000000000..35bf0f21d7 --- /dev/null +++ b/instrumentation/undertow-1.4/bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/undertow/KeyHolder.java @@ -0,0 +1,37 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.bootstrap.undertow; + +import java.util.IdentityHashMap; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; + +/** + * Undertow's {@code io.undertow.server.HttpServerExchange} uses {@code + * io.undertow.util.AttachmentKey} as a key for storing arbitrary data. It uses {@link + * IdentityHashMap} and thus all keys are compared for equality by object reference. This means that + * we cannot hold an instance of {@code io.undertow.util.AttachmentKey} in a static field of the + * corresponding Tracer, as we usually do. Tracers are loaded into user's classloaders and thus it + * is totally possible to have several instances of tracers. Each of those instances will have a + * separate value in a static field and {@code io.undertow.server.HttpServerExchange} will treat + * them as different keys then. + * + *

That is why this class exists and resides in a separate package. This package is treated in a + * special way and is always loaded by bootstrap classloader. This makes sure that this class is + * available to all tracers from every classloader. + * + *

But at the same time, being loaded by bootstrap classloader, this class itself cannot initiate + * the loading of {@code io.undertow.util.AttachmentKey} class. Class has to be loaded by any + * classloader that has it, e.g. by the classloader of a Tracer that uses this key holder. After + * that, all Tracers, loaded by all classloaders, will be able to use exactly the same sole + * instance of the key. + */ +// TODO allow instrumentation to have their own classes that should go to bootstrap classloader +public final class KeyHolder { + public static final ConcurrentMap, Object> contextKeys = new ConcurrentHashMap<>(); + + private KeyHolder() {} +} diff --git a/instrumentation/undertow-1.4/bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/undertow/UndertowActiveHandlers.java b/instrumentation/undertow-1.4/bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/undertow/UndertowActiveHandlers.java new file mode 100644 index 0000000000..cb05944fb7 --- /dev/null +++ b/instrumentation/undertow-1.4/bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/undertow/UndertowActiveHandlers.java @@ -0,0 +1,48 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.bootstrap.undertow; + +import io.opentelemetry.context.Context; +import io.opentelemetry.context.ContextKey; +import java.util.concurrent.atomic.AtomicInteger; + +/** Helper container for keeping track of request processing state in undertow. */ +public final class UndertowActiveHandlers { + private static final ContextKey CONTEXT_KEY = + ContextKey.named("opentelemetry-undertow-active-handlers"); + + private UndertowActiveHandlers() {} + + /** + * Attach to context. + * + * @param context server context + * @param initialValue initial value for counter + * @return new context + */ + public static Context init(Context context, int initialValue) { + return context.with(CONTEXT_KEY, new AtomicInteger(initialValue)); + } + + /** + * Increment counter. + * + * @param context server context + */ + public static void increment(Context context) { + context.get(CONTEXT_KEY).incrementAndGet(); + } + + /** + * Decrement counter. + * + * @param context server context + * @return value of counter after decrementing it + */ + public static int decrementAndGet(Context context) { + return context.get(CONTEXT_KEY).decrementAndGet(); + } +} diff --git a/instrumentation/undertow-1.4/javaagent/build.gradle.kts b/instrumentation/undertow-1.4/javaagent/build.gradle.kts index d4a2ea8419..fb03180077 100644 --- a/instrumentation/undertow-1.4/javaagent/build.gradle.kts +++ b/instrumentation/undertow-1.4/javaagent/build.gradle.kts @@ -13,4 +13,6 @@ muzzle { dependencies { library("io.undertow:undertow-core:2.0.0.Final") + + compileOnly(project(":instrumentation:undertow-1.4:bootstrap")) } diff --git a/instrumentation/undertow-1.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/undertow/UndertowHttpServerTracer.java b/instrumentation/undertow-1.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/undertow/UndertowHttpServerTracer.java index 4d540c741f..311e74c4e2 100644 --- a/instrumentation/undertow-1.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/undertow/UndertowHttpServerTracer.java +++ b/instrumentation/undertow-1.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/undertow/UndertowHttpServerTracer.java @@ -12,8 +12,8 @@ import io.opentelemetry.context.propagation.TextMapGetter; import io.opentelemetry.instrumentation.api.servlet.AppServerBridge; import io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming; import io.opentelemetry.instrumentation.api.tracer.HttpServerTracer; -import io.opentelemetry.javaagent.instrumentation.api.undertow.KeyHolder; -import io.opentelemetry.javaagent.instrumentation.api.undertow.UndertowActiveHandlers; +import io.opentelemetry.javaagent.bootstrap.undertow.KeyHolder; +import io.opentelemetry.javaagent.bootstrap.undertow.UndertowActiveHandlers; import io.undertow.server.DefaultResponseListener; import io.undertow.server.HttpServerExchange; import io.undertow.util.AttachmentKey; diff --git a/javaagent/build.gradle.kts b/javaagent/build.gradle.kts index 11576942af..566db40437 100644 --- a/javaagent/build.gradle.kts +++ b/javaagent/build.gradle.kts @@ -115,7 +115,9 @@ tasks { } } -val licenseReportDependencies by configurations.creating +val licenseReportDependencies by configurations.creating { + extendsFrom(shadowInclude) +} dependencies { testCompileOnly(project(":javaagent-bootstrap")) @@ -126,6 +128,7 @@ dependencies { testImplementation("io.opentracing.contrib.dropwizard:dropwizard-opentracing:0.2.2") shadowInclude(project(":javaagent-bootstrap")) + shadowInclude(project(":instrumentation", configuration = "bootstrap")) // We only have compileOnly dependencies on these to make sure they don"t leak into POMs. licenseReportDependencies("com.github.ben-manes.caffeine:caffeine") { @@ -137,10 +140,8 @@ dependencies { // but I couldn"t get that to work licenseReportDependencies(project(":javaagent-tooling")) licenseReportDependencies(project(":javaagent-extension-api")) - licenseReportDependencies(project(":javaagent-bootstrap")) } - licenseReport { outputDir = rootProject.file("licenses").absolutePath diff --git a/settings.gradle.kts b/settings.gradle.kts index 09de8984e0..d1a6d3499f 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -308,6 +308,7 @@ include(":instrumentation:tomcat:tomcat-7.0:javaagent") include(":instrumentation:tomcat:tomcat-10.0:javaagent") include(":instrumentation:tomcat:tomcat-common:javaagent") include(":instrumentation:twilio-6.6:javaagent") +include(":instrumentation:undertow-1.4:bootstrap") include(":instrumentation:undertow-1.4:javaagent") include(":instrumentation:vaadin-14.2:javaagent") include(":instrumentation:vaadin-14.2:testing") diff --git a/testing/agent-for-testing/build.gradle.kts b/testing/agent-for-testing/build.gradle.kts index 275f24e25e..a474121fdc 100644 --- a/testing/agent-for-testing/build.gradle.kts +++ b/testing/agent-for-testing/build.gradle.kts @@ -68,6 +68,7 @@ tasks { dependencies { // Dependencies to include without obfuscation. shadowInclude(project(":javaagent-bootstrap")) + shadowInclude(project(":instrumentation", configuration = "bootstrap")) testImplementation(project(":testing-common")) testImplementation("io.opentelemetry:opentelemetry-api")