make adding the db.statement tag configurable in jdbc calls (#1103)

* make adding the db.statement tag configurable in jdbc calls

* add the db statement setting to the Slick tests

Author: ivantopo

instrumentation/kamon-jdbc/src/main/resources/reference.conf

@@ -6,7 +6,13 @@ kamon.instrumentation.jdbc {
 
   # Parses sql to generate nicer operation name
   # Might have a negative performance impact
-  parse-sql-for-operation-name = false
+  parse-sql-for-operation-name = no
+
+  # Decides when to include the db.statement tag in Spans. The possible values are:
+  #   - always: always adds the db.statement tag, regardless of the statement type
+  #   - prepared: adds the db.statement tag on prepared statements only
+  #   - never: completely disables adding the db.statement tag to JDBC spans
+  add-db-statement-as-span-tag = always
 
   # Defines additional instrumentation actions to be taken when Slow or Failed JDBC statements are detected.
   #

instrumentation/kamon-jdbc/src/main/scala/kamon/instrumentation/jdbc/StatementMonitor.scala

@@ -25,6 +25,7 @@ import kamon.trace.Span
 import kanela.agent.bootstrap.stack.CallStackDepth
 import net.sf.jsqlparser.parser.CCJSqlParserUtil
 
+import java.sql.PreparedStatement
 import java.time.Instant
 import java.time.temporal.ChronoUnit
 
@@ -38,14 +39,34 @@ object StatementMonitor extends LoggingSupport {
     val GenericExecute = "execute"
   }
 
-  @volatile private var parseSqlOperationName: Boolean = loadSqlParsingConfig(Kamon.config())
+  @volatile private var parseSqlOperationName: Boolean = false
+  @volatile private var addStatementSQL: Boolean = true
+  @volatile private var addPreparedStatementSQL: Boolean = true
 
-  Kamon.onReconfigure { config =>
-    parseSqlOperationName = loadSqlParsingConfig(config)
-  }
+  Kamon.onReconfigure(c => updateSettings(c))
+  updateSettings(Kamon.config())
+
+  private def updateSettings(config: Config): Unit = {
+    val jdbcInstrumentationConfig = config.getConfig("kamon.instrumentation.jdbc")
+    parseSqlOperationName = jdbcInstrumentationConfig.getBoolean("parse-sql-for-operation-name")
+
+    jdbcInstrumentationConfig.getString("add-db-statement-as-span-tag") match {
+      case "always" =>
+        addStatementSQL = true
+        addPreparedStatementSQL = true
+
+      case "prepared" =>
+        addStatementSQL = false
+        addPreparedStatementSQL = true
 
-  private def loadSqlParsingConfig(config: Config) = {
-    config.getBoolean("kamon.instrumentation.jdbc.parse-sql-for-operation-name")
+      case "never" =>
+        addStatementSQL = false
+        addPreparedStatementSQL = false
+
+      case unrecognizedValue =>
+        logger.warn(s"Unrecognized value [${unrecognizedValue}] for the [add-db-statement-as-span-tag] setting. " +
+                    s"Falling back to [always]")
+    }
   }
 
   def start(statement: Any, sql: String, statementType: String): Option[Invocation] = {
@@ -83,7 +104,10 @@ object StatementMonitor extends LoggingSupport {
         statementType
       }
 
-      val clientSpan = Kamon.clientSpanBuilder(spanName, "jdbc").tag("db.statement", sql)
+      val clientSpan = Kamon.clientSpanBuilder(spanName, "jdbc")
+
+      if(addStatementSQL || (statement.isInstanceOf[PreparedStatement] && addPreparedStatementSQL))
+        clientSpan.tag("db.statement", sql)
 
       databaseTags.spanTags.iterator().foreach(t => clientSpan.tag(t.key, databaseTags.spanTags.get(Lookups.coerce(t.key))))
       databaseTags.metricTags.iterator().foreach(t => clientSpan.tagMetrics(t.key, databaseTags.metricTags.get(Lookups.coerce(t.key))))

instrumentation/kamon-jdbc/src/test/scala/kamon/instrumentation/jdbc/SlickInstrumentationSpec.scala

@@ -37,6 +37,8 @@ class SlickInstrumentationSpec extends AnyWordSpec with Matchers with Eventually
 
   "the Slick instrumentation" should {
     "propagate the current Context to the AsyncExecutor on Slick" in {
+      applyConfig("kamon.instrumentation.jdbc.add-db-statement-as-span-tag=always")
+      
       val db = setup(Database.forConfig("slick-h2"))
       val parent = Kamon.spanBuilder("parent").start()
 

instrumentation/kamon-jdbc/src/test/scala/kamon/instrumentation/jdbc/StatementInstrumentationSpec.scala

@@ -79,6 +79,8 @@ class StatementInstrumentationSpec extends AnyWordSpec
 
       s"instrumenting the ${driver.name} driver" should {
         "generate Spans on calls to .execute() in prepared statements" in {
+          applyConfig("kamon.instrumentation.jdbc.add-db-statement-as-span-tag=always")
+
           val select = s"SELECT * FROM Address where Nr = ?"
           val statement = connection.prepareStatement(select)
           statement.setLong(1, 1)
@@ -215,6 +217,54 @@ class StatementInstrumentationSpec extends AnyWordSpec
             .toLowerCase() should include("notatable")
         }
 
+        "include the db.statement tag in prepared statements only when enabled via configuration" in {
+          applyConfig("kamon.instrumentation.jdbc.add-db-statement-as-span-tag=prepared")
+
+          connection
+            .createStatement()
+            .execute("SELECT * FROM Address where Nr = 1")
+
+          eventually(timeout(scaled(5 seconds)), interval(200 millis)) {
+            val span = commonSpanValidations(testSpanReporter(), driver)
+            span.tags.get(option("db.statement")) shouldBe empty
+          }
+
+
+          val ps = connection.prepareStatement("INSERT INTO Address VALUES(?, ?)")
+          ps.setInt(1, 1)
+          ps.setString(2, "test")
+          ps.execute()
+
+          eventually(timeout(scaled(5 seconds)), interval(200 millis)) {
+            val span = commonSpanValidations(testSpanReporter(), driver)
+            span.tags.get(plain("db.statement")) shouldBe "INSERT INTO Address VALUES(?, ?)"
+          }
+        }
+
+        "not include the db.statement tag when disabled via configuration" in {
+          applyConfig("kamon.instrumentation.jdbc.add-db-statement-as-span-tag=never")
+
+          connection
+            .createStatement()
+            .execute("SELECT * FROM Address where Nr = 1")
+
+          eventually(timeout(scaled(5 seconds)), interval(200 millis)) {
+            val span = commonSpanValidations(testSpanReporter(), driver)
+            span.tags.get(option("db.statement")) shouldBe empty
+          }
+
+
+          val ps = connection.prepareStatement("INSERT INTO Address VALUES(?, ?)")
+          ps.setInt(1, 1)
+          ps.setString(2, "test")
+          ps.execute()
+
+          eventually(timeout(scaled(5 seconds)), interval(200 millis)) {
+            val span = commonSpanValidations(testSpanReporter(), driver)
+            span.tags.get(option("db.statement")) shouldBe empty
+          }
+        }
+
         "track in-flight operations" in {
           if (driver.supportSleeping) {
             val vendorTags = TagSet.of("db.vendor", driver.vendor)
@@ -241,6 +291,7 @@ class StatementInstrumentationSpec extends AnyWordSpec
             }
 
           }
+
           connection.close()
           driver.cleanup()
         }