Fix sqlite-jdbc insrumentation for > 3.32.3.2 kamon-io#1008

This is more of a temporary workaround than a fix. It disables instrumentation on the non-prepared statement versions of queries
Falmarri · Apr 27, 2021 · 666017d · 666017d
1 parent a6ebaca
commit 666017d
Show file tree

Hide file tree

Showing 3 changed files with 71 additions and 12 deletions.
diff --git a/build.sbt b/build.sbt
@@ -266,7 +266,8 @@ lazy val `kamon-jdbc` = (project in file("instrumentation/kamon-jdbc"))
       logbackClassic % "test",
       "com.typesafe.slick"       %% "slick-hikaricp"            % "3.3.2" % "test",
       "com.h2database"            % "h2"                        % "1.4.182" % "test",
-      "org.xerial"                % "sqlite-jdbc"               % "3.27.2.1" % "test",
+      "org.xerial"                % "sqlite-jdbc"               % "3.34.0" % "test",
+      "org.apache.commons"        % "commons-lang3"             % "3.12.0" % "test",
       "ch.vorburger.mariaDB4j"    % "mariaDB4j"                 % "2.4.0" % "test"
     )
   ).dependsOn(`kamon-core`, `kamon-executors`, `kamon-testkit` % "test")

diff --git a/...ation/kamon-jdbc/src/main/scala/kamon/instrumentation/jdbc/StatementInstrumentation.scala b/...ation/kamon-jdbc/src/main/scala/kamon/instrumentation/jdbc/StatementInstrumentation.scala
@@ -27,11 +27,15 @@ import kamon.trace.Hooks
 import kanela.agent.api.instrumentation.InstrumentationBuilder
 import kanela.agent.api.instrumentation.bridge.FieldBridge
 import kanela.agent.libs.net.bytebuddy.asm.Advice
-
 import scala.util.Try
+
+import kamon.instrumentation.jdbc.StatementMonitor.{Invocation, StatementTypes}
+import kamon.instrumentation.jdbc.utils.LoggingSupport
+import kanela.agent.libs.net.bytebuddy.asm.Advice.Thrown
+import kanela.agent.libs.net.bytebuddy.description.`type`.TypeDescription
 import kanela.agent.libs.net.bytebuddy.matcher.ElementMatchers._
 
-class StatementInstrumentation extends InstrumentationBuilder {
+class StatementInstrumentation extends InstrumentationBuilder with LoggingSupport {
 
   private val withOneStringArgument = withArgument(0, classOf[String])
 
@@ -56,13 +60,19 @@ class StatementInstrumentation extends InstrumentationBuilder {
     .mixin(classOf[HasDatabaseTags.Mixin])
     .mixin(classOf[HasConnectionPoolTelemetry.Mixin])
 
-  onTypesMatching(hasSuperType(named("java.sql.Statement")).and(not(nameStartsWith("com.zaxxer.hikari"))))
+  onTypesMatching(
+    hasSuperType[TypeDescription](named("java.sql.Statement"))
+      .and(not(nameStartsWith[TypeDescription]("com.zaxxer.hikari")))
+  )
     .advise(method("execute").and(withOneStringArgument), classOf[StatementExecuteMethodAdvisor])
     .advise(method("executeQuery").and(withOneStringArgument), classOf[StatementExecuteQueryMethodAdvisor])
     .advise(method("executeUpdate").and(withOneStringArgument), classOf[StatementExecuteUpdateMethodAdvisor])
     .advise(anyMethods("executeBatch", "executeLargeBatch"), classOf[StatementExecuteBatchMethodAdvisor])
 
-  onTypesMatching(hasSuperType(named("java.sql.PreparedStatement")).and(not(nameStartsWith("com.zaxxer.hikari"))))
+  onTypesMatching(
+    hasSuperType[TypeDescription](named("java.sql.PreparedStatement"))
+      .and(not(nameStartsWith[TypeDescription]("com.zaxxer.hikari")))
+  )
     .advise(method("execute"), classOf[PreparedStatementExecuteMethodAdvisor])
     .advise(method("executeQuery"), classOf[PreparedStatementExecuteQueryMethodAdvisor])
     .advise(method("executeUpdate"), classOf[PreparedStatementExecuteUpdateMethodAdvisor])
@@ -78,11 +88,30 @@ class StatementInstrumentation extends InstrumentationBuilder {
     .advise(method("executeQuery"), classOf[PreparedStatementExecuteQueryMethodAdvisor])
     .advise(method("executeUpdate"), classOf[PreparedStatementExecuteUpdateMethodAdvisor])
 
-  onType("org.sqlite.jdbc3.JDBC3Statement")
-    .advise(method("execute").and(withOneStringArgument), classOf[StatementExecuteMethodAdvisor])
-    .advise(method("executeQuery").and(withOneStringArgument), classOf[StatementExecuteQueryMethodAdvisor])
-    .advise(method("executeUpdate").and(withOneStringArgument), classOf[StatementExecuteUpdateMethodAdvisor])
-    .advise(anyMethods("executeBatch", "executeLargeBatch"), classOf[StatementExecuteBatchMethodAdvisor])
+
+  try {
+    val prop = new Properties()
+    prop.load(this.getClass().getResourceAsStream("sqlite-jdbc.properties"))
+    val sqliteVersion = prop.getProperty("version", "0").split('.').map(_.toInt).toList
+
+    sqliteVersion match {
+      case epoch :: maj :: _ if epoch <= 3 && maj < 32 =>
+        onType("org.sqlite.jdbc3.JDBC3Statement")
+          .advise(method("execute").and(withOneStringArgument), classOf[StatementExecuteMethodAdvisor])
+          .advise(method("executeQuery").and(withOneStringArgument), classOf[StatementExecuteQueryMethodAdvisor])
+          .advise(method("executeUpdate").and(withOneStringArgument), classOf[StatementExecuteUpdateMethodAdvisor])
+          .advise(anyMethods("executeBatch", "executeLargeBatch"), classOf[StatementExecuteBatchMethodAdvisor])
+      case 3 :: 32 :: 3 :: p :: _ if p < 3 =>
+        onType("org.sqlite.jdbc3.JDBC3Statement")
+          .advise(method("execute").and(withOneStringArgument), classOf[StatementExecuteMethodAdvisor])
+          .advise(method("executeQuery").and(withOneStringArgument), classOf[StatementExecuteQueryMethodAdvisor])
+          .advise(method("executeUpdate").and(withOneStringArgument), classOf[StatementExecuteUpdateMethodAdvisor])
+          .advise(anyMethods("executeBatch", "executeLargeBatch"), classOf[StatementExecuteBatchMethodAdvisor])
+      case _ =>
+        logWarn("Could not instrument org.sqlite.jdbc3.JDBC3Statement. Prepared statements will be instrumented, but direct SQL calls will not")
+    }
+
+  }
 
   /**
     * Since Postgres is reusing the same statement in the implementation if isAlive, we need to "refresh" the
@@ -95,6 +124,35 @@ class StatementInstrumentation extends InstrumentationBuilder {
 
 }
 
+/**
+  * Advisor for java.sql.Statement::execute
+  */
+class SqliteStatementExecuteMethodAdvisor
+object SqliteStatementExecuteMethodAdvisor {
+  @Advice.OnMethodEnter(suppress = classOf[Throwable])
+  def executeStart(@Advice.This statement: Any, @Advice.Argument(0) sql: String, @Advice.Argument(1) autoCommit: Boolean): Option[Invocation] = {
+    StatementMonitor.start(statement, sql, StatementTypes.GenericExecute)
+  }
+
+  @Advice.OnMethodExit(onThrowable = classOf[Throwable], suppress = classOf[Throwable])
+  def executeEnd(@Advice.Enter invocation: Option[Invocation], @Thrown throwable: Throwable): Unit = {
+    invocation.foreach(_.close(throwable))
+  }
+}
+
+class SqliteCoreStatementExecuteMethodAdvisor
+object SqliteCoreStatementExecuteMethodAdvisor {
+  @Advice.OnMethodEnter(suppress = classOf[Throwable])
+  def executeStart(@Advice.This statement: Any, @Advice.Argument(0) sql: String, @Advice.Argument(1) autoCommit: Boolean): Option[Invocation] = {
+    StatementMonitor.start(statement, sql, StatementTypes.GenericExecute)
+  }
+
+  @Advice.OnMethodExit(onThrowable = classOf[Throwable], suppress = classOf[Throwable])
+  def executeEnd(@Advice.Enter invocation: Option[Invocation], @Thrown throwable: Throwable): Unit = {
+    invocation.foreach(_.close(throwable))
+  }
+}
+
 case class DatabaseTags(metricTags: TagSet, spanTags: TagSet)
 
 trait HasDatabaseTags {
@@ -198,4 +256,4 @@ object PgConnectionIsAliveAdvice {
       }
     }
   }
-}
+}
diff --git a/instrumentation/kamon-jdbc/src/test/resources/logback.xml b/instrumentation/kamon-jdbc/src/test/resources/logback.xml
@@ -9,4 +9,4 @@
     <root level="OFF">
         <appender-ref ref="STDOUT"/>
     </root>
-</configuration>
+</configuration>