diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/DriverInstrumentation.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/DriverInstrumentation.java index 17ac8d7629a..c5c266ed312 100644 --- a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/DriverInstrumentation.java +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/DriverInstrumentation.java @@ -64,6 +64,7 @@ public void methodAdvice(MethodTransformer transformer) { } public static class DriverAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) public static void addDBInfo( @Advice.Argument(0) final String url, @@ -75,8 +76,10 @@ public static void addDBInfo( } String connectionUrl = url; Properties connectionProps = props; - if (null == props - || !Boolean.parseBoolean(props.getProperty("oracle.jdbc.useShardingDriverConnection"))) { + if (JDBCDecorator.FETCH_DB_METADATA_ON_CONNECT + && (null == props + || !Boolean.parseBoolean( + props.getProperty("oracle.jdbc.useShardingDriverConnection")))) { try { DatabaseMetaData metaData = connection.getMetaData(); connectionUrl = metaData.getURL(); diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java index ee0dffc67c3..2e5e414f787 100644 --- a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java @@ -28,6 +28,7 @@ import java.sql.SQLException; import java.sql.Statement; import java.util.HashSet; +import java.util.Properties; import java.util.Set; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -59,6 +60,10 @@ public class JDBCDecorator extends DatabaseClientDecorator { DBM_PROPAGATION_MODE.equals(DBM_PROPAGATION_MODE_FULL); public static final boolean DBM_TRACE_PREPARED_STATEMENTS = Config.get().isDbmTracePreparedStatements(); + private static final boolean FETCH_DB_METADATA_ON_QUERY = + Config.get().isDbMetadataFetchingOnQueryEnabled(); + public static final boolean FETCH_DB_METADATA_ON_CONNECT = + Config.get().isDbMetadataFetchingOnConnectEnabled(); private volatile boolean warnedAboutDBMPropagationMode = false; // to log a warning only once @@ -181,7 +186,7 @@ public static DBInfo parseDBInfo( } catch (Throwable ignore) { } if (dbInfo == null) { - // couldn't find DBInfo anywhere, so fall back to default + // couldn't find DBInfo from a previous call anywhere, so we try to fetch it from the DB dbInfo = parseDBInfoFromConnection(connection); } // store the DBInfo on the outermost connection instance to avoid future searches @@ -200,7 +205,7 @@ public String getDbService(final DBInfo dbInfo) { } public static DBInfo parseDBInfoFromConnection(final Connection connection) { - if (connection == null) { + if (connection == null || !FETCH_DB_METADATA_ON_QUERY) { // we can log here, but it risks to be too verbose return DBInfo.DEFAULT; } @@ -209,16 +214,19 @@ public static DBInfo parseDBInfoFromConnection(final Connection connection) { final DatabaseMetaData metaData = connection.getMetaData(); final String url; if (metaData != null && (url = metaData.getURL()) != null) { + Properties clientInfo = null; try { - dbInfo = JDBCConnectionUrlParser.extractDBInfo(url, connection.getClientInfo()); + clientInfo = connection.getClientInfo(); } catch (final Throwable ex) { - // getClientInfo is likely not allowed. - dbInfo = JDBCConnectionUrlParser.extractDBInfo(url, null); + // getClientInfo is likely not allowed, we can still extract info from the url alone + log.debug("Could not get client info from DB", ex); } + dbInfo = JDBCConnectionUrlParser.extractDBInfo(url, clientInfo); } else { dbInfo = DBInfo.DEFAULT; } } catch (final SQLException se) { + log.debug("Could not get metadata from DB", se); dbInfo = DBInfo.DEFAULT; } return dbInfo; diff --git a/dd-java-agent/instrumentation/jdbc/src/test/groovy/JDBCDecoratorParseDBInfoTestBase.groovy b/dd-java-agent/instrumentation/jdbc/src/test/groovy/JDBCDecoratorParseDBInfoTestBase.groovy new file mode 100644 index 00000000000..6efb257d605 --- /dev/null +++ b/dd-java-agent/instrumentation/jdbc/src/test/groovy/JDBCDecoratorParseDBInfoTestBase.groovy @@ -0,0 +1,104 @@ +import static datadog.trace.api.config.TraceInstrumentationConfig.DB_METADATA_FETCHING_ON_QUERY + +import datadog.trace.agent.test.InstrumentationSpecification +import datadog.trace.bootstrap.instrumentation.jdbc.DBInfo +import datadog.trace.instrumentation.jdbc.JDBCDecorator +import java.sql.Connection +import test.TestDatabaseMetaData + +/** + * Base test class for parseDBInfoFromConnection with different flag configurations + */ +abstract class JDBCDecoratorParseDBInfoTestBase extends InstrumentationSpecification { + + def "test parseDBInfoFromConnection with null connection"() { + when: + def result = JDBCDecorator.parseDBInfoFromConnection(null) + + then: + result == DBInfo.DEFAULT + } + + def "test parseDBInfoFromConnection with null ClientInfo"() { + setup: + def metadata = new TestDatabaseMetaData() + metadata.setURL("jdbc:postgresql://testhost:5432/testdb") + def connection = Mock(Connection) { + getMetaData() >> metadata + getClientInfo() >> null + } + + when: + def result = JDBCDecorator.parseDBInfoFromConnection(connection) + + then: + if (shouldFetchMetadata()) { + result.type == "postgresql" + result.host == "testhost" + result.port == 5432 + result.db == "testdb" + } else { + result == DBInfo.DEFAULT + } + } + + def "test parseDBInfoFromConnection regular case"() { + setup: + def metadata = new TestDatabaseMetaData() + metadata.setURL("jdbc:postgresql://testhost:5432/testdb") + def clientInfo = new Properties() + clientInfo.setProperty("warehouse", "my-test-warehouse") // we'll check that property to know if clientInfo were used + def connection = Mock(Connection) { + getMetaData() >> (shouldFetchMetadata() ? metadata : { assert false }) + } + + when: + def result = JDBCDecorator.parseDBInfoFromConnection(connection) + + then: + if (shouldFetchMetadata()) { + result.type == "postgresql" + result.host == "testhost" + result.port == 5432 + result.db == "testdb" + result.warehouse == "my-test-warehouse" + } else { + result == DBInfo.DEFAULT + } + } + + abstract boolean shouldFetchMetadata() +} + +/** + * Test with both flags enabled (default behavior) + */ +class JDBCDecoratorParseDBInfoWithMetadataForkedTest extends JDBCDecoratorParseDBInfoTestBase { + + @Override + void configurePreAgent() { + super.configurePreAgent() + injectSysConfig(DB_METADATA_FETCHING_ON_QUERY, "true") + } + + @Override + boolean shouldFetchMetadata() { + return true + } +} + +class JDBCDecoratorParseDBInfoWithoutCallsForkedTest extends JDBCDecoratorParseDBInfoTestBase { + + @Override + void configurePreAgent() { + super.configurePreAgent() + injectSysConfig(DB_METADATA_FETCHING_ON_QUERY, "false") + } + + @Override + boolean shouldFetchMetadata() { + return false + } +} + + diff --git a/dd-trace-api/src/main/java/datadog/trace/api/config/TraceInstrumentationConfig.java b/dd-trace-api/src/main/java/datadog/trace/api/config/TraceInstrumentationConfig.java index e96900c073d..6022ef6cf39 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/config/TraceInstrumentationConfig.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/config/TraceInstrumentationConfig.java @@ -63,6 +63,10 @@ public final class TraceInstrumentationConfig { "trace.db.client.split-by-instance.type.suffix"; public static final String DB_CLIENT_HOST_SPLIT_BY_HOST = "trace.db.client.split-by-host"; + public static final String DB_METADATA_FETCHING_ON_QUERY = "trace.db.metadata.fetching.on.query"; + public static final String DB_METADATA_FETCHING_ON_CONNECT = + "trace.db.metadata.fetching.on.connect"; + public static final String JDBC_PREPARED_STATEMENT_CLASS_NAME = "trace.jdbc.prepared.statement.class.name"; diff --git a/internal-api/src/main/java/datadog/trace/api/Config.java b/internal-api/src/main/java/datadog/trace/api/Config.java index b643371221b..d8034f45b4e 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/internal-api/src/main/java/datadog/trace/api/Config.java @@ -511,6 +511,8 @@ import static datadog.trace.api.config.TraceInstrumentationConfig.DB_DBM_INJECT_SQL_BASEHASH; import static datadog.trace.api.config.TraceInstrumentationConfig.DB_DBM_PROPAGATION_MODE_MODE; import static datadog.trace.api.config.TraceInstrumentationConfig.DB_DBM_TRACE_PREPARED_STATEMENTS; +import static datadog.trace.api.config.TraceInstrumentationConfig.DB_METADATA_FETCHING_ON_CONNECT; +import static datadog.trace.api.config.TraceInstrumentationConfig.DB_METADATA_FETCHING_ON_QUERY; import static datadog.trace.api.config.TraceInstrumentationConfig.ELASTICSEARCH_BODY_AND_PARAMS_ENABLED; import static datadog.trace.api.config.TraceInstrumentationConfig.ELASTICSEARCH_BODY_ENABLED; import static datadog.trace.api.config.TraceInstrumentationConfig.ELASTICSEARCH_PARAMS_ENABLED; @@ -1076,6 +1078,8 @@ public static String getHostName() { private final boolean dbmInjectSqlBaseHash; private final String dbmPropagationMode; private final boolean dbmTracePreparedStatements; + private final boolean dbMetadataFetchingOnQuery; + private final boolean dbMetadataFetchingOnConnect; private final boolean dynamicInstrumentationEnabled; private final String dynamicInstrumentationSnapshotUrl; @@ -1626,6 +1630,9 @@ private Config(final ConfigProvider configProvider, final InstrumenterConfig ins DB_CLIENT_HOST_SPLIT_BY_INSTANCE_TYPE_SUFFIX, DEFAULT_DB_CLIENT_HOST_SPLIT_BY_INSTANCE_TYPE_SUFFIX); + dbMetadataFetchingOnQuery = configProvider.getBoolean(DB_METADATA_FETCHING_ON_QUERY, true); + dbMetadataFetchingOnConnect = configProvider.getBoolean(DB_METADATA_FETCHING_ON_CONNECT, true); + dbClientSplitByHost = configProvider.getBoolean( DB_CLIENT_HOST_SPLIT_BY_HOST, DEFAULT_DB_CLIENT_HOST_SPLIT_BY_HOST); @@ -3146,6 +3153,14 @@ public boolean isDbClientSplitByHost() { return dbClientSplitByHost; } + public boolean isDbMetadataFetchingOnQueryEnabled() { + return dbMetadataFetchingOnQuery; + } + + public boolean isDbMetadataFetchingOnConnectEnabled() { + return dbMetadataFetchingOnConnect; + } + public Set getSplitByTags() { return splitByTags; } @@ -5551,6 +5566,10 @@ public String toString() { + dbClientSplitByInstanceTypeSuffix + ", dbClientSplitByHost=" + dbClientSplitByHost + + ", dbMetadataFetchingEnabled=" + + dbMetadataFetchingOnQuery + + ", dbMetadataFetchingOnConnect=" + + dbMetadataFetchingOnConnect + ", dbmInjectSqlBaseHash=" + dbmInjectSqlBaseHash + ", dbmPropagationMode=" diff --git a/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy index 8d2bb2a76f7..b99474da231 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy @@ -1,12 +1,5 @@ package datadog.trace.api -import datadog.trace.api.env.FixedCapturedEnvironment -import datadog.trace.bootstrap.config.provider.AgentArgsInjector -import datadog.trace.bootstrap.config.provider.ConfigConverter -import datadog.trace.bootstrap.config.provider.ConfigProvider -import datadog.trace.test.util.DDSpecification -import datadog.trace.util.throwable.FatalAgentMisconfigurationError - import static datadog.trace.api.ConfigDefaults.DEFAULT_HTTP_CLIENT_ERROR_STATUSES import static datadog.trace.api.ConfigDefaults.DEFAULT_HTTP_SERVER_ERROR_STATUSES import static datadog.trace.api.ConfigDefaults.DEFAULT_PARTIAL_FLUSH_MIN_SPANS @@ -22,10 +15,10 @@ import static datadog.trace.api.DDTags.SERVICE import static datadog.trace.api.DDTags.SERVICE_TAG import static datadog.trace.api.TracePropagationStyle.B3MULTI import static datadog.trace.api.TracePropagationStyle.B3SINGLE +import static datadog.trace.api.TracePropagationStyle.BAGGAGE import static datadog.trace.api.TracePropagationStyle.DATADOG import static datadog.trace.api.TracePropagationStyle.HAYSTACK import static datadog.trace.api.TracePropagationStyle.TRACECONTEXT -import static datadog.trace.api.TracePropagationStyle.BAGGAGE import static datadog.trace.api.config.CiVisibilityConfig.CIVISIBILITY_AGENTLESS_ENABLED import static datadog.trace.api.config.CiVisibilityConfig.CIVISIBILITY_ENABLED import static datadog.trace.api.config.DebuggerConfig.DYNAMIC_INSTRUMENTATION_CLASSFILE_DUMP_ENABLED @@ -51,16 +44,16 @@ import static datadog.trace.api.config.GeneralConfig.GLOBAL_TAGS import static datadog.trace.api.config.GeneralConfig.HEALTH_METRICS_ENABLED import static datadog.trace.api.config.GeneralConfig.HEALTH_METRICS_STATSD_HOST import static datadog.trace.api.config.GeneralConfig.HEALTH_METRICS_STATSD_PORT +import static datadog.trace.api.config.GeneralConfig.INSTRUMENTATION_SOURCE import static datadog.trace.api.config.GeneralConfig.JDK_SOCKET_ENABLED import static datadog.trace.api.config.GeneralConfig.PERF_METRICS_ENABLED import static datadog.trace.api.config.GeneralConfig.SERVICE_NAME import static datadog.trace.api.config.GeneralConfig.SITE +import static datadog.trace.api.config.GeneralConfig.SSI_INJECTION_ENABLED +import static datadog.trace.api.config.GeneralConfig.SSI_INJECTION_FORCE import static datadog.trace.api.config.GeneralConfig.TAGS import static datadog.trace.api.config.GeneralConfig.TRACER_METRICS_IGNORED_RESOURCES import static datadog.trace.api.config.GeneralConfig.VERSION -import static datadog.trace.api.config.GeneralConfig.SSI_INJECTION_ENABLED -import static datadog.trace.api.config.GeneralConfig.SSI_INJECTION_FORCE -import static datadog.trace.api.config.GeneralConfig.INSTRUMENTATION_SOURCE import static datadog.trace.api.config.JmxFetchConfig.JMX_FETCH_CHECK_PERIOD import static datadog.trace.api.config.JmxFetchConfig.JMX_FETCH_ENABLED import static datadog.trace.api.config.JmxFetchConfig.JMX_FETCH_METRICS_CONFIGS @@ -69,8 +62,8 @@ import static datadog.trace.api.config.JmxFetchConfig.JMX_FETCH_STATSD_HOST import static datadog.trace.api.config.JmxFetchConfig.JMX_FETCH_STATSD_PORT import static datadog.trace.api.config.JmxFetchConfig.JMX_TAGS import static datadog.trace.api.config.LlmObsConfig.LLMOBS_AGENTLESS_ENABLED -import static datadog.trace.api.config.LlmObsConfig.LLMOBS_ML_APP import static datadog.trace.api.config.LlmObsConfig.LLMOBS_ENABLED +import static datadog.trace.api.config.LlmObsConfig.LLMOBS_ML_APP import static datadog.trace.api.config.ProfilingConfig.PROFILING_AGENTLESS import static datadog.trace.api.config.ProfilingConfig.PROFILING_API_KEY_FILE_OLD import static datadog.trace.api.config.ProfilingConfig.PROFILING_API_KEY_FILE_VERY_OLD @@ -111,10 +104,6 @@ import static datadog.trace.api.config.TracerConfig.HTTP_CLIENT_ERROR_STATUSES import static datadog.trace.api.config.TracerConfig.HTTP_SERVER_ERROR_STATUSES import static datadog.trace.api.config.TracerConfig.ID_GENERATION_STRATEGY import static datadog.trace.api.config.TracerConfig.PARTIAL_FLUSH_ENABLED -import static datadog.trace.api.config.TracerConfig.TRACE_EXPERIMENTAL_FEATURES_ENABLED -import static datadog.trace.api.config.TracerConfig.TRACE_LONG_RUNNING_ENABLED -import static datadog.trace.api.config.TracerConfig.TRACE_LONG_RUNNING_FLUSH_INTERVAL -import static datadog.trace.api.config.TracerConfig.TRACE_LONG_RUNNING_INITIAL_FLUSH_INTERVAL import static datadog.trace.api.config.TracerConfig.PARTIAL_FLUSH_MIN_SPANS import static datadog.trace.api.config.TracerConfig.PRIORITIZATION_TYPE import static datadog.trace.api.config.TracerConfig.PRIORITY_SAMPLING @@ -127,8 +116,12 @@ import static datadog.trace.api.config.TracerConfig.SPAN_TAGS import static datadog.trace.api.config.TracerConfig.SPLIT_BY_TAGS import static datadog.trace.api.config.TracerConfig.TRACE_AGENT_PORT import static datadog.trace.api.config.TracerConfig.TRACE_AGENT_URL -import static datadog.trace.api.config.TracerConfig.TRACE_PROPAGATION_EXTRACT_FIRST +import static datadog.trace.api.config.TracerConfig.TRACE_EXPERIMENTAL_FEATURES_ENABLED +import static datadog.trace.api.config.TracerConfig.TRACE_LONG_RUNNING_ENABLED +import static datadog.trace.api.config.TracerConfig.TRACE_LONG_RUNNING_FLUSH_INTERVAL +import static datadog.trace.api.config.TracerConfig.TRACE_LONG_RUNNING_INITIAL_FLUSH_INTERVAL import static datadog.trace.api.config.TracerConfig.TRACE_PROPAGATION_BEHAVIOR_EXTRACT +import static datadog.trace.api.config.TracerConfig.TRACE_PROPAGATION_EXTRACT_FIRST import static datadog.trace.api.config.TracerConfig.TRACE_RATE_LIMIT import static datadog.trace.api.config.TracerConfig.TRACE_REPORT_HOSTNAME import static datadog.trace.api.config.TracerConfig.TRACE_RESOLVER_ENABLED @@ -138,6 +131,13 @@ import static datadog.trace.api.config.TracerConfig.TRACE_SAMPLING_SERVICE_RULES import static datadog.trace.api.config.TracerConfig.TRACE_X_DATADOG_TAGS_MAX_LENGTH import static datadog.trace.api.config.TracerConfig.WRITER_TYPE +import datadog.trace.api.env.FixedCapturedEnvironment +import datadog.trace.bootstrap.config.provider.AgentArgsInjector +import datadog.trace.bootstrap.config.provider.ConfigConverter +import datadog.trace.bootstrap.config.provider.ConfigProvider +import datadog.trace.test.util.DDSpecification +import datadog.trace.util.throwable.FatalAgentMisconfigurationError + class ConfigTest extends DDSpecification { private static final String PREFIX = "dd." private static final DD_API_KEY_ENV = "DD_API_KEY" @@ -2813,4 +2813,56 @@ class ConfigTest extends DDSpecification { "datadoghq.eu" | "https://instrumentation-telemetry-intake.datadoghq.eu/api/v2/apmtelemetry" "datad0g.com" | "https://all-http-intake.logs.datad0g.com/api/v2/apmtelemetry" } + + def "db metadata fetching enabled with sys = #sys env = #env"() { + setup: + if (sys != null) { + System.setProperty("dd.trace.db.metadata.fetching.on.query", sys) + } + if (env != null) { + environmentVariables.set("DD_TRACE_DB_METADATA_FETCHING_ON_QUERY", env) + } + + when: + def config = new Config() + + then: + config.isDbMetadataFetchingOnQueryEnabled() == expected + + where: + sys | env | expected + null | null | true // default is true + null | "true" | true + null | "false" | false + "true" | null | true + "false" | null | false + "true" | "false" | true // sys prop takes precedence + "false" | "true" | false // sys prop takes precedence + } + + def "db client info fetching enabled with sys = #sys env = #env"() { + setup: + if (sys != null) { + System.setProperty("dd.trace.db.metadata.fetching.on.connect", sys) + } + if (env != null) { + environmentVariables.set("DD_TRACE_DB_METADATA_FETCHING_ON_CONNECT", env) + } + + when: + def config = new Config() + + then: + config.isDbMetadataFetchingOnConnectEnabled() == expected + + where: + sys | env | expected + null | null | true // default is true + null | "true" | true + null | "false" | false + "true" | null | true + "false" | null | false + "true" | "false" | true // sys prop takes precedence + "false" | "true" | false // sys prop takes precedence + } }