Catch SQLException when calling JDBC API from agent (#1035)

* avoid SQLException on unsupported Statement(s) * code cleanup * add few other sync & null checks where relevant Keeping it 'as-is' is required in servlet plugin, because tests fail for an unknown reason yet.
elastic · Mar 3, 2020 · 2e9e312 · 2e9e312
1 parent 71752a9
commit 2e9e312
Show file tree

Hide file tree

Showing 42 changed files with 1,089 additions and 410 deletions.
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
@@ -38,6 +38,20 @@ endif::[]
 [float]
 ===== Bug fixes
 
+[[release-notes-x.x.x]]
+==== 1.14.0 - (Next)
+
+[float]
+===== Breaking changes
+
+[float]
+===== Features
+* `Span#captureException` and `Transaction#captureException` in public Api return reported error id. - {pull}[#1015]
+
+[float]
+===== Bug fixes
+* properly handle `java.sql.SQLException` for unsupported JDBC features {pull}[#1035] https://github.com/elastic/apm-agent-java/issues/1025[#1025]
+
 [[release-notes-1.x]]
 === Java Agent version 1.x
 
@@ -58,7 +72,6 @@ endif::[]
 for more details) - {pull}1009[#1009]
 * Add support for log correlation for log4j and log4j2, even when not used in combination with slf4j.
   See <<supported-logging-frameworks>> for details.
-* `Span#captureException` and `Transaction#captureException` in public Api return reported error id. - {pull}[#1015]
 
 [float]
 ===== Bug Fixes

diff --git a/apm-agent-core/src/test/java/co/elastic/apm/agent/AbstractInstrumentationTest.java b/apm-agent-core/src/test/java/co/elastic/apm/agent/AbstractInstrumentationTest.java
@@ -11,9 +11,9 @@
  * the Apache License, Version 2.0 (the "License"); you may
  * not use this file except in compliance with the License.
  * You may obtain a copy of the License at
- * 
+ *
  *   http://www.apache.org/licenses/LICENSE-2.0
- * 
+ *
  * Unless required by applicable law or agreed to in writing,
  * software distributed under the License is distributed on an
  * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
@@ -44,18 +44,17 @@
 import static org.assertj.core.api.Assertions.assertThat;
 
 public abstract class AbstractInstrumentationTest {
+
     protected static ElasticApmTracer tracer;
     protected static MockReporter reporter;
     protected static ConfigurationRegistry config;
-
     private static TestObjectPoolFactory objectPoolFactory;
 
     @BeforeAll
     @BeforeClass
     public static void beforeAll() {
         objectPoolFactory = new TestObjectPoolFactory();
 
-
         reporter = new MockReporter();
         config = SpyConfiguration.createSpyConfig();
 
@@ -77,27 +76,11 @@ public static void afterAll() {
         ElasticApmAgent.reset();
     }
 
-    public static void reset() {
-        SpyConfiguration.reset(config);
-        reporter.reset();
-    }
-
-    public static ElasticApmTracer getTracer() {
-        return tracer;
-    }
-
-    public static MockReporter getReporter() {
-        return reporter;
-    }
-
-    public static ConfigurationRegistry getConfig() {
-        return config;
-    }
-
     @Before
     @BeforeEach
-    public final void resetReporter() {
-        reset();
+    public final void resetConfigAndReporter() {
+        SpyConfiguration.reset(config);
+        reporter.reset();
     }
 
     @After

diff --git a/...plugin/src/main/java/co/elastic/apm/agent/plugin/api/CaptureExceptionInstrumentation.java b/...plugin/src/main/java/co/elastic/apm/agent/plugin/api/CaptureExceptionInstrumentation.java
@@ -11,9 +11,9 @@
  * the Apache License, Version 2.0 (the "License"); you may
  * not use this file except in compliance with the License.
  * You may obtain a copy of the License at
- * 
+ *
  *   http://www.apache.org/licenses/LICENSE-2.0
- * 
+ *
  * Unless required by applicable law or agreed to in writing,
  * software distributed under the License is distributed on an
  * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY

diff --git a/.../main/java/co/elastic/apm/agent/es/restclient/ElasticsearchRestClientInstrumentation.java b/.../main/java/co/elastic/apm/agent/es/restclient/ElasticsearchRestClientInstrumentation.java
@@ -11,9 +11,9 @@
  * the Apache License, Version 2.0 (the "License"); you may
  * not use this file except in compliance with the License.
  * You may obtain a copy of the License at
- * 
+ *
  *   http://www.apache.org/licenses/LICENSE-2.0
- * 
+ *
  * Unless required by applicable law or agreed to in writing,
  * software distributed under the License is distributed on an
  * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
@@ -45,10 +45,14 @@ public abstract class ElasticsearchRestClientInstrumentation extends ElasticApmI
     public static HelperClassManager<ElasticsearchRestClientInstrumentationHelper<HttpEntity, Response, ResponseListener>> esClientInstrHelperManager;
 
     public ElasticsearchRestClientInstrumentation(ElasticApmTracer tracer) {
-        esClientInstrHelperManager = HelperClassManager.ForAnyClassLoader.of(tracer,
-            "co.elastic.apm.agent.es.restclient.ElasticsearchRestClientInstrumentationHelperImpl",
-            "co.elastic.apm.agent.es.restclient.ResponseListenerWrapper",
-            "co.elastic.apm.agent.es.restclient.ElasticsearchRestClientInstrumentationHelperImpl$ResponseListenerAllocator");
+        synchronized (ElasticsearchRestClientInstrumentation.class) {
+            if (esClientInstrHelperManager == null) {
+                esClientInstrHelperManager = HelperClassManager.ForAnyClassLoader.of(tracer,
+                    "co.elastic.apm.agent.es.restclient.ElasticsearchRestClientInstrumentationHelperImpl",
+                    "co.elastic.apm.agent.es.restclient.ResponseListenerWrapper",
+                    "co.elastic.apm.agent.es.restclient.ElasticsearchRestClientInstrumentationHelperImpl$ResponseListenerAllocator");
+            }
+        }
     }
 
     @Override

diff --git a/...ns/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/ConnectionInstrumentation.java b/...ns/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/ConnectionInstrumentation.java
@@ -11,9 +11,9 @@
  * the Apache License, Version 2.0 (the "License"); you may
  * not use this file except in compliance with the License.
  * You may obtain a copy of the License at
- * 
+ *
  *   http://www.apache.org/licenses/LICENSE-2.0
- * 
+ *
  * Unless required by applicable law or agreed to in writing,
  * software distributed under the License is distributed on an
  * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
@@ -24,8 +24,8 @@
  */
 package co.elastic.apm.agent.jdbc;
 
-import co.elastic.apm.agent.bci.ElasticApmInstrumentation;
 import co.elastic.apm.agent.bci.VisibleForAdvice;
+import co.elastic.apm.agent.impl.ElasticApmTracer;
 import co.elastic.apm.agent.jdbc.helper.JdbcHelper;
 import net.bytebuddy.asm.Advice;
 import net.bytebuddy.description.NamedElement;
@@ -35,8 +35,6 @@
 
 import java.sql.Connection;
 import java.sql.PreparedStatement;
-import java.util.Collection;
-import java.util.Collections;
 
 import static net.bytebuddy.matcher.ElementMatchers.hasSuperType;
 import static net.bytebuddy.matcher.ElementMatchers.isInterface;
@@ -52,14 +50,26 @@
  * Matches the various {@link Connection#prepareCall} and {@link Connection#prepareStatement} methods
  * and keeps a reference to from the resulting {@link java.sql.CallableStatement} or {@link PreparedStatement} to the sql.
  */
-public class ConnectionInstrumentation extends ElasticApmInstrumentation {
+public class ConnectionInstrumentation extends JdbcInstrumentation {
 
-    static final String JDBC_INSTRUMENTATION_GROUP = "jdbc";
+    public ConnectionInstrumentation(ElasticApmTracer tracer) {
+        super(tracer);
+    }
 
     @VisibleForAdvice
     @Advice.OnMethodExit(suppress = Throwable.class)
-    public static void storeSql(@Advice.Return final PreparedStatement statement, @Advice.Argument(0) String sql) {
-        JdbcHelper.mapStatementToSql(statement, sql);
+    public static void storeSql(@Advice.Return PreparedStatement statement,
+                                @Advice.Argument(0) String sql) {
+
+        if (jdbcHelperManager == null) {
+            return;
+        }
+
+        JdbcHelper jdbcHelper = jdbcHelperManager.getForClassLoaderOfClass(PreparedStatement.class);
+        if (jdbcHelper != null) {
+            jdbcHelper.mapStatementToSql(statement, sql);
+        }
+
     }
 
     @Override
@@ -81,9 +91,4 @@ public ElementMatcher<? super MethodDescription> getMethodMatcher() {
             .and(isPublic());
     }
 
-    @Override
-    public Collection<String> getInstrumentationGroupNames() {
-        return Collections.singleton(JDBC_INSTRUMENTATION_GROUP);
-    }
-
 }
diff --git a/...-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/JdbcInstrumentation.java b/...-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/JdbcInstrumentation.java
@@ -0,0 +1,60 @@
+/*-
+ * #%L
+ * Elastic APM Java agent
+ * %%
+ * Copyright (C) 2018 - 2020 Elastic and contributors
+ * %%
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ * #L%
+ */
+package co.elastic.apm.agent.jdbc;
+
+import co.elastic.apm.agent.bci.ElasticApmInstrumentation;
+import co.elastic.apm.agent.bci.HelperClassManager;
+import co.elastic.apm.agent.bci.VisibleForAdvice;
+import co.elastic.apm.agent.impl.ElasticApmTracer;
+import co.elastic.apm.agent.jdbc.helper.JdbcHelper;
+
+import javax.annotation.Nullable;
+import java.util.Collection;
+import java.util.Collections;
+
+public abstract class JdbcInstrumentation extends ElasticApmInstrumentation {
+
+    private static final Collection<String> JDBC_GROUPS = Collections.singleton("jdbc");
+
+    @VisibleForAdvice
+    @Nullable
+    public static HelperClassManager<JdbcHelper> jdbcHelperManager = null;
+
+    public JdbcInstrumentation(ElasticApmTracer tracer) {
+        synchronized (JdbcInstrumentation.class) {
+            if (jdbcHelperManager == null) {
+                jdbcHelperManager = HelperClassManager.ForSingleClassLoader.of(tracer,
+                    "co.elastic.apm.agent.jdbc.helper.JdbcHelperImpl",
+                    "co.elastic.apm.agent.jdbc.helper.JdbcHelperImpl$1");
+            }
+        }
+    }
+
+    @Override
+    public final Collection<String> getInstrumentationGroupNames() {
+        return JDBC_GROUPS;
+    }
+
+}