diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index cbf6c2f288..f5c09b6aa5 100644 --- 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/apm-agent-plugins/apm-urlconnection-plugin/src/main/java/co/elastic/apm/agent/urlconnection/HttpUrlConnectionInstrumentation.java b/apm-agent-plugins/apm-urlconnection-plugin/src/main/java/co/elastic/apm/agent/urlconnection/HttpUrlConnectionInstrumentation.java index 6101e4225f..85155b989f 100644 --- a/apm-agent-plugins/apm-urlconnection-plugin/src/main/java/co/elastic/apm/agent/urlconnection/HttpUrlConnectionInstrumentation.java +++ b/apm-agent-plugins/apm-urlconnection-plugin/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/apm-agent-plugins/apm-urlconnection-plugin/src/test/java/co/elastic/apm/agent/urlconnection/HttpUrlConnectionInstrumentationTest.java b/apm-agent-plugins/apm-urlconnection-plugin/src/test/java/co/elastic/apm/agent/urlconnection/HttpUrlConnectionInstrumentationTest.java index e091d49251..1200f0ad9a 100644 --- a/apm-agent-plugins/apm-urlconnection-plugin/src/test/java/co/elastic/apm/agent/urlconnection/HttpUrlConnectionInstrumentationTest.java +++ b/apm-agent-plugins/apm-urlconnection-plugin/src/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();