Skip to content

Commit 330463e

Browse files
lauritlaurit
committed
Don't use unsafe on jdk23+
1 parent a4ebb07 commit 330463e

File tree

10 files changed

+457
-28
lines changed

10 files changed

+457
-28
lines changed

‎.github/workflows/build-common.yml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -329,6 +329,7 @@ jobs:
329329
-PtestJavaVersion=${{ matrix.test-java-version }}
330330
-PtestJavaVM=${{ matrix.vm }}
331331
-PtestIndy=${{ matrix.test-indy }}
332+
-PdenyUnsafe=${{ matrix.test-java-version >= 23 && 'true' || 'false' }}
332333
-Porg.gradle.java.installations.paths=${{ steps.setup-test-java.outputs.path }}
333334
-Porg.gradle.java.installations.auto-download=false
334335
${{ inputs.no-build-cache && ' --no-build-cache' || '' }}

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

Lines changed: 33 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ plugins {
66
id("io.opentelemetry.instrumentation.javaagent-shadowing")
77
}
88

9+
val denyUnsafe = gradle.startParameter.projectProperties["denyUnsafe"] == "true"
10+
911
dependencies {
1012
/*
1113
Dependencies added to this configuration will be found by the muzzle gradle plugin during code
@@ -72,30 +74,37 @@ class JavaagentTestArgumentsProvider(
7274
@PathSensitive(PathSensitivity.RELATIVE)
7375
val shadowJar: File,
7476
) : CommandLineArgumentProvider {
75-
override fun asArguments(): Iterable<String> = listOf(
76-
"-Dotel.javaagent.debug=true",
77-
"-javaagent:${agentShadowJar.absolutePath}",
78-
// make the path to the javaagent available to tests
79-
"-Dotel.javaagent.testing.javaagent-jar-path=${agentShadowJar.absolutePath}",
80-
"-Dotel.javaagent.experimental.initializer.jar=${shadowJar.absolutePath}",
81-
"-Dotel.javaagent.testing.additional-library-ignores.enabled=false",
82-
"-Dotel.javaagent.testing.fail-on-context-leak=${findProperty("failOnContextLeak") != false}",
83-
// prevent sporadic gradle deadlocks, see SafeLogger for more details
84-
"-Dotel.javaagent.testing.transform-safe-logging.enabled=true",
85-
// Reduce noise in assertion messages since we don't need to verify this in most tests. We check
86-
// in smoke tests instead.
87-
"-Dotel.javaagent.add-thread-details=false",
88-
"-Dotel.javaagent.experimental.indy=${findProperty("testIndy") == "true"}",
89-
// suppress repeated logging of "No metric data to export - skipping export."
90-
// since PeriodicMetricReader is configured with a short interval
91-
"-Dio.opentelemetry.javaagent.slf4j.simpleLogger.log.io.opentelemetry.sdk.metrics.export.PeriodicMetricReader=INFO",
92-
// suppress a couple of verbose ClassNotFoundException stack traces logged at debug level
93-
"-Dio.opentelemetry.javaagent.slf4j.simpleLogger.log.io.grpc.internal.ServerImplBuilder=INFO",
94-
"-Dio.opentelemetry.javaagent.slf4j.simpleLogger.log.io.grpc.internal.ManagedChannelImplBuilder=INFO",
95-
"-Dio.opentelemetry.javaagent.slf4j.simpleLogger.log.io.perfmark.PerfMark=INFO",
96-
"-Dio.opentelemetry.javaagent.slf4j.simpleLogger.log.io.grpc.Context=INFO",
97-
"-Dotel.java.experimental.span-attributes.copy-from-baggage.include=test-baggage-key-1,test-baggage-key-2"
98-
)
77+
override fun asArguments(): Iterable<String> {
78+
val list = mutableListOf(
79+
"-Dotel.javaagent.debug=true",
80+
"-javaagent:${agentShadowJar.absolutePath}",
81+
// make the path to the javaagent available to tests
82+
"-Dotel.javaagent.testing.javaagent-jar-path=${agentShadowJar.absolutePath}",
83+
"-Dotel.javaagent.experimental.initializer.jar=${shadowJar.absolutePath}",
84+
"-Dotel.javaagent.testing.additional-library-ignores.enabled=false",
85+
"-Dotel.javaagent.testing.fail-on-context-leak=${findProperty("failOnContextLeak") != false}",
86+
// prevent sporadic gradle deadlocks, see SafeLogger for more details
87+
"-Dotel.javaagent.testing.transform-safe-logging.enabled=true",
88+
// Reduce noise in assertion messages since we don't need to verify this in most tests. We check
89+
// in smoke tests instead.
90+
"-Dotel.javaagent.add-thread-details=false",
91+
"-Dotel.javaagent.experimental.indy=${findProperty("testIndy") == "true"}",
92+
// suppress repeated logging of "No metric data to export - skipping export."
93+
// since PeriodicMetricReader is configured with a short interval
94+
"-Dio.opentelemetry.javaagent.slf4j.simpleLogger.log.io.opentelemetry.sdk.metrics.export.PeriodicMetricReader=INFO",
95+
// suppress a couple of verbose ClassNotFoundException stack traces logged at debug level
96+
"-Dio.opentelemetry.javaagent.slf4j.simpleLogger.log.io.grpc.internal.ServerImplBuilder=INFO",
97+
"-Dio.opentelemetry.javaagent.slf4j.simpleLogger.log.io.grpc.internal.ManagedChannelImplBuilder=INFO",
98+
"-Dio.opentelemetry.javaagent.slf4j.simpleLogger.log.io.perfmark.PerfMark=INFO",
99+
"-Dio.opentelemetry.javaagent.slf4j.simpleLogger.log.io.grpc.Context=INFO",
100+
"-Dotel.java.experimental.span-attributes.copy-from-baggage.include=test-baggage-key-1,test-baggage-key-2"
101+
)
102+
if (denyUnsafe) {
103+
list += "-Dsun.misc.unsafe.memory.access=deny"
104+
list += "-Dotel.instrumentation.deny-unsafe.enabled=true"
105+
}
106+
return list
107+
}
99108
}
100109

101110
// need to run this after evaluate because testSets plugin adds new test tasks

‎muzzle/src/main/java/io/opentelemetry/javaagent/tooling/HelperInjector.java‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -305,6 +305,9 @@ private static Map<String, byte[]> resolve(Map<String, Supplier<byte[]>> classes
305305
return result;
306306
}
307307

308+
// disable using unsafe for jdk 23+ where it prints a warning
309+
private static final boolean canUseUnsafe =
310+
Double.parseDouble(System.getProperty("java.specification.version")) < 23;
308311
private static final String virtualFieldPackage =
309312
VirtualFieldAccessorMarker.class.getPackage().getName() + ".";
310313

@@ -347,10 +350,11 @@ private void injectBootstrapClassLoader(Map<String, Supplier<byte[]>> inject) th
347350
}
348351
}
349352

350-
// TODO by default, we use unsafe to define rest of the classes into boot loader
351-
// can be disabled with -Dnet.bytebuddy.safe=true
352-
// use -Dsun.misc.unsafe.memory.access=debug to check where unsafe is used
353-
if (ClassInjector.UsingUnsafe.isAvailable() && !classnameToBytes.isEmpty()) {
353+
if (classnameToBytes.isEmpty()) {
354+
return;
355+
}
356+
357+
if (canUseUnsafe && ClassInjector.UsingUnsafe.isAvailable()) {
354358
ClassInjector.UsingUnsafe.ofBootLoader().injectRaw(classnameToBytes);
355359
return;
356360
}

‎testing/agent-exporter/build.gradle.kts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import org.gradle.kotlin.dsl.implementation
2+
13
plugins {
24
id("com.gradleup.shadow")
35
id("otel.java-conventions")
@@ -15,6 +17,8 @@ dependencies {
1517
compileOnly(project(":javaagent-extension-api"))
1618
compileOnly(project(":javaagent-bootstrap"))
1719
compileOnly(project(":javaagent-tooling"))
20+
// Used by byte-buddy but not brought in as a transitive dependency.
21+
compileOnly("com.google.code.findbugs:annotations")
1822

1923
implementation("io.opentelemetry:opentelemetry-exporter-otlp-common")
2024
compileOnly("io.opentelemetry:opentelemetry-api-incubator")

‎testing/agent-exporter/src/main/java/io/opentelemetry/javaagent/testing/instrumentation/DefaultStreamMessageInstrumentation.java‎

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
/*
2+
* Copyright The OpenTelemetry Authors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package io.opentelemetry.javaagent.testing.instrumentation;
7+
8+
import static net.bytebuddy.matcher.ElementMatchers.named;
9+
10+
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
11+
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
12+
import java.util.concurrent.ConcurrentLinkedQueue;
13+
import net.bytebuddy.asm.AsmVisitorWrapper;
14+
import net.bytebuddy.description.field.FieldDescription;
15+
import net.bytebuddy.description.field.FieldList;
16+
import net.bytebuddy.description.method.MethodList;
17+
import net.bytebuddy.description.type.TypeDescription;
18+
import net.bytebuddy.implementation.Implementation;
19+
import net.bytebuddy.matcher.ElementMatcher;
20+
import net.bytebuddy.pool.TypePool;
21+
import org.objectweb.asm.ClassVisitor;
22+
import org.objectweb.asm.MethodVisitor;
23+
import org.objectweb.asm.Opcodes;
24+
import org.objectweb.asm.Type;
25+
26+
public class DefaultStreamMessageInstrumentation implements TypeInstrumentation {
27+
@Override
28+
public ElementMatcher<TypeDescription> typeMatcher() {
29+
return named("io.opentelemetry.testing.internal.armeria.common.stream.DefaultStreamMessage");
30+
}
31+
32+
@Override
33+
public void transform(TypeTransformer transformer) {
34+
// in the constructor of DefaultStreamMessage replace new MpscChunkedArrayQueue with new
35+
// ConcurrentLinkedQueue
36+
transformer.applyTransformer(
37+
(builder, typeDescription, classLoader, javaModule, protectionDomain) ->
38+
builder.visit(
39+
new AsmVisitorWrapper.AbstractBase() {
40+
41+
@Override
42+
public ClassVisitor wrap(
43+
TypeDescription instrumentedType,
44+
ClassVisitor classVisitor,
45+
Implementation.Context implementationContext,
46+
TypePool typePool,
47+
FieldList<FieldDescription.InDefinedShape> fields,
48+
MethodList<?> methods,
49+
int writerFlags,
50+
int readerFlags) {
51+
return new ClassVisitor(Opcodes.ASM9, classVisitor) {
52+
@Override
53+
public MethodVisitor visitMethod(
54+
int access,
55+
String name,
56+
String descriptor,
57+
String signature,
58+
String[] exceptions) {
59+
MethodVisitor mv =
60+
super.visitMethod(access, name, descriptor, signature, exceptions);
61+
if (!"<init>".equals(name)) {
62+
return mv;
63+
}
64+
65+
return new MethodVisitor(api, mv) {
66+
@Override
67+
public void visitTypeInsn(int opcode, String type) {
68+
if (opcode == Opcodes.NEW
69+
&& type.endsWith("jctools/queues/MpscChunkedArrayQueue")) {
70+
type = Type.getInternalName(ConcurrentLinkedQueue.class);
71+
}
72+
super.visitTypeInsn(opcode, type);
73+
}
74+
75+
@Override
76+
public void visitMethodInsn(
77+
int opcode,
78+
String owner,
79+
String name,
80+
String descriptor,
81+
boolean isInterface) {
82+
if (opcode == Opcodes.INVOKESPECIAL
83+
&& owner.endsWith("jctools/queues/MpscChunkedArrayQueue")
84+
&& name.equals("<init>")
85+
&& descriptor.equals("(II)V")) {
86+
owner = Type.getInternalName(ConcurrentLinkedQueue.class);
87+
// original constructor takes two arguments, remove them from the
88+
// stack
89+
super.visitInsn(Opcodes.POP);
90+
super.visitInsn(Opcodes.POP);
91+
super.visitMethodInsn(
92+
opcode,
93+
Type.getInternalName(ConcurrentLinkedQueue.class),
94+
name,
95+
"()V",
96+
isInterface);
97+
} else {
98+
super.visitMethodInsn(opcode, owner, name, descriptor, isInterface);
99+
}
100+
}
101+
};
102+
}
103+
};
104+
}
105+
}));
106+
}
107+
}

‎testing/agent-exporter/src/main/java/io/opentelemetry/javaagent/testing/instrumentation/DenyUnsafeInstrumentationModule.java‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
/*
2+
* Copyright The OpenTelemetry Authors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package io.opentelemetry.javaagent.testing.instrumentation;
7+
8+
import static java.util.Arrays.asList;
9+
10+
import com.google.auto.service.AutoService;
11+
import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
12+
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
13+
import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
14+
import java.util.List;
15+
16+
/**
17+
* Instrument libraries used for testing to remove usages of unsafe so we could test the agent with
18+
* --sun-misc-unsafe-memory-access=deny
19+
*/
20+
@AutoService(InstrumentationModule.class)
21+
public class DenyUnsafeInstrumentationModule extends InstrumentationModule {
22+
23+
public DenyUnsafeInstrumentationModule() {
24+
super("deny-unsafe");
25+
}
26+
27+
@Override
28+
public List<TypeInstrumentation> typeInstrumentations() {
29+
return asList(
30+
new ProtoUtf8UnsafeProcessorInstrumentation(),
31+
new ExceptionSamplerInstrumentation(),
32+
new ServerInstrumentation(),
33+
new DefaultStreamMessageInstrumentation());
34+
}
35+
36+
@Override
37+
public boolean defaultEnabled(ConfigProperties config) {
38+
return false;
39+
}
40+
}

‎testing/agent-exporter/src/main/java/io/opentelemetry/javaagent/testing/instrumentation/ExceptionSamplerInstrumentation.java‎

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
/*
2+
* Copyright The OpenTelemetry Authors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package io.opentelemetry.javaagent.testing.instrumentation;
7+
8+
import static net.bytebuddy.matcher.ElementMatchers.named;
9+
10+
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
11+
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
12+
import java.util.concurrent.ConcurrentHashMap;
13+
import net.bytebuddy.asm.AsmVisitorWrapper;
14+
import net.bytebuddy.description.field.FieldDescription;
15+
import net.bytebuddy.description.field.FieldList;
16+
import net.bytebuddy.description.method.MethodList;
17+
import net.bytebuddy.description.type.TypeDescription;
18+
import net.bytebuddy.implementation.Implementation;
19+
import net.bytebuddy.matcher.ElementMatcher;
20+
import net.bytebuddy.pool.TypePool;
21+
import org.objectweb.asm.ClassVisitor;
22+
import org.objectweb.asm.MethodVisitor;
23+
import org.objectweb.asm.Opcodes;
24+
import org.objectweb.asm.Type;
25+
26+
public class ExceptionSamplerInstrumentation implements TypeInstrumentation {
27+
@Override
28+
public ElementMatcher<TypeDescription> typeMatcher() {
29+
return named("io.opentelemetry.testing.internal.armeria.common.ExceptionSampler");
30+
}
31+
32+
@Override
33+
public void transform(TypeTransformer transformer) {
34+
// in the constructor of ExceptionSampler replace new NonBlockingHashMap with new
35+
// ConcurrentHashMap
36+
transformer.applyTransformer(
37+
(builder, typeDescription, classLoader, javaModule, protectionDomain) ->
38+
builder.visit(
39+
new AsmVisitorWrapper.AbstractBase() {
40+
41+
@Override
42+
public ClassVisitor wrap(
43+
TypeDescription instrumentedType,
44+
ClassVisitor classVisitor,
45+
Implementation.Context implementationContext,
46+
TypePool typePool,
47+
FieldList<FieldDescription.InDefinedShape> fields,
48+
MethodList<?> methods,
49+
int writerFlags,
50+
int readerFlags) {
51+
return new ClassVisitor(Opcodes.ASM9, classVisitor) {
52+
@Override
53+
public MethodVisitor visitMethod(
54+
int access,
55+
String name,
56+
String descriptor,
57+
String signature,
58+
String[] exceptions) {
59+
MethodVisitor mv =
60+
super.visitMethod(access, name, descriptor, signature, exceptions);
61+
if (!"<init>".equals(name)) {
62+
return mv;
63+
}
64+
65+
return new MethodVisitor(api, mv) {
66+
@Override
67+
public void visitTypeInsn(int opcode, String type) {
68+
if (opcode == Opcodes.NEW
69+
&& type.endsWith("jctools/maps/NonBlockingHashMap")) {
70+
type = Type.getInternalName(ConcurrentHashMap.class);
71+
}
72+
super.visitTypeInsn(opcode, type);
73+
}
74+
75+
@Override
76+
public void visitMethodInsn(
77+
int opcode,
78+
String owner,
79+
String name,
80+
String descriptor,
81+
boolean isInterface) {
82+
if (opcode == Opcodes.INVOKESPECIAL
83+
&& owner.endsWith("jctools/maps/NonBlockingHashMap")
84+
&& name.equals("<init>")) {
85+
owner = Type.getInternalName(ConcurrentHashMap.class);
86+
}
87+
super.visitMethodInsn(opcode, owner, name, descriptor, isInterface);
88+
}
89+
};
90+
}
91+
};
92+
}
93+
}));
94+
}
95+
}

0 commit comments

Comments
 (0)