Skip to content

Adding class exclusion from instrumentation config #1187

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
eyalkoren merged 4 commits into from
May 19, 2020
Merged

Adding class exclusion from instrumentation config #1187

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b927334
Adding class exclusing from instrumentation config
eyalkoren May 17, 2020
16d769d
Fixing Mockito typing issues
eyalkoren May 18, 2020
ab4865d
Merge remote-tracking branch 'upstream/master' into adding-class-excl…
eyalkoren May 19, 2020
59d396b
Change config name and changelog
eyalkoren May 19, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions apm-agent-core/src/main/java/co/elastic/apm/agent/bci/ElasticApmAgent.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -397,8 +397,6 @@ public static synchronized void reset() {
}

private static AgentBuilder getAgentBuilder(final ByteBuddy byteBuddy, final CoreConfiguration coreConfiguration, Logger logger, AgentBuilder.DescriptionStrategy descriptionStrategy, boolean premain) {
final List<WildcardMatcher> classesExcludedFromInstrumentation = coreConfiguration.getClassesExcludedFromInstrumentation();

AgentBuilder.LocationStrategy locationStrategy = AgentBuilder.LocationStrategy.ForClassLoader.WEAK;
if (agentJarFile != null) {
try {
Expand Down Expand Up @@ -472,7 +470,13 @@ private static AgentBuilder getAgentBuilder(final ByteBuddy byteBuddy, final Cor
.or(new ElementMatcher.Junction.AbstractBase<TypeDescription>() {
@Override
public boolean matches(TypeDescription target) {
return WildcardMatcher.anyMatch(classesExcludedFromInstrumentation, target.getName()) != null;
return WildcardMatcher.anyMatch(coreConfiguration.getDefaultClassesExcludedFromInstrumentation(), target.getName()) != null;
}
})
.or(new ElementMatcher.Junction.AbstractBase<TypeDescription>() {
@Override
public boolean matches(TypeDescription target) {
return WildcardMatcher.anyMatch(coreConfiguration.getClassesExcludedFromInstrumentation(), target.getName()) != null;
}
})
.disableClassFormatChanges();
Expand Down
17 changes: 16 additions & 1 deletion apm-agent-core/src/main/java/co/elastic/apm/agent/configuration/CoreConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -334,11 +334,22 @@ public class CoreConfiguration extends ConfigurationOptionProvider {
.builder(new ListValueConverter<>(new WildcardMatcherValueConverter()), List.class)
.key("classes_excluded_from_instrumentation")
.configurationCategory(CORE_CATEGORY)
.description("Use to exclude specific classes from being instrumented. In order to exclude entire packages, \n" +
"use wildcards, as in: `com.project.exclude.*`" +
"\n" +
WildcardMatcher.DOCUMENTATION)
.dynamic(false)
.buildWithDefault(Collections.<WildcardMatcher>emptyList());

private final ConfigurationOption<List<WildcardMatcher>> defaultClassesExcludedFromInstrumentation = ConfigurationOption
.builder(new ListValueConverter<>(new WildcardMatcherValueConverter()), List.class)
.key("default_classes_excluded_from_instrumentation")

@felixbarny felixbarny May 19, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe classes_excluded_from_instrumentation_default so that the prefix is the same? Feel free to ignore though

.configurationCategory(CORE_CATEGORY)
.tags("internal")
.description("\n" +
"\n" +
WildcardMatcher.DOCUMENTATION)
.dynamic(true)
.dynamic(false)
.buildWithDefault(Arrays.asList(
WildcardMatcher.valueOf("(?-i)org.infinispan*"),
WildcardMatcher.valueOf("(?-i)org.apache.xerces*"),
Expand Down Expand Up @@ -638,6 +649,10 @@ public List<WildcardMatcher> getClassesExcludedFromInstrumentation() {
return classesExcludedFromInstrumentation.get();
}

public List<WildcardMatcher> getDefaultClassesExcludedFromInstrumentation() {
return defaultClassesExcludedFromInstrumentation.get();
}

public List<WildcardMatcher> getMethodsExcludedFromInstrumentation() {
return methodsExcludedFromInstrumentation.get();
}
Expand Down
9 changes: 9 additions & 0 deletions apm-agent-core/src/test/java/co/elastic/apm/agent/MockTracer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,15 @@ public static synchronized MockInstrumentationSetup getOrCreateInstrumentationTr
);
}

public static synchronized void resetTracer() {
ElasticApmTracer tracer = ElasticApmInstrumentation.tracer;
if (tracer != null) {
ElasticApmAgent.reset();
tracer.stop();
ElasticApmInstrumentation.tracer = null;
}
}

/**
* Like {@link #create(ConfigurationRegistry)} but with a {@link SpyConfiguration#createSpyConfig() mocked ConfigurationRegistry}.
*
Expand Down
90 changes: 66 additions & 24 deletions apm-agent-core/src/test/java/co/elastic/apm/agent/bci/InstrumentationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import co.elastic.apm.agent.configuration.SpyConfiguration;
import co.elastic.apm.agent.impl.ElasticApmTracer;
import co.elastic.apm.agent.impl.ElasticApmTracerBuilder;
import co.elastic.apm.agent.matcher.WildcardMatcher;
import net.bytebuddy.agent.ByteBuddyAgent;
import net.bytebuddy.asm.Advice;
import net.bytebuddy.description.method.MethodDescription;
Expand All @@ -37,6 +38,7 @@
import net.bytebuddy.matcher.ElementMatchers;
import org.apache.commons.math.util.MathUtils;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.stagemonitor.configuration.ConfigurationRegistry;

Expand All @@ -50,79 +52,119 @@

class InstrumentationTest {

private final ElasticApmTracer tracer = MockTracer.create();
private ElasticApmTracer tracer;
private ConfigurationRegistry configurationRegistry;
private CoreConfiguration coreConfig;

@BeforeEach
void setup() {
tracer = MockTracer.create();
configurationRegistry = SpyConfiguration.createSpyConfig();
coreConfig = configurationRegistry.getConfig(CoreConfiguration.class);
}

@AfterEach
void afterAll() {
ElasticApmAgent.reset();
void reset() {
MockTracer.resetTracer();
}

@Test
void testIntercept() {
init(SpyConfiguration.createSpyConfig(), List.of(new TestInstrumentation()));
init(configurationRegistry, List.of(new TestInstrumentation()));
assertThat(interceptMe()).isEqualTo("intercepted");
}

@Test
void testDisabled() {
final ConfigurationRegistry config = SpyConfiguration.createSpyConfig();
when(config.getConfig(CoreConfiguration.class).getDisabledInstrumentations()).thenReturn(Collections.singletonList("test"));
init(config, List.of(new TestInstrumentation()));
when(coreConfig.getDisabledInstrumentations()).
thenReturn(Collections.singletonList("test"));
init(configurationRegistry, List.of(new TestInstrumentation()));
assertThat(interceptMe()).isEmpty();
}

@Test
void testEnsureInstrumented() {
init(SpyConfiguration.createSpyConfig(), List.of());
init(configurationRegistry, List.of());
assertThat(interceptMe()).isEmpty();
ElasticApmAgent.ensureInstrumented(getClass(), List.of(TestInstrumentation.class));
assertThat(interceptMe()).isEqualTo("intercepted");
}

@Test
void testReInitEnableOneInstrumentation() {
final ConfigurationRegistry config = SpyConfiguration.createSpyConfig();
CoreConfiguration configConfig = config.getConfig(CoreConfiguration.class);
when(configConfig.getDisabledInstrumentations()).thenReturn(Collections.singletonList("test"));
init(config, List.of(new TestInstrumentation()));
when(coreConfig.getDisabledInstrumentations()).thenReturn(Collections.singletonList("test"));
init(configurationRegistry, List.of(new TestInstrumentation()));
assertThat(interceptMe()).isEmpty();

when(configConfig.getDisabledInstrumentations()).thenReturn(List.of());
when(coreConfig.getDisabledInstrumentations()).thenReturn(List.of());
ElasticApmAgent.doReInitInstrumentation(List.of(new TestInstrumentation()));
assertThat(interceptMe()).isEqualTo("intercepted");
}

@Test
void testDefaultDisabledInstrumentation() {
final ConfigurationRegistry config = SpyConfiguration.createSpyConfig();

init(config, List.of(new TestInstrumentation()));
init(configurationRegistry, List.of(new TestInstrumentation()));
assertThat(interceptMe()).isEqualTo("intercepted");

when(config.getConfig(CoreConfiguration.class).getDisabledInstrumentations()).thenReturn(Collections.singletonList("experimental"));
when(coreConfig.getDisabledInstrumentations())
.thenReturn(Collections.singletonList("experimental"));
ElasticApmAgent.doReInitInstrumentation(List.of(new TestInstrumentation()));
assertThat(interceptMe()).isEmpty();
}

@Test
void testLegacyDefaultDisabledInstrumentation() {
final ConfigurationRegistry config = SpyConfiguration.createSpyConfig();
void testExcludedClassesFromInstrumentation() {
when(coreConfig.getClassesExcludedFromInstrumentation())
.thenReturn(List.of(WildcardMatcher.valueOf("co.elastic.apm.agent.bci.InstrumentationTest")));
init(configurationRegistry, List.of(new TestInstrumentation()));
ElasticApmAgent.doReInitInstrumentation(List.of(new TestInstrumentation()));
assertThat(interceptMe()).isEmpty();
}

@Test
void testExcludedPackageFromInstrumentation() {
when(coreConfig.getClassesExcludedFromInstrumentation())
.thenReturn(List.of(WildcardMatcher.valueOf("co.elastic.apm.agent.bci.*")));
init(configurationRegistry, List.of(new TestInstrumentation()));
ElasticApmAgent.doReInitInstrumentation(List.of(new TestInstrumentation()));
assertThat(interceptMe()).isEmpty();
}

init(config, List.of(new TestInstrumentation()));
@Test
void testExcludedDefaultClassesFromInstrumentation() {
when(coreConfig.getDefaultClassesExcludedFromInstrumentation())
.thenReturn(List.of(WildcardMatcher.valueOf("co.elastic.apm.agent.bci.InstrumentationTest")));
init(configurationRegistry, List.of(new TestInstrumentation()));
ElasticApmAgent.doReInitInstrumentation(List.of(new TestInstrumentation()));
assertThat(interceptMe()).isEmpty();
}

@Test
void testExcludedDefaultPackageFromInstrumentation() {
when(coreConfig.getDefaultClassesExcludedFromInstrumentation())
.thenReturn(List.of(WildcardMatcher.valueOf("co.elastic.apm.agent.bci.*")));
init(configurationRegistry, List.of(new TestInstrumentation()));
ElasticApmAgent.doReInitInstrumentation(List.of(new TestInstrumentation()));
assertThat(interceptMe()).isEmpty();
}

@Test
void testLegacyDefaultDisabledInstrumentation() {
init(configurationRegistry, List.of(new TestInstrumentation()));
assertThat(interceptMe()).isEqualTo("intercepted");

when(config.getConfig(CoreConfiguration.class).getDisabledInstrumentations()).thenReturn(Collections.singletonList("incubating"));
when(coreConfig.getDisabledInstrumentations())
.thenReturn(Collections.singletonList("incubating"));
ElasticApmAgent.doReInitInstrumentation(List.of(new TestInstrumentation()));
assertThat(interceptMe()).isEmpty();
}

@Test
void testReInitDisableAllInstrumentations() {
final ConfigurationRegistry config = SpyConfiguration.createSpyConfig();
init(config, List.of(new TestInstrumentation()));
init(configurationRegistry, List.of(new TestInstrumentation()));
assertThat(interceptMe()).isEqualTo("intercepted");

when(config.getConfig(CoreConfiguration.class).isInstrument()).thenReturn(false);
when(coreConfig.isInstrument()).thenReturn(false);
ElasticApmAgent.doReInitInstrumentation(List.of(new TestInstrumentation()));
assertThat(interceptMe()).isEmpty();
}
Expand Down
22 changes: 11 additions & 11 deletions apm-agent-core/src/test/java/co/elastic/apm/agent/impl/context/BodyProcessorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
*
* http://www.apache.org/licenses/LICENSE-2.0
*
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
Expand All @@ -36,7 +36,7 @@

import static co.elastic.apm.agent.impl.context.AbstractContext.REDACTED_CONTEXT_STRING;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.when;
import static org.mockito.Mockito.doReturn;

class BodyProcessorTest {

Expand All @@ -54,7 +54,7 @@ void setUp() {

@Test
void processBeforeReport_Transaction_EventTypeAll() {
when(config.getCaptureBody()).thenReturn(CoreConfiguration.EventType.ALL);
doReturn(CoreConfiguration.EventType.ALL).when(config).getCaptureBody();

final Transaction transaction = processTransaction();

Expand All @@ -64,7 +64,7 @@ void processBeforeReport_Transaction_EventTypeAll() {

@Test
void processBeforeReport_Transaction_EventTypeTransaction() {
when(config.getCaptureBody()).thenReturn(CoreConfiguration.EventType.TRANSACTIONS);
doReturn(CoreConfiguration.EventType.TRANSACTIONS).when(config).getCaptureBody();

final Transaction transaction = processTransaction();

Expand All @@ -74,7 +74,7 @@ void processBeforeReport_Transaction_EventTypeTransaction() {

@Test
void processBeforeReport_Transaction_EventTypeError() {
when(config.getCaptureBody()).thenReturn(CoreConfiguration.EventType.ERRORS);
doReturn(CoreConfiguration.EventType.ERRORS).when(config).getCaptureBody();

final Transaction transaction = processTransaction();

Expand All @@ -84,7 +84,7 @@ void processBeforeReport_Transaction_EventTypeError() {

@Test
void processBeforeReport_Transaction_EventTypeOff() {
when(config.getCaptureBody()).thenReturn(CoreConfiguration.EventType.OFF);
doReturn(CoreConfiguration.EventType.OFF).when(config).getCaptureBody();

final Transaction transaction = processTransaction();

Expand All @@ -94,7 +94,7 @@ void processBeforeReport_Transaction_EventTypeOff() {

@Test
void processBeforeReport_Error_EventTypeAll() {
when(config.getCaptureBody()).thenReturn(CoreConfiguration.EventType.ALL);
doReturn(CoreConfiguration.EventType.ALL).when(config).getCaptureBody();

final ErrorCapture error = processError();

Expand All @@ -104,7 +104,7 @@ void processBeforeReport_Error_EventTypeAll() {

@Test
void processBeforeReport_Error_EventTypeTransaction() {
when(config.getCaptureBody()).thenReturn(CoreConfiguration.EventType.TRANSACTIONS);
doReturn(CoreConfiguration.EventType.TRANSACTIONS).when(config).getCaptureBody();

final ErrorCapture error = processError();

Expand All @@ -114,7 +114,7 @@ void processBeforeReport_Error_EventTypeTransaction() {

@Test
void processBeforeReport_Error_EventTypeError() {
when(config.getCaptureBody()).thenReturn(CoreConfiguration.EventType.ERRORS);
doReturn(CoreConfiguration.EventType.ERRORS).when(config).getCaptureBody();

final ErrorCapture error = processError();

Expand All @@ -124,7 +124,7 @@ void processBeforeReport_Error_EventTypeError() {

@Test
void processBeforeReport_Error_EventTypeOff() {
when(config.getCaptureBody()).thenReturn(CoreConfiguration.EventType.OFF);
doReturn(CoreConfiguration.EventType.OFF).when(config).getCaptureBody();

final ErrorCapture error = processError();

Expand Down
2 changes: 1 addition & 1 deletion ...m-grpc-plugin/src/test/java/co/elastic/apm/agent/grpc/AbstractGrpcContextHeadersTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ void simpleClientContextPropagation() {
Transaction transaction2 = transactions.stream()
.filter((t) -> !t.equals(transaction1))
.findFirst()
.orElseThrow();
.orElseThrow(() -> null);

Span span = reporter.getFirstSpan();

Expand Down
3 changes: 2 additions & 1 deletion ...t-plugins/apm-jms-plugin/src/test/java/co/elastic/apm/agent/jms/JmsInstrumentationIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@
import static java.util.concurrent.TimeUnit.MILLISECONDS;
import static org.assertj.core.api.Assertions.assertThat;
import static org.awaitility.Awaitility.await;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.when;

@RunWith(Parameterized.class)
Expand Down Expand Up @@ -125,7 +126,7 @@ public void startTransaction() throws Exception {
startAndActivateTransaction(null);
brokerFacade.beforeTest();
noopQ = brokerFacade.createQueue("NOOP");
when(coreConfiguration.getCaptureBody()).thenReturn(CoreConfiguration.EventType.ALL);
doReturn(CoreConfiguration.EventType.ALL).when(coreConfiguration).getCaptureBody();
}

private void startAndActivateTransaction(@Nullable Sampler sampler) {
Expand Down
5 changes: 3 additions & 2 deletions ...n/apm-kafka-base-plugin/src/test/java/co/elastic/apm/agent/kafka/KafkaLegacyClientIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,12 @@
import static java.util.concurrent.TimeUnit.MILLISECONDS;
import static org.assertj.core.api.Assertions.assertThat;
import static org.awaitility.Awaitility.await;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.when;

/**
* This test is disabled because may fail on CI, maybe because of running in parallel to the current client test.
* It is still useful to be ran locally to test the legacy client.
* It is still useful to run locally to test the legacy client.
* <p>
* Each test sends a message to a request topic and waits on a reply message. This serves two purposes:
* 1. reduce waits to a minimum within tests
Expand Down Expand Up @@ -210,7 +211,7 @@ public void testSendTwoRecords_PartiallyIterate() {

@Test
public void testBodyCaptureEnabled() {
when(coreConfiguration.getCaptureBody()).thenReturn(CoreConfiguration.EventType.ALL);
doReturn(CoreConfiguration.EventType.ALL).when(coreConfiguration).getCaptureBody();
testScenario = TestScenario.BODY_CAPTURE_ENABLED;
consumerThread.setIterationMode(RecordIterationMode.ITERABLE_FOR);
sendTwoRecordsAndConsumeReplies();
Expand Down
3 changes: 2 additions & 1 deletion ...fka-plugin/apm-kafka-headers-plugin/src/test/java/co/elastic/apm/agent/kafka/KafkaIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
import static java.util.concurrent.TimeUnit.MILLISECONDS;
import static org.assertj.core.api.Assertions.assertThat;
import static org.awaitility.Awaitility.await;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.when;

/**
Expand Down Expand Up @@ -230,7 +231,7 @@ public void testSendTwoRecords_PartiallyIterate() {

@Test
public void testBodyCaptureEnabled() {
when(coreConfiguration.getCaptureBody()).thenReturn(CoreConfiguration.EventType.ALL);
doReturn(CoreConfiguration.EventType.ALL).when(coreConfiguration).getCaptureBody();
testScenario = TestScenario.BODY_CAPTURE_ENABLED;
consumerThread.setIterationMode(RecordIterationMode.ITERABLE_FOR);
sendTwoRecordsAndConsumeReplies();
Expand Down
Loading