Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,6 @@ public static class Builder {

public Builder type(String type) {
this.type = type;
// Those DBs use the full text of the query including the comments as a cache key,
// so we disable full propagation support for them to avoid destroying the cache.
if (type.equals("oracle")) this.fullPropagationSupport = false;
return this;
}

Expand Down
2 changes: 2 additions & 0 deletions dd-java-agent/instrumentation/jdbc/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,12 @@ dependencies {
testImplementation group: 'mysql', name: 'mysql-connector-java', version: '8.0.23'
testImplementation group: 'org.postgresql', name: 'postgresql', version: '[9.4,42.2.18]'
testImplementation group: 'com.microsoft.sqlserver', name: 'mssql-jdbc', version: '10.2.0.jre8'
testImplementation group: 'com.oracle.database.jdbc', name: 'ojdbc8', version: '19.19.0.0'

testImplementation group: 'org.testcontainers', name:'mysql', version: libs.versions.testcontainers.get()
testImplementation group: 'org.testcontainers', name:'postgresql', version: libs.versions.testcontainers.get()
testImplementation group: 'org.testcontainers', name:'mssqlserver', version: libs.versions.testcontainers.get()
testImplementation group: 'org.testcontainers', name:'oracle-xe', version: '1.20.4'

testRuntimeOnly project(':dd-java-agent:instrumentation:iast-instrumenter')

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,9 @@ public static AgentScope onEnter(@Advice.This final Statement statement) {
} else if (DECORATE.isPostgres(dbInfo) && DBM_TRACE_PREPARED_STATEMENTS) {
span = startSpan(DATABASE_QUERY);
DECORATE.setApplicationName(span, connection);
} else if (DECORATE.isOracle(dbInfo)) {
span = startSpan(DATABASE_QUERY);
DECORATE.setAction(span, connection);
} else {
span = startSpan(DATABASE_QUERY);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ public class JDBCDecorator extends DatabaseClientDecorator<DBInfo> {
public static final String DBM_PROPAGATION_MODE_STATIC = "service";
public static final String DBM_PROPAGATION_MODE_FULL = "full";

public static final String DD_INSTRUMENTATION_PREFIX = "_DD_";

public static final String DBM_PROPAGATION_MODE = Config.get().getDBMPropagationMode();
public static final boolean INJECT_COMMENT =
DBM_PROPAGATION_MODE.equals(DBM_PROPAGATION_MODE_FULL)
Expand Down Expand Up @@ -252,6 +254,10 @@ public String traceParent(AgentSpan span, int samplingPriority) {
return sb.toString();
}

public boolean isOracle(final DBInfo dbInfo) {
return "oracle".equals(dbInfo.getType());
}

public boolean isPostgres(final DBInfo dbInfo) {
return dbInfo.getType().startsWith("postgres");
}
Expand All @@ -260,6 +266,38 @@ public boolean isSqlServer(final DBInfo dbInfo) {
return "sqlserver".equals(dbInfo.getType());
}

/**
* Executes `connection.setClientInfo("OCSID.ACTION", traceContext)` statement on the Oracle DB to
* set the trace parent in `v$session.action`. This is used because it isn't possible to propagate
* trace parent with the comment.
*
* @param span The span of the instrumented statement
* @param connection The same connection as the one that will be used for the actual statement
*/
public void setAction(AgentSpan span, Connection connection) {
try {

Integer priority = span.forceSamplingDecision();
if (priority == null) {
return;
}
final String traceContext = DD_INSTRUMENTATION_PREFIX + DECORATE.traceParent(span, priority);

connection.setClientInfo("OCSID.ACTION", traceContext);

} catch (Throwable e) {
log.debug(
"Failed to set extra DBM data in application_name for trace {}. "
+ "To disable this behavior, set trace_prepared_statements to 'false'. "
+ "See https://docs.datadoghq.com/database_monitoring/connect_dbm_and_apm/ for more info. {}",
span.getTraceId().toHexString(),
e);
DECORATE.onError(span, e);
} finally {
span.setTag("_dd.dbm_trace_injected", true);
}
}

/**
* Executes a `SET CONTEXT_INFO` statement on the DB with the active trace ID and the given span
* ID. This context will be "attached" to future queries on the same connection. See <a
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,12 +87,20 @@ public static AgentScope onEnter(
boolean injectTraceContext = DECORATE.shouldInjectTraceContext(dbInfo);
final AgentSpan span;
final boolean isSqlServer = DECORATE.isSqlServer(dbInfo);
final boolean isOracle = DECORATE.isOracle(dbInfo);

if (isSqlServer && INJECT_COMMENT && injectTraceContext) {
// The span ID is pre-determined so that we can reference it when setting the context
final long spanID = DECORATE.setContextInfo(connection, dbInfo);
// we then force that pre-determined span ID for the span covering the actual query
span = AgentTracer.get().buildSpan(DATABASE_QUERY).withSpanId(spanID).start();
if (INJECT_COMMENT && injectTraceContext) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: What about having only one startSpan() as default case, and grouping custom span decoration together like:

if (INJECT_COMMENT && injectTraceContext && (isSqlServer || isOracle)) {
  if (isSqlServer) { 
    // DECORATE + build/start
  } else if (isOracle) {
    // start + DECORATE
  }
 } else {
   startSpan()
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understood. Is it OK to merge the PR as it is, and do the change when we remove the instrumentation span for SQL Server? The line 96 (span = AgentTracer.get().buildSpan(DATABASE_QUERY).withSpanId(spanID).start();) will then become span = startSpan(DATABASE_QUERY);, so we'll be able to remove span = startSpan(DATABASE_QUERY); from all branches.

if (isSqlServer) {
// The span ID is pre-determined so that we can reference it when setting the context
final long spanID = DECORATE.setContextInfo(connection, dbInfo);
// we then force that pre-determined span ID for the span covering the actual query
span = AgentTracer.get().buildSpan(DATABASE_QUERY).withSpanId(spanID).start();
} else if (isOracle) {
span = startSpan(DATABASE_QUERY);
DECORATE.setAction(span, connection);
} else {
span = startSpan(DATABASE_QUERY);
}
} else {
span = startSpan(DATABASE_QUERY);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ abstract class JDBCDecoratorTest extends AgentTestRunner {

where:
dbType | expectedByType
"oracle" | false
"oracle" | true
"sqlserver" | true
"mysql" | true
"postgresql" | true
Expand Down
Loading
Loading