Fix IndexOutOfBounds in apache http clients (#4575)

* Fix IndexOutOfBounds in apache http clients * aws test has an extra span now that http client instrumentation doesn't fail with execption
open-telemetry · Nov 3, 2021 · bc7b125 · bc7b125
1 parent 6694f85
commit bc7b125
Show file tree

Hide file tree

Showing 10 changed files with 48 additions and 6 deletions.
diff --git a/...pentelemetry/javaagent/instrumentation/apachehttpasyncclient/ApacheHttpClientRequest.java b/...pentelemetry/javaagent/instrumentation/apachehttpasyncclient/ApacheHttpClientRequest.java
@@ -48,7 +48,7 @@ static List<String> headersToList(Header[] headers) {
     }
     List<String> headersList = new ArrayList<>(headers.length);
     for (int i = 0; i < headers.length; ++i) {
-      headersList.set(i, headers[i].getValue());
+      headersList.add(headers[i].getValue());
     }
     return headersList;
   }

diff --git a/...ion/apache-httpasyncclient-4.1/javaagent/src/test/groovy/ApacheHttpAsyncClientTest.groovy b/...ion/apache-httpasyncclient-4.1/javaagent/src/test/groovy/ApacheHttpAsyncClientTest.groovy
@@ -34,6 +34,11 @@ abstract class ApacheHttpAsyncClientTest extends HttpClientTest<HttpUriRequest>
     client.start()
   }
 
+  @Override
+  String userAgent() {
+    return "httpasyncclient"
+  }
+
   @Override
   Integer responseCodeOnRedirectError() {
     return 302
@@ -42,6 +47,7 @@ abstract class ApacheHttpAsyncClientTest extends HttpClientTest<HttpUriRequest>
   @Override
   HttpUriRequest buildRequest(String method, URI uri, Map<String, String> headers) {
     def request = createRequest(method, uri)
+    request.addHeader("user-agent", userAgent())
     headers.entrySet().each {
       request.setHeader(new BasicHeader(it.key, it.value))
     }

diff --git a/...pentelemetry/javaagent/instrumentation/apachehttpclient/v4_0/ApacheHttpClientRequest.java b/...pentelemetry/javaagent/instrumentation/apachehttpclient/v4_0/ApacheHttpClientRequest.java
@@ -54,7 +54,7 @@ static List<String> headersToList(Header[] headers) {
     }
     List<String> headersList = new ArrayList<>(headers.length);
     for (int i = 0; i < headers.length; ++i) {
-      headersList.set(i, headers[i].getValue());
+      headersList.add(headers[i].getValue());
     }
     return headersList;
   }

diff --git a/...he-httpclient/apache-httpclient-4.0/javaagent/src/test/groovy/ApacheHttpClientTest.groovy b/...he-httpclient/apache-httpclient-4.0/javaagent/src/test/groovy/ApacheHttpClientTest.groovy
@@ -49,9 +49,15 @@ abstract class ApacheHttpClientTest<T extends HttpRequest> extends HttpClientTes
     client.getConnectionManager().shutdown()
   }
 
+  @Override
+  String userAgent() {
+    return "apachehttpclient"
+  }
+
   @Override
   T buildRequest(String method, URI uri, Map<String, String> headers) {
     def request = createRequest(method, uri)
+    request.addHeader("user-agent", userAgent())
     headers.entrySet().each {
       request.setHeader(new BasicHeader(it.key, it.value))
     }

diff --git a/.../java/io/opentelemetry/instrumentation/apachehttpclient/v4_3/ApacheHttpClientRequest.java b/.../java/io/opentelemetry/instrumentation/apachehttpclient/v4_3/ApacheHttpClientRequest.java
@@ -53,7 +53,7 @@ static List<String> headersToList(Header[] headers) {
     }
     List<String> headersList = new ArrayList<>(headers.length);
     for (int i = 0; i < headers.length; ++i) {
-      headersList.set(i, headers[i].getValue());
+      headersList.add(headers[i].getValue());
     }
     return headersList;
   }

diff --git a/...groovy/io/opentelemetry/instrumentation/apachehttpclient/v4_3/ApacheHttpClientTest.groovy b/...groovy/io/opentelemetry/instrumentation/apachehttpclient/v4_3/ApacheHttpClientTest.groovy
@@ -24,6 +24,11 @@ abstract class ApacheHttpClientTest<T extends HttpRequest> extends HttpClientTes
 
   abstract protected CloseableHttpClient createClient()
 
+  @Override
+  String userAgent() {
+    return "apachehttpclient"
+  }
+
   @Override
   Integer responseCodeOnRedirectError() {
     return 302
@@ -35,6 +40,7 @@ abstract class ApacheHttpClientTest<T extends HttpRequest> extends HttpClientTes
   @Override
   T buildRequest(String method, URI uri, Map<String, String> headers) {
     def request = createRequest(method, uri)
+    request.addHeader("user-agent", userAgent())
     headers.entrySet().each {
       request.setHeader(new BasicHeader(it.key, it.value))
     }

diff --git a/...aagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientHttpAttributesExtractor.java b/...aagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientHttpAttributesExtractor.java
@@ -138,7 +138,7 @@ private static List<String> headersToList(Header[] headers) {
     }
     List<String> headersList = new ArrayList<>(headers.length);
     for (int i = 0; i < headers.length; ++i) {
-      headersList.set(i, headers[i].getValue());
+      headersList.add(headers[i].getValue());
     }
     return headersList;
   }

diff --git a/...he-httpclient/apache-httpclient-5.0/javaagent/src/test/groovy/ApacheHttpClientTest.groovy b/...he-httpclient/apache-httpclient-5.0/javaagent/src/test/groovy/ApacheHttpClientTest.groovy
@@ -40,9 +40,15 @@ abstract class ApacheHttpClientTest<T extends HttpRequest> extends HttpClientTes
     client = builder.build()
   }
 
+  @Override
+  String userAgent() {
+    return "apachehttpclient"
+  }
+
   @Override
   T buildRequest(String method, URI uri, Map<String, String> headers) {
     def request = createRequest(method, uri)
+    request.addHeader("user-agent", userAgent())
     headers.entrySet().each {
       request.setHeader(new BasicHeader(it.key, it.value))
     }

diff --git a/...tSqs/groovy/io/opentelemetry/javaagent/instrumentation/awssdk/v1_11/SqsTracingTest.groovy b/...tSqs/groovy/io/opentelemetry/javaagent/instrumentation/awssdk/v1_11/SqsTracingTest.groovy
@@ -14,4 +14,9 @@ class SqsTracingTest extends AbstractSqsTracingTest implements AgentTestTrait {
   AmazonSQSAsyncClientBuilder configureClient(AmazonSQSAsyncClientBuilder client) {
     return client
   }
+
+  @Override
+  boolean hasHttpClientSpan() {
+    true
+  }
 }
diff --git a/...c/main/groovy/io/opentelemetry/instrumentation/awssdk/v1_11/AbstractSqsTracingTest.groovy b/...c/main/groovy/io/opentelemetry/instrumentation/awssdk/v1_11/AbstractSqsTracingTest.groovy
@@ -50,6 +50,10 @@ abstract class AbstractSqsTracingTest extends InstrumentationSpecification {
     }
   }
 
+  boolean hasHttpClientSpan() {
+    false
+  }
+
   def "simple sqs producer-consumer services"() {
     setup:
     client.createQueue("testSdkSqs")
@@ -83,7 +87,7 @@ abstract class AbstractSqsTracingTest extends InstrumentationSpecification {
           }
         }
       }
-      trace(1, 2) {
+      trace(1, hasHttpClientSpan() ? 3 : 2) {
         span(0) {
           name "SQS.SendMessage"
           kind PRODUCER
@@ -103,7 +107,16 @@ abstract class AbstractSqsTracingTest extends InstrumentationSpecification {
             "net.transport" IP_TCP
           }
         }
-        span(1) {
+        def offset = 0
+        if (hasHttpClientSpan()) {
+          offset = 1
+          span(1) {
+            name "HTTP POST"
+            kind CLIENT
+            childOf span(0)
+          }
+        }
+        span(1 + offset) {
           name "SQS.ReceiveMessage"
           kind CONSUMER
           childOf span(0)