use static final variable to decide if we should append SQL comment

Author: na-ji

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

@@ -127,7 +127,7 @@ public static String onEnter(
             traceConfig(activeSpan()).getServiceMapping().getOrDefault(dbService, dbService);
       }
 
-      boolean append = DECORATE.shouldAppendSqlComment() || "sqlserver".equals(dbInfo.getType());
+      boolean append = DECORATE.DBM_ALWAYS_APPEND_SQL_COMMENT || "sqlserver".equals(dbInfo.getType());
       sql =
           SQLCommenter.inject(
               sql, dbService, dbInfo.getType(), dbInfo.getHost(), dbInfo.getDb(), null, append);

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

@@ -59,6 +59,8 @@ public class JDBCDecorator extends DatabaseClientDecorator<DBInfo> {
       DBM_PROPAGATION_MODE.equals(DBM_PROPAGATION_MODE_FULL);
   public static final boolean DBM_TRACE_PREPARED_STATEMENTS =
       Config.get().isDbmTracePreparedStatements();
+  public static final boolean DBM_ALWAYS_APPEND_SQL_COMMENT =
+      Config.get().isDbmAlwaysAppendSqlComment();
 
   private volatile boolean warnedAboutDBMPropagationMode = false; // to log a warning only once
 
@@ -423,8 +425,4 @@ public boolean shouldInjectSQLComment() {
     return Config.get().getDbmPropagationMode().equals(DBM_PROPAGATION_MODE_FULL)
         || Config.get().getDbmPropagationMode().equals(DBM_PROPAGATION_MODE_STATIC);
   }
-
-  public boolean shouldAppendSqlComment() {
-    return Config.get().isDbmAlwaysAppendSqlComment();
-  }
 }

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

@@ -127,7 +127,7 @@ public static AgentScope onEnter(
           final boolean injectTraceInComment = injectTraceContext && !isSqlServer && !isOracle;
 
           // prepend mode will prepend the SQL comment to the raw sql query
-          boolean appendComment = DECORATE.shouldAppendSqlComment();
+          boolean appendComment = DECORATE.DBM_ALWAYS_APPEND_SQL_COMMENT;
 
           // There is a bug in the SQL Server JDBC driver that prevents
           // the generated keys from being returned when the

dd-java-agent/instrumentation/jdbc/src/test/groovy/DBMInjectionForkedTest.groovy

@@ -5,8 +5,7 @@ import test.TestConnection
 import test.TestPreparedStatement
 import test.TestStatement
 
-class DBMInjectionForkedTest extends InstrumentationSpecification {
-
+abstract class InjectionTest extends InstrumentationSpecification {
   @Override
   void configurePreAgent() {
     super.configurePreAgent()
@@ -21,7 +20,9 @@ class DBMInjectionForkedTest extends InstrumentationSpecification {
 
   static query = "SELECT 1"
   static serviceInjection = "ddps='my_service_name',dddbs='remapped_testdb'"
+}
 
+class DBMInjectionForkedTest extends InjectionTest {
   def "prepared stmt"() {
     setup:
     def connection = new TestConnection(false)
@@ -35,30 +36,32 @@ class DBMInjectionForkedTest extends InstrumentationSpecification {
     assert statement.sql == "/*${serviceInjection}*/ ${query}"
   }
 
-  def "append comment on prepared stmt"() {
+  def "single query"() {
     setup:
-    injectSysConfig(TraceInstrumentationConfig.DB_DBM_ALWAYS_APPEND_SQL_COMMENT, "true")
     def connection = new TestConnection(false)
 
     when:
-    def statement = connection.prepareStatement(query) as TestPreparedStatement
-    statement.execute()
+    def statement = connection.createStatement() as TestStatement
+    statement.executeQuery(query)
 
     then:
-    // even in full propagation mode, we cannot inject trace info in prepared statements
-    assert statement.sql == "${query} /*${serviceInjection}*/"
+    assert statement.sql == "/*${serviceInjection},traceparent='00-00000000000000000000000000000004-0000000000000003-01'*/ ${query}"
   }
+}
 
-  def "single query"() {
+class DBMAppendInjectionForkedTest extends InjectionTest {
+  def "append comment on prepared stmt"() {
     setup:
+    injectSysConfig(TraceInstrumentationConfig.DB_DBM_ALWAYS_APPEND_SQL_COMMENT, "true")
     def connection = new TestConnection(false)
 
     when:
-    def statement = connection.createStatement() as TestStatement
-    statement.executeQuery(query)
+    def statement = connection.prepareStatement(query) as TestPreparedStatement
+    statement.execute()
 
     then:
-    assert statement.sql == "/*${serviceInjection},traceparent='00-00000000000000000000000000000006-0000000000000005-01'*/ ${query}"
+    // even in full propagation mode, we cannot inject trace info in prepared statements
+    assert statement.sql == "${query} /*${serviceInjection}*/"
   }
 
   def "append comment on single query"() {
@@ -71,6 +74,6 @@ class DBMInjectionForkedTest extends InstrumentationSpecification {
     statement.executeQuery(query)
 
     then:
-    assert statement.sql == "${query} /*${serviceInjection},traceparent='00-00000000000000000000000000000008-0000000000000007-01'*/"
+    assert statement.sql == "${query} /*${serviceInjection},traceparent='00-00000000000000000000000000000004-0000000000000003-01'*/"
   }
 }

dd-java-agent/instrumentation/jdbc/src/test/groovy/SQLServerInjectionForkedTest.groovy

@@ -1,13 +1,10 @@
-
-
 import datadog.trace.agent.test.InstrumentationSpecification
 import datadog.trace.api.config.TraceInstrumentationConfig
 import test.TestConnection
 import test.TestDatabaseMetaData
 import test.TestStatement
 
-class SQLServerInjectionForkedTest extends InstrumentationSpecification {
-
+abstract class InjectionForkedTest extends InstrumentationSpecification {
   @Override
   void configurePreAgent() {
     super.configurePreAgent()
@@ -18,7 +15,9 @@ class SQLServerInjectionForkedTest extends InstrumentationSpecification {
 
   static query = "SELECT 1"
   static serviceInjection = "ddps='my_service_name',dddbs='sqlserver',ddh='localhost',dddb='testdb'"
+}
 
+class SQLServerInjectionForkedTest extends InjectionForkedTest {
   def "SQL Server no trace injection with full propagation mode"() {
     setup:
     def connection = new TestConnection(false)
@@ -51,7 +50,9 @@ class SQLServerInjectionForkedTest extends InstrumentationSpecification {
     then:
     assert statement.sql == "${query} /*${serviceInjection}*/"
   }
+}
 
+class SQLServerAppendInjectionForkedTest extends InjectionForkedTest {
   def "SQL Server append comment when configured to do so"() {
     setup:
     injectSysConfig(TraceInstrumentationConfig.DB_DBM_ALWAYS_APPEND_SQL_COMMENT, "true")