Skip to content

Commit c627efa

Browse files
dougqhclaude
dougqh
and
claude
committed
Reduce per-query allocations in SQLCommenter.inject()
Optimize the hot path of SQLCommenter.inject() which runs on every JDBC statement execution when DBM is enabled: - Avoid getFirstWord() substring allocation by using index-based firstWordStartsWith() and firstWordEqualsIgnoreCase() that check the first word in-place via String.regionMatches() - Eliminate hasDDComment() substring allocation by adding a range-based containsTraceComment(sql, fromIndex, toIndex) overload that searches for trace comment markers directly in the SQL string without extracting a substring - Pre-compute marker strings (e.g. "ddps=", "dddbs=") as static finals instead of concatenating them on every containsTraceComment() call - Cache the static comment string (everything except traceParent and peerService) per (dbService, hostname, dbName, Config) in a ConcurrentHashMap inside SharedDBCommenter, so repeated calls in DBM "static" mode skip URLEncoder.encode() and StringBuilder work Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent c13e821 commit c627efa

2 files changed

Lines changed: 213 additions & 22 deletions

File tree

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

Lines changed: 156 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import java.io.UnsupportedEncodingException;
1010
import java.net.URLEncoder;
1111
import java.nio.charset.StandardCharsets;
12+
import java.util.concurrent.ConcurrentHashMap;
1213
import org.slf4j.Logger;
1314
import org.slf4j.LoggerFactory;
1415

@@ -32,17 +33,63 @@ public class SharedDBCommenter {
3233
private static final String TRACEPARENT = encode("traceparent");
3334
private static final String DD_SERVICE_HASH = encode("ddsh");
3435

36+
/**
37+
* Cache for static comment strings (those without traceParent or peerService). The key combines
38+
* dbService, hostname, dbName, and Config identity to ensure correctness if Config is replaced.
39+
*/
40+
private static final ConcurrentHashMap<StaticCommentKey, String> staticCommentCache =
41+
new ConcurrentHashMap<>();
42+
43+
// Pre-computed marker strings for trace comment detection
44+
private static final String PARENT_SERVICE_EQ = PARENT_SERVICE + "=";
45+
private static final String DATABASE_SERVICE_EQ = DATABASE_SERVICE + "=";
46+
private static final String DD_HOSTNAME_EQ = DD_HOSTNAME + "=";
47+
private static final String DD_DB_NAME_EQ = DD_DB_NAME + "=";
48+
private static final String DD_PEER_SERVICE_EQ = DD_PEER_SERVICE + "=";
49+
private static final String DD_ENV_EQ = DD_ENV + "=";
50+
private static final String DD_VERSION_EQ = DD_VERSION + "=";
51+
private static final String TRACEPARENT_EQ = TRACEPARENT + "=";
52+
private static final String DD_SERVICE_HASH_EQ = DD_SERVICE_HASH + "=";
53+
3554
// Used by SQLCommenter and MongoCommentInjector to avoid duplicate comment injection
3655
public static boolean containsTraceComment(String commentContent) {
37-
return commentContent.contains(PARENT_SERVICE + "=")
38-
|| commentContent.contains(DATABASE_SERVICE + "=")
39-
|| commentContent.contains(DD_HOSTNAME + "=")
40-
|| commentContent.contains(DD_DB_NAME + "=")
41-
|| commentContent.contains(DD_PEER_SERVICE + "=")
42-
|| commentContent.contains(DD_ENV + "=")
43-
|| commentContent.contains(DD_VERSION + "=")
44-
|| commentContent.contains(TRACEPARENT + "=")
45-
|| commentContent.contains(DD_SERVICE_HASH + "=");
56+
return commentContent.contains(PARENT_SERVICE_EQ)
57+
|| commentContent.contains(DATABASE_SERVICE_EQ)
58+
|| commentContent.contains(DD_HOSTNAME_EQ)
59+
|| commentContent.contains(DD_DB_NAME_EQ)
60+
|| commentContent.contains(DD_PEER_SERVICE_EQ)
61+
|| commentContent.contains(DD_ENV_EQ)
62+
|| commentContent.contains(DD_VERSION_EQ)
63+
|| commentContent.contains(TRACEPARENT_EQ)
64+
|| commentContent.contains(DD_SERVICE_HASH_EQ);
65+
}
66+
67+
/**
68+
* Checks for trace comment markers within a range of the given string, without allocating a
69+
* substring. Searches within [fromIndex, toIndex) of the source string.
70+
*/
71+
public static boolean containsTraceComment(String sql, int fromIndex, int toIndex) {
72+
return containsInRange(sql, PARENT_SERVICE_EQ, fromIndex, toIndex)
73+
|| containsInRange(sql, DATABASE_SERVICE_EQ, fromIndex, toIndex)
74+
|| containsInRange(sql, DD_HOSTNAME_EQ, fromIndex, toIndex)
75+
|| containsInRange(sql, DD_DB_NAME_EQ, fromIndex, toIndex)
76+
|| containsInRange(sql, DD_PEER_SERVICE_EQ, fromIndex, toIndex)
77+
|| containsInRange(sql, DD_ENV_EQ, fromIndex, toIndex)
78+
|| containsInRange(sql, DD_VERSION_EQ, fromIndex, toIndex)
79+
|| containsInRange(sql, TRACEPARENT_EQ, fromIndex, toIndex)
80+
|| containsInRange(sql, DD_SERVICE_HASH_EQ, fromIndex, toIndex);
81+
}
82+
83+
/** Checks if {@code target} appears within the range [fromIndex, toIndex) of {@code source}. */
84+
private static boolean containsInRange(String source, String target, int fromIndex, int toIndex) {
85+
int targetLen = target.length();
86+
int limit = toIndex - targetLen;
87+
for (int i = fromIndex; i <= limit; i++) {
88+
if (source.regionMatches(i, target, 0, targetLen)) {
89+
return true;
90+
}
91+
}
92+
return false;
4693
}
4794

4895
// Build database comment content without comment delimiters such as /* */
@@ -69,6 +116,60 @@ public static String buildComment(
69116
return sb.length() > 0 ? sb.toString() : null;
70117
}
71118

119+
/**
120+
* Builds the static portion of a database comment that does not change per-span. This includes
121+
* parentService, databaseService, hostname, dbName, env, version, and serviceHash. The dynamic
122+
* parts (peerService, traceParent) are excluded and must be appended separately.
123+
*
124+
* <p>Results are cached per (dbService, hostname, dbName, Config) combination to avoid redundant
125+
* URLEncoder.encode() calls and StringBuilder work on every query execution.
126+
*
127+
* @return the static comment prefix, or null if no static fields are set
128+
*/
129+
public static String buildStaticComment(String dbService, String hostname, String dbName) {
130+
Config config = Config.get();
131+
StaticCommentKey key = new StaticCommentKey(dbService, hostname, dbName, config);
132+
String cached = staticCommentCache.get(key);
133+
if (cached != null) {
134+
return cached;
135+
}
136+
137+
StringBuilder sb = new StringBuilder();
138+
139+
int initSize = 0;
140+
append(sb, PARENT_SERVICE, config.getServiceName(), initSize);
141+
append(sb, DATABASE_SERVICE, dbService, initSize);
142+
append(sb, DD_HOSTNAME, hostname, initSize);
143+
append(sb, DD_DB_NAME, dbName, initSize);
144+
// peerService is per-span, skip here
145+
append(sb, DD_ENV, config.getEnv(), initSize);
146+
append(sb, DD_VERSION, config.getVersion(), initSize);
147+
// traceParent is per-span, skip here
148+
149+
if (config.isDbmInjectSqlBaseHash() && config.isExperimentalPropagateProcessTagsEnabled()) {
150+
append(sb, DD_SERVICE_HASH, BaseHash.getBaseHashStr(), initSize);
151+
}
152+
153+
String result = sb.length() > 0 ? sb.toString() : null;
154+
if (result != null) {
155+
staticCommentCache.putIfAbsent(key, result);
156+
}
157+
return result;
158+
}
159+
160+
/** Returns true if the current active span has a non-null, non-empty peer service tag. */
161+
public static boolean hasPeerService() {
162+
AgentSpan span = activeSpan();
163+
if (span != null) {
164+
Object peerService = span.getTag(Tags.PEER_SERVICE);
165+
if (peerService != null) {
166+
String str = peerService.toString();
167+
return str != null && !str.isEmpty();
168+
}
169+
}
170+
return false;
171+
}
172+
72173
private static String getPeerService() {
73174
AgentSpan span = activeSpan();
74175
Object peerService = null;
@@ -104,4 +205,50 @@ private static void append(StringBuilder sb, String key, String value, int initS
104205
}
105206
sb.append(key).append(EQUALS).append(QUOTE).append(encodedValue).append(QUOTE);
106207
}
208+
209+
/**
210+
* Cache key for static comment lookup. Uses Config identity (rather than individual field values)
211+
* to ensure the cache is automatically invalidated if Config is replaced (as happens in tests).
212+
* In production, Config is created once, so identity comparison is both correct and cheap.
213+
*/
214+
private static final class StaticCommentKey {
215+
private final String dbService;
216+
private final String hostname;
217+
private final String dbName;
218+
private final Config config;
219+
private final int hashCode;
220+
221+
StaticCommentKey(String dbService, String hostname, String dbName, Config config) {
222+
this.dbService = dbService;
223+
this.hostname = hostname;
224+
this.dbName = dbName;
225+
this.config = config;
226+
int h = 17;
227+
h = 31 * h + (dbService != null ? dbService.hashCode() : 0);
228+
h = 31 * h + (hostname != null ? hostname.hashCode() : 0);
229+
h = 31 * h + (dbName != null ? dbName.hashCode() : 0);
230+
h = 31 * h + System.identityHashCode(config);
231+
this.hashCode = h;
232+
}
233+
234+
@Override
235+
public boolean equals(Object o) {
236+
if (this == o) return true;
237+
if (!(o instanceof StaticCommentKey)) return false;
238+
StaticCommentKey that = (StaticCommentKey) o;
239+
return config == that.config
240+
&& eq(dbService, that.dbService)
241+
&& eq(hostname, that.hostname)
242+
&& eq(dbName, that.dbName);
243+
}
244+
245+
private static boolean eq(String a, String b) {
246+
return a == null ? b == null : a.equals(b);
247+
}
248+
249+
@Override
250+
public int hashCode() {
251+
return hashCode;
252+
}
253+
}
107254
}

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

Lines changed: 57 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,11 @@ public class SQLCommenter {
1515
private static final int BUFFER_EXTRA = 4;
1616
private static final int SQL_COMMENT_OVERHEAD = SPACE_CHARS + COMMENT_DELIMITERS + BUFFER_EXTRA;
1717

18+
/**
19+
* Returns the first non-whitespace word in the SQL string. The result is a substring allocation,
20+
* so callers that only need to check prefix/equality should use {@link #firstWordStartsWith} or
21+
* {@link #firstWordEqualsIgnoreCase} instead.
22+
*/
1823
protected static String getFirstWord(String sql) {
1924
int beginIndex = 0;
2025
while (beginIndex < sql.length() && Character.isWhitespace(sql.charAt(beginIndex))) {
@@ -27,6 +32,38 @@ protected static String getFirstWord(String sql) {
2732
return sql.substring(beginIndex, endIndex);
2833
}
2934

35+
/**
36+
* Checks if the first non-whitespace word in sql starts with the given prefix, without allocating
37+
* a substring.
38+
*/
39+
private static boolean firstWordStartsWith(String sql, String prefix) {
40+
int beginIndex = 0;
41+
int len = sql.length();
42+
while (beginIndex < len && Character.isWhitespace(sql.charAt(beginIndex))) {
43+
beginIndex++;
44+
}
45+
return sql.regionMatches(beginIndex, prefix, 0, prefix.length());
46+
}
47+
48+
/**
49+
* Checks if the first non-whitespace word in sql equals the given target (case-insensitive),
50+
* without allocating a substring.
51+
*/
52+
private static boolean firstWordEqualsIgnoreCase(String sql, String target) {
53+
int beginIndex = 0;
54+
int len = sql.length();
55+
while (beginIndex < len && Character.isWhitespace(sql.charAt(beginIndex))) {
56+
beginIndex++;
57+
}
58+
int endIndex = beginIndex;
59+
while (endIndex < len && !Character.isWhitespace(sql.charAt(endIndex))) {
60+
endIndex++;
61+
}
62+
int wordLen = endIndex - beginIndex;
63+
return wordLen == target.length()
64+
&& sql.regionMatches(true, beginIndex, target, 0, target.length());
65+
}
66+
3067
public static String inject(
3168
String sql,
3269
String dbService,
@@ -40,25 +77,25 @@ public static String inject(
4077
}
4178
boolean appendComment = preferAppend;
4279
if (dbType != null) {
43-
final String firstWord = getFirstWord(sql);
80+
boolean startsWithBrace = firstWordStartsWith(sql, "{");
4481

4582
// The Postgres JDBC parser doesn't allow SQL comments anywhere in a JDBC
4683
// callable statements
4784
// https://github.com/pgjdbc/pgjdbc/blob/master/pgjdbc/src/main/java/org/postgresql/core/Parser.java#L1038
4885
// TODO: Could we inject the comment after the JDBC has been converted to
4986
// standard SQL?
50-
if (firstWord.startsWith("{") && dbType.startsWith("postgres")) {
87+
if (startsWithBrace && dbType.startsWith("postgres")) {
5188
return sql;
5289
}
5390

5491
// Append the comment for mysql JDBC callable statements
55-
if (firstWord.startsWith("{") && "mysql".equals(dbType)) {
92+
if (startsWithBrace && "mysql".equals(dbType)) {
5693
appendComment = true;
5794
}
5895

5996
// Both Postgres and MySQL are unhappy with anything before CALL in a stored
6097
// procedure invocation, but they seem ok with it after so we force append mode
61-
if (firstWord.equalsIgnoreCase("call")) {
98+
if (firstWordEqualsIgnoreCase(sql, "call")) {
6299
appendComment = true;
63100
}
64101

@@ -71,8 +108,18 @@ public static String inject(
71108
return sql;
72109
}
73110

74-
String commentContent =
75-
SharedDBCommenter.buildComment(dbService, dbType, hostname, dbName, traceParent);
111+
// When there are no per-span dynamic fields (traceParent, peerService), use the cached
112+
// static comment to avoid redundant URLEncoder.encode() calls and StringBuilder allocations.
113+
// This is the common path in DBM "static" propagation mode.
114+
boolean hasDynamic =
115+
(traceParent != null && !traceParent.isEmpty()) || SharedDBCommenter.hasPeerService();
116+
String commentContent;
117+
if (!hasDynamic) {
118+
commentContent = SharedDBCommenter.buildStaticComment(dbService, hostname, dbName);
119+
} else {
120+
commentContent =
121+
SharedDBCommenter.buildComment(dbService, dbType, hostname, dbName, traceParent);
122+
}
76123

77124
if (commentContent == null) {
78125
return sql;
@@ -112,11 +159,6 @@ private static boolean hasDDComment(String sql, boolean appendComment) {
112159
return false;
113160
}
114161

115-
String commentContent = extractCommentContent(sql, appendComment);
116-
return SharedDBCommenter.containsTraceComment(commentContent);
117-
}
118-
119-
private static String extractCommentContent(String sql, boolean appendComment) {
120162
int startIdx;
121163
int endIdx;
122164
if (appendComment) {
@@ -127,9 +169,11 @@ private static String extractCommentContent(String sql, boolean appendComment) {
127169
endIdx = sql.indexOf(CLOSE_COMMENT);
128170
}
129171
if (startIdx != -1 && endIdx != -1 && endIdx > startIdx) {
130-
return sql.substring(startIdx + OPEN_COMMENT_LEN, endIdx);
172+
// Check for trace comment markers directly in the SQL without allocating a substring.
173+
// We search within the bounds [startIdx + OPEN_COMMENT_LEN, endIdx) of the original string.
174+
return SharedDBCommenter.containsTraceComment(sql, startIdx + OPEN_COMMENT_LEN, endIdx);
131175
}
132-
return "";
176+
return false;
133177
}
134178

135179
/**

0 commit comments

Comments
 (0)