From 9a1bc179cf1a5690522665c94e6ae7f981d4ba66 Mon Sep 17 00:00:00 2001 From: EddeCCC Date: Wed, 11 Sep 2024 10:59:28 +0200 Subject: [PATCH] extend ClassInstrumentationConfiguration (wip) --- .../cache/process/BatchInstrumenter.java | 9 ++++-- .../instrumentation/InstrumentationState.java | 21 +++++++++--- .../ClassInstrumentationConfiguration.java | 32 +++++++++++++++---- .../model/InstrumentationScope.java | 32 +++++++++++++++++++ .../agent/resolver/ConfigurationResolver.java | 26 ++++++++++++++- .../agent/resolver/scope/ScopeResolver.java | 20 ++++++++++-- .../transformation/DynamicTransformer.java | 4 ++- .../DynamicTransformerTest.java | 2 +- 8 files changed, 127 insertions(+), 19 deletions(-) create mode 100644 src/main/java/rocks/inspectit/gepard/agent/internal/instrumentation/model/InstrumentationScope.java diff --git a/src/main/java/rocks/inspectit/gepard/agent/instrumentation/cache/process/BatchInstrumenter.java b/src/main/java/rocks/inspectit/gepard/agent/instrumentation/cache/process/BatchInstrumenter.java index fe25bcd..92a2780 100644 --- a/src/main/java/rocks/inspectit/gepard/agent/instrumentation/cache/process/BatchInstrumenter.java +++ b/src/main/java/rocks/inspectit/gepard/agent/instrumentation/cache/process/BatchInstrumenter.java @@ -7,6 +7,7 @@ import org.slf4j.LoggerFactory; import rocks.inspectit.gepard.agent.instrumentation.cache.PendingClassesCache; import rocks.inspectit.gepard.agent.internal.instrumentation.InstrumentationState; +import rocks.inspectit.gepard.agent.internal.instrumentation.model.ClassInstrumentationConfiguration; import rocks.inspectit.gepard.agent.internal.schedule.NamedRunnable; import rocks.inspectit.gepard.agent.resolver.ConfigurationResolver; @@ -70,10 +71,12 @@ Set> getNextBatch(int batchSize) { checkedClassesCount++; try { - boolean shouldInstrument = configurationResolver.shouldInstrument(clazz); - boolean isInstrumented = instrumentationState.isInstrumented(clazz); + ClassInstrumentationConfiguration instrumentationConfiguration = + configurationResolver.getActiveConfiguration(clazz); + boolean shouldRetransform = + instrumentationState.shouldRetransform(clazz, instrumentationConfiguration); - if (shouldInstrument != isInstrumented) classesToRetransform.add(clazz); + if (shouldRetransform) classesToRetransform.add(clazz); } catch (Exception e) { log.error("Could not check instrumentation status for {}", clazz.getName(), e); } diff --git a/src/main/java/rocks/inspectit/gepard/agent/internal/instrumentation/InstrumentationState.java b/src/main/java/rocks/inspectit/gepard/agent/internal/instrumentation/InstrumentationState.java index b319455..f35d578 100644 --- a/src/main/java/rocks/inspectit/gepard/agent/internal/instrumentation/InstrumentationState.java +++ b/src/main/java/rocks/inspectit/gepard/agent/internal/instrumentation/InstrumentationState.java @@ -25,6 +25,19 @@ public static InstrumentationState create() { return new InstrumentationState(); } + public boolean shouldRetransform( + Class clazz, ClassInstrumentationConfiguration currentConfig) { + ClassInstrumentationConfiguration activeConfig = + activeInstrumentations.asMap().entrySet().stream() + .filter(entry -> entry.getKey().isEqualTo(clazz)) // find class + .map(Map.Entry::getValue) // get configuration + .findAny() + .orElse(null); + + if (Objects.nonNull(activeConfig)) return !activeConfig.equals(currentConfig); + return currentConfig.isActive(); + } + /** * Checks, if the provided class is already instrumented. * @@ -39,7 +52,7 @@ public boolean isInstrumented(Class clazz) { .findAny() .orElse(null); - if (Objects.nonNull(activeConfig)) return activeConfig.isInstrumented(); + // if (Objects.nonNull(activeConfig)) return activeConfig.isInstrumented(); return false; } @@ -53,7 +66,7 @@ public boolean isInstrumented(InstrumentedType instrumentedType) { ClassInstrumentationConfiguration activeConfig = activeInstrumentations.getIfPresent(instrumentedType); - if (Objects.nonNull(activeConfig)) return activeConfig.isInstrumented(); + // if (Objects.nonNull(activeConfig)) return activeConfig.isInstrumented(); return false; } @@ -62,8 +75,8 @@ public boolean isInstrumented(InstrumentedType instrumentedType) { * * @param type the instrumented type */ - public void addInstrumentedType(InstrumentedType type) { - ClassInstrumentationConfiguration newConfig = new ClassInstrumentationConfiguration(true); + public void addInstrumentedType( + InstrumentedType type, ClassInstrumentationConfiguration newConfig) { activeInstrumentations.put(type, newConfig); } diff --git a/src/main/java/rocks/inspectit/gepard/agent/internal/instrumentation/model/ClassInstrumentationConfiguration.java b/src/main/java/rocks/inspectit/gepard/agent/internal/instrumentation/model/ClassInstrumentationConfiguration.java index 93d345f..a299f5a 100644 --- a/src/main/java/rocks/inspectit/gepard/agent/internal/instrumentation/model/ClassInstrumentationConfiguration.java +++ b/src/main/java/rocks/inspectit/gepard/agent/internal/instrumentation/model/ClassInstrumentationConfiguration.java @@ -1,19 +1,37 @@ package rocks.inspectit.gepard.agent.internal.instrumentation.model; +import java.util.Collections; +import java.util.Objects; +import java.util.Set; + /** * Stores the instrumentation configuration for a specific class. Currently, a class can only be * instrumented or not. Later, we could add a list of active rules for example. */ -public class ClassInstrumentationConfiguration { +public record ClassInstrumentationConfiguration(Set activeScopes) { + + /** The configuration representing that no instrumentation of the class if performed. */ + public static final ClassInstrumentationConfiguration NO_INSTRUMENTATION = + new ClassInstrumentationConfiguration(Collections.emptySet()); - /** Currently, only true */ - private final boolean isInstrumented; + @Override + public boolean equals(Object o) { + if (o instanceof ClassInstrumentationConfiguration otherConfig) + return activeScopes.equals(otherConfig.activeScopes); + return false; + } - public ClassInstrumentationConfiguration(boolean isInstrumented) { - this.isInstrumented = isInstrumented; + @Override + public int hashCode() { + return Objects.hash(activeScopes); } - public boolean isInstrumented() { - return isInstrumented; + /** + * Checks, if this configuration induces bytecode changes to the target class. + * + * @return true, if this configuration expects instrumentation + */ + public boolean isActive() { + return !activeScopes.isEmpty(); } } diff --git a/src/main/java/rocks/inspectit/gepard/agent/internal/instrumentation/model/InstrumentationScope.java b/src/main/java/rocks/inspectit/gepard/agent/internal/instrumentation/model/InstrumentationScope.java new file mode 100644 index 0000000..1730bfd --- /dev/null +++ b/src/main/java/rocks/inspectit/gepard/agent/internal/instrumentation/model/InstrumentationScope.java @@ -0,0 +1,32 @@ +package rocks.inspectit.gepard.agent.internal.instrumentation.model; + +import java.util.List; +import java.util.Objects; +import rocks.inspectit.gepard.agent.internal.configuration.model.instrumentation.Scope; + +/** + * @param fqn + * @param methods + */ +public record InstrumentationScope(String fqn, List methods) { + + /** + * @param scope + * @return + */ + public static InstrumentationScope create(Scope scope) { + return new InstrumentationScope(scope.getFqn(), scope.getMethods()); + } + + @Override + public boolean equals(Object o) { + if (o instanceof InstrumentationScope otherScope) + return fqn.equals(otherScope.fqn) && methods.equals(otherScope.methods); + return false; + } + + @Override + public int hashCode() { + return Objects.hash(fqn, methods); + } +} diff --git a/src/main/java/rocks/inspectit/gepard/agent/resolver/ConfigurationResolver.java b/src/main/java/rocks/inspectit/gepard/agent/resolver/ConfigurationResolver.java index 09ab8e9..9dbde22 100644 --- a/src/main/java/rocks/inspectit/gepard/agent/resolver/ConfigurationResolver.java +++ b/src/main/java/rocks/inspectit/gepard/agent/resolver/ConfigurationResolver.java @@ -1,9 +1,12 @@ package rocks.inspectit.gepard.agent.resolver; +import java.util.Set; import net.bytebuddy.description.method.MethodDescription; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; import rocks.inspectit.gepard.agent.internal.configuration.model.instrumentation.InstrumentationConfiguration; +import rocks.inspectit.gepard.agent.internal.instrumentation.model.ClassInstrumentationConfiguration; +import rocks.inspectit.gepard.agent.internal.instrumentation.model.InstrumentationScope; import rocks.inspectit.gepard.agent.resolver.scope.ScopeResolver; /** @@ -27,6 +30,27 @@ public static ConfigurationResolver create(ConfigurationHolder holder) { return new ConfigurationResolver(holder); } + /** + * @param clazz + * @return + */ + public ClassInstrumentationConfiguration getActiveConfiguration(Class clazz) { + String className = clazz.getName(); + Set activeScopes = scopeResolver.getActiveScopes(className); + + if (activeScopes.isEmpty()) return ClassInstrumentationConfiguration.NO_INSTRUMENTATION; + return new ClassInstrumentationConfiguration(activeScopes); + } + + public ClassInstrumentationConfiguration getActiveConfiguration(TypeDescription typeDescription) { + String className = typeDescription.getName(); + Set activeScopes = scopeResolver.getActiveScopes(className); + + // TODO Das sollte eigtl nicht passieren können? + if (activeScopes.isEmpty()) return ClassInstrumentationConfiguration.NO_INSTRUMENTATION; + return new ClassInstrumentationConfiguration(activeScopes); + } + /** * Checks, if the provided class requires instrumentation. * @@ -34,6 +58,7 @@ public static ConfigurationResolver create(ConfigurationHolder holder) { * @return true, if the provided class should be instrumented */ public boolean shouldInstrument(Class clazz) { + // TODO REMOVE String className = clazz.getName(); return shouldInstrument(className); } @@ -57,7 +82,6 @@ public boolean shouldInstrument(TypeDescription type) { */ public ElementMatcher.Junction getMethodMatcher( TypeDescription typeDescription) { - return scopeResolver.buildMethodMatcher(typeDescription); } diff --git a/src/main/java/rocks/inspectit/gepard/agent/resolver/scope/ScopeResolver.java b/src/main/java/rocks/inspectit/gepard/agent/resolver/scope/ScopeResolver.java index 1b8c0c6..15a0a47 100644 --- a/src/main/java/rocks/inspectit/gepard/agent/resolver/scope/ScopeResolver.java +++ b/src/main/java/rocks/inspectit/gepard/agent/resolver/scope/ScopeResolver.java @@ -1,13 +1,17 @@ package rocks.inspectit.gepard.agent.resolver.scope; import java.util.Collection; +import java.util.Collections; import java.util.List; +import java.util.Set; +import java.util.stream.Collectors; import net.bytebuddy.description.method.MethodDescription; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; import net.bytebuddy.matcher.ElementMatchers; import rocks.inspectit.gepard.agent.internal.configuration.model.instrumentation.InstrumentationConfiguration; import rocks.inspectit.gepard.agent.internal.configuration.model.instrumentation.Scope; +import rocks.inspectit.gepard.agent.internal.instrumentation.model.InstrumentationScope; import rocks.inspectit.gepard.agent.resolver.ConfigurationHolder; import rocks.inspectit.gepard.agent.resolver.matcher.CustomElementMatchers; import rocks.inspectit.gepard.agent.resolver.matcher.MatcherChainBuilder; @@ -24,6 +28,20 @@ public ScopeResolver(ConfigurationHolder holder) { this.holder = holder; } + /** + * @param fullyQualifiedName + * @return + */ + public Set getActiveScopes(String fullyQualifiedName) { + if (shouldIgnore(fullyQualifiedName)) return Collections.emptySet(); + + List scopes = getAllMatchingScopes(fullyQualifiedName); + return scopes.stream() + .filter(Scope::isEnabled) + .map(InstrumentationScope::create) + .collect(Collectors.toSet()); + } + /** * Checks, if the provided class name requires instrumentation. * @@ -31,7 +49,6 @@ public ScopeResolver(ConfigurationHolder holder) { * @return true, if the provided class should be instrumented */ public boolean shouldInstrument(String fullyQualifiedName) { - List scopes = getScopes(); return !shouldIgnore(fullyQualifiedName) @@ -47,7 +64,6 @@ public boolean shouldInstrument(String fullyQualifiedName) { */ public ElementMatcher.Junction buildMethodMatcher( TypeDescription typeDescription) { - List scopes = getAllMatchingScopes(typeDescription.getName()); if (containsAllMethodsScope(scopes)) { diff --git a/src/main/java/rocks/inspectit/gepard/agent/transformation/DynamicTransformer.java b/src/main/java/rocks/inspectit/gepard/agent/transformation/DynamicTransformer.java index cb77915..9e2c824 100644 --- a/src/main/java/rocks/inspectit/gepard/agent/transformation/DynamicTransformer.java +++ b/src/main/java/rocks/inspectit/gepard/agent/transformation/DynamicTransformer.java @@ -12,6 +12,7 @@ import org.slf4j.LoggerFactory; import rocks.inspectit.gepard.agent.internal.instrumentation.InstrumentationState; import rocks.inspectit.gepard.agent.internal.instrumentation.InstrumentedType; +import rocks.inspectit.gepard.agent.internal.instrumentation.model.ClassInstrumentationConfiguration; import rocks.inspectit.gepard.agent.resolver.ConfigurationResolver; import rocks.inspectit.gepard.agent.transformation.advice.InspectitAdvice; @@ -60,7 +61,8 @@ public DynamicType.Builder transform( builder = builder.visit(Advice.to(InspectitAdvice.class).on(methodMatcher)); // Mark type as instrumented - instrumentationState.addInstrumentedType(currentType); + ClassInstrumentationConfiguration config = resolver.getActiveConfiguration(typeDescription); + instrumentationState.addInstrumentedType(currentType, config); } else if (instrumentationState.isInstrumented(currentType)) { log.debug("Removing transformation from {}", typeDescription.getName()); // Mark type as uninstrumented or deinstrumented diff --git a/src/test/java/rocks/inspectit/gepard/agent/transformation/DynamicTransformerTest.java b/src/test/java/rocks/inspectit/gepard/agent/transformation/DynamicTransformerTest.java index 03434ad..b0d5e8e 100644 --- a/src/test/java/rocks/inspectit/gepard/agent/transformation/DynamicTransformerTest.java +++ b/src/test/java/rocks/inspectit/gepard/agent/transformation/DynamicTransformerTest.java @@ -33,7 +33,7 @@ void testTransformTransformsOnShouldInstrumentTrue() { transformer.transform(builder, typeDescription, TEST_CLASS.getClassLoader(), null, null); verify(builder).visit(any()); - verify(instrumentationState).addInstrumentedType(any()); + verify(instrumentationState).addInstrumentedType(any(), any()); } @Test