Fix a bug in the field backed VirtualField implementation (#4310)

* Fix a bug in the field backed VirtualField implementation

* Multiple interface fields
This commit is contained in:
Mateusz Rzeszutek 2021-10-06 18:40:53 +02:00 committed by GitHub
parent b1493dfe8f
commit fda4779127
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 82 additions and 28 deletions

View File

@ -26,7 +26,6 @@ import io.opentelemetry.javaagent.instrumentation.api.internal.InstrumentedTaskC
import io.opentelemetry.javaagent.tooling.asyncannotationsupport.WeakRefAsyncOperationEndStrategies;
import io.opentelemetry.javaagent.tooling.bootstrap.BootstrapPackagesBuilderImpl;
import io.opentelemetry.javaagent.tooling.config.ConfigInitializer;
import io.opentelemetry.javaagent.tooling.field.FieldBackedImplementationInstaller;
import io.opentelemetry.javaagent.tooling.ignore.IgnoredClassLoadersMatcher;
import io.opentelemetry.javaagent.tooling.ignore.IgnoredTypesBuilderImpl;
import io.opentelemetry.javaagent.tooling.ignore.IgnoredTypesMatcher;
@ -133,8 +132,6 @@ public class AgentInstaller {
runBeforeAgentListeners(agentListeners, config);
FieldBackedImplementationInstaller.resetContextMatchers();
AgentBuilder agentBuilder =
new AgentBuilder.Default()
.disableClassFormatChanges()

View File

@ -49,14 +49,15 @@ final class FieldAccessorInterfacesGenerator {
String typeName, String fieldTypeName) {
// We are using Object class name instead of fieldTypeName here because this gets injected
// onto Bootstrap classloader where context class may be unavailable
TypeDescription fieldTypeDesc = new TypeDescription.ForLoadedType(Object.class);
TypeDescription fieldTypeDesc = TypeDescription.OBJECT;
return byteBuddy
.makeInterface()
.merge(SyntheticState.SYNTHETIC)
.name(getFieldAccessorInterfaceName(typeName, fieldTypeName))
.defineMethod(getRealGetterName(typeName), fieldTypeDesc, Visibility.PUBLIC)
.defineMethod(getRealGetterName(typeName, fieldTypeName), fieldTypeDesc, Visibility.PUBLIC)
.withoutCode()
.defineMethod(getRealSetterName(typeName), TypeDescription.VOID, Visibility.PUBLIC)
.defineMethod(
getRealSetterName(typeName, fieldTypeName), TypeDescription.VOID, Visibility.PUBLIC)
.withParameter(fieldTypeDesc, "value")
.withoutCode()
.make();

View File

@ -53,8 +53,7 @@ import net.bytebuddy.utility.JavaModule;
* <em>FieldBackedImplementation$VirtualField$Runnable$RunnableState12345.getVirtualField(Runnable.class,
* RunnableState.class)</em>
*/
public final class FieldBackedImplementationInstaller
implements VirtualFieldImplementationInstaller {
final class FieldBackedImplementationInstaller implements VirtualFieldImplementationInstaller {
private static final TransformSafeLogger logger =
TransformSafeLogger.getLogger(FieldBackedImplementationInstaller.class);
@ -161,13 +160,6 @@ public final class FieldBackedImplementationInstaller
private static final Set<Map.Entry<String, String>> INSTALLED_VIRTUAL_FIELD_MATCHERS =
new HashSet<>();
/** Clear set that prevents multiple matchers for same context class. */
public static void resetContextMatchers() {
synchronized (INSTALLED_VIRTUAL_FIELD_MATCHERS) {
INSTALLED_VIRTUAL_FIELD_MATCHERS.clear();
}
}
@Override
public AgentBuilder.Identified.Extendable injectFields(
AgentBuilder.Identified.Extendable builder) {

View File

@ -38,15 +38,18 @@ final class GeneratedVirtualFieldNames {
+ Utils.convertToInnerClassName(fieldTypeName);
}
static String getRealFieldName(String typeName) {
return "__opentelemetryVirtualField$" + Utils.convertToInnerClassName(typeName);
static String getRealFieldName(String typeName, String fieldTypeName) {
return "__opentelemetryVirtualField$"
+ Utils.convertToInnerClassName(typeName)
+ "$"
+ Utils.convertToInnerClassName(fieldTypeName);
}
static String getRealGetterName(String typeName) {
return "get" + getRealFieldName(typeName);
static String getRealGetterName(String typeName, String fieldTypeName) {
return "get" + getRealFieldName(typeName, fieldTypeName);
}
static String getRealSetterName(String typeName) {
return "set" + getRealFieldName(typeName);
static String getRealSetterName(String typeName, String fieldTypeName) {
return "set" + getRealFieldName(typeName, fieldTypeName);
}
}

View File

@ -67,10 +67,10 @@ final class RealFieldInjector implements AsmVisitorWrapper {
return new ClassVisitor(Opcodes.ASM7, classVisitor) {
// We are using Object class name instead of fieldTypeName here because this gets
// injected onto Bootstrap classloader where context class may be unavailable
private final TypeDescription fieldType = new TypeDescription.ForLoadedType(Object.class);
private final String fieldName = getRealFieldName(typeName);
private final String getterMethodName = getRealGetterName(typeName);
private final String setterMethodName = getRealSetterName(typeName);
private final TypeDescription fieldType = TypeDescription.OBJECT;
private final String fieldName = getRealFieldName(typeName, fieldTypeName);
private final String getterMethodName = getRealGetterName(typeName, fieldTypeName);
private final String setterMethodName = getRealSetterName(typeName, fieldTypeName);
private final TypeDescription interfaceType =
fieldAccessorInterfaces.find(typeName, fieldTypeName);
private boolean foundField = false;

View File

@ -154,7 +154,7 @@ final class VirtualFieldImplementationsGenerator {
* @param name name of the method being visited
*/
private void generateRealGetMethod(String name) {
String getterName = getRealGetterName(typeName);
String getterName = getRealGetterName(typeName, fieldTypeName);
Label elseLabel = new Label();
MethodVisitor mv = getMethodVisitor(name);
mv.visitCode();
@ -207,7 +207,7 @@ final class VirtualFieldImplementationsGenerator {
* @param name name of the method being visited
*/
private void generateRealPutMethod(String name) {
String setterName = getRealSetterName(typeName);
String setterName = getRealSetterName(typeName, fieldTypeName);
Label elseLabel = new Label();
Label endLabel = new Label();
MethodVisitor mv = getMethodVisitor(name);

View File

@ -12,6 +12,7 @@ import io.opentelemetry.instrumentation.api.field.VirtualField;
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
import library.KeyClass;
import library.KeyInterface;
import net.bytebuddy.asm.Advice;
import net.bytebuddy.description.type.TypeDescription;
import net.bytebuddy.matcher.ElementMatcher;
@ -35,6 +36,8 @@ public class ContextTestInstrumentation implements TypeInstrumentation {
named("putContextCount"), this.getClass().getName() + "$PutApiUsageAdvice");
transformer.applyAdviceToMethod(
named("removeContextCount"), this.getClass().getName() + "$RemoveApiUsageAdvice");
transformer.applyAdviceToMethod(
named("useMultipleFields"), this.getClass().getName() + "$UseMultipleFieldsAdvice");
}
@SuppressWarnings("unused")
@ -90,4 +93,20 @@ public class ContextTestInstrumentation implements TypeInstrumentation {
virtualField.set(thiz, null);
}
}
@SuppressWarnings("unused")
public static class UseMultipleFieldsAdvice {
@Advice.OnMethodExit
public static void methodExit(@Advice.This KeyClass thiz) {
VirtualField<KeyClass, Context> field1 = VirtualField.find(KeyClass.class, Context.class);
VirtualField<KeyClass, Integer> field2 = VirtualField.find(KeyClass.class, Integer.class);
VirtualField<KeyInterface, Integer> interfaceField =
VirtualField.find(KeyInterface.class, Integer.class);
Context context = field1.get(thiz);
int count = context == null ? 0 : context.count;
field2.set(thiz, count);
interfaceField.set(thiz, count);
}
}
}

View File

@ -85,6 +85,36 @@ class FieldBackedImplementationTest extends AgentInstrumentationSpecification {
UntransformableKeyClass | keyClass.getSimpleName() | false
}
def "multiple fields are injected"() {
setup:
List<Field> fields = []
for (Field field : KeyClass.getDeclaredFields()) {
if (field.getName().startsWith("__opentelemetry")) {
fields.add(field)
}
}
List<Class<?>> interfaces = []
for (Class iface : KeyClass.getInterfaces()) {
if (iface.name.startsWith('io.opentelemetry.javaagent.bootstrap.instrumentation.context.FieldBackedImplementationInstaller$VirtualFieldAccessor')) {
interfaces.add(iface)
}
}
expect:
fields.size() == 3
fields.forEach { field ->
assert Modifier.isPrivate(field.modifiers)
assert Modifier.isTransient(field.modifiers)
assert field.synthetic
}
interfaces.size() == 3
interfaces.forEach { iface ->
assert iface.synthetic
}
}
def "correct api usage stores state in map"() {
when:
instance1.incrementContextCount()

View File

@ -5,7 +5,7 @@
package library;
public class KeyClass {
public class KeyClass implements KeyInterface {
public boolean isInstrumented() {
// implementation replaced with test instrumentation
return false;
@ -28,4 +28,8 @@ public class KeyClass {
public void removeContextCount() {
// implementation replaced with test instrumentation
}
public void useMultipleFields() {
// implementation replaced with test instrumentation
}
}

View File

@ -0,0 +1,8 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/
package library;
public interface KeyInterface {}