Skip to content

Commit

Permalink
Fix redefinition failure on openj9 (open-telemetry#5009)
Browse files Browse the repository at this point in the history
* Fix redefinition failure on openj9

* isntead of remembering the list of interfaces the class has remember whether it has any of the virutal field marker interfaces

* address review comment

* ensure virtual field detection works when internal-reflection instrumentation is disabled
  • Loading branch information
laurit authored and RashmiRam committed May 23, 2022
1 parent b066264 commit c89bec2
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 8 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
plugins {
id("otel.javaagent-bootstrap")
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package io.opentelemetry.javaagent.instrumentation.internal.reflection;

import io.opentelemetry.javaagent.bootstrap.VirtualFieldAccessorMarker;
import io.opentelemetry.javaagent.bootstrap.VirtualFieldDetector;
import io.opentelemetry.javaagent.bootstrap.VirtualFieldInstalledMarker;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
Expand Down Expand Up @@ -60,16 +61,23 @@ public static Class<?>[] filterInterfaces(Class<?>[] interfaces, Class<?> contai
return interfaces;
}
List<Class<?>> result = new ArrayList<>(interfaces.length);
boolean hasVirtualFieldMarker = false;
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$"))) {
hasVirtualFieldMarker = true;
continue;
}
result.add(interfaceClass);
}

if (hasVirtualFieldMarker) {
VirtualFieldDetector.markVirtualFieldsPresent(containingClass);
}

return result.toArray(new Class<?>[0]);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.javaagent.bootstrap;

import io.opentelemetry.instrumentation.api.cache.Cache;
import java.util.Arrays;

/** Helper class for detecting whether given class has virtual fields. */
public final class VirtualFieldDetector {

private static final Cache<Class<?>, Boolean> classesWithVirtualFields = Cache.weak();

private VirtualFieldDetector() {}

/**
* Detect whether given class has virtual fields. This method looks for virtual fields only from
* the specified class not its super classes.
*
* @param clazz a class
* @return true if given class has virtual fields
*/
public static boolean hasVirtualFields(Class<?> clazz) {
// clazz.getInterfaces() needs to be called before reading from classesWithVirtualFields
// as the call to clazz.getInterfaces() triggers adding clazz to that map via instrumentation
// calling VirtualFieldDetector#markVirtualFieldsPresent() from Class#getInterfaces()
Class<?>[] interfaces = clazz.getInterfaces();
// to avoid breaking in case internal-reflection instrumentation is disabled check whether
// interfaces array contains virtual field marker interface
if (Arrays.asList(interfaces).contains(VirtualFieldInstalledMarker.class)) {
return true;
}
return classesWithVirtualFields.get(clazz) != null;
}

public static void markVirtualFieldsPresent(Class<?> clazz) {
classesWithVirtualFields.put(clazz, Boolean.TRUE);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,12 @@

import io.opentelemetry.instrumentation.api.config.Config;
import io.opentelemetry.javaagent.bootstrap.InstrumentationHolder;
import io.opentelemetry.javaagent.bootstrap.VirtualFieldInstalledMarker;
import io.opentelemetry.javaagent.bootstrap.VirtualFieldDetector;
import io.opentelemetry.javaagent.tooling.HelperInjector;
import io.opentelemetry.javaagent.tooling.TransformSafeLogger;
import io.opentelemetry.javaagent.tooling.instrumentation.InstrumentationModuleInstaller;
import io.opentelemetry.javaagent.tooling.muzzle.VirtualFieldMappings;
import java.lang.instrument.Instrumentation;
import java.util.Arrays;
import java.util.Collection;
import java.util.HashSet;
import java.util.Map;
Expand Down Expand Up @@ -216,14 +215,10 @@ private static AgentBuilder.RawMatcher safeToInjectFieldsMatcher() {
/*
* The idea here is that we can add fields if class is just being loaded
* (classBeingRedefined == null) and we have to add same fields again if class we added
* fields before is being transformed again. Note: here we assume that Class#getInterfaces()
* returns list of interfaces defined immediately on a given class, not inherited from its
* parents. It looks like current JVM implementation does exactly this but javadoc is not
* explicit about that.
* fields before is being transformed again.
*/
return classBeingRedefined == null
|| Arrays.asList(classBeingRedefined.getInterfaces())
.contains(VirtualFieldInstalledMarker.class);
|| VirtualFieldDetector.hasVirtualFields(classBeingRedefined);
};
}

Expand Down

0 comments on commit c89bec2

Please sign in to comment.