Skip to content

Commit a26939e

Browse files
dougqhclaude
andcommitted
Remove static comment cache per review feedback
Remove the ConcurrentHashMap-based static comment cache and StaticCommentKey class from SharedDBCommenter. Always use the existing buildComment() path. Keep the pre-computed marker constants and allocation-free containsTraceComment overload. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent c627efa commit a26939e

2 files changed

Lines changed: 2 additions & 120 deletions

File tree

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

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

@@ -33,13 +32,6 @@ public class SharedDBCommenter {
3332
private static final String TRACEPARENT = encode("traceparent");
3433
private static final String DD_SERVICE_HASH = encode("ddsh");
3534

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-
4335
// Pre-computed marker strings for trace comment detection
4436
private static final String PARENT_SERVICE_EQ = PARENT_SERVICE + "=";
4537
private static final String DATABASE_SERVICE_EQ = DATABASE_SERVICE + "=";
@@ -116,60 +108,6 @@ public static String buildComment(
116108
return sb.length() > 0 ? sb.toString() : null;
117109
}
118110

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-
173111
private static String getPeerService() {
174112
AgentSpan span = activeSpan();
175113
Object peerService = null;
@@ -205,50 +143,4 @@ private static void append(StringBuilder sb, String key, String value, int initS
205143
}
206144
sb.append(key).append(EQUALS).append(QUOTE).append(encodedValue).append(QUOTE);
207145
}
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-
}
254146
}

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

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -108,18 +108,8 @@ public static String inject(
108108
return sql;
109109
}
110110

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-
}
111+
String commentContent =
112+
SharedDBCommenter.buildComment(dbService, dbType, hostname, dbName, traceParent);
123113

124114
if (commentContent == null) {
125115
return sql;

0 commit comments

Comments
 (0)