Fix multiple spans being created for HTTPUrlConnection HEAD requests

elastic · Oct 10, 2023 · 7f11616 · 7f11616
1 parent 61dfa35
commit 7f11616
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 2 deletions.
diff --git a/...in/src/main/java/co/elastic/apm/agent/urlconnection/HttpUrlConnectionInstrumentation.java b/...in/src/main/java/co/elastic/apm/agent/urlconnection/HttpUrlConnectionInstrumentation.java
@@ -79,15 +79,19 @@ public static class AdviceClass {
             @Advice.OnMethodEnter(suppress = Throwable.class, inline = false)
             public static Object enter(@Advice.This HttpURLConnection thiz,
                                        @Advice.FieldValue("connected") boolean connected,
+                                       @Advice.FieldValue("responseCode") int responseCode,
                                        @Advice.Origin String signature) {
 
+                //With HEAD requests the connected stays false
+                boolean actuallyConnected = connected || responseCode != -1;
+
                 boolean isNestedCall = callDepth.isNestedCallAndIncrement();
                 ElasticContext<?> activeContext = tracer.currentContext();
                 AbstractSpan<?> parentSpan = activeContext.getSpan();
                 Span<?> span = null;
                 if (parentSpan != null) {
                     span = inFlightSpans.get(thiz);
-                    if (span == null && !connected) {
+                    if (span == null && !actuallyConnected) {
                         final URL url = thiz.getURL();
                         span = HttpClientHelper.startHttpClientSpan(activeContext, thiz.getRequestMethod(), url.toString(), url.getProtocol(), url.getHost(), url.getPort());
                     }
@@ -98,7 +102,7 @@ public static Object enter(@Advice.This HttpURLConnection thiz,
                     }
                 }
 
-                if (!isNestedCall && !connected) {
+                if (!isNestedCall && !actuallyConnected) {
                     tracer.currentContext().propagateContext(thiz, UrlConnectionPropertyAccessor.instance(), UrlConnectionPropertyAccessor.instance());
                 }
 

diff --git a/...rc/test/java/co/elastic/apm/agent/urlconnection/HttpUrlConnectionInstrumentationTest.java b/...rc/test/java/co/elastic/apm/agent/urlconnection/HttpUrlConnectionInstrumentationTest.java
@@ -81,6 +81,16 @@ public void testMultipleConnect() throws Exception {
         expectSpan("/").verify();
     }
 
+    @Test
+    public void testMultipleGetResponseCodeWithHead() throws Exception {
+        final HttpURLConnection urlConnection = (HttpURLConnection) new URL(getBaseUrl() + "/").openConnection();
+        urlConnection.setRequestMethod("HEAD");
+        //Call twice to make sure we don't create two spans
+        urlConnection.getResponseCode();
+        urlConnection.getResponseCode();
+        expectSpan("/").verify();
+    }
+
     @Test
     public void testGetOutputStream() throws Exception {
         final HttpURLConnection urlConnection = (HttpURLConnection) new URL(getBaseUrl() + "/").openConnection();