Skip to content

Commit d17a0d7

Browse files
trasktrask
authored
Instrument SpanKey directly (#5933)
* Instrument SpanKey directly * feedback * Make muzzle work * Revert unrelated change
1 parent 2f4d2ab commit d17a0d7

File tree

12 files changed

+235
-51
lines changed

12 files changed

+235
-51
lines changed

‎conventions/src/main/kotlin/io.opentelemetry.instrumentation.javaagent-shadowing.gradle.kts

+2-1
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,9 @@ tasks.withType<ShadowJar>().configureEach {
3333
relocate("io.opentelemetry.extension.aws", "io.opentelemetry.javaagent.shaded.io.opentelemetry.extension.aws")
3434
relocate("io.opentelemetry.extension.kotlin", "io.opentelemetry.javaagent.shaded.io.opentelemetry.extension.kotlin")
3535

36-
// this is for instrumentation on opentelemetry-api itself
36+
// this is for instrumentation of opentelemetry-api and opentelemetry-instrumentation-api
3737
relocate("application.io.opentelemetry", "io.opentelemetry")
38+
relocate("application.io.opentelemetry.instrumentation.api", "io.opentelemetry.instrumentation.api")
3839

3940
// this is for instrumentation on java.util.logging (since java.util.logging itself is shaded above)
4041
relocate("application.java.util.logging", "java.util.logging")

‎gradle-plugins/src/main/kotlin/io.opentelemetry.instrumentation.muzzle-check.gradle.kts

+5-2
Original file line numberDiff line numberDiff line change
@@ -101,8 +101,9 @@ tasks.withType<ShadowJar>().configureEach {
101101
relocate("io.opentelemetry.extension.aws", "io.opentelemetry.javaagent.shaded.io.opentelemetry.extension.aws")
102102
relocate("io.opentelemetry.extension.kotlin", "io.opentelemetry.javaagent.shaded.io.opentelemetry.extension.kotlin")
103103

104-
// this is for instrumentation on opentelemetry-api itself
104+
// this is for instrumentation of opentelemetry-api and opentelemetry-instrumentation-api
105105
relocate("application.io.opentelemetry", "io.opentelemetry")
106+
relocate("application.io.opentelemetry.instrumentation.api", "io.opentelemetry.instrumentation.api")
106107

107108
// this is for instrumentation on java.util.logging (since java.util.logging itself is shaded above)
108109
relocate("application.java.util.logging", "java.util.logging")
@@ -285,7 +286,8 @@ fun addMuzzleTask(muzzleDirective: MuzzleDirective, versionArtifact: Artifact?,
285286
val instrumentationCL = createInstrumentationClassloader()
286287
val userCL = createClassLoaderForTask(config)
287288
withContextClassLoader(instrumentationCL) {
288-
MuzzleGradlePluginUtil.assertInstrumentationMuzzled(instrumentationCL, userCL, muzzleDirective.assertPass.get())
289+
MuzzleGradlePluginUtil.assertInstrumentationMuzzled(instrumentationCL, userCL,
290+
muzzleDirective.excludedInstrumentationModules.get(), muzzleDirective.assertPass.get())
289291
}
290292

291293
for (thread in Thread.getAllStackTraces().keys) {
@@ -348,6 +350,7 @@ fun inverseOf(muzzleDirective: MuzzleDirective, system: RepositorySystem, sessio
348350
versions.set(version)
349351
assertPass.set(!muzzleDirective.assertPass.get())
350352
excludedDependencies.set(muzzleDirective.excludedDependencies)
353+
excludedInstrumentationModules.set(muzzleDirective.excludedInstrumentationModules)
351354
}
352355
inverseDirectives.add(inverseDirective)
353356
}

‎gradle-plugins/src/main/kotlin/io/opentelemetry/javaagent/muzzle/MuzzleDirective.kt

+11
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ abstract class MuzzleDirective {
2020
abstract val skipVersions: SetProperty<String>
2121
abstract val additionalDependencies: ListProperty<Any>
2222
abstract val excludedDependencies: ListProperty<String>
23+
abstract val excludedInstrumentationModules: ListProperty<String>
2324
abstract val assertPass: Property<Boolean>
2425
abstract val assertInverse: Property<Boolean>
2526
internal abstract val coreJdk: Property<Boolean> // use coreJdk() function below to enable
@@ -30,6 +31,7 @@ abstract class MuzzleDirective {
3031
skipVersions.convention(emptySet())
3132
additionalDependencies.convention(listOf())
3233
excludedDependencies.convention(listOf())
34+
excludedInstrumentationModules.convention(listOf())
3335
assertPass.convention(false)
3436
assertInverse.convention(false)
3537
coreJdk.convention(false)
@@ -58,6 +60,15 @@ abstract class MuzzleDirective {
5860
excludedDependencies.add(excludeString!!)
5961
}
6062

63+
/**
64+
* Excludes an instrumentation module from the current muzzle test.
65+
*
66+
* @param excludeString An instrumentation module class name to exclude
67+
*/
68+
fun excludeInstrumentationModule(excludeString: String) {
69+
excludedInstrumentationModules.add(excludeString)
70+
}
71+
6172
fun skip(vararg version: String?) {
6273
skipVersions.addAll(*version)
6374
}

‎gradle-plugins/src/main/kotlin/io/opentelemetry/javaagent/muzzle/matcher/MuzzleGradlePluginUtil.kt

+6-2
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,9 @@ class MuzzleGradlePluginUtil {
5252
* version passes different {@code userClassLoader}.
5353
*/
5454
@Suppress("UNCHECKED_CAST")
55-
fun assertInstrumentationMuzzled(agentClassLoader: ClassLoader, userClassLoader: ClassLoader, assertPass: Boolean) {
55+
fun assertInstrumentationMuzzled(agentClassLoader: ClassLoader, userClassLoader: ClassLoader,
56+
excludedInstrumentationModules: List<String>, assertPass: Boolean) {
57+
5658
val matcherClass = agentClassLoader.loadClass("io.opentelemetry.javaagent.tooling.muzzle.ClassLoaderMatcher")
5759

5860
// We cannot reference Mismatch class directly here, because we are loaded from a different
@@ -65,7 +67,9 @@ class MuzzleGradlePluginUtil {
6567
.invoke(null, userClassLoader, assertPass)
6668
as Map<String, List<Any>>
6769

68-
allMismatches.forEach { moduleName, mismatches ->
70+
allMismatches.filterKeys {
71+
!excludedInstrumentationModules.contains(it)
72+
}.forEach { (moduleName, mismatches) ->
6973
val passed = mismatches.isEmpty()
7074
if (passed && !assertPass) {
7175
System.err.println("MUZZLE PASSED $moduleName BUT FAILURE WAS EXPECTED")

‎instrumentation/opentelemetry-api/opentelemetry-api-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/opentelemetryapi/context/InstrumentationApiContextBridging.java

-42
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,10 @@
1111
import java.lang.invoke.MethodHandles;
1212
import java.lang.invoke.MethodType;
1313
import java.util.ArrayList;
14-
import java.util.Arrays;
1514
import java.util.List;
1615
import java.util.function.Function;
1716
import javax.annotation.Nullable;
1817

19-
@SuppressWarnings({"unchecked", "rawtypes"})
2018
final class InstrumentationApiContextBridging {
2119

2220
static List<ContextKeyBridge<?, ?>> instrumentationApiBridges() {
@@ -48,30 +46,6 @@ final class InstrumentationApiContextBridging {
4846
// no old instrumentation-api on classpath
4947
}
5048

51-
List<String> spanKeyNames =
52-
Arrays.asList(
53-
// span kind keys
54-
"KIND_SERVER_KEY",
55-
"KIND_CLIENT_KEY",
56-
"KIND_CONSUMER_KEY",
57-
"KIND_PRODUCER_KEY",
58-
// semantic convention keys
59-
"HTTP_SERVER_KEY",
60-
"RPC_SERVER_KEY",
61-
"HTTP_CLIENT_KEY",
62-
"RPC_CLIENT_KEY",
63-
"DB_CLIENT_KEY",
64-
"PRODUCER_KEY",
65-
"CONSUMER_RECEIVE_KEY",
66-
"CONSUMER_PROCESS_KEY");
67-
68-
for (String spanKeyName : spanKeyNames) {
69-
ContextKeyBridge<?, ?> spanKeyBridge = spanKeyBridge(spanKeyName);
70-
if (spanKeyBridge != null) {
71-
bridges.add(spanKeyBridge);
72-
}
73-
}
74-
7549
ContextKeyBridge<?, ?> httpRouteHolderBridge = httpRouteStateBridge();
7650
if (httpRouteHolderBridge != null) {
7751
bridges.add(httpRouteHolderBridge);
@@ -80,22 +54,6 @@ final class InstrumentationApiContextBridging {
8054
return bridges;
8155
}
8256

83-
@Nullable
84-
private static ContextKeyBridge<Span, io.opentelemetry.api.trace.Span> spanKeyBridge(
85-
String name) {
86-
try {
87-
return new ContextKeyBridge<>(
88-
"application.io.opentelemetry.instrumentation.api.internal.SpanKey",
89-
"io.opentelemetry.instrumentation.api.internal.SpanKey",
90-
name,
91-
Bridging::toApplication,
92-
Bridging::toAgentOrNull);
93-
} catch (Throwable e) {
94-
// instrumentation-api may be absent on the classpath, just skip
95-
return null;
96-
}
97-
}
98-
9957
private static final Class<?> AGENT_HTTP_ROUTE_STATE;
10058
private static final MethodHandle AGENT_CREATE;
10159
private static final MethodHandle AGENT_GET_UPDATED_BY_SOURCE_ORDER;

‎instrumentation/opentelemetry-instrumentation-api/javaagent/build.gradle.kts

+27
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,27 @@ plugins {
22
id("otel.javaagent-instrumentation")
33
}
44

5+
// note that muzzle is not run against the current SNAPSHOT instrumentation-api, but this is ok
6+
// because the tests are run against the current SNAPSHOT instrumentation-api which will catch any
7+
// muzzle issues in SNAPSHOT instrumentation-api
8+
9+
muzzle {
10+
pass {
11+
group.set("io.opentelemetry.instrumentation")
12+
module.set("opentelemetry-instrumentation-api")
13+
// currently all 1.0.0+ versions are alpha so they are all skipped
14+
// if you want to test them anyways, comment out "alpha" from the exclusions in AcceptableVersions.kt
15+
versions.set("[1.14.0-alpha,)")
16+
assertInverse.set(true)
17+
excludeInstrumentationModule("io.opentelemetry.javaagent.instrumentation.opentelemetryapi.OpenTelemetryApiInstrumentationModule")
18+
}
19+
}
20+
521
dependencies {
622
implementation(project(":instrumentation:opentelemetry-api:opentelemetry-api-1.0:javaagent"))
723

824
compileOnly(project(":opentelemetry-api-shaded-for-instrumenting", configuration = "shadow"))
25+
compileOnly(project(":opentelemetry-instrumentation-api-shaded-for-instrumenting", configuration = "shadow"))
926

1027
testImplementation(project(":instrumentation-api-semconv"))
1128
testImplementation(project(":instrumentation:opentelemetry-instrumentation-api:testing"))
@@ -25,6 +42,16 @@ testing {
2542
}
2643

2744
configurations.configureEach {
45+
if (name.startsWith("muzzle-Assert")) {
46+
// some names also start with "muzzle-AssertFail", which is conveniently the same length
47+
val ver = name.substring("muzzle-AssertPass-io.opentelemetry.instrumentation-opentelemetry-instrumentation-api-".length)
48+
resolutionStrategy {
49+
dependencySubstitution {
50+
substitute(module("io.opentelemetry.instrumentation:opentelemetry-instrumentation-api"))
51+
.using(module("io.opentelemetry.instrumentation:opentelemetry-instrumentation-api:$ver"))
52+
}
53+
}
54+
}
2855
if (name.startsWith("testOldServerSpan")) {
2956
resolutionStrategy {
3057
dependencySubstitution {

‎instrumentation/opentelemetry-instrumentation-api/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/instrumentationapi/InstrumentationApiInstrumentationModule.java

+3-4
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
package io.opentelemetry.javaagent.instrumentation.instrumentationapi;
77

88
import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed;
9-
import static java.util.Collections.singletonList;
9+
import static java.util.Arrays.asList;
1010

1111
import com.google.auto.service.AutoService;
1212
import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
@@ -23,12 +23,11 @@ public InstrumentationApiInstrumentationModule() {
2323

2424
@Override
2525
public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
26-
return hasClassesNamed(
27-
"application.io.opentelemetry.instrumentation.api.internal.HttpRouteState");
26+
return hasClassesNamed("application.io.opentelemetry.instrumentation.api.internal.SpanKey");
2827
}
2928

3029
@Override
3130
public List<TypeInstrumentation> typeInstrumentations() {
32-
return singletonList(new HttpRouteStateInstrumentation());
31+
return asList(new HttpRouteStateInstrumentation(), new SpanKeyInstrumentation());
3332
}
3433
}

‎instrumentation/opentelemetry-instrumentation-api/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/instrumentationapi/SpanKeyBridging.java

+53
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
/*
2+
* Copyright The OpenTelemetry Authors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package io.opentelemetry.javaagent.instrumentation.instrumentationapi;
7+
8+
import application.io.opentelemetry.instrumentation.api.internal.SpanKey;
9+
import java.util.HashMap;
10+
import java.util.Map;
11+
import javax.annotation.Nullable;
12+
13+
public final class SpanKeyBridging {
14+
15+
private static final Map<SpanKey, io.opentelemetry.instrumentation.api.internal.SpanKey>
16+
agentSpanKeys = createMapping();
17+
18+
private static Map<SpanKey, io.opentelemetry.instrumentation.api.internal.SpanKey>
19+
createMapping() {
20+
21+
Map<SpanKey, io.opentelemetry.instrumentation.api.internal.SpanKey> map = new HashMap<>();
22+
map.put(SpanKey.KIND_SERVER, io.opentelemetry.instrumentation.api.internal.SpanKey.KIND_SERVER);
23+
map.put(SpanKey.KIND_CLIENT, io.opentelemetry.instrumentation.api.internal.SpanKey.KIND_CLIENT);
24+
map.put(
25+
SpanKey.KIND_CONSUMER, io.opentelemetry.instrumentation.api.internal.SpanKey.KIND_CONSUMER);
26+
map.put(
27+
SpanKey.KIND_PRODUCER, io.opentelemetry.instrumentation.api.internal.SpanKey.KIND_PRODUCER);
28+
29+
map.put(SpanKey.HTTP_SERVER, io.opentelemetry.instrumentation.api.internal.SpanKey.HTTP_SERVER);
30+
map.put(SpanKey.RPC_SERVER, io.opentelemetry.instrumentation.api.internal.SpanKey.RPC_SERVER);
31+
32+
map.put(SpanKey.HTTP_CLIENT, io.opentelemetry.instrumentation.api.internal.SpanKey.HTTP_CLIENT);
33+
map.put(SpanKey.RPC_CLIENT, io.opentelemetry.instrumentation.api.internal.SpanKey.RPC_CLIENT);
34+
map.put(SpanKey.DB_CLIENT, io.opentelemetry.instrumentation.api.internal.SpanKey.DB_CLIENT);
35+
36+
map.put(SpanKey.PRODUCER, io.opentelemetry.instrumentation.api.internal.SpanKey.PRODUCER);
37+
map.put(
38+
SpanKey.CONSUMER_RECEIVE,
39+
io.opentelemetry.instrumentation.api.internal.SpanKey.CONSUMER_RECEIVE);
40+
map.put(
41+
SpanKey.CONSUMER_PROCESS,
42+
io.opentelemetry.instrumentation.api.internal.SpanKey.CONSUMER_PROCESS);
43+
return map;
44+
}
45+
46+
@Nullable
47+
public static io.opentelemetry.instrumentation.api.internal.SpanKey toAgentOrNull(
48+
SpanKey applicationSpanKey) {
49+
return agentSpanKeys.get(applicationSpanKey);
50+
}
51+
52+
private SpanKeyBridging() {}
53+
}

‎instrumentation/opentelemetry-instrumentation-api/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/instrumentationapi/SpanKeyInstrumentation.java

+104
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
/*
2+
* Copyright The OpenTelemetry Authors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package io.opentelemetry.javaagent.instrumentation.instrumentationapi;
7+
8+
import static net.bytebuddy.matcher.ElementMatchers.named;
9+
import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
10+
11+
import application.io.opentelemetry.api.trace.Span;
12+
import application.io.opentelemetry.context.Context;
13+
import application.io.opentelemetry.instrumentation.api.internal.SpanKey;
14+
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
15+
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
16+
import io.opentelemetry.javaagent.instrumentation.opentelemetryapi.context.AgentContextStorage;
17+
import io.opentelemetry.javaagent.instrumentation.opentelemetryapi.trace.Bridging;
18+
import net.bytebuddy.asm.Advice;
19+
import net.bytebuddy.description.type.TypeDescription;
20+
import net.bytebuddy.matcher.ElementMatcher;
21+
22+
final class SpanKeyInstrumentation implements TypeInstrumentation {
23+
24+
@Override
25+
public ElementMatcher<TypeDescription> typeMatcher() {
26+
return named("application.io.opentelemetry.instrumentation.api.internal.SpanKey");
27+
}
28+
29+
@Override
30+
public void transform(TypeTransformer transformer) {
31+
transformer.applyAdviceToMethod(
32+
named("storeInContext")
33+
.and(takesArgument(0, named("application.io.opentelemetry.context.Context")))
34+
.and(takesArgument(1, named("application.io.opentelemetry.api.trace.Span"))),
35+
this.getClass().getName() + "$StoreInContextAdvice");
36+
transformer.applyAdviceToMethod(
37+
named("fromContextOrNull")
38+
.and(takesArgument(0, named("application.io.opentelemetry.context.Context"))),
39+
this.getClass().getName() + "$FromContextOrNullAdvice");
40+
}
41+
42+
@SuppressWarnings("unused")
43+
public static class StoreInContextAdvice {
44+
@Advice.OnMethodEnter(skipOn = Advice.OnDefaultValue.class)
45+
public static Object onEnter() {
46+
return null;
47+
}
48+
49+
@Advice.OnMethodExit(suppress = Throwable.class)
50+
public static void onExit(
51+
@Advice.This SpanKey applicationSpanKey,
52+
@Advice.Argument(0) Context applicationContext,
53+
@Advice.Argument(1) Span applicationSpan,
54+
@Advice.Return(readOnly = false) Context newApplicationContext) {
55+
56+
io.opentelemetry.instrumentation.api.internal.SpanKey agentSpanKey =
57+
SpanKeyBridging.toAgentOrNull(applicationSpanKey);
58+
if (agentSpanKey == null) {
59+
return;
60+
}
61+
62+
io.opentelemetry.context.Context agentContext =
63+
AgentContextStorage.getAgentContext(applicationContext);
64+
65+
io.opentelemetry.api.trace.Span agentSpan = Bridging.toAgentOrNull(applicationSpan);
66+
if (agentSpan == null) {
67+
return;
68+
}
69+
70+
io.opentelemetry.context.Context newAgentContext =
71+
agentSpanKey.storeInContext(agentContext, agentSpan);
72+
73+
newApplicationContext = AgentContextStorage.toApplicationContext(newAgentContext);
74+
}
75+
}
76+
77+
@SuppressWarnings("unused")
78+
public static class FromContextOrNullAdvice {
79+
@Advice.OnMethodEnter(skipOn = Advice.OnDefaultValue.class)
80+
public static Object onEnter() {
81+
return null;
82+
}
83+
84+
@Advice.OnMethodExit(suppress = Throwable.class)
85+
public static void onExit(
86+
@Advice.This SpanKey applicationSpanKey,
87+
@Advice.Argument(0) Context applicationContext,
88+
@Advice.Return(readOnly = false) Span applicationSpan) {
89+
90+
io.opentelemetry.instrumentation.api.internal.SpanKey agentSpanKey =
91+
SpanKeyBridging.toAgentOrNull(applicationSpanKey);
92+
if (agentSpanKey == null) {
93+
return;
94+
}
95+
96+
io.opentelemetry.context.Context agentContext =
97+
AgentContextStorage.getAgentContext(applicationContext);
98+
99+
io.opentelemetry.api.trace.Span agentSpan = agentSpanKey.fromContextOrNull(agentContext);
100+
101+
applicationSpan = agentSpan == null ? null : Bridging.toApplication(agentSpan);
102+
}
103+
}
104+
}

‎javaagent/build.gradle.kts

+1
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ dependencies {
6767
baseJavaagentLibs(project(":instrumentation:opentelemetry-annotations-1.0:javaagent"))
6868
baseJavaagentLibs(project(":instrumentation:opentelemetry-api:opentelemetry-api-1.0:javaagent"))
6969
baseJavaagentLibs(project(":instrumentation:opentelemetry-api:opentelemetry-api-1.4:javaagent"))
70+
baseJavaagentLibs(project(":instrumentation:opentelemetry-instrumentation-api:javaagent"))
7071
baseJavaagentLibs(project(":instrumentation:executors:javaagent"))
7172
baseJavaagentLibs(project(":instrumentation:internal:internal-class-loader:javaagent"))
7273
baseJavaagentLibs(project(":instrumentation:internal:internal-eclipse-osgi-3.6:javaagent"))

0 commit comments

Comments
 (0)