Skip to content

Commit 417308c

Browse files
dougqhclaude
andcommitted
Address review: containsTraceComment takes a SubSequence; callers build the view
Per review on #11737: move SubSequence.of to the call sites (SQLCommenter builds the comment-region view; the String overload wraps the whole content) and make SharedDBCommenter.containsTraceComment take a SubSequence instead of (String, from, to). Behavior unchanged (SubSequence.contains -> Strings.regionContains); test + dup-comment-benchmark Javadoc updated to the new shape. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 24d8ea0 commit 417308c

4 files changed

Lines changed: 18 additions & 15 deletions

File tree

dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/dbm/SharedDBCommenter.java

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -58,16 +58,15 @@ public class SharedDBCommenter {
5858
// passes the already-extracted comment body; SQLCommenter uses the range overload to check it
5959
// in place. Both run the same nine "<key>=" needle checks.
6060
public static boolean containsTraceComment(String commentContent) {
61-
return containsTraceComment(commentContent, 0, commentContent.length());
61+
return containsTraceComment(SubSequence.of(commentContent, 0));
6262
}
6363

6464
/**
65-
* Range overload: true if {@code sql} contains a trace-comment needle fully within {@code [from,
66-
* to)} -- checks the comment body in place, with no substring allocation of the region.
65+
* True if {@code comment} contains a trace-comment needle. Callers pass a zero-copy {@link
66+
* SubSequence} view of the comment body, so the check runs in place with no substring allocation
67+
* of the region.
6768
*/
68-
public static boolean containsTraceComment(String sql, int from, int to) {
69-
// Zero-copy view of the comment body; reads like ordinary String.contains, no substring.
70-
SubSequence comment = SubSequence.of(sql, from, to);
69+
public static boolean containsTraceComment(SubSequence comment) {
7170
return comment.contains(PARENT_SERVICE_EQ)
7271
|| comment.contains(DATABASE_SERVICE_EQ)
7372
|| comment.contains(DD_HOSTNAME_EQ)

dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/instrumentation/dbm/SharedDBCommenterContainsTraceCommentTest.java

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,14 @@
44
import static org.junit.jupiter.api.Assertions.assertFalse;
55
import static org.junit.jupiter.api.Assertions.assertTrue;
66

7+
import datadog.trace.util.SubSequence;
78
import org.junit.jupiter.api.Test;
89

910
/**
1011
* DB-level behavior of {@link SharedDBCommenter#containsTraceComment}: the nine "<key>=" needle
11-
* set, the {@code String} delegate, and the range overload checking the comment body in place. (The
12-
* {@code [from, to)} boundary semantics are unit-tested on {@code Strings.regionContains}.)
12+
* set, the {@code String} delegate, and the {@link SubSequence} overload checking the comment body
13+
* in place. (The {@code [from, to)} boundary semantics are unit-tested on {@code
14+
* Strings.regionContains}.)
1315
*/
1416
class SharedDBCommenterContainsTraceCommentTest {
1517

@@ -25,15 +27,15 @@ void range_needleInsideCommentBody() {
2527
String sql = "SELECT 1 /*ddps='svc',dde='test'*/";
2628
int from = sql.indexOf("/*") + 2;
2729
int to = sql.indexOf("*/");
28-
assertTrue(containsTraceComment(sql, from, to));
30+
assertTrue(containsTraceComment(SubSequence.of(sql, from, to)));
2931
}
3032

3133
@Test
3234
void range_nonDdCommentBody() {
3335
String sql = "SELECT 1 /* just a customer comment */";
3436
int from = sql.indexOf("/*") + 2;
3537
int to = sql.indexOf("*/");
36-
assertFalse(containsTraceComment(sql, from, to));
38+
assertFalse(containsTraceComment(SubSequence.of(sql, from, to)));
3739
}
3840

3941
@Test
@@ -43,6 +45,6 @@ void range_ddNeedleOutsideCommentRegionNotMatched() {
4345
String sql = "ddps='x' /* clean */";
4446
int from = sql.indexOf("/*") + 2;
4547
int to = sql.indexOf("*/");
46-
assertFalse(containsTraceComment(sql, from, to));
48+
assertFalse(containsTraceComment(SubSequence.of(sql, from, to)));
4749
}
4850
}

dd-java-agent/instrumentation/jdbc/src/jmh/java/datadog/trace/instrumentation/jdbc/SQLCommenterDuplicateCommentBenchmark.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@
1414
*
1515
* <p><b>What we're measuring.</b> The guard used to materialize {@code sql.substring(commentStart,
1616
* commentEnd)} (the comment body) just to scan it for trace-comment needles. (B) checks the comment
17-
* region in place via {@code SharedDBCommenter.containsTraceComment(sql, from, to)} -- no
18-
* substring.
17+
* region in place via {@code SharedDBCommenter.containsTraceComment(SubSequence.of(sql, from, to))}
18+
* -- no substring.
1919
*
2020
* <p><b>Isolation.</b> The substring only happens when the SQL already carries a comment in the
2121
* checked position; for a DD comment {@code inject} then returns early. Passing {@code dbType=null}

dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/SQLCommenter.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package datadog.trace.instrumentation.jdbc;
22

33
import datadog.trace.bootstrap.instrumentation.dbm.SharedDBCommenter;
4+
import datadog.trace.util.SubSequence;
45

56
public class SQLCommenter {
67
// SQL-specific constants, rest defined in SharedDBCommenter
@@ -122,8 +123,9 @@ private static boolean hasDDComment(String sql, boolean appendComment) {
122123
endIdx = sql.indexOf(CLOSE_COMMENT);
123124
}
124125
if (startIdx != -1 && endIdx != -1 && endIdx > startIdx) {
125-
// Check the comment body in place -- no substring of the comment region.
126-
return SharedDBCommenter.containsTraceComment(sql, startIdx + OPEN_COMMENT_LEN, endIdx);
126+
// Check the comment body in place via a zero-copy view -- no substring of the comment region.
127+
return SharedDBCommenter.containsTraceComment(
128+
SubSequence.of(sql, startIdx + OPEN_COMMENT_LEN, endIdx));
127129
}
128130
return false;
129131
}

0 commit comments

Comments
 (0)