Skip to content

Commit 7434123

Browse files
tobiasstadlerfelixbarnySylvainJuge
authored
Support to selectively enable instrumentations (#2292)
* Added support to selectively enable instrumentations (#1740) * Added #2292 to the changelog * Review suggestions by @felixbarny * Added Tests for CoreConfiguration#isInstrumentationEnabled * Review suggestions by @felixbarny * Improved code Co-authored-by: Felix Barnsteiner <[email protected]> * Updated configuration.asciidoc * Improve documentation as suggested by @SylvainJuge Co-authored-by: SylvainJuge <[email protected]> * Added test for enable_experimental_instrumentations * Fixed enable_experimental_instrumentations * Updated configuration.asciidoc Co-authored-by: Felix Barnsteiner <[email protected]> Co-authored-by: SylvainJuge <[email protected]>
1 parent aa57415 commit 7434123

File tree

10 files changed

+292
-54
lines changed

10 files changed

+292
-54
lines changed

‎CHANGELOG.asciidoc‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ endif::[]
2525
2626
[float]
2727
===== Features
28+
* Added support to selectively enable instrumentations - {pull}2292[#2292]
2829
2930
[float]
3031
===== Bug fixes

‎apm-agent-core/src/main/java/co/elastic/apm/agent/bci/ElasticApmAgent.java‎

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -335,21 +335,7 @@ private static AgentBuilder initAgentBuilder(ElasticApmTracer tracer, Instrument
335335
}
336336

337337
private static boolean isIncluded(ElasticApmInstrumentation advice, CoreConfiguration coreConfiguration) {
338-
ArrayList<String> disabledInstrumentations = new ArrayList<>(coreConfiguration.getDisabledInstrumentations());
339-
// Supporting the deprecated `incubating` tag for backward compatibility
340-
if (disabledInstrumentations.contains("incubating")) {
341-
disabledInstrumentations.add("experimental");
342-
}
343-
return !isGroupDisabled(disabledInstrumentations, advice.getInstrumentationGroupNames()) && isInstrumentationEnabled(advice, coreConfiguration);
344-
}
345-
346-
private static boolean isGroupDisabled(Collection<String> disabledInstrumentations, Collection<String> instrumentationGroupNames) {
347-
for (String instrumentationGroupName : instrumentationGroupNames) {
348-
if (disabledInstrumentations.contains(instrumentationGroupName)) {
349-
return true;
350-
}
351-
}
352-
return false;
338+
return isInstrumentationEnabled(advice, coreConfiguration) && coreConfiguration.isInstrumentationEnabled(advice.getInstrumentationGroupNames());
353339
}
354340

355341
private static boolean isInstrumentationEnabled(ElasticApmInstrumentation advice, CoreConfiguration coreConfiguration) {

‎apm-agent-core/src/main/java/co/elastic/apm/agent/configuration/CoreConfiguration.java‎

Lines changed: 70 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -31,17 +31,20 @@
3131
import co.elastic.apm.agent.matcher.WildcardMatcherValueConverter;
3232
import org.stagemonitor.configuration.ConfigurationOption;
3333
import org.stagemonitor.configuration.ConfigurationOptionProvider;
34+
import org.stagemonitor.configuration.converter.AbstractValueConverter;
3435
import org.stagemonitor.configuration.converter.MapValueConverter;
36+
import org.stagemonitor.configuration.converter.SetValueConverter;
3537
import org.stagemonitor.configuration.converter.StringValueConverter;
3638
import org.stagemonitor.configuration.source.ConfigurationSource;
3739

3840
import javax.annotation.Nullable;
39-
import java.util.ArrayList;
4041
import java.util.Arrays;
4142
import java.util.Collection;
4243
import java.util.Collections;
44+
import java.util.LinkedHashSet;
4345
import java.util.List;
4446
import java.util.Map;
47+
import java.util.Set;
4548
import java.util.concurrent.TimeUnit;
4649

4750
import static co.elastic.apm.agent.configuration.validation.RangeValidator.isInRange;
@@ -232,7 +235,38 @@ public class CoreConfiguration extends ConfigurationOptionProvider {
232235
WildcardMatcher.valueOf("set-cookie")
233236
));
234237

235-
private final ConfigurationOption<Collection<String>> disabledInstrumentations = ConfigurationOption.stringsOption()
238+
private final ConfigurationOption<Collection<String>> enabledInstrumentations = ConfigurationOption.stringsOption()
239+
.key("enable_instrumentations")
240+
.configurationCategory(CORE_CATEGORY)
241+
.description("A list of instrumentations which should be selectively enabled.\n" +
242+
"Valid options are ${allInstrumentationGroupNames}.\n" +
243+
"When set to non-empty value, only listed instrumentations will be enabled if they are not disabled through <<config-disable-instrumentations>> or <<config-enable-experimental-instrumentations>>.\n" +
244+
"When not set or empty (default), all instrumentations enabled by default will be enabled unless they are disabled through <<config-disable-instrumentations>> or <<config-enable-experimental-instrumentations>>.\n" +
245+
"\n" +
246+
"NOTE: Changing this value at runtime can slow down the application temporarily.")
247+
.dynamic(true)
248+
.tags("added[1.28.0]")
249+
.buildWithDefault(Collections.<String>emptyList());
250+
251+
private final ConfigurationOption<Collection<String>> disabledInstrumentations = ConfigurationOption.builder(new AbstractValueConverter<Collection<String>>() {
252+
@Override
253+
public Collection<String> convert(String s) {
254+
Collection<String> values = SetValueConverter.STRINGS_VALUE_CONVERTER.convert(s);
255+
if (values.contains("incubating")) {
256+
Set<String> legacyValues = new LinkedHashSet<String>(values);
257+
legacyValues.add("experimental");
258+
259+
return Collections.unmodifiableSet(legacyValues);
260+
}
261+
262+
return values;
263+
}
264+
265+
@Override
266+
public String toString(Collection<String> value) {
267+
return SetValueConverter.STRINGS_VALUE_CONVERTER.toString(value);
268+
}
269+
}, Collection.class)
236270
.key("disable_instrumentations")
237271
.aliasKeys("disabled_instrumentations")
238272
.configurationCategory(CORE_CATEGORY)
@@ -636,7 +670,7 @@ public boolean isInstrument() {
636670
}
637671

638672
public List<ConfigurationOption<?>> getInstrumentationOptions() {
639-
return Arrays.asList(instrument, traceMethods, disabledInstrumentations, enableExperimentalInstrumentations);
673+
return Arrays.asList(instrument, traceMethods, enabledInstrumentations, disabledInstrumentations, enableExperimentalInstrumentations);
640674
}
641675

642676
public String getServiceName() {
@@ -687,14 +721,40 @@ public List<WildcardMatcher> getSanitizeFieldNames() {
687721
return sanitizeFieldNames.get();
688722
}
689723

690-
public Collection<String> getDisabledInstrumentations() {
691-
List<String> disabled = new ArrayList<>(disabledInstrumentations.get());
692-
if (enableExperimentalInstrumentations.get()) {
693-
disabled.remove("experimental");
694-
} else {
695-
disabled.add("experimental");
724+
public boolean isInstrumentationEnabled(String instrumentationGroupName) {
725+
final Collection<String> enabledInstrumentationGroupNames = enabledInstrumentations.get();
726+
final Collection<String> disabledInstrumentationGroupNames = disabledInstrumentations.get();
727+
return (enabledInstrumentationGroupNames.isEmpty() || enabledInstrumentationGroupNames.contains(instrumentationGroupName)) &&
728+
!disabledInstrumentationGroupNames.contains(instrumentationGroupName) &&
729+
(enableExperimentalInstrumentations.get() || !instrumentationGroupName.equals("experimental"));
730+
}
731+
732+
public boolean isInstrumentationEnabled(Collection<String> instrumentationGroupNames) {
733+
return isGroupEnabled(instrumentationGroupNames) &&
734+
!isGroupDisabled(instrumentationGroupNames);
735+
}
736+
737+
private boolean isGroupEnabled(Collection<String> instrumentationGroupNames) {
738+
final Collection<String> enabledInstrumentationGroupNames = enabledInstrumentations.get();
739+
if (enabledInstrumentationGroupNames.isEmpty()) {
740+
return true;
741+
}
742+
for (String instrumentationGroupName : instrumentationGroupNames) {
743+
if (enabledInstrumentationGroupNames.contains(instrumentationGroupName)) {
744+
return true;
745+
}
746+
}
747+
return false;
748+
}
749+
750+
private boolean isGroupDisabled(Collection<String> instrumentationGroupNames) {
751+
Collection<String> disabledInstrumentationGroupNames = disabledInstrumentations.get();
752+
for (String instrumentationGroupName : instrumentationGroupNames) {
753+
if (disabledInstrumentationGroupNames.contains(instrumentationGroupName)) {
754+
return true;
755+
}
696756
}
697-
return disabled;
757+
return !enableExperimentalInstrumentations.get() && instrumentationGroupNames.contains("experimental");
698758
}
699759

700760
public List<WildcardMatcher> getUnnestExceptions() {

‎apm-agent-core/src/test/java/co/elastic/apm/agent/bci/InstrumentationTest.java‎

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@
6060
import java.util.concurrent.Executors;
6161
import java.util.concurrent.atomic.AtomicInteger;
6262

63+
import static co.elastic.apm.agent.util.MockitoMatchers.containsValue;
6364
import static net.bytebuddy.implementation.bytecode.assign.Assigner.Typing.DYNAMIC;
6465
import static net.bytebuddy.matcher.ElementMatchers.any;
6566
import static net.bytebuddy.matcher.ElementMatchers.isConstructor;
@@ -69,6 +70,7 @@
6970
import static org.assertj.core.api.Assertions.assertThat;
7071
import static org.assertj.core.api.Assertions.assertThatThrownBy;
7172
import static org.awaitility.Awaitility.await;
73+
import static org.mockito.ArgumentMatchers.anyCollection;
7274
import static org.mockito.Mockito.doReturn;
7375

7476
class InstrumentationTest {
@@ -146,7 +148,7 @@ public String assignToReturn(String foo, String bar) {
146148

147149
@Test
148150
void testDisabled() {
149-
doReturn(Collections.singletonList("test")).when(coreConfig).getDisabledInstrumentations();
151+
doReturn(false).when(coreConfig).isInstrumentationEnabled(containsValue("test"));
150152
init(List.of(new TestInstrumentation()));
151153
assertThat(interceptMe()).isEmpty();
152154
}
@@ -176,11 +178,11 @@ void testConcurrentEnsureInstrumented() throws InterruptedException {
176178

177179
@Test
178180
void testReInitEnableOneInstrumentation() {
179-
doReturn(Collections.singletonList("test")).when(coreConfig).getDisabledInstrumentations();
181+
doReturn(false).when(coreConfig).isInstrumentationEnabled(containsValue("test"));
180182
init(List.of(new TestInstrumentation()));
181183
assertThat(interceptMe()).isEmpty();
182184

183-
doReturn(List.of()).when(coreConfig).getDisabledInstrumentations();
185+
doReturn(true).when(coreConfig).isInstrumentationEnabled(anyCollection());
184186
ElasticApmAgent.doReInitInstrumentation(List.of(new TestInstrumentation()));
185187
assertThat(interceptMe()).isEqualTo("intercepted");
186188
}
@@ -190,7 +192,7 @@ void testDefaultDisabledInstrumentation() {
190192
init(List.of(new TestInstrumentation()));
191193
assertThat(interceptMe()).isEqualTo("intercepted");
192194

193-
doReturn(Collections.singletonList("experimental")).when(coreConfig).getDisabledInstrumentations();
195+
doReturn(false).when(coreConfig).isInstrumentationEnabled(containsValue("experimental"));
194196
ElasticApmAgent.doReInitInstrumentation(List.of(new TestInstrumentation()));
195197
assertThat(interceptMe()).isEmpty();
196198
}
@@ -231,16 +233,6 @@ void testExcludedDefaultPackageFromInstrumentation() {
231233
assertThat(interceptMe()).isEmpty();
232234
}
233235

234-
@Test
235-
void testLegacyDefaultDisabledInstrumentation() {
236-
init(List.of(new TestInstrumentation()));
237-
assertThat(interceptMe()).isEqualTo("intercepted");
238-
239-
doReturn(Collections.singletonList("incubating")).when(coreConfig).getDisabledInstrumentations();
240-
ElasticApmAgent.doReInitInstrumentation(List.of(new TestInstrumentation()));
241-
assertThat(interceptMe()).isEmpty();
242-
}
243-
244236
@Test
245237
void testReInitDisableAllInstrumentations() {
246238
init(List.of(new TestInstrumentation()));

‎apm-agent-core/src/test/java/co/elastic/apm/agent/configuration/CoreConfigurationTest.java‎

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
/*
2+
* Licensed to Elasticsearch B.V. under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch B.V. licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package co.elastic.apm.agent.configuration;
20+
21+
import org.junit.jupiter.api.Test;
22+
import org.stagemonitor.configuration.ConfigurationRegistry;
23+
import org.stagemonitor.configuration.source.SimpleSource;
24+
25+
import java.util.Arrays;
26+
import java.util.Collections;
27+
28+
import static org.assertj.core.api.Assertions.assertThat;
29+
30+
class CoreConfigurationTest {
31+
32+
@Test
33+
void testWithoutDisabledAndEnabledInstrumentations() {
34+
CoreConfiguration config = getCoreConfiguration("", "");
35+
assertThat(config.isInstrumentationEnabled("foo")).isTrue();
36+
assertThat(config.isInstrumentationEnabled(Collections.singletonList("foo"))).isTrue();
37+
assertThat(config.isInstrumentationEnabled(Arrays.asList("foo", "bar"))).isTrue();
38+
}
39+
40+
@Test
41+
void testWithDisabledInstrumentations() {
42+
CoreConfiguration config = getCoreConfiguration("", "foo");
43+
assertThat(config.isInstrumentationEnabled("foo")).isFalse();
44+
assertThat(config.isInstrumentationEnabled("bar")).isTrue();
45+
assertThat(config.isInstrumentationEnabled(Collections.singletonList("foo"))).isFalse();
46+
assertThat(config.isInstrumentationEnabled(Collections.singletonList("bar"))).isTrue();
47+
assertThat(config.isInstrumentationEnabled(Arrays.asList("foo", "bar"))).isFalse();
48+
}
49+
50+
@Test
51+
void testWithEnabledInstrumentations() {
52+
CoreConfiguration config = getCoreConfiguration("foo", "");
53+
assertThat(config.isInstrumentationEnabled("foo")).isTrue();
54+
assertThat(config.isInstrumentationEnabled("bar")).isFalse();
55+
assertThat(config.isInstrumentationEnabled(Collections.singletonList("foo"))).isTrue();
56+
assertThat(config.isInstrumentationEnabled(Collections.singletonList("bar"))).isFalse();
57+
assertThat(config.isInstrumentationEnabled(Arrays.asList("foo", "bar"))).isTrue();
58+
}
59+
60+
@Test
61+
void testWithDisabledAndEnabledInstrumentations() {
62+
CoreConfiguration config = getCoreConfiguration("foo", "foo");
63+
assertThat(config.isInstrumentationEnabled("foo")).isFalse();
64+
assertThat(config.isInstrumentationEnabled("bar")).isFalse();
65+
assertThat(config.isInstrumentationEnabled(Collections.singletonList("foo"))).isFalse();
66+
assertThat(config.isInstrumentationEnabled(Collections.singletonList("bar"))).isFalse();
67+
assertThat(config.isInstrumentationEnabled(Arrays.asList("foo", "bar"))).isFalse();
68+
}
69+
70+
@Test
71+
void testWithEnabledInstrumentationsButDisabledExperimentalInstrumentations() {
72+
CoreConfiguration config = getCoreConfiguration("experimental", "");
73+
assertThat(config.isInstrumentationEnabled("experimental")).isFalse();
74+
assertThat(config.isInstrumentationEnabled(Collections.singletonList("experimental"))).isFalse();
75+
}
76+
77+
@Test
78+
void testWithEnabledInstrumentationsAndEnabledExperimentalInstrumentations() {
79+
CoreConfiguration config = ConfigurationRegistry.builder()
80+
.addOptionProvider(new CoreConfiguration())
81+
.addConfigSource(SimpleSource.forTest("enable_instrumentations", "experimental"))
82+
.addConfigSource(SimpleSource.forTest("disable_instrumentations", ""))
83+
.addConfigSource(SimpleSource.forTest("enable_experimental_instrumentations", "true"))
84+
.build()
85+
.getConfig(CoreConfiguration.class);
86+
87+
assertThat(config.isInstrumentationEnabled("experimental")).isTrue();
88+
assertThat(config.isInstrumentationEnabled(Collections.singletonList("experimental"))).isTrue();
89+
assertThat(config.isInstrumentationEnabled("foobar")).isFalse();
90+
assertThat(config.isInstrumentationEnabled(Collections.singletonList("foobar"))).isFalse();
91+
}
92+
93+
@Test
94+
void testLegacyDefaultDisabledInstrumentation() {
95+
CoreConfiguration config = getCoreConfiguration("", "incubating");
96+
assertThat(config.isInstrumentationEnabled("experimental")).isFalse();
97+
assertThat(config.isInstrumentationEnabled(Collections.singletonList("experimental"))).isFalse();
98+
}
99+
100+
private static CoreConfiguration getCoreConfiguration(String enabledInstrumentations, String disabledInstrumentations) {
101+
ConfigurationRegistry configurationRegistry = ConfigurationRegistry.builder()
102+
.addOptionProvider(new CoreConfiguration())
103+
.addConfigSource(SimpleSource.forTest("enable_instrumentations", enabledInstrumentations))
104+
.addConfigSource(SimpleSource.forTest("disable_instrumentations", disabledInstrumentations))
105+
.build();
106+
107+
return configurationRegistry.getConfig(CoreConfiguration.class);
108+
}
109+
}

‎apm-agent-core/src/test/java/co/elastic/apm/agent/util/MockitoMatchers.java‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
/*
2+
* Licensed to Elasticsearch B.V. under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch B.V. licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package co.elastic.apm.agent.util;
20+
21+
import org.mockito.ArgumentMatcher;
22+
import org.mockito.internal.progress.ThreadSafeMockingProgress;
23+
24+
import java.util.Collection;
25+
26+
import static java.util.Collections.emptyList;
27+
28+
public class MockitoMatchers {
29+
30+
private static class ContainsValue<T> implements ArgumentMatcher<Collection<T>> {
31+
32+
private final T value;
33+
34+
public ContainsValue(T value) {
35+
this.value = value;
36+
}
37+
38+
@Override
39+
public boolean matches(Collection<T> collection) {
40+
return collection.contains(value);
41+
}
42+
}
43+
44+
public static <T> Collection<T> containsValue(T t) {
45+
ThreadSafeMockingProgress.mockingProgress().getArgumentMatcherStorage().reportMatcher(new ContainsValue(t));
46+
return emptyList();
47+
}
48+
}

0 commit comments

Comments
 (0)