Fix multiple spans being created for HTTPUrlConnection HEAD requests (e…

…lastic#3353) * Fix multiple spans being created for HTTPUrlConnection HEAD requests * Added changelog --------- Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
schikin · Oct 11, 2023 · 5a7309d · 5a7309d
1 parent 273dc58
commit 5a7309d
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 2 deletions.
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
@@ -31,6 +31,10 @@ Use subheadings with the "=====" level for adding notes for unreleased changes:
 
 === Unreleased
 
+[float]
+===== Bug fixes
+* Fixed too many spans being created for `HTTPUrlConnection` requests with method `HEAD` - {pull}3353[#3353]
+
 [[release-notes-1.x]]
 === Java Agent version 1.x
 

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();