Allow transforming classes with missing field types (#8393)

javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/AgentInstaller.java

@@ -31,7 +31,6 @@ import io.opentelemetry.javaagent.extension.ignore.IgnoredTypesConfigurer;
 import io.opentelemetry.javaagent.tooling.asyncannotationsupport.WeakRefAsyncOperationEndStrategies;
 import io.opentelemetry.javaagent.tooling.bootstrap.BootstrapPackagesBuilderImpl;
 import io.opentelemetry.javaagent.tooling.bootstrap.BootstrapPackagesConfigurer;
-import io.opentelemetry.javaagent.tooling.bytebuddy.SafeTypeStrategy;
 import io.opentelemetry.javaagent.tooling.config.AgentConfig;
 import io.opentelemetry.javaagent.tooling.config.ConfigPropertiesBridge;
 import io.opentelemetry.javaagent.tooling.config.EarlyInitAgentConfig;
@@ -59,6 +58,8 @@ import net.bytebuddy.agent.builder.ResettableClassFileTransformer;
 import net.bytebuddy.description.type.TypeDefinition;
 import net.bytebuddy.description.type.TypeDescription;
 import net.bytebuddy.dynamic.DynamicType;
+import net.bytebuddy.dynamic.VisibilityBridgeStrategy;
+import net.bytebuddy.dynamic.scaffold.InstrumentedType;
 import net.bytebuddy.dynamic.scaffold.MethodGraph;
 import net.bytebuddy.utility.JavaModule;
 
@@ -131,11 +132,12 @@ public class AgentInstaller {
         new AgentBuilder.Default(
                 // default method graph compiler inspects the class hierarchy, we don't need it, so
                 // we use a simpler and faster strategy instead
-                new ByteBuddy().with(MethodGraph.Compiler.ForDeclaredMethods.INSTANCE))
+                new ByteBuddy()
+                    .with(MethodGraph.Compiler.ForDeclaredMethods.INSTANCE)
+                    .with(VisibilityBridgeStrategy.Default.NEVER)
+                    .with(InstrumentedType.Factory.Default.FROZEN))
+            .with(AgentBuilder.TypeStrategy.Default.DECORATE)
             .disableClassFormatChanges()
-            // disableClassFormatChanges sets type strategy to TypeStrategy.Default.REDEFINE_FROZEN
-            // we'll wrap it with our own strategy
-            .with(new SafeTypeStrategy(AgentBuilder.TypeStrategy.Default.REDEFINE_FROZEN))
             .with(AgentBuilder.RedefinitionStrategy.RETRANSFORMATION)
             .with(new RedefinitionDiscoveryStrategy())
             .with(AgentBuilder.DescriptionStrategy.Default.POOL_ONLY)

javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/bytebuddy/SafeTypeStrategy.java

@@ -1,86 +0,0 @@
-/*
- * Copyright The OpenTelemetry Authors
- * SPDX-License-Identifier: Apache-2.0
- */
-
-package io.opentelemetry.javaagent.tooling.bytebuddy;
-
-import static net.bytebuddy.matcher.ElementMatchers.failSafe;
-import static net.bytebuddy.matcher.ElementMatchers.hasParameters;
-import static net.bytebuddy.matcher.ElementMatchers.hasType;
-import static net.bytebuddy.matcher.ElementMatchers.isVisibleTo;
-import static net.bytebuddy.matcher.ElementMatchers.not;
-import static net.bytebuddy.matcher.ElementMatchers.returns;
-import static net.bytebuddy.matcher.ElementMatchers.whereNone;
-
-import java.security.ProtectionDomain;
-import net.bytebuddy.ByteBuddy;
-import net.bytebuddy.agent.builder.AgentBuilder;
-import net.bytebuddy.description.method.MethodDescription;
-import net.bytebuddy.description.method.MethodList;
-import net.bytebuddy.description.type.TypeDescription;
-import net.bytebuddy.dynamic.ClassFileLocator;
-import net.bytebuddy.dynamic.DynamicType;
-import net.bytebuddy.dynamic.scaffold.inline.MethodNameTransformer;
-import net.bytebuddy.utility.JavaModule;
-
-/**
- * Wrapper for {@link AgentBuilder.TypeStrategy} that excludes methods with missing return or
- * parameter types. By default, byte-buddy fails transforming such classes.
- */
-public final class SafeTypeStrategy implements AgentBuilder.TypeStrategy {
-  private final AgentBuilder.TypeStrategy delegate;
-
-  public SafeTypeStrategy(AgentBuilder.TypeStrategy delegate) {
-    this.delegate = delegate;
-  }
-
-  @Override
-  public DynamicType.Builder<?> builder(
-      TypeDescription typeDescription,
-      ByteBuddy byteBuddy,
-      ClassFileLocator classFileLocator,
-      MethodNameTransformer methodNameTransformer,
-      ClassLoader classLoader,
-      JavaModule module,
-      ProtectionDomain protectionDomain) {
-    // type description wrapper that removes methods with missing return or parameter types
-    TypeDescription newTypeDescription =
-        new TypeDescription.AbstractBase.OfSimpleType.WithDelegation() {
-
-          @Override
-          public String getName() {
-            return delegate().getName();
-          }
-
-          @Override
-          protected TypeDescription delegate() {
-            return typeDescription;
-          }
-
-          @Override
-          public MethodList<MethodDescription.InDefinedShape> getDeclaredMethods() {
-            MethodList<MethodDescription.InDefinedShape> methodList = super.getDeclaredMethods();
-            return filterMethods(methodList, typeDescription);
-          }
-        };
-
-    return delegate.builder(
-        newTypeDescription,
-        byteBuddy,
-        classFileLocator,
-        methodNameTransformer,
-        classLoader,
-        module,
-        protectionDomain);
-  }
-
-  private static <T extends MethodDescription> MethodList<T> filterMethods(
-      MethodList<T> methodList, TypeDescription viewPoint) {
-    // filter out methods missing return or parameter types
-    return methodList.filter(
-        failSafe(
-            returns(isVisibleTo(viewPoint))
-                .and(hasParameters(whereNone(hasType(not(isVisibleTo(viewPoint))))))));
-  }
-}

javaagent-tooling/src/testMissingType/java/io/opentelemetry/javaagent/tooling/bytebuddy/MissingTypeTest.java

@@ -36,8 +36,8 @@ class MissingTypeTest {
     AgentBuilder builder =
         new AgentBuilder.Default(
                 new ByteBuddy().with(MethodGraph.Compiler.ForDeclaredMethods.INSTANCE))
+            .with(AgentBuilder.TypeStrategy.Default.DECORATE)
             .disableClassFormatChanges()
-            .with(new SafeTypeStrategy(AgentBuilder.TypeStrategy.Default.REDEFINE_FROZEN))
             .with(AgentBuilder.RedefinitionStrategy.RETRANSFORMATION)
             .with(
                 new AgentBuilder.Listener.Adapter() {
@@ -92,8 +92,10 @@ class MissingTypeTest {
   }
 
   // com.google.common.base.Joiner is missing from runtime class path
-  @SuppressWarnings({"UnusedMethod", "MethodCanBeStatic"})
+  @SuppressWarnings({"UnusedMethod", "UnusedVariable", "MethodCanBeStatic"})
   private static class SomeClass {
+    public Joiner joiner;
+
     public static boolean isInstrumented() {
       return false;
     }