Skip to content

Commit e1ceda8

Browse files
labbatilabbati
committed
Improve config usage as instance based on CR
1 parent 1dc8467 commit e1ceda8

10 files changed

Lines changed: 102 additions & 63 deletions

File tree

‎dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/decorator/BaseDecoratorTest.groovy‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import io.opentracing.tag.Tags
99
import spock.lang.Shared
1010
import spock.lang.Specification
1111

12-
import static datadog.trace.agent.test.utils.ConfigUtils.withSystemProperty
1312
import static io.opentracing.log.Fields.ERROR_OBJECT
1413

1514
class BaseDecoratorTest extends Specification {
@@ -168,19 +167,24 @@ class BaseDecoratorTest extends Specification {
168167
false | true | 1.0
169168
}
170169

171-
def "test analytics rate enabled"() {
172-
when:
173-
BaseDecorator dec = withSystemProperty("dd.${integName}.analytics.enabled", "true") {
174-
withSystemProperty("dd.${integName}.analytics.sample-rate", "$sampleRate") {
175-
ConfigUtils.resetConfig()
176-
newDecorator(enabled)
177-
}
170+
def "test analytics rate enabled:#enabled, integration:#integName, sampleRate:#sampleRate"() {
171+
setup:
172+
ConfigUtils.updateConfig {
173+
System.properties.setProperty("dd.${integName}.analytics.enabled", "true")
174+
System.properties.setProperty("dd.${integName}.analytics.sample-rate", "$sampleRate")
178175
}
179176

177+
when:
178+
BaseDecorator dec = newDecorator(enabled)
179+
180180
then:
181181
dec.traceAnalyticsEnabled == expectedEnabled
182182
dec.traceAnalyticsSampleRate == (Float) expectedRate
183183

184+
cleanup:
185+
System.clearProperty("dd.${integName}.analytics.enabled")
186+
System.clearProperty("dd.${integName}.analytics.sample-rate")
187+
184188
where:
185189
enabled | integName | sampleRate | expectedEnabled | expectedRate
186190
false | "" | "" | false | 1.0

‎dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/test/DefaultInstrumenterTest.groovy‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,9 @@ class DefaultInstrumenterTest extends Specification {
8080

8181
def "configure default sys prop as #value"() {
8282
setup:
83-
System.setProperty("dd.integrations.enabled", value)
84-
ConfigUtils.resetConfig()
83+
ConfigUtils.updateConfig {
84+
System.setProperty("dd.integrations.enabled", value)
85+
}
8586
def target = new TestDefaultInstrumenter("test")
8687
target.instrument(new AgentBuilder.Default())
8788

‎dd-java-agent/instrumentation/java-concurrent/src/test/groovy/ExecutorInstrumentationTest.groovy‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,9 @@ import java.util.concurrent.TimeoutException
2626
class ExecutorInstrumentationTest extends AgentTestRunner {
2727

2828
static {
29-
ConfigUtils.makeConfigInstanceModifiable()
30-
System.setProperty("dd.trace.executors", "ExecutorInstrumentationTest\$CustomThreadPoolExecutor")
31-
ConfigUtils.resetConfig()
29+
ConfigUtils.updateConfig {
30+
System.setProperty("dd.trace.executors", "ExecutorInstrumentationTest\$CustomThreadPoolExecutor")
31+
}
3232
}
3333

3434
@Shared

‎dd-java-agent/instrumentation/slf4j-mdc/src/test/groovy/Slf4jMDCTest.groovy‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,9 @@ import java.util.concurrent.atomic.AtomicReference
99

1010
class Slf4jMDCTest extends AgentTestRunner {
1111
static {
12-
ConfigUtils.makeConfigInstanceModifiable()
13-
System.setProperty("dd.logs.injection", "true")
14-
ConfigUtils.resetConfig()
12+
ConfigUtils.updateConfig {
13+
System.setProperty("dd.logs.injection", "true")
14+
}
1515
}
1616

1717
def "mdc shows trace and span ids for active scope"() {

‎dd-java-agent/instrumentation/trace-annotation/src/test/groovy/ConfiguredTraceAnnotationsTest.groovy‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,14 @@ import dd.test.trace.annotation.SayTracedHello
55

66
import java.util.concurrent.Callable
77

8-
import static datadog.trace.agent.test.utils.ConfigUtils.withSystemProperty
98
import static datadog.trace.instrumentation.trace_annotation.TraceAnnotationsInstrumentation.DEFAULT_ANNOTATIONS
109

1110
class ConfiguredTraceAnnotationsTest extends AgentTestRunner {
1211

1312
static {
14-
ConfigUtils.makeConfigInstanceModifiable()
15-
// nr annotation not included here, so should be disabled.
16-
System.setProperty("dd.trace.annotations", "package.Class\$Name;${OuterClass.InterestingMethod.name}")
17-
ConfigUtils.resetConfig()
13+
ConfigUtils.updateConfig {
14+
System.setProperty("dd.trace.annotations", "package.Class\$Name;${OuterClass.InterestingMethod.name}")
15+
}
1816
}
1917

2018
def specCleanup() {
@@ -49,13 +47,16 @@ class ConfiguredTraceAnnotationsTest extends AgentTestRunner {
4947

5048
def "test configuration #value"() {
5149
setup:
52-
def config = withSystemProperty("dd.trace.annotations", value) {
53-
ConfigUtils.resetConfig()
54-
new TraceAnnotationsInstrumentation().additionalTraceAnnotations
50+
ConfigUtils.updateConfig {
51+
if (value) {
52+
System.properties.setProperty("dd.trace.annotations", value)
53+
} else {
54+
System.clearProperty("dd.trace.annotations")
55+
}
5556
}
5657

5758
expect:
58-
config == expected.toSet()
59+
new TraceAnnotationsInstrumentation().additionalTraceAnnotations == expected.toSet()
5960

6061
where:
6162
value | expected

‎dd-java-agent/instrumentation/trace-annotation/src/test/groovy/TraceAnnotationsTest.groovy‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,9 @@ import java.util.concurrent.Callable
1010
class TraceAnnotationsTest extends AgentTestRunner {
1111

1212
static {
13-
ConfigUtils.makeConfigInstanceModifiable()
14-
System.clearProperty("dd.trace.annotations")
15-
ConfigUtils.resetConfig(true)
13+
ConfigUtils.updateConfig {
14+
System.clearProperty("dd.trace.annotations")
15+
}
1616
refreshTestTracer()
1717
}
1818

‎dd-java-agent/instrumentation/trace-annotation/src/test/groovy/TraceConfigTest.groovy‎

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,19 +4,18 @@ import datadog.trace.instrumentation.trace_annotation.TraceConfigInstrumentation
44

55
import java.util.concurrent.Callable
66

7-
import static datadog.trace.agent.test.utils.ConfigUtils.withSystemProperty
8-
97
class TraceConfigTest extends AgentTestRunner {
108

119
static {
12-
ConfigUtils.makeConfigInstanceModifiable()
13-
System.setProperty("dd.trace.methods", "package.ClassName[method1,method2];${ConfigTracedCallable.name}[call]")
14-
ConfigUtils.resetConfig()
10+
ConfigUtils.updateConfig {
11+
System.setProperty("dd.trace.methods", "package.ClassName[method1,method2];${ConfigTracedCallable.name}[call]")
12+
}
1513
}
1614

1715
def specCleanup() {
18-
System.clearProperty("dd.trace.methods")
19-
ConfigUtils.resetConfig()
16+
ConfigUtils.updateConfig {
17+
System.clearProperty("dd.trace.methods")
18+
}
2019
}
2120

2221
class ConfigTracedCallable implements Callable<String> {
@@ -46,15 +45,19 @@ class TraceConfigTest extends AgentTestRunner {
4645

4746
def "test configuration #value"() {
4847
setup:
49-
def config = null
50-
withSystemProperty("dd.trace.methods", value) {
51-
ConfigUtils.resetConfig()
52-
def instrumentation = new TraceConfigInstrumentation()
53-
config = instrumentation.classMethodsToTrace
48+
ConfigUtils.updateConfig {
49+
if (value) {
50+
System.properties.setProperty("dd.trace.methods", value)
51+
} else {
52+
System.clearProperty("dd.trace.methods")
53+
}
5454
}
55+
5556
expect:
57+
new TraceConfigInstrumentation().classMethodsToTrace == expected
5658

57-
config == expected
59+
cleanup:
60+
System.clearProperty("dd.trace.methods")
5861

5962
where:
6063
value | expected

‎dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/utils/ConfigUtils.groovy‎

Lines changed: 15 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -23,22 +23,6 @@ class ConfigUtils {
2323
static final RUNTIME_ID_FIELD = Config.getDeclaredField("runtimeId")
2424
}
2525

26-
// TODO: ideally all users of this should switch to using Config object (and withConfigOverride) instead.
27-
@Deprecated
28-
@SneakyThrows
29-
static <T extends Object> Object withSystemProperty(final String name, final String value, final Callable<T> r) {
30-
if (value == null) {
31-
System.clearProperty(name)
32-
} else {
33-
System.setProperty(name, value)
34-
}
35-
try {
36-
return r.call()
37-
} finally {
38-
System.clearProperty(name)
39-
}
40-
}
41-
4226
@SneakyThrows
4327
synchronized static <T extends Object> Object withConfigOverride(final String name, final String value, final Callable<T> r) {
4428
// Ensure the class was retransformed properly in AgentTestRunner.makeConfigInstanceModifiable()
@@ -59,13 +43,25 @@ class ConfigUtils {
5943
}
6044
}
6145

46+
/**
47+
* Provides an callback to set up the testing environment and reset the global configuration after system properties and envs are set.
48+
*
49+
* @param r
50+
* @return
51+
*/
52+
static updateConfig(final Callable r) {
53+
makeConfigInstanceModifiable()
54+
r.call()
55+
resetConfig()
56+
}
57+
6258
/**
6359
* Calling will reset the runtimeId too, so it might cause problems around runtimeId verification.
6460
* If you are testing runtimeId provide <code>preserveRuntimeId = false</code> to copy the previous runtimeId
65-
* tot he new config instance.
61+
* to the new config instance.
6662
*/
67-
static void resetConfig(preserveRuntimeId = false) {
68-
// Ensure the class was retransformed properly in AgentTestRunner.makeConfigInstanceModifiable()
63+
static void resetConfig(preserveRuntimeId = true) {
64+
// Ensure the class was re-transformed properly in AgentTestRunner.makeConfigInstanceModifiable()
6965
assert Modifier.isPublic(ConfigInstance.FIELD.getModifiers())
7066
assert Modifier.isStatic(ConfigInstance.FIELD.getModifiers())
7167
assert Modifier.isVolatile(ConfigInstance.FIELD.getModifiers())

‎dd-java-agent/testing/src/test/groovy/AgentTestRunnerTest.groovy‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import com.google.common.reflect.ClassPath
22
import datadog.trace.agent.test.AgentTestRunner
33
import datadog.trace.agent.test.SpockRunner
44
import datadog.trace.agent.test.utils.ClasspathUtils
5+
import datadog.trace.agent.test.utils.ConfigUtils
56
import datadog.trace.agent.test.utils.GlobalTracerUtils
67
import datadog.trace.agent.tooling.Constants
78
import io.opentracing.Span
@@ -10,7 +11,6 @@ import spock.lang.Shared
1011

1112
import java.lang.reflect.Field
1213

13-
import static datadog.trace.agent.test.utils.ConfigUtils.resetConfig
1414
import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace
1515
import static datadog.trace.api.Config.TRACE_CLASSES_EXCLUDE
1616

@@ -23,8 +23,9 @@ class AgentTestRunnerTest extends AgentTestRunner {
2323
private Class sharedSpanClass
2424

2525
static {
26-
System.setProperty("dd." + TRACE_CLASSES_EXCLUDE, "config.exclude.packagename.*, config.exclude.SomeClass,config.exclude.SomeClass\$NestedClass")
27-
resetConfig()
26+
ConfigUtils.updateConfig {
27+
System.setProperty("dd." + TRACE_CLASSES_EXCLUDE, "config.exclude.packagename.*, config.exclude.SomeClass,config.exclude.SomeClass\$NestedClass")
28+
}
2829

2930
// when test class initializes, opentracing should be set up, but not the agent.
3031
OT_LOADER = io.opentracing.Tracer.getClassLoader()

‎dd-trace-api/src/main/java/datadog/trace/api/Config.java‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -467,6 +467,17 @@ private Map<String, String> getRuntimeTags() {
467467

468468
public boolean isIntegrationEnabled(
469469
final SortedSet<String> integrationNames, final boolean defaultEnabled) {
470+
return integrationEnabled(integrationNames, defaultEnabled);
471+
}
472+
473+
/**
474+
* @deprecated This method should only be used internally. Use the instance getter instead {@link #isIntegrationEnabled(SortedSet, boolean)}.
475+
* @param integrationNames
476+
* @param defaultEnabled
477+
* @return
478+
*/
479+
public static boolean integrationEnabled(
480+
final SortedSet<String> integrationNames, final boolean defaultEnabled) {
470481
// If default is enabled, we want to enable individually,
471482
// if default is disabled, we want to disable individually.
472483
boolean anyEnabled = defaultEnabled;
@@ -484,6 +495,17 @@ public boolean isIntegrationEnabled(
484495

485496
public boolean isJmxFetchIntegrationEnabled(
486497
final SortedSet<String> integrationNames, final boolean defaultEnabled) {
498+
return jmxFetchIntegrationEnabled(integrationNames, defaultEnabled);
499+
}
500+
501+
/**
502+
* @deprecated This method should only be used internally. Use the instance getter instead {@link #isJmxFetchIntegrationEnabled(SortedSet, boolean)}.
503+
* @param integrationNames
504+
* @param defaultEnabled
505+
* @return
506+
*/
507+
public static boolean jmxFetchIntegrationEnabled(
508+
final SortedSet<String> integrationNames, final boolean defaultEnabled) {
487509
// If default is enabled, we want to enable individually,
488510
// if default is disabled, we want to disable individually.
489511
boolean anyEnabled = defaultEnabled;
@@ -501,6 +523,17 @@ public boolean isJmxFetchIntegrationEnabled(
501523

502524
public boolean isTraceAnalyticsIntegrationEnabled(
503525
final SortedSet<String> integrationNames, final boolean defaultEnabled) {
526+
return traceAnalyticsIntegrationEnabled(integrationNames, defaultEnabled);
527+
}
528+
529+
/**
530+
* @deprecated This method should only be used internally. Use the instance getter instead {@link #isTraceAnalyticsIntegrationEnabled(SortedSet, boolean)}.
531+
* @param integrationNames
532+
* @param defaultEnabled
533+
* @return
534+
*/
535+
public static boolean traceAnalyticsIntegrationEnabled(
536+
final SortedSet<String> integrationNames, final boolean defaultEnabled) {
504537
// If default is enabled, we want to enable individually,
505538
// if default is disabled, we want to disable individually.
506539
boolean anyEnabled = defaultEnabled;

0 commit comments

Comments
 (0)