diff --git a/javaagent-tooling/build.gradle.kts b/javaagent-tooling/build.gradle.kts index 530f1da46301..b3161ac85ac3 100644 --- a/javaagent-tooling/build.gradle.kts +++ b/javaagent-tooling/build.gradle.kts @@ -2,6 +2,7 @@ import net.ltgt.gradle.errorprone.errorprone plugins { id("otel.java-conventions") + id("otel.nullaway-conventions") id("otel.publish-conventions") id("otel.jmh-conventions") } diff --git a/javaagent-tooling/javaagent-tooling-java9/build.gradle.kts b/javaagent-tooling/javaagent-tooling-java9/build.gradle.kts index d1a54f2e7e7f..3adec20f45ac 100644 --- a/javaagent-tooling/javaagent-tooling-java9/build.gradle.kts +++ b/javaagent-tooling/javaagent-tooling-java9/build.gradle.kts @@ -1,5 +1,6 @@ plugins { id("otel.java-conventions") + id("otel.nullaway-conventions") id("otel.publish-conventions") } diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/AgentInstaller.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/AgentInstaller.java index 959ce738e27a..7b423641d90c 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/AgentInstaller.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/AgentInstaller.java @@ -10,6 +10,7 @@ import static io.opentelemetry.javaagent.tooling.SafeServiceLoader.loadOrdered; import static io.opentelemetry.javaagent.tooling.Utils.getResourceName; import static java.util.Collections.emptyList; +import static java.util.Objects.requireNonNull; import static java.util.logging.Level.FINE; import static java.util.logging.Level.SEVERE; import static net.bytebuddy.matcher.ElementMatchers.any; @@ -153,7 +154,9 @@ private static void installBytebuddyAgent( AutoConfiguredOpenTelemetrySdk autoConfiguredSdk = installOpenTelemetrySdk(extensionClassLoader); - ConfigProperties sdkConfig = AutoConfigureUtil.getConfig(autoConfiguredSdk); + ConfigProperties sdkConfig = + requireNonNull( + AutoConfigureUtil.getConfig(autoConfiguredSdk), "SDK config must not be null"); setBootstrapPackages(sdkConfig, extensionClassLoader); ConfiguredResourceAttributesHolder.initialize( @@ -287,7 +290,7 @@ private static AgentBuilder configureIgnoredTypes( IgnoredTypesBuilderImpl builder = new IgnoredTypesBuilderImpl(); for (IgnoredTypesConfigurer configurer : loadOrdered(IgnoredTypesConfigurer.class, extensionClassLoader)) { - configurer.configure(builder, config != null ? config : EmptyConfigProperties.INSTANCE); + configurer.configure(builder, config); } Trie ignoredTasksTrie = builder.buildIgnoredTasksTrie(); diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/AgentStarterImpl.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/AgentStarterImpl.java index cc809ab3a4e6..0d05599b4b15 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/AgentStarterImpl.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/AgentStarterImpl.java @@ -18,6 +18,7 @@ import java.lang.instrument.UnmodifiableClassException; import java.security.ProtectionDomain; import java.util.ServiceLoader; +import javax.annotation.Nullable; import org.objectweb.asm.ClassReader; import org.objectweb.asm.ClassVisitor; import org.objectweb.asm.ClassWriter; @@ -34,7 +35,7 @@ public class AgentStarterImpl implements AgentStarter { private final Instrumentation instrumentation; private final File javaagentFile; private final boolean isSecurityManagerSupportEnabled; - private ClassLoader extensionClassLoader; + @Nullable private ClassLoader extensionClassLoader; public AgentStarterImpl( Instrumentation instrumentation, @@ -142,6 +143,7 @@ private static void logUnrecognizedLoggerImplWarning(String loggerImplementation + "'. The agent will use the no-op implementation."); } + @Nullable @Override public ClassLoader getExtensionClassLoader() { return extensionClassLoader; @@ -156,6 +158,7 @@ private static class LaunchHelperClassFileTransformer implements ClassFileTransf boolean hookInserted = false; boolean transformed = false; + @Nullable @Override public byte[] transform( ClassLoader loader, @@ -202,6 +205,7 @@ public void visitCode() { private static class InetAddressClassFileTransformer implements ClassFileTransformer { boolean hookInserted = false; + @Nullable @Override public byte[] transform( ClassLoader loader, @@ -257,6 +261,7 @@ public void visitMethodInsn( private static class AgentBuilderDefaultClassFileTransformer implements ClassFileTransformer { + @Nullable @Override public byte[] transform( ClassLoader loader, @@ -291,6 +296,7 @@ public FieldVisitor visitField( private static class CallbackRegistrationClassFileTransformer implements ClassFileTransformer { + @Nullable @Override public byte[] transform( ClassLoader loader, diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/DefineClassHandler.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/DefineClassHandler.java index 32fb05fa3dbc..51d9992aebce 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/DefineClassHandler.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/DefineClassHandler.java @@ -11,6 +11,7 @@ import io.opentelemetry.javaagent.bootstrap.DefineClassHelper.Handler; import java.util.HashSet; import java.util.Set; +import javax.annotation.Nullable; import org.objectweb.asm.ClassReader; public class DefineClassHandler implements Handler { @@ -27,7 +28,7 @@ public DefineClassContext beforeDefineClass( if (classBytes == null || (classBytes.length == 40 && new String(classBytes, ISO_8859_1).startsWith("J9ROMCLASSCOOKIE"))) { - return null; + return DefineClassContextImpl.NOP; } Set superNames = new HashSet<>(); @@ -85,9 +86,7 @@ private static void addSuperNames(Set superNames, Class clazz) { @Override public void afterDefineClass(DefineClassContext context) { - if (context != null) { - context.exit(); - } + context.exit(); } /** @@ -107,17 +106,24 @@ public static Set getSuperTypes() { } private static class DefineClassContextImpl implements DefineClassContext { - private static final DefineClassContextImpl NOP = new DefineClassContextImpl(); - - private final DefineClassContextImpl previous; - String failedClassDotName; - Set superDotNames; + // NOP is returned from beforeDefineClass without calling enter() (no frame pushed), + // so exit() must not mutate the ThreadLocal — otherwise a nested J9 defineClass would + // clobber an outer frame that is still active. + private static final DefineClassContextImpl NOP = + new DefineClassContextImpl() { + @Override + public void exit() {} + }; + + @Nullable private final DefineClassContextImpl previous; + @Nullable String failedClassDotName; + @Nullable Set superDotNames; private DefineClassContextImpl() { previous = null; } - private DefineClassContextImpl(DefineClassContextImpl previous) { + private DefineClassContextImpl(@Nullable DefineClassContextImpl previous) { this.previous = previous; } @@ -130,7 +136,7 @@ static DefineClassContextImpl enter() { @Override public void exit() { - defineClassContext.set(previous); + defineClassContext.set(previous != null ? previous : NOP); } } } diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/ExtensionClassLoader.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/ExtensionClassLoader.java index 97431f2a5306..4068f5d080a1 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/ExtensionClassLoader.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/ExtensionClassLoader.java @@ -109,7 +109,7 @@ private static void includeEmbeddedExtensionsIfFound(List extensions, File } } - private static File ensureTempDirectoryExists(File tempDirectory) throws IOException { + private static File ensureTempDirectoryExists(@Nullable File tempDirectory) throws IOException { if (tempDirectory == null) { tempDirectory = Files.createTempDirectory("otel-extensions").toFile(); tempDirectory.deleteOnExit(); diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/InputStreamUrlConnection.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/InputStreamUrlConnection.java index 53095f784c49..1717a67416be 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/InputStreamUrlConnection.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/InputStreamUrlConnection.java @@ -9,6 +9,7 @@ import java.net.URL; import java.net.URLConnection; import java.security.Permission; +import javax.annotation.Nullable; public class InputStreamUrlConnection extends URLConnection { private final InputStream inputStream; @@ -30,6 +31,7 @@ public InputStream getInputStream() { return inputStream; } + @Nullable @Override public Permission getPermission() { // No permissions needed because all classes are in memory diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/OpenTelemetryInstaller.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/OpenTelemetryInstaller.java index e4b51a440087..23106f0b2df5 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/OpenTelemetryInstaller.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/OpenTelemetryInstaller.java @@ -6,6 +6,7 @@ package io.opentelemetry.javaagent.tooling; import static java.util.Arrays.asList; +import static java.util.Collections.emptyMap; import io.opentelemetry.api.GlobalOpenTelemetry; import io.opentelemetry.instrumentation.config.bridge.ConfigPropertiesBackedConfigProvider; @@ -16,6 +17,7 @@ import io.opentelemetry.sdk.autoconfigure.SdkAutoconfigureAccess; import io.opentelemetry.sdk.autoconfigure.internal.AutoConfigureUtil; import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; +import io.opentelemetry.sdk.autoconfigure.spi.internal.DefaultConfigProperties; import io.opentelemetry.sdk.common.CompletableResultCode; public final class OpenTelemetryInstaller { @@ -36,15 +38,17 @@ public static AutoConfiguredOpenTelemetrySdk installOpenTelemetrySdk( .build(); OpenTelemetrySdk sdk = autoConfiguredSdk.getOpenTelemetrySdk(); ConfigProperties configProperties = AutoConfigureUtil.getConfig(autoConfiguredSdk); - boolean declarativeConfigUsed = configProperties == null; - if (!declarativeConfigUsed) { + if (configProperties != null) { // Provide a fake declarative configuration based on config properties // so that declarative configuration API can be used everywhere sdk = new ExtendedOpenTelemetrySdkWrapper( sdk, ConfigPropertiesBackedConfigProvider.create(configProperties)); AgentDistributionConfig.set(AgentDistributionConfig.fromConfigProperties(configProperties)); + } else { + // Declarative config path: no ConfigProperties available, use empty defaults + configProperties = DefaultConfigProperties.createFromMap(emptyMap()); } setForceFlush(sdk); diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/RemappingUrlConnection.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/RemappingUrlConnection.java index 742470f67cea..9a256d275c9a 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/RemappingUrlConnection.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/RemappingUrlConnection.java @@ -15,6 +15,7 @@ import java.security.Permission; import java.util.jar.JarEntry; import java.util.jar.JarFile; +import javax.annotation.Nullable; import org.objectweb.asm.ClassReader; import org.objectweb.asm.ClassWriter; import org.objectweb.asm.commons.ClassRemapper; @@ -50,7 +51,7 @@ public class RemappingUrlConnection extends URLConnection { private final JarFile delegateJarFile; private final JarEntry entry; - private byte[] cacheClassBytes; + @Nullable private byte[] cacheClassBytes; public RemappingUrlConnection(URL url, JarFile delegateJarFile, JarEntry entry) { super(url); @@ -89,6 +90,7 @@ private static byte[] remapClassBytes(InputStream in) throws IOException { return cw.toByteArray(); } + @Nullable @Override public Permission getPermission() { // No permissions needed because all classes are in memory diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/SafeServiceLoader.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/SafeServiceLoader.java index 935b899a734e..e78328568183 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/SafeServiceLoader.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/SafeServiceLoader.java @@ -12,8 +12,10 @@ import java.util.Comparator; import java.util.Iterator; import java.util.List; +import java.util.NoSuchElementException; import java.util.ServiceLoader; import java.util.logging.Logger; +import javax.annotation.Nullable; public final class SafeServiceLoader { @@ -30,13 +32,14 @@ public final class SafeServiceLoader { */ // Because we want to catch exception per iteration @SuppressWarnings("ForEachIterable") - public static List load(Class serviceClass, ClassLoader classLoader) { + public static List load(Class serviceClass, @Nullable ClassLoader classLoader) { List result = new ArrayList<>(); ServiceLoader services = ServiceLoader.load(serviceClass, classLoader); for (Iterator iterator = new SafeIterator<>(services.iterator()); iterator.hasNext(); ) { - T service = iterator.next(); - if (service != null) { - result.add(service); + try { + result.add(iterator.next()); + } catch (NoSuchElementException ignored) { + // UnsupportedClassVersionError was thrown and handled by SafeIterator } } return result; @@ -47,7 +50,7 @@ public static List load(Class serviceClass, ClassLoader classLoader) { * comparing their {@link Ordered#order()}. */ public static List loadOrdered( - Class serviceClass, ClassLoader classLoader) { + Class serviceClass, @Nullable ClassLoader classLoader) { List result = load(serviceClass, classLoader); result.sort(Comparator.comparing(Ordered::order)); return result; @@ -89,7 +92,7 @@ public T next() { return delegate.next(); } catch (UnsupportedClassVersionError unsupportedClassVersionError) { handleUnsupportedClassVersionError(unsupportedClassVersionError); - return null; + throw new NoSuchElementException(unsupportedClassVersionError.getMessage()); } } } diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/Utils.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/Utils.java index 2664623751b6..5134879c534b 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/Utils.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/Utils.java @@ -11,6 +11,7 @@ import io.opentelemetry.javaagent.bootstrap.AgentClassLoader.BootstrapClassLoaderProxy; import io.opentelemetry.javaagent.bootstrap.AgentInitializer; import java.security.PrivilegedAction; +import javax.annotation.Nullable; import net.bytebuddy.description.method.MethodDescription; import net.bytebuddy.description.type.TypeDefinition; @@ -24,6 +25,7 @@ public static ClassLoader getAgentClassLoader() { return AgentInstaller.class.getClassLoader(); } + @Nullable public static ClassLoader getExtensionsClassLoader() { return AgentInitializer.getExtensionsClassLoader(); } diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/config/ConfigurationFile.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/config/ConfigurationFile.java index d2bb9025fe7a..1ef637d47aea 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/config/ConfigurationFile.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/config/ConfigurationFile.java @@ -22,7 +22,7 @@ final class ConfigurationFile { - private static Map configFileContents; + @Nullable private static Map configFileContents; // this class is used early, and must not use logging in most of its methods // in case any file loading/parsing error occurs, we save the error message and log it later, when diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/config/JavaagentDistributionAccessCustomizerProvider.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/config/JavaagentDistributionAccessCustomizerProvider.java index c50a69e314b4..9f447e878822 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/config/JavaagentDistributionAccessCustomizerProvider.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/config/JavaagentDistributionAccessCustomizerProvider.java @@ -21,6 +21,7 @@ import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.DistributionPropertyModel; import java.io.IOException; import java.util.logging.Logger; +import javax.annotation.Nullable; /** * Allows access to the Javaagent distribution node, which cannot be accessed using the {@link @@ -60,7 +61,7 @@ public void customize(DeclarativeConfigurationCustomizer customizer) { }); } - private static AgentDistributionConfig parseConfig(DistributionModel distribution) { + private static AgentDistributionConfig parseConfig(@Nullable DistributionModel distribution) { if (distribution != null) { DistributionPropertyModel javaagent = distribution.getAdditionalProperties().get("javaagent"); if (javaagent != null) { diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/field/FieldBackedImplementationInstaller.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/field/FieldBackedImplementationInstaller.java index d6e1e7de02a7..0b7a3687a66a 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/field/FieldBackedImplementationInstaller.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/field/FieldBackedImplementationInstaller.java @@ -7,6 +7,7 @@ import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasSuperType; import static io.opentelemetry.javaagent.tooling.field.GeneratedVirtualFieldNames.getFieldAccessorInterfaceName; +import static java.util.Objects.requireNonNull; import static java.util.logging.Level.FINEST; import static net.bytebuddy.matcher.ElementMatchers.isAbstract; import static net.bytebuddy.matcher.ElementMatchers.named; @@ -27,6 +28,7 @@ import java.util.HashSet; import java.util.Map; import java.util.Set; +import javax.annotation.Nullable; import net.bytebuddy.ByteBuddy; import net.bytebuddy.agent.builder.AgentBuilder; import net.bytebuddy.asm.AsmVisitorWrapper; @@ -78,8 +80,10 @@ public FieldBackedImplementationInstaller( Class instrumenterClass, VirtualFieldMappings virtualFieldMappings) { this.instrumenterClass = instrumenterClass; this.virtualFieldMappings = virtualFieldMappings; - // This class is used only when running with javaagent, thus this calls is safe - this.instrumentation = InstrumentationHolder.getInstrumentation(); + // This class is used only when running with javaagent, thus this call is safe + this.instrumentation = + requireNonNull( + InstrumentationHolder.getInstrumentation(), "Instrumentation must not be null"); ByteBuddy byteBuddy = new ByteBuddy(); fieldAccessorInterfaces = @@ -146,6 +150,7 @@ private AgentBuilder.Transformer bootstrapHelperInjector( final HelperInjector injector = HelperInjector.forDynamicTypes(getClass().getSimpleName(), helpers, instrumentation); + @Nullable @Override public DynamicType.Builder transform( DynamicType.Builder builder, diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/field/VirtualFieldImplementationsGenerator.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/field/VirtualFieldImplementationsGenerator.java index 14f7417e8862..8e1115f27223 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/field/VirtualFieldImplementationsGenerator.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/field/VirtualFieldImplementationsGenerator.java @@ -17,6 +17,7 @@ import io.opentelemetry.javaagent.tooling.muzzle.VirtualFieldMappings; import java.util.HashMap; import java.util.Map; +import javax.annotation.Nullable; import net.bytebuddy.ByteBuddy; import net.bytebuddy.ClassFileVersion; import net.bytebuddy.asm.AsmVisitorWrapper; @@ -119,6 +120,7 @@ public ClassVisitor wrap( private final boolean frames = implementationContext.getClassFileVersion().isAtLeast(ClassFileVersion.JAVA_V6); + @Nullable @Override public MethodVisitor visitMethod( int access, String name, String descriptor, String signature, String[] exceptions) { @@ -275,25 +277,28 @@ private VirtualFieldImplementationTemplate(Cache map) { this.map = map; } + @Nullable @Override public Object get(Object object) { return realGet(object); } @Override - public void set(Object object, Object fieldValue) { + public void set(Object object, @Nullable Object fieldValue) { realPut(object, fieldValue); } + @Nullable private Object realGet(Object key) { // to be generated return null; } - private void realPut(Object key, Object value) { + private void realPut(Object key, @Nullable Object value) { // to be generated } + @Nullable private Object mapGet(Object key) { return map.get(key); } diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/InstrumentationLoader.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/InstrumentationLoader.java index 71d6faf68679..c08a7d23e9d9 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/InstrumentationLoader.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/InstrumentationLoader.java @@ -6,6 +6,7 @@ package io.opentelemetry.javaagent.tooling.instrumentation; import static io.opentelemetry.javaagent.tooling.SafeServiceLoader.loadOrdered; +import static java.util.Objects.requireNonNull; import static java.util.logging.Level.FINE; import static java.util.logging.Level.SEVERE; @@ -15,6 +16,7 @@ import io.opentelemetry.javaagent.tooling.AgentExtension; import io.opentelemetry.javaagent.tooling.Utils; import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; +import java.lang.instrument.Instrumentation; import java.util.logging.Logger; import net.bytebuddy.agent.builder.AgentBuilder; @@ -22,14 +24,16 @@ public class InstrumentationLoader implements AgentExtension { private static final Logger logger = Logger.getLogger(InstrumentationLoader.class.getName()); - private final InstrumentationModuleInstaller instrumentationModuleInstaller = - new InstrumentationModuleInstaller(InstrumentationHolder.getInstrumentation()); - @Override public AgentBuilder extend(AgentBuilder agentBuilder, ConfigProperties config) { + InstrumentationModuleInstaller instrumentationModuleInstaller = + new InstrumentationModuleInstaller(InstrumentationHolder.getInstrumentation()); int numberOfLoadedModules = 0; + ClassLoader extensionsClassLoader = + requireNonNull( + Utils.getExtensionsClassLoader(), "Extensions class loader must not be null"); for (InstrumentationModule instrumentationModule : - loadOrdered(InstrumentationModule.class, Utils.getExtensionsClassLoader())) { + loadOrdered(InstrumentationModule.class, extensionsClassLoader)) { if (logger.isLoggable(FINE)) { logger.log( FINE, diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/InstrumentationModuleInstaller.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/InstrumentationModuleInstaller.java index fa5ad09fa5c3..2590d1ef7080 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/InstrumentationModuleInstaller.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/InstrumentationModuleInstaller.java @@ -6,6 +6,7 @@ package io.opentelemetry.javaagent.tooling.instrumentation; import static java.util.Collections.emptyList; +import static java.util.Objects.requireNonNull; import static java.util.logging.Level.FINE; import static java.util.logging.Level.WARNING; import static net.bytebuddy.matcher.ElementMatchers.isAnnotatedWith; @@ -61,7 +62,7 @@ public final class InstrumentationModuleInstaller { VirtualFieldImplementationInstallerFactory.getInstance(); public InstrumentationModuleInstaller(Instrumentation instrumentation) { - this.instrumentation = instrumentation; + this.instrumentation = requireNonNull(instrumentation, "Instrumentation must not be null"); } // Need to call deprecated API for backward compatibility with modules that haven't migrated diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/MuzzleMatcher.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/MuzzleMatcher.java index 8b70e5ea1f5b..bb218f87b249 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/MuzzleMatcher.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/MuzzleMatcher.java @@ -5,6 +5,7 @@ package io.opentelemetry.javaagent.tooling.instrumentation; +import static java.util.Objects.requireNonNull; import static java.util.logging.Level.FINE; import static java.util.logging.Level.WARNING; import static net.bytebuddy.dynamic.loading.ClassLoadingStrategy.BOOTSTRAP_LOADER; @@ -18,7 +19,7 @@ import io.opentelemetry.javaagent.tooling.muzzle.ReferenceMatcher; import java.security.ProtectionDomain; import java.util.List; -import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; import java.util.function.UnaryOperator; import java.util.logging.Level; import java.util.logging.Logger; @@ -39,9 +40,8 @@ class MuzzleMatcher implements AgentBuilder.RawMatcher { private final InstrumentationModule instrumentationModule; private final UnaryOperator classLoaderTransformer; private final Level muzzleLogLevel; - private final AtomicBoolean initialized = new AtomicBoolean(false); + private final AtomicReference referenceMatcher = new AtomicReference<>(); private final Cache matchCache = Cache.weak(); - private volatile ReferenceMatcher referenceMatcher; MuzzleMatcher( TransformSafeLogger instrumentationLogger, @@ -105,9 +105,11 @@ private boolean doesMatch(ClassLoader classLoader) { // ReferenceMatcher is lazily created to avoid unnecessarily loading the muzzle references from // the module during the agent setup private ReferenceMatcher getReferenceMatcher() { - if (initialized.compareAndSet(false, true)) { - referenceMatcher = ReferenceMatcher.of(instrumentationModule); + ReferenceMatcher result = referenceMatcher.get(); + if (result == null) { + referenceMatcher.compareAndSet(null, ReferenceMatcher.of(instrumentationModule)); + result = requireNonNull(referenceMatcher.get()); } - return referenceMatcher; + return result; } } diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/http/RegexUrlTemplateCustomizerInitializer.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/http/RegexUrlTemplateCustomizerInitializer.java index 6192a79e74cf..7b46b27e5d37 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/http/RegexUrlTemplateCustomizerInitializer.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/http/RegexUrlTemplateCustomizerInitializer.java @@ -17,6 +17,7 @@ import java.util.logging.Logger; import java.util.regex.Pattern; import java.util.regex.PatternSyntaxException; +import javax.annotation.Nullable; @AutoService(BeforeAgentListener.class) public final class RegexUrlTemplateCustomizerInitializer implements BeforeAgentListener { @@ -48,6 +49,7 @@ public void beforeAgent(AutoConfiguredOpenTelemetrySdk autoConfiguredOpenTelemet }); } + @Nullable private static Pattern toPattern(String patternString) { try { // ensure that pattern matches the whole url diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyBootstrap.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyBootstrap.java index 3f09dfe7fdbf..a0b547fa5e23 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyBootstrap.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyBootstrap.java @@ -124,6 +124,7 @@ private static CallSite bootstrap( () -> internalBootstrap(lookup, adviceMethodName, adviceMethodType, args)); } + @Nullable private static CallSite internalBootstrap( MethodHandles.Lookup lookup, String adviceMethodName, @@ -210,7 +211,8 @@ private static CallSite bootstrapAdvice( // Update the callsite of those to run the actual instrumentation logger.log( FINE, - "Fixing nested instrumentation invokedynamic instruction bootstrapping for instrumented class {0} and advice {1}.{2}, the instrumentation should be active now", + "Fixing nested instrumentation invokedynamic instruction bootstrapping for instrumented" + + " class {0} and advice {1}.{2}, the instrumentation should be active now", new Object[] {lookup.lookupClass().getName(), adviceClassName, adviceMethodName}); nestedBootstrapCallSite.setTarget(methodHandle); MutableCallSite.syncAll(new MutableCallSite[] {nestedBootstrapCallSite}); diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/InstrumentationModuleClassLoader.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/InstrumentationModuleClassLoader.java index c9f149938c9e..479f48633bd1 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/InstrumentationModuleClassLoader.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/InstrumentationModuleClassLoader.java @@ -72,7 +72,7 @@ public class InstrumentationModuleClassLoader extends ClassLoader { private final Map additionalInjectedClasses; private final ClassLoader agentOrExtensionCl; - private volatile MethodHandles.Lookup cachedLookup; + @Nullable private volatile MethodHandles.Lookup cachedLookup; @Nullable private final ClassLoader instrumentedCl; @@ -261,6 +261,7 @@ private boolean shouldLoadFromAgent(String dotClassName) { return true; } + @Nullable private static Class tryLoad(@Nullable ClassLoader cl, String name) { try { return Class.forName(name, false, cl); diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/VirtualFieldChecker.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/VirtualFieldChecker.java index 2ddbae84f225..07204e0f5f14 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/VirtualFieldChecker.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/VirtualFieldChecker.java @@ -7,6 +7,7 @@ import io.opentelemetry.instrumentation.api.util.VirtualField; import io.opentelemetry.javaagent.extension.instrumentation.internal.AsmApi; +import javax.annotation.Nullable; import net.bytebuddy.asm.Advice; import org.objectweb.asm.ClassReader; import org.objectweb.asm.MethodVisitor; @@ -61,6 +62,7 @@ public void visitMethodInsn( }); } + @Nullable private static AnnotationNode getAnnotationNode(MethodNode source, Type type) { if (source.visibleAnnotations != null) { for (AnnotationNode annotationNode : source.visibleAnnotations) { diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/util/ClassLoaderMap.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/util/ClassLoaderMap.java index 22b8c710be3f..68f73bc01bff 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/util/ClassLoaderMap.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/util/ClassLoaderMap.java @@ -16,6 +16,7 @@ import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.function.Supplier; +import javax.annotation.Nullable; import net.bytebuddy.ByteBuddy; import net.bytebuddy.description.modifier.Ownership; import net.bytebuddy.description.modifier.Visibility; @@ -35,6 +36,7 @@ static Injector defaultInjector() { return defaultInjector; } + @Nullable public static Object get(ClassLoader classLoader, Injector classInjector, Object key) { return getClassLoaderData(classLoader, classInjector, false).get(key); } diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/util/ClassLoaderValue.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/util/ClassLoaderValue.java index 1fa9317f2cd2..9ee537e95f46 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/util/ClassLoaderValue.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/util/ClassLoaderValue.java @@ -7,6 +7,7 @@ import io.opentelemetry.javaagent.tooling.util.ClassLoaderMap.Injector; import java.util.function.Supplier; +import javax.annotation.Nullable; /** * Associate value with a class loader. Added value will behave as if it was stored in a field in @@ -27,6 +28,7 @@ public ClassLoaderValue() { this.classInjector = classInjector; } + @Nullable @SuppressWarnings("unchecked") // we lose generic type when storing it in ClassLoaderMap public T get(ClassLoader classLoader) { return (T) ClassLoaderMap.get(classLoader, classInjector, this); diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/util/Trie.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/util/Trie.java index a5f5ab48bf9e..71231e2486d7 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/util/Trie.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/util/Trie.java @@ -22,16 +22,17 @@ static Builder builder() { * will return {@code 10}. */ @Nullable - default V getOrNull(CharSequence str) { - return getOrDefault(str, null); - } + V getOrNull(CharSequence str); /** * Returns the value associated with the longest matched prefix, or the {@code defaultValue} if * there wasn't a match. For example: for a trie containing an {@code ("abc", 10)} entry {@code * trie.getOrDefault("abcd", -1)} will return {@code 10}. */ - V getOrDefault(CharSequence str, V defaultValue); + default V getOrDefault(CharSequence str, V defaultValue) { + V result = getOrNull(str); + return result != null ? result : defaultValue; + } /** Returns {@code true} if this trie contains the prefix {@code str}. */ default boolean contains(CharSequence str) { diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/util/TrieImpl.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/util/TrieImpl.java index 97ed4768db96..a06678cf2432 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/util/TrieImpl.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/util/TrieImpl.java @@ -20,10 +20,11 @@ private TrieImpl(Node root) { this.root = root; } + @Nullable @Override - public V getOrDefault(CharSequence str, V defaultValue) { + public V getOrNull(CharSequence str) { Node node = root; - V lastMatchedValue = defaultValue; + V lastMatchedValue = null; for (int i = 0; i < str.length(); ++i) { char c = str.charAt(i); @@ -42,9 +43,9 @@ public V getOrDefault(CharSequence str, V defaultValue) { static final class Node { final char[] chars; final Node[] children; - final V value; + @Nullable final V value; - Node(char[] chars, Node[] children, V value) { + Node(char[] chars, Node[] children, @Nullable V value) { this.chars = chars; this.children = children; this.value = value; @@ -89,7 +90,7 @@ public Trie build() { static final class NodeBuilder { final Map> children = new HashMap<>(); - V value; + @Nullable V value; Node build() { int size = children.size(); diff --git a/javaagent/build.gradle.kts b/javaagent/build.gradle.kts index 435ca4ebcd88..b226797f2d85 100644 --- a/javaagent/build.gradle.kts +++ b/javaagent/build.gradle.kts @@ -10,6 +10,7 @@ plugins { id("com.github.jk1.dependency-license-report") id("otel.java-conventions") + id("otel.nullaway-conventions") id("otel.publish-conventions") id("io.opentelemetry.instrumentation.javaagent-shadowing") id("org.spdx.sbom") diff --git a/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/HelperInjector.java b/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/HelperInjector.java index 70b4adb293ce..00508df65829 100644 --- a/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/HelperInjector.java +++ b/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/HelperInjector.java @@ -124,10 +124,14 @@ public HelperInjector( String requestingName, List helperClassNames, List helperResources, - ClassLoader helpersSource, + @Nullable ClassLoader helpersSource, @Nullable Instrumentation instrumentation) { this.requestingName = requestingName; + if (!helperClassNames.isEmpty()) { + requireNonNull( + helpersSource, "helpersSource must not be null when helperClassNames is non-empty"); + } List helpers = helperClassNames.stream() .distinct()