Skip to content

Commit 808761a

Browse files
AlexeyKuznetsov-DDAlexeyKuznetsov-DD
committed
Refactored protocol version to be enum instead of string.
1 parent 6f232d3 commit 808761a

13 files changed

Lines changed: 163 additions & 270 deletions

File tree

‎communication/src/main/java/datadog/communication/ddagent/DDAgentFeaturesDiscovery.java‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import datadog.metrics.api.Recording;
1818
import datadog.metrics.impl.statsd.DDAgentStatsDClientManager;
1919
import datadog.trace.api.BaseHash;
20+
import datadog.trace.api.ProtocolVersion;
2021
import datadog.trace.api.telemetry.LogCollector;
2122
import datadog.trace.util.Strings;
2223
import java.nio.ByteBuffer;
@@ -108,17 +109,12 @@ public DDAgentFeaturesDiscovery(
108109
OkHttpClient client,
109110
Monitoring monitoring,
110111
HttpUrl agentUrl,
111-
String traceAgentProtocolVersion,
112+
ProtocolVersion protocolVersion,
112113
boolean metricsEnabled) {
113114
this.client = client;
114115
this.agentBaseUrl = agentUrl;
115116
this.metricsEnabled = metricsEnabled;
116-
this.traceEndpoints =
117-
"1.0".equals(traceAgentProtocolVersion)
118-
? new String[] {V1_ENDPOINT, V05_ENDPOINT, V04_ENDPOINT, V03_ENDPOINT}
119-
: ("0.5".equals(traceAgentProtocolVersion)
120-
? new String[] {V05_ENDPOINT, V04_ENDPOINT, V03_ENDPOINT}
121-
: new String[] {V04_ENDPOINT, V03_ENDPOINT});
117+
this.traceEndpoints = protocolVersion.traceEndpoints();
122118
this.discoveryTimer = monitoring.newTimer("trace.agent.discovery.time");
123119
this.discoveryState = new State();
124120
}

‎communication/src/test/groovy/datadog/communication/ddagent/DDAgentFeaturesDiscoveryTest.groovy‎

Lines changed: 27 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@ import static datadog.communication.ddagent.DDAgentFeaturesDiscovery.V07_CONFIG_
88
import static datadog.communication.ddagent.DDAgentFeaturesDiscovery.V1_ENDPOINT
99
import static datadog.communication.http.OkHttpUtils.DATADOG_CONTAINER_ID
1010
import static datadog.communication.http.OkHttpUtils.DATADOG_CONTAINER_TAGS_HASH
11+
import static datadog.trace.api.ProtocolVersion.V0_4
12+
import static datadog.trace.api.ProtocolVersion.V0_5
13+
import static datadog.trace.api.ProtocolVersion.V1_0
1114

1215
import datadog.common.container.ContainerInfo
1316
import datadog.metrics.api.Monitoring
@@ -79,16 +82,15 @@ class DDAgentFeaturesDiscoveryTest extends DDSpecification {
7982

8083
where:
8184
protocol | expectedTraceEndpoint
82-
"0.4" | V04_ENDPOINT
83-
"0.5" | V05_ENDPOINT
84-
"1.0" | V1_ENDPOINT
85-
"xxx" | V04_ENDPOINT
85+
V0_4 | V04_ENDPOINT
86+
V0_5 | V05_ENDPOINT
87+
V1_0 | V1_ENDPOINT
8688
}
8789

8890
def "Should change discovery state atomically after discovery happened"() {
8991
setup:
9092
OkHttpClient client = Mock(OkHttpClient)
91-
DDAgentFeaturesDiscovery features = new DDAgentFeaturesDiscovery(client, monitoring, agentUrl, "0.5", true)
93+
DDAgentFeaturesDiscovery features = new DDAgentFeaturesDiscovery(client, monitoring, agentUrl, V0_5, true)
9294

9395
when: "/info available"
9496
features.discover()
@@ -114,7 +116,7 @@ class DDAgentFeaturesDiscoveryTest extends DDSpecification {
114116
def "test parse /info response with discoverIfOutdated"() {
115117
setup:
116118
OkHttpClient client = Mock(OkHttpClient)
117-
DDAgentFeaturesDiscovery features = new DDAgentFeaturesDiscovery(client, monitoring, agentUrl, "0.5", true)
119+
DDAgentFeaturesDiscovery features = new DDAgentFeaturesDiscovery(client, monitoring, agentUrl, V0_5, true)
118120

119121
when: "/info available"
120122
features.discoverIfOutdated()
@@ -142,7 +144,7 @@ class DDAgentFeaturesDiscoveryTest extends DDSpecification {
142144
def "test parse /info response with client dropping"() {
143145
setup:
144146
OkHttpClient client = Mock(OkHttpClient)
145-
DDAgentFeaturesDiscovery features = new DDAgentFeaturesDiscovery(client, monitoring, agentUrl, "0.5", true)
147+
DDAgentFeaturesDiscovery features = new DDAgentFeaturesDiscovery(client, monitoring, agentUrl, V0_5, true)
146148

147149
when: "/info available"
148150
features.discover()
@@ -160,7 +162,7 @@ class DDAgentFeaturesDiscoveryTest extends DDSpecification {
160162
def "test parse /info response with data streams unavailable"() {
161163
setup:
162164
OkHttpClient client = Mock(OkHttpClient)
163-
DDAgentFeaturesDiscovery features = new DDAgentFeaturesDiscovery(client, monitoring, agentUrl, "0.5", true)
165+
DDAgentFeaturesDiscovery features = new DDAgentFeaturesDiscovery(client, monitoring, agentUrl, V0_5, true)
164166

165167
when: "/info available"
166168
features.discover()
@@ -179,7 +181,7 @@ class DDAgentFeaturesDiscoveryTest extends DDSpecification {
179181
def "test parse /info response with long running spans available"() {
180182
setup:
181183
OkHttpClient client = Mock(OkHttpClient)
182-
DDAgentFeaturesDiscovery features = new DDAgentFeaturesDiscovery(client, monitoring, agentUrl, "0.5", true)
184+
DDAgentFeaturesDiscovery features = new DDAgentFeaturesDiscovery(client, monitoring, agentUrl, V0_5, true)
183185

184186
when: "/info available"
185187
features.discover()
@@ -193,7 +195,7 @@ class DDAgentFeaturesDiscoveryTest extends DDSpecification {
193195
def "test fallback when /info empty"() {
194196
setup:
195197
OkHttpClient client = Mock(OkHttpClient)
196-
DDAgentFeaturesDiscovery features = new DDAgentFeaturesDiscovery(client, monitoring, agentUrl, "0.4", true)
198+
DDAgentFeaturesDiscovery features = new DDAgentFeaturesDiscovery(client, monitoring, agentUrl, V0_4, true)
197199

198200
when: "/info is empty"
199201
features.discover()
@@ -215,7 +217,7 @@ class DDAgentFeaturesDiscoveryTest extends DDSpecification {
215217
def "test fallback when /info not found"() {
216218
setup:
217219
OkHttpClient client = Mock(OkHttpClient)
218-
DDAgentFeaturesDiscovery features = new DDAgentFeaturesDiscovery(client, monitoring, agentUrl, "0.5", true)
220+
DDAgentFeaturesDiscovery features = new DDAgentFeaturesDiscovery(client, monitoring, agentUrl, V0_5, true)
219221

220222
when: "/info unavailable"
221223
features.discover()
@@ -237,7 +239,7 @@ class DDAgentFeaturesDiscoveryTest extends DDSpecification {
237239
def "test fallback when /info not found and agent returns ok"() {
238240
setup:
239241
OkHttpClient client = Mock(OkHttpClient)
240-
DDAgentFeaturesDiscovery features = new DDAgentFeaturesDiscovery(client, monitoring, agentUrl, "0.5", true)
242+
DDAgentFeaturesDiscovery features = new DDAgentFeaturesDiscovery(client, monitoring, agentUrl, V0_5, true)
241243

242244
when: "/info unavailable"
243245
features.discover()
@@ -257,7 +259,7 @@ class DDAgentFeaturesDiscoveryTest extends DDSpecification {
257259
def "test fallback when /info not found and v0.5 disabled"() {
258260
setup:
259261
OkHttpClient client = Mock(OkHttpClient)
260-
DDAgentFeaturesDiscovery features = new DDAgentFeaturesDiscovery(client, monitoring, agentUrl, "0.4", true)
262+
DDAgentFeaturesDiscovery features = new DDAgentFeaturesDiscovery(client, monitoring, agentUrl, V0_4, true)
261263

262264
when: "/info unavailable"
263265
features.discover()
@@ -278,7 +280,7 @@ class DDAgentFeaturesDiscoveryTest extends DDSpecification {
278280
def "test fallback when /info not found and v0.5 unavailable agent side"() {
279281
setup:
280282
OkHttpClient client = Mock(OkHttpClient)
281-
DDAgentFeaturesDiscovery features = new DDAgentFeaturesDiscovery(client, monitoring, agentUrl, "0.5", true)
283+
DDAgentFeaturesDiscovery features = new DDAgentFeaturesDiscovery(client, monitoring, agentUrl, V0_5, true)
282284

283285
when: "/info unavailable"
284286
features.discover()
@@ -299,7 +301,7 @@ class DDAgentFeaturesDiscoveryTest extends DDSpecification {
299301
def "test fallback on very old agent"() {
300302
setup:
301303
OkHttpClient client = Mock(OkHttpClient)
302-
DDAgentFeaturesDiscovery features = new DDAgentFeaturesDiscovery(client, monitoring, agentUrl, "0.5", true)
304+
DDAgentFeaturesDiscovery features = new DDAgentFeaturesDiscovery(client, monitoring, agentUrl, V0_5, true)
303305

304306
when: "/info unavailable"
305307
features.discover()
@@ -321,7 +323,7 @@ class DDAgentFeaturesDiscoveryTest extends DDSpecification {
321323
def "disabling metrics disables metrics and dropping"() {
322324
setup:
323325
OkHttpClient client = Mock(OkHttpClient)
324-
DDAgentFeaturesDiscovery features = new DDAgentFeaturesDiscovery(client, monitoring, agentUrl, "0.5", false)
326+
DDAgentFeaturesDiscovery features = new DDAgentFeaturesDiscovery(client, monitoring, agentUrl, V0_5, false)
325327

326328
when: "/info unavailable"
327329
features.discover()
@@ -357,7 +359,7 @@ class DDAgentFeaturesDiscoveryTest extends DDSpecification {
357359
def "discovery of metrics endpoint after agent upgrade enables dropping and metrics"() {
358360
setup:
359361
OkHttpClient client = Mock(OkHttpClient)
360-
DDAgentFeaturesDiscovery features = new DDAgentFeaturesDiscovery(client, monitoring, agentUrl, "0.4", true)
362+
DDAgentFeaturesDiscovery features = new DDAgentFeaturesDiscovery(client, monitoring, agentUrl, V0_4, true)
361363

362364
when: "/info unavailable"
363365
features.discover()
@@ -385,7 +387,7 @@ class DDAgentFeaturesDiscoveryTest extends DDSpecification {
385387
def "disappearance of info endpoint after agent downgrade disables metrics and dropping"() {
386388
setup:
387389
OkHttpClient client = Mock(OkHttpClient)
388-
DDAgentFeaturesDiscovery features = new DDAgentFeaturesDiscovery(client, monitoring, agentUrl, "0.4", true)
390+
DDAgentFeaturesDiscovery features = new DDAgentFeaturesDiscovery(client, monitoring, agentUrl, V0_4, true)
389391

390392
when: "/info available"
391393
features.discover()
@@ -414,7 +416,7 @@ class DDAgentFeaturesDiscoveryTest extends DDSpecification {
414416
def "disappearance of metrics endpoint after agent downgrade disables metrics and dropping"() {
415417
setup:
416418
OkHttpClient client = Mock(OkHttpClient)
417-
DDAgentFeaturesDiscovery features = new DDAgentFeaturesDiscovery(client, monitoring, agentUrl, "0.4", true)
419+
DDAgentFeaturesDiscovery features = new DDAgentFeaturesDiscovery(client, monitoring, agentUrl, V0_4, true)
418420

419421
when: "/info available"
420422
features.discover()
@@ -444,7 +446,7 @@ class DDAgentFeaturesDiscoveryTest extends DDSpecification {
444446
def "test parse /info response with telemetry proxy"() {
445447
setup:
446448
OkHttpClient client = Mock(OkHttpClient)
447-
DDAgentFeaturesDiscovery features = new DDAgentFeaturesDiscovery(client, monitoring, agentUrl, "0.5", true)
449+
DDAgentFeaturesDiscovery features = new DDAgentFeaturesDiscovery(client, monitoring, agentUrl, V0_5, true)
448450

449451
when: "/info available"
450452
features.discover()
@@ -461,7 +463,7 @@ class DDAgentFeaturesDiscoveryTest extends DDSpecification {
461463
def "test parse /info response with old EVP proxy"() {
462464
setup:
463465
OkHttpClient client = Mock(OkHttpClient)
464-
DDAgentFeaturesDiscovery features = new DDAgentFeaturesDiscovery(client, monitoring, agentUrl, "0.5", true)
466+
DDAgentFeaturesDiscovery features = new DDAgentFeaturesDiscovery(client, monitoring, agentUrl, V0_5, true)
465467

466468
when: "/info available"
467469
features.discover()
@@ -480,7 +482,7 @@ class DDAgentFeaturesDiscoveryTest extends DDSpecification {
480482
def "test parse /info response with peer tag back propagation"() {
481483
setup:
482484
OkHttpClient client = Mock(OkHttpClient)
483-
DDAgentFeaturesDiscovery features = new DDAgentFeaturesDiscovery(client, monitoring, agentUrl, "0.5", true)
485+
DDAgentFeaturesDiscovery features = new DDAgentFeaturesDiscovery(client, monitoring, agentUrl, V0_5, true)
484486

485487
when: "/info available"
486488
features.discover()
@@ -513,7 +515,7 @@ class DDAgentFeaturesDiscoveryTest extends DDSpecification {
513515
def "test metrics disabled for agent version below 7.65"() {
514516
setup:
515517
OkHttpClient client = Mock(OkHttpClient)
516-
DDAgentFeaturesDiscovery features = new DDAgentFeaturesDiscovery(client, monitoring, agentUrl, "0.5", true)
518+
DDAgentFeaturesDiscovery features = new DDAgentFeaturesDiscovery(client, monitoring, agentUrl, V0_5, true)
517519

518520
when: "agent version is below 7.65"
519521
features.discover()
@@ -547,7 +549,7 @@ class DDAgentFeaturesDiscoveryTest extends DDSpecification {
547549
def "test metrics disabled for agent with unparseable version"() {
548550
setup:
549551
OkHttpClient client = Mock(OkHttpClient)
550-
DDAgentFeaturesDiscovery features = new DDAgentFeaturesDiscovery(client, monitoring, agentUrl, "0.5", true)
552+
DDAgentFeaturesDiscovery features = new DDAgentFeaturesDiscovery(client, monitoring, agentUrl, V0_5, true)
551553

552554
when: "agent version is unparseable"
553555
features.discover()
@@ -573,7 +575,7 @@ class DDAgentFeaturesDiscoveryTest extends DDSpecification {
573575
def "should send container id as header on the info request and parse the hash in the response"() {
574576
setup:
575577
OkHttpClient client = Mock(OkHttpClient)
576-
DDAgentFeaturesDiscovery features = new DDAgentFeaturesDiscovery(client, monitoring, agentUrl, "0.5", true)
578+
DDAgentFeaturesDiscovery features = new DDAgentFeaturesDiscovery(client, monitoring, agentUrl, V0_5, true)
577579
def oldContainerId = ContainerInfo.get().getContainerId()
578580
def oldContainerTagsHash = ContainerInfo.get().getContainerTagsHash()
579581
ContainerInfo.get().setContainerId("test")

‎dd-java-agent/appsec/src/jmh/java/datadog/appsec/benchmark/AppSecBenchmark.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package datadog.appsec.benchmark;
22

3+
import static datadog.trace.api.ProtocolVersion.V0_4;
34
import static datadog.trace.api.gateway.Events.EVENTS;
45
import static java.util.concurrent.TimeUnit.MICROSECONDS;
56
import static java.util.concurrent.TimeUnit.SECONDS;
@@ -187,7 +188,7 @@ public Call clone() {
187188

188189
static class StubDDAgentFeaturesDiscovery extends DDAgentFeaturesDiscovery {
189190
public StubDDAgentFeaturesDiscovery(OkHttpClient client) {
190-
super(client, Monitoring.DISABLED, HttpUrl.get("http://localhost:8080/"), "0.4", false);
191+
super(client, Monitoring.DISABLED, HttpUrl.get("http://localhost:8080/"), V0_4, false);
191192
}
192193

193194
@Override

‎dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/datastreams/MockFeaturesDiscovery.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,14 @@
22

33
import datadog.communication.ddagent.DDAgentFeaturesDiscovery;
44
import datadog.metrics.api.Monitoring;
5+
import datadog.trace.api.ProtocolVersion;
56

67
// TODO Ideally, DDAgentFeaturesDiscovery would be an interface to create a proper testable stubs
78
public class MockFeaturesDiscovery extends DDAgentFeaturesDiscovery {
89
private final boolean supportsDataStreams;
910

1011
public MockFeaturesDiscovery(boolean supportsDataStreams) {
11-
super(null, Monitoring.DISABLED, null, "0.5", true);
12+
super(null, Monitoring.DISABLED, null, ProtocolVersion.V0_5, true);
1213
this.supportsDataStreams = supportsDataStreams;
1314
}
1415

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ public final class ConfigDefaults {
9191
static final boolean DEFAULT_JMX_FETCH_ENABLED = true;
9292

9393
// forced to use 1.0 for testing only, rollback to "0.4" before merging to master;
94-
static final String DEFAULT_TRACE_AGENT_PROTOCOL_VERSION = "1.0";
94+
static final String DEFAULT_TRACE_AGENT_PROTOCOL_VERSION = ProtocolVersion.V1_0.asConfigValue();
9595

9696
static final boolean DEFAULT_CLIENT_IP_ENABLED = false;
9797

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

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
package datadog.trace.api;
2+
3+
import java.util.Locale;
4+
5+
public enum ProtocolVersion {
6+
V0_4("0.4", new String[] {"v0.4/traces", "v0.3/traces"}),
7+
V0_5("0.5", new String[] {"v0.5/traces", "v0.4/traces", "v0.3/traces"}),
8+
V1_0("1.0", new String[] {"v1.0/traces", "v0.5/traces", "v0.4/traces", "v0.3/traces"});
9+
10+
private final String configValue;
11+
private final String[] traceEndpoints;
12+
13+
ProtocolVersion(String configValue, String[] traceEndpoints) {
14+
this.configValue = configValue;
15+
this.traceEndpoints = traceEndpoints;
16+
}
17+
18+
public String asConfigValue() {
19+
return configValue;
20+
}
21+
22+
public String[] traceEndpoints() {
23+
return traceEndpoints;
24+
}
25+
26+
public static ProtocolVersion fromConfigValue(String value) {
27+
if (value == null) {
28+
return V0_4;
29+
}
30+
31+
String normalized = value.toLowerCase(Locale.ROOT);
32+
if (V1_0.configValue.equals(normalized)) {
33+
return V1_0;
34+
}
35+
36+
if (V0_5.configValue.equals(normalized)) {
37+
return V0_5;
38+
}
39+
40+
return V0_4;
41+
}
42+
43+
public static ProtocolVersion fromTraceEndpoint(String endpoint) {
44+
if (endpoint == null) {
45+
return null;
46+
}
47+
48+
String normalized = endpoint.toLowerCase(Locale.ROOT);
49+
if (normalized.endsWith(V1_0.traceEndpoints[0])) {
50+
return V1_0;
51+
}
52+
53+
if (normalized.endsWith(V0_5.traceEndpoints[0])) {
54+
return V0_5;
55+
}
56+
57+
return V0_4;
58+
}
59+
}

‎dd-trace-core/src/jmh/java/datadog/trace/common/metrics/ConflatingMetricsAggregatorBenchmark.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package datadog.trace.common.metrics;
22

3+
import static datadog.trace.api.ProtocolVersion.V0_4;
34
import static java.util.concurrent.TimeUnit.MICROSECONDS;
45
import static java.util.concurrent.TimeUnit.SECONDS;
56

@@ -72,7 +73,7 @@ static class FixedAgentFeaturesDiscovery extends DDAgentFeaturesDiscovery {
7273

7374
public FixedAgentFeaturesDiscovery(Set<String> peerTags, Set<String> spanKinds) {
7475
// create a fixed discovery with metrics enabled
75-
super(null, Monitoring.DISABLED, null, "0.4", true);
76+
super(null, Monitoring.DISABLED, null, V0_4, true);
7677
this.peerTags = peerTags;
7778
this.spanKinds = spanKinds;
7879
}

‎dd-trace-core/src/main/java/datadog/trace/common/writer/DDAgentWriter.java‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import datadog.communication.ddagent.DDAgentFeaturesDiscovery;
1010
import datadog.metrics.api.Monitoring;
1111
import datadog.trace.api.Config;
12+
import datadog.trace.api.ProtocolVersion;
1213
import datadog.trace.common.sampling.SingleSpanSampler;
1314
import datadog.trace.common.writer.ddagent.DDAgentApi;
1415
import datadog.trace.common.writer.ddagent.DDAgentMapperDiscovery;
@@ -37,7 +38,7 @@ public static class DDAgentWriterBuilder {
3738
HealthMetrics healthMetrics = HealthMetrics.NO_OP;
3839
int flushIntervalMilliseconds = 1000;
3940
Monitoring monitoring = Monitoring.DISABLED;
40-
String traceAgentProtocolVersion = Config.get().getTraceAgentProtocolVersion();
41+
ProtocolVersion protocolVersion = Config.get().getTraceAgentProtocolVersion();
4142
boolean metricsReportingEnabled = Config.get().isTracerMetricsEnabled();
4243
private int flushTimeout = 1;
4344
private TimeUnit flushTimeoutUnit = TimeUnit.SECONDS;
@@ -103,11 +104,15 @@ public DDAgentWriterBuilder monitoring(Monitoring monitoring) {
103104
return this;
104105
}
105106

106-
public DDAgentWriterBuilder traceAgentProtocolVersion(String traceAgentProtocolVersion) {
107-
this.traceAgentProtocolVersion = traceAgentProtocolVersion;
107+
public DDAgentWriterBuilder traceAgentProtocolVersion(ProtocolVersion protocolVersion) {
108+
this.protocolVersion = protocolVersion;
108109
return this;
109110
}
110111

112+
public DDAgentWriterBuilder traceAgentProtocolVersion(String traceAgentProtocolVersion) {
113+
return traceAgentProtocolVersion(ProtocolVersion.fromConfigValue(traceAgentProtocolVersion));
114+
}
115+
111116
public DDAgentWriterBuilder metricsReportingEnabled(boolean metricsReportingEnabled) {
112117
this.metricsReportingEnabled = metricsReportingEnabled;
113118
return this;
@@ -143,7 +148,7 @@ public DDAgentWriter build() {
143148
if (null == featureDiscovery) {
144149
featureDiscovery =
145150
new DDAgentFeaturesDiscovery(
146-
client, monitoring, agentUrl, traceAgentProtocolVersion, metricsReportingEnabled);
151+
client, monitoring, agentUrl, protocolVersion, metricsReportingEnabled);
147152
}
148153
if (null == agentApi) {
149154
agentApi =

0 commit comments

Comments
 (0)