Skip to content

Commit 7608cdf

Browse files
bric3bric3
authored
Only enable client side stats if the host agent is at least 7.65.0 (#10041)
* fix: Only enable client side stats if the host agent is at least 7.65.0 * fix: PR review, moved version check outside hotpath * fix: Use the revert check isVersionAtLeast -> isVersionBelow * chore: Make spotless happy
1 parent e6b5e68 commit 7608cdf

File tree

6 files changed

+203
-3
lines changed

6 files changed

+203
-3
lines changed

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

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
package datadog.communication.ddagent;
2+
3+
import org.slf4j.Logger;
4+
import org.slf4j.LoggerFactory;
5+
6+
public class AgentVersion {
7+
8+
private static final Logger log = LoggerFactory.getLogger(AgentVersion.class);
9+
10+
/**
11+
* Checks if the given version string represents a version that is below the specified major,
12+
* minor, and patch version.
13+
*
14+
* @param version the version string to check (e.g., "7.64.0")
15+
* @param maxMajor maximum major version (exclusive)
16+
* @param maxMinor maximum minor version (exclusive)
17+
* @param maxPatch maximum patch version (exclusive)
18+
* @return true if version is below the specified maximum (or if not parseable), false otherwise
19+
*/
20+
public static boolean isVersionBelow(String version, int maxMajor, int maxMinor, int maxPatch) {
21+
if (version == null || version.isEmpty()) {
22+
return true;
23+
}
24+
25+
try {
26+
// Parse version string in format "major.minor.patch" (e.g., "7.65.0")
27+
// Assumes the 'version' is below if it can't be parsed.
28+
int majorDot = version.indexOf('.');
29+
if (majorDot == -1) {
30+
return true;
31+
}
32+
33+
int major = Integer.parseInt(version.substring(0, majorDot));
34+
35+
if (major < maxMajor) {
36+
return true;
37+
} else if (major > maxMajor) {
38+
return false;
39+
}
40+
41+
// major == maxMajor
42+
int minorDot = version.indexOf('.', majorDot + 1);
43+
if (minorDot == -1) {
44+
return true;
45+
}
46+
47+
int minor = Integer.parseInt(version.substring(majorDot + 1, minorDot));
48+
if (minor < maxMinor) {
49+
return true;
50+
} else if (minor > maxMinor) {
51+
return false;
52+
}
53+
54+
// major == maxMajor && minor == maxMinor
55+
// Find end of patch version (may have suffix like "-rc.1")
56+
int patchEnd = minorDot + 1;
57+
while (patchEnd < version.length() && Character.isDigit(version.charAt(patchEnd))) {
58+
patchEnd++;
59+
}
60+
61+
int patch = Integer.parseInt(version.substring(minorDot + 1, patchEnd));
62+
if (patch != maxPatch) {
63+
return patch < maxPatch;
64+
} else {
65+
// If there's a suffix (like "-rc.1"), consider it below the non-suffixed version
66+
return patchEnd < version.length();
67+
}
68+
} catch (NumberFormatException | IndexOutOfBoundsException e) {
69+
return true;
70+
}
71+
}
72+
}

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ private static class State {
8787
String dataStreamsEndpoint;
8888
boolean supportsLongRunning;
8989
boolean supportsDropping;
90+
boolean supportsClientSideStats;
9091
String state;
9192
String configEndpoint;
9293
String debuggerLogEndpoint;
@@ -306,6 +307,7 @@ private boolean processInfoResponse(State newState, String response) {
306307
Boolean.TRUE.equals(map.getOrDefault("long_running_spans", false));
307308

308309
if (metricsEnabled) {
310+
newState.supportsClientSideStats = !AgentVersion.isVersionBelow(newState.version, 7, 65, 0);
309311
Object canDrop = map.get("client_drop_p0s");
310312
newState.supportsDropping =
311313
null != canDrop
@@ -358,7 +360,8 @@ private static void discoverStatsDPort(final Map<String, Object> info) {
358360
public boolean supportsMetrics() {
359361
return metricsEnabled
360362
&& null != discoveryState.metricsEndpoint
361-
&& discoveryState.supportsDropping;
363+
&& discoveryState.supportsDropping
364+
&& discoveryState.supportsClientSideStats;
362365
}
363366

364367
public boolean supportsDebugger() {

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

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -498,6 +498,67 @@ class DDAgentFeaturesDiscoveryTest extends DDSpecification {
498498
)
499499
}
500500

501+
def "test metrics disabled for agent version below 7.65"() {
502+
setup:
503+
OkHttpClient client = Mock(OkHttpClient)
504+
DDAgentFeaturesDiscovery features = new DDAgentFeaturesDiscovery(client, monitoring, agentUrl, true, true)
505+
506+
when: "agent version is below 7.65"
507+
features.discover()
508+
509+
then:
510+
1 * client.newCall(_) >> { Request request ->
511+
def response = """
512+
{
513+
"version": "${version}",
514+
"endpoints": ["/v0.5/traces", "/v0.6/stats"],
515+
"client_drop_p0s": true,
516+
"config": {}
517+
}
518+
"""
519+
infoResponse(request, response)
520+
}
521+
features.getMetricsEndpoint() == V6_METRICS_ENDPOINT
522+
features.supportsDropping() == true
523+
features.supportsMetrics() == expected
524+
525+
where:
526+
version | expected
527+
"7.64.0" | false
528+
"7.64.9" | false
529+
"7.64.9-rc.1" | false
530+
"7.65.0" | true
531+
"7.65.1" | true
532+
"7.70.0" | true
533+
"8.0.0" | true
534+
}
535+
536+
def "test metrics disabled for agent with unparseable version"() {
537+
setup:
538+
OkHttpClient client = Mock(OkHttpClient)
539+
DDAgentFeaturesDiscovery features = new DDAgentFeaturesDiscovery(client, monitoring, agentUrl, true, true)
540+
541+
when: "agent version is unparseable"
542+
features.discover()
543+
544+
then:
545+
1 * client.newCall(_) >> { Request request ->
546+
def response = """
547+
{
548+
"version": "${version}",
549+
"endpoints": ["/v0.5/traces", "/v0.6/stats"],
550+
"client_drop_p0s": true,
551+
"config": {}
552+
}
553+
"""
554+
infoResponse(request, response)
555+
}
556+
!features.supportsMetrics()
557+
558+
where:
559+
version << ["invalid", "7", "7.65", "", null]
560+
}
561+
501562
def "should send container id as header on the info request and parse the hash in the response"() {
502563
setup:
503564
OkHttpClient client = Mock(OkHttpClient)

‎communication/src/test/java/datadog/communication/ddagent/AgentVersionTest.java‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
package datadog.communication.ddagent;
2+
3+
import static org.junit.jupiter.api.Assertions.*;
4+
5+
import org.junit.jupiter.api.Test;
6+
7+
class AgentVersionTest {
8+
9+
@Test
10+
void testIsVersionBelow_VersionBelowThreshold() {
11+
assertTrue(AgentVersion.isVersionBelow("7.64.0", 7, 65, 0));
12+
assertTrue(AgentVersion.isVersionBelow("7.64.9", 7, 65, 0));
13+
assertTrue(AgentVersion.isVersionBelow("6.99.99", 7, 65, 0));
14+
assertTrue(AgentVersion.isVersionBelow("7.0.0", 7, 65, 0));
15+
}
16+
17+
@Test
18+
void testIsVersionBelow_VersionEqualToThreshold() {
19+
assertFalse(AgentVersion.isVersionBelow("7.65.0", 7, 65, 0));
20+
}
21+
22+
@Test
23+
void testIsVersionBelow_VersionAboveThreshold() {
24+
assertFalse(AgentVersion.isVersionBelow("7.65.1", 7, 65, 0));
25+
assertFalse(AgentVersion.isVersionBelow("7.66.0", 7, 65, 0));
26+
assertFalse(AgentVersion.isVersionBelow("8.0.0", 7, 65, 0));
27+
assertFalse(AgentVersion.isVersionBelow("7.65.10", 7, 65, 0));
28+
}
29+
30+
@Test
31+
void testIsVersionBelow_MajorVersionComparison() {
32+
assertTrue(AgentVersion.isVersionBelow("6.100.100", 7, 0, 0));
33+
assertFalse(AgentVersion.isVersionBelow("8.0.0", 7, 100, 100));
34+
}
35+
36+
@Test
37+
void testIsVersionBelow_MinorVersionComparison() {
38+
assertTrue(AgentVersion.isVersionBelow("7.64.100", 7, 65, 0));
39+
assertFalse(AgentVersion.isVersionBelow("7.66.0", 7, 65, 100));
40+
}
41+
42+
@Test
43+
void testIsVersionBelow_WithSuffix() {
44+
assertTrue(AgentVersion.isVersionBelow("7.64.0-rc.1", 7, 65, 0));
45+
assertTrue(AgentVersion.isVersionBelow("7.65.0-rc.1", 7, 65, 0));
46+
assertFalse(AgentVersion.isVersionBelow("7.65.1-snapshot", 7, 65, 0));
47+
}
48+
49+
@Test
50+
void testIsVersionBelow_NullOrEmpty() {
51+
assertTrue(AgentVersion.isVersionBelow(null, 7, 65, 0));
52+
assertTrue(AgentVersion.isVersionBelow("", 7, 65, 0));
53+
}
54+
55+
@Test
56+
void testIsVersionBelow_InvalidFormat() {
57+
assertTrue(AgentVersion.isVersionBelow("invalid", 7, 65, 0));
58+
assertTrue(AgentVersion.isVersionBelow("7", 7, 65, 0));
59+
assertTrue(AgentVersion.isVersionBelow("7.", 7, 65, 0));
60+
assertTrue(AgentVersion.isVersionBelow("7.65", 7, 65, 0));
61+
assertTrue(AgentVersion.isVersionBelow("7.65.", 7, 65, 0));
62+
assertTrue(AgentVersion.isVersionBelow("a.b.c", 7, 65, 0));
63+
}
64+
}

‎communication/src/test/resources/agent-features/agent-info-with-client-dropping.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
{
2-
"version": "0.99.0",
2+
"version": "7.65.0",
33
"git_commit": "fab047e10",
44
"build_date": "2020-12-04 15:57:06.74187 +0200 EET m=+0.029001792",
55
"endpoints": [

‎dd-trace-core/src/test/groovy/datadog/trace/common/metrics/MetricsReliabilityTest.groovy‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ class MetricsReliabilityTest extends DDCoreSpecification {
3535
httpServer {
3636
handlers {
3737
get("/info") {
38-
final def res = '{"endpoints":[' + (state.agentMetricsAvailable ? '"/v0.6/stats", ' : '') + '"/v0.4/traces"], "client_drop_p0s" : true}'
38+
final def res = '{"version":"7.65.0","endpoints":[' + (state.agentMetricsAvailable ? '"/v0.6/stats", ' : '') + '"/v0.4/traces"], "client_drop_p0s" : true}'
3939
state.hash = Strings.sha256(res)
4040
response.send(res)
4141
state.latch.countDown()

0 commit comments

Comments
 (0)