Remove virtual field interfaces from reflection results

instrumentation/internal/internal-reflection/javaagent-integration-tests/build.gradle.kts

@@ -4,4 +4,5 @@ plugins {
 
 dependencies {
   testInstrumentation(project(":instrumentation:internal:internal-reflection:javaagent"))
+  testCompileOnly(project(":javaagent-bootstrap"))
 }

instrumentation/internal/internal-reflection/javaagent-integration-tests/src/test/groovy/ReflectionTest.groovy

@@ -4,7 +4,8 @@
  */
 
 import io.opentelemetry.instrumentation.test.AgentInstrumentationSpecification
-
+import io.opentelemetry.javaagent.bootstrap.VirtualFieldAccessorMarker
+import io.opentelemetry.javaagent.bootstrap.VirtualFieldInstalledMarker
 import java.lang.reflect.Field
 import java.lang.reflect.Method
 
@@ -37,16 +38,20 @@ class ReflectionTest extends AgentInstrumentationSpecification {
     methodFound == false
 
     and:
-    def interfaceClass = TestClass.getInterfaces().find {
-      it.getName().contains("VirtualFieldAccessor\$")
-    }
-    interfaceClass != null
-    def interfaceMethodFound = false
-    for (Method method : interfaceClass.getDeclaredMethods()) {
-      if (method.getName().contains("__opentelemetry")) {
-        interfaceMethodFound = true
-      }
-    }
-    interfaceMethodFound == false
+    // although marker interfaces are removed from getInterfaces() result class is still assignable
+    // to them
+    VirtualFieldInstalledMarker.isAssignableFrom(TestClass)
+    VirtualFieldAccessorMarker.isAssignableFrom(TestClass)
+    TestClass.getInterfaces().length == 2
+    TestClass.getInterfaces() == [Runnable, Serializable]
+  }
+
+  def "test generated serialVersionUID"() {
+    // expected value is computed with serialver utility that comes with jdk
+    expect:
+    ObjectStreamClass.lookup(TestClass).getSerialVersionUID() == -1508684692096503670L
+
+    and:
+    TestClass.getDeclaredFields().length == 0
   }
 }

instrumentation/internal/internal-reflection/javaagent-integration-tests/src/test/java/TestClass.java

@@ -3,7 +3,9 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-public class TestClass implements Runnable {
+import java.io.Serializable;
+
+public class TestClass implements Runnable, Serializable {
 
   @Override
   public void run() {}

instrumentation/internal/internal-reflection/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/reflection/ClassInstrumentation.java

@@ -0,0 +1,105 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.instrumentation.internal.reflection;
+
+import static net.bytebuddy.matcher.ElementMatchers.named;
+
+import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
+import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
+import net.bytebuddy.asm.AsmVisitorWrapper;
+import net.bytebuddy.description.field.FieldDescription;
+import net.bytebuddy.description.field.FieldList;
+import net.bytebuddy.description.method.MethodList;
+import net.bytebuddy.description.type.TypeDescription;
+import net.bytebuddy.implementation.Implementation;
+import net.bytebuddy.matcher.ElementMatcher;
+import net.bytebuddy.pool.TypePool;
+import org.objectweb.asm.ClassVisitor;
+import org.objectweb.asm.ClassWriter;
+import org.objectweb.asm.MethodVisitor;
+import org.objectweb.asm.Opcodes;
+import org.objectweb.asm.Type;
+
+public class ClassInstrumentation implements TypeInstrumentation {
+  @Override
+  public ElementMatcher<TypeDescription> typeMatcher() {
+    return named("java.lang.Class");
+  }
+
+  @Override
+  public void transform(TypeTransformer transformer) {
+    transformer.applyTransformer(
+        (builder, typeDescription, classLoader, module) ->
+            builder.visit(
+                new AsmVisitorWrapper() {
+                  @Override
+                  public int mergeWriter(int flags) {
+                    return flags | ClassWriter.COMPUTE_MAXS;
+                  }
+
+                  @Override
+                  public int mergeReader(int flags) {
+                    return flags;
+                  }
+
+                  @Override
+                  public ClassVisitor wrap(
+                      TypeDescription instrumentedType,
+                      ClassVisitor classVisitor,
+                      Implementation.Context implementationContext,
+                      TypePool typePool,
+                      FieldList<FieldDescription.InDefinedShape> fields,
+                      MethodList<?> methods,
+                      int writerFlags,
+                      int readerFlags) {
+                    return new ClassClassVisitor(classVisitor);
+                  }
+                }));
+  }
+
+  private static class ClassClassVisitor extends ClassVisitor {
+
+    ClassClassVisitor(ClassVisitor cv) {
+      super(Opcodes.ASM7, cv);
+    }
+
+    @Override
+    public MethodVisitor visitMethod(
+        int access, String name, String descriptor, String signature, String[] exceptions) {
+      MethodVisitor mv = super.visitMethod(access, name, descriptor, signature, exceptions);
+      if ("getInterfaces".equals(name)
+          && ("()[Ljava/lang/Class;".equals(descriptor)
+              || "(Z)[Ljava/lang/Class;".equals(descriptor))) {
+        mv =
+            new MethodVisitor(api, mv) {
+              @Override
+              public void visitMethodInsn(
+                  int opcode, String owner, String name, String descriptor, boolean isInterface) {
+                super.visitMethodInsn(opcode, owner, name, descriptor, isInterface);
+                // filter the result of call to getInterfaces0, which is used on hotspot, and
+                // J9VMInternals.getInterfaces which is used on openj9
+                if (((opcode == Opcodes.INVOKEVIRTUAL || opcode == Opcodes.INVOKESPECIAL)
+                        && "getInterfaces0".equals(name)
+                        && "()[Ljava/lang/Class;".equals(descriptor))
+                    || (opcode == Opcodes.INVOKESTATIC
+                        && "getInterfaces".equals(name)
+                        && "java/lang/J9VMInternals".equals(owner)
+                        && "(Ljava/lang/Class;)[Ljava/lang/Class;".equals(descriptor))) {
+                  mv.visitVarInsn(Opcodes.ALOAD, 0);
+                  mv.visitMethodInsn(
+                      Opcodes.INVOKESTATIC,
+                      Type.getInternalName(ReflectionHelper.class),
+                      "filterInterfaces",
+                      "([Ljava/lang/Class;Ljava/lang/Class;)[Ljava/lang/Class;",
+                      false);
+                }
+              }
+            };
+      }
+      return mv;
+    }
+  }
+}

instrumentation/internal/internal-reflection/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/reflection/ReflectionHelper.java

@@ -5,6 +5,7 @@
 
 package io.opentelemetry.javaagent.instrumentation.internal.reflection;
 
+import io.opentelemetry.javaagent.bootstrap.VirtualFieldAccessorMarker;
 import io.opentelemetry.javaagent.bootstrap.VirtualFieldInstalledMarker;
 import java.lang.reflect.Field;
 import java.lang.reflect.Method;
@@ -18,6 +19,7 @@ private ReflectionHelper() {}
   public static Field[] filterFields(Class<?> containingClass, Field[] fields) {
     if (fields.length == 0
         || !VirtualFieldInstalledMarker.class.isAssignableFrom(containingClass)) {
+      // nothing to filter when class does not have any added virtual fields
       return fields;
     }
     List<Field> result = new ArrayList<>(fields.length);
@@ -36,6 +38,7 @@ public static Method[] filterMethods(Class<?> containingClass, Method[] methods)
       return methods;
     } else if (containingClass.isInterface()
         && containingClass.isSynthetic()
+        && VirtualFieldAccessorMarker.class.isAssignableFrom(containingClass)
         && containingClass.getName().contains("VirtualFieldAccessor$")) {
       // hide all methods from virtual field accessor interfaces
       return new Method[0];
@@ -55,4 +58,25 @@ public static Method[] filterMethods(Class<?> containingClass, Method[] methods)
     }
     return result.toArray(new Method[0]);
   }
+
+  @SuppressWarnings("unused")
+  public static Class<?>[] filterInterfaces(Class<?>[] interfaces, Class<?> containingClass) {
+    if (interfaces.length == 0
+        || !VirtualFieldInstalledMarker.class.isAssignableFrom(containingClass)) {
+      // nothing to filter when class does not have any added virtual fields
+      return interfaces;
+    }
+    List<Class<?>> result = new ArrayList<>(interfaces.length);
+    for (Class<?> interfaceClass : interfaces) {
+      // filter out virtual field marker and accessor interfaces
+      if (interfaceClass == VirtualFieldInstalledMarker.class
+          || (VirtualFieldAccessorMarker.class.isAssignableFrom(interfaceClass)
+              && interfaceClass.isSynthetic()
+              && interfaceClass.getName().contains("VirtualFieldAccessor$"))) {
+        continue;
+      }
+      result.add(interfaceClass);
+    }
+    return result.toArray(new Class<?>[0]);
+  }
 }

instrumentation/internal/internal-reflection/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/reflection/ReflectionIgnoredTypesConfigurer.java

@@ -17,5 +17,6 @@ public class ReflectionIgnoredTypesConfigurer implements IgnoredTypesConfigurer
   public void configure(Config config, IgnoredTypesBuilder builder) {
     builder.allowClass("jdk.internal.reflect.Reflection");
     builder.allowClass("sun.reflect.Reflection");
+    builder.allowClass("java.lang.Class");
   }
 }

instrumentation/internal/internal-reflection/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/reflection/ReflectionInstrumentationModule.java

@@ -5,7 +5,7 @@
 
 package io.opentelemetry.javaagent.instrumentation.internal.reflection;
 
-import static java.util.Collections.singletonList;
+import static java.util.Arrays.asList;
 
 import com.google.auto.service.AutoService;
 import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
@@ -26,6 +26,6 @@ public boolean defaultEnabled() {
 
   @Override
   public List<TypeInstrumentation> typeInstrumentations() {
-    return singletonList(new ReflectionInstrumentation());
+    return asList(new ClassInstrumentation(), new ReflectionInstrumentation());
   }
 }

javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/VirtualFieldAccessorMarker.java

@@ -0,0 +1,9 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.bootstrap;
+
+/** A marker interface implemented by virtual field accessor classes. */
+public interface VirtualFieldAccessorMarker {}

javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/field/FieldAccessorInterfacesGenerator.java

@@ -9,6 +9,7 @@
 import static io.opentelemetry.javaagent.tooling.field.GeneratedVirtualFieldNames.getRealGetterName;
 import static io.opentelemetry.javaagent.tooling.field.GeneratedVirtualFieldNames.getRealSetterName;
 
+import io.opentelemetry.javaagent.bootstrap.VirtualFieldAccessorMarker;
 import io.opentelemetry.javaagent.tooling.muzzle.VirtualFieldMappings;
 import java.util.HashMap;
 import java.util.Map;
@@ -54,6 +55,7 @@ private DynamicType.Unloaded<?> makeFieldAccessorInterface(
         .makeInterface()
         .merge(SyntheticState.SYNTHETIC)
         .name(getFieldAccessorInterfaceName(typeName, fieldTypeName))
+        .implement(VirtualFieldAccessorMarker.class)
         .defineMethod(
             getRealGetterName(typeName, fieldTypeName),
             fieldTypeDesc,