add traceparent header to apache-httpclinet

jvm/apache-httpclient/README.md

@@ -4,8 +4,20 @@
 
 Add `M3TracingHttpInterceptor` as request/response interceptor of HttpClient.
 
-```java
+### httpclient 4.2-
+
+```java:
 // CAUTION: Must setup as BOTH interceptor otherwise it may cause memory leak.
 httpclient.addRequestInterceptor(M3TracingHttpInterceptor.INSTANCE);
 httpclient.addResponseInterceptor(M3TracingHttpInterceptor.INSTANCE);
 ```
+
+### httpclient 4.3+
+
+```java
+CloseableHttpClient httpClient = HttpClientBuilder.create()
+    .addInterceptorFirst((HttpRequestInterceptor) M3TracingHttpInterceptor.INSTANCE)
+    .addInterceptorLast((HttpResponseInterceptor) M3TracingHttpInterceptor.INSTANCE)
+    .build();
+```
+                }

jvm/apache-httpclient/src/main/kotlin/com/m3/tracing/apache/httpclient/ApacheHttpRequestInfo.kt

@@ -0,0 +1,26 @@
+package com.m3.tracing.apache.httpclient
+
+import com.m3.tracing.http.HttpRequestInfo
+import com.m3.tracing.http.HttpRequestMetadataKey
+import org.apache.http.HttpRequest
+import org.apache.http.client.methods.HttpUriRequest
+import org.apache.http.client.protocol.HttpClientContext
+import org.apache.http.protocol.HttpContext
+
+open class ApacheHttpRequestInfo(protected val req: HttpRequest, protected val context: HttpContext) : HttpRequestInfo {
+    override fun tryGetHeader(name: String): String? = req.getFirstHeader(name)?.value
+    override fun trySetHeader(name: String, value: String) = req.setHeader(name, value)
+
+    @Suppress("UNCHECKED_CAST", "IMPLICIT_ANY")
+    override fun <T> tryGetMetadata(key: HttpRequestMetadataKey<T>): T? = when (key) {
+        HttpRequestMetadataKey.Method -> req.requestLine?.method as T?
+        HttpRequestMetadataKey.Path -> getPath() as T?
+        HttpRequestMetadataKey.Host -> if (context is HttpClientContext) context.targetHost?.hostName as T? else null
+        HttpRequestMetadataKey.Url -> if (context is HttpClientContext) "${context.targetHost}${getPath() ?: ""}" as T? else null
+
+        else -> null
+    }
+
+    // HttpRequest.requestLine.url contains querystring, so need to be removed
+    private fun getPath(): String? = if (req is HttpUriRequest) req.uri?.path else req.requestLine?.uri?.split("?")?.elementAt(0)
+}

jvm/apache-httpclient/src/main/kotlin/com/m3/tracing/apache/httpclient/M3TracingHttpInterceptor.kt

@@ -3,11 +3,11 @@ package com.m3.tracing.apache.httpclient
 import com.m3.tracing.M3Tracer
 import com.m3.tracing.M3TracerFactory
 import com.m3.tracing.TraceSpan
+import com.m3.tracing.http.HttpRequestMetadataKey
 import org.apache.http.HttpRequest
 import org.apache.http.HttpRequestInterceptor
 import org.apache.http.HttpResponse
 import org.apache.http.HttpResponseInterceptor
-import org.apache.http.client.methods.HttpUriRequest
 import org.apache.http.protocol.HttpContext
 import org.slf4j.LoggerFactory
 
@@ -19,22 +19,24 @@ open class M3TracingHttpInterceptor(
         protected val tracer: M3Tracer
 ) : HttpRequestInterceptor, HttpResponseInterceptor {
     companion object {
-        @JvmStatic
+        @JvmField
         public val INSTANCE = M3TracingHttpInterceptor()
 
         private val currentSpan = ThreadLocal<TraceSpan>()
         private val logger = LoggerFactory.getLogger(M3TracingHttpInterceptor::class.java)
     }
 
-    constructor(): this(M3TracerFactory.get())
+    constructor() : this(M3TracerFactory.get())
 
     override fun process(request: HttpRequest, context: HttpContext) {
-        val span = tracer.startSpan(createSpanName(request))
+        val requestInfo = ApacheHttpRequestInfo(request, context)
+        val span = tracer.processOutgoingHttpRequest(requestInfo)
         currentSpan.set(span) // Set to ThreadLocal ASAP to prevent leak
 
         doQuietly {
-            span["method"] = request.requestLine.method
-            span["uri"] = request.requestLine.uri
+            span["client"] = "m3-tracing:apache-httpclient"
+            span["method"] = requestInfo.tryGetMetadata(HttpRequestMetadataKey.Method)
+            span["path"] = requestInfo.tryGetMetadata(HttpRequestMetadataKey.Path)
         }
     }
 
@@ -49,17 +51,6 @@ open class M3TracingHttpInterceptor(
         span.close()
     }
 
-
-    private fun createSpanName(request: HttpRequest): String {
-        // Intentionally excluded queryString because it might contain dynamic string
-        // Dynamic span name makes runningSpan table so huge
-        return if (request is HttpUriRequest) {
-            "HTTP ${request.method} ${request.uri.host}"
-        } else {
-            "HTTP ${request.requestLine.method}"
-        }
-    }
-
     /**
      * `On Error Resume Next` in 21st century.
      */
@@ -70,4 +61,4 @@ open class M3TracingHttpInterceptor(
             logger.error("Failed to update Span.", e)
         }
     }
-}
+}

jvm/apache-httpclient/src/test/kotlin/com/m3/tracing/apache/httpclient/ApacheHttpRequestInfoTest.kt

@@ -0,0 +1,140 @@
+package com.m3.tracing.apache.httpclient
+
+import com.google.common.truth.Truth
+import com.m3.tracing.http.HttpRequestMetadataKey
+import org.apache.http.HttpHost
+import org.apache.http.HttpRequest
+import org.apache.http.RequestLine
+import org.apache.http.client.methods.HttpUriRequest
+import org.apache.http.client.protocol.HttpClientContext
+import org.apache.http.protocol.HttpContext
+import org.junit.jupiter.api.Test
+import org.junit.jupiter.api.extension.ExtendWith
+import org.mockito.Mock
+import org.mockito.Mockito
+import org.mockito.Mockito.mock
+import org.mockito.junit.jupiter.MockitoExtension
+import java.net.URI
+
+@ExtendWith(MockitoExtension::class)
+class ApacheHttpRequestInfoTest {
+
+    @Mock
+    lateinit var uriRequest: HttpUriRequest
+
+    @Mock
+    lateinit var nonUriRequest: HttpRequest
+
+    @Mock
+    lateinit var clientContext: HttpClientContext
+
+    @Mock
+    lateinit var nonClientContext: HttpContext
+
+    @Test
+    fun `each attribute is set properly for HttpUriRequest`() {
+
+        val requestLine = mock(RequestLine::class.java)
+        val uri = mock(URI::class.java)
+        val host = mock(HttpHost::class.java)
+
+        Mockito.`when`(uriRequest.requestLine).thenReturn(requestLine)
+        Mockito.`when`(uriRequest.uri).thenReturn(uri)
+        Mockito.`when`(clientContext.targetHost).thenReturn(host)
+        Mockito.`when`(uri.path).thenReturn("/foo/bar.html")
+        Mockito.`when`(requestLine.method).thenReturn("GET")
+        Mockito.`when`(host.hostName).thenReturn("example.com")
+        Mockito.`when`(host.toString()).thenReturn("http://example.com")
+
+        val req = ApacheHttpRequestInfo(uriRequest, clientContext)
+
+        Truth.assertThat(req.tryGetMetadata(HttpRequestMetadataKey.Host)).isEqualTo("example.com")
+        Truth.assertThat(req.tryGetMetadata(HttpRequestMetadataKey.Method)).isEqualTo("GET")
+        Truth.assertThat(req.tryGetMetadata(HttpRequestMetadataKey.Path)).isEqualTo("/foo/bar.html")
+        Truth.assertThat(req.tryGetMetadata(HttpRequestMetadataKey.Url)).isEqualTo("http://example.com/foo/bar.html")
+    }
+
+    @Test
+    fun `each attribute is set properly for Non-HttpUriRequest`() {
+
+        val requestLine = mock(RequestLine::class.java)
+
+        Mockito.`when`(nonUriRequest.requestLine).thenReturn(requestLine)
+        Mockito.`when`(requestLine.method).thenReturn("GET")
+        Mockito.`when`(requestLine.uri).thenReturn("/foo/bar.html?param=value")
+
+        var req = ApacheHttpRequestInfo(nonUriRequest, nonClientContext)
+
+        Truth.assertThat(req.tryGetMetadata(HttpRequestMetadataKey.Host)).isNull()
+        Truth.assertThat(req.tryGetMetadata(HttpRequestMetadataKey.Method)).isEqualTo("GET")
+        Truth.assertThat(req.tryGetMetadata(HttpRequestMetadataKey.Path)).isEqualTo("/foo/bar.html")
+        Truth.assertThat(req.tryGetMetadata(HttpRequestMetadataKey.Url)).isNull()
+
+        Mockito.`when`(requestLine.uri).thenReturn("/")
+
+        req = ApacheHttpRequestInfo(nonUriRequest, nonClientContext)
+
+        Truth.assertThat(req.tryGetMetadata(HttpRequestMetadataKey.Host)).isNull()
+        Truth.assertThat(req.tryGetMetadata(HttpRequestMetadataKey.Method)).isEqualTo("GET")
+        Truth.assertThat(req.tryGetMetadata(HttpRequestMetadataKey.Path)).isEqualTo("/")
+        Truth.assertThat(req.tryGetMetadata(HttpRequestMetadataKey.Url)).isNull()
+    }
+
+    @Test
+    fun `no exception when attributes in HttpUriRequest are null`() {
+
+        val requestLine = mock(RequestLine::class.java)
+        val uri = mock(URI::class.java)
+        val host = mock(HttpHost::class.java)
+
+        Mockito.`when`(uriRequest.requestLine).thenReturn(requestLine)
+        Mockito.`when`(uriRequest.uri).thenReturn(uri)
+        Mockito.`when`(clientContext.targetHost).thenReturn(host)
+        Mockito.`when`(uri.path).thenReturn(null)
+        Mockito.`when`(requestLine.method).thenReturn(null)
+        Mockito.`when`(host.hostName).thenReturn(null)
+        Mockito.`when`(host.toString()).thenReturn(null)
+
+        var req = ApacheHttpRequestInfo(uriRequest, clientContext)
+
+        Truth.assertThat(req.tryGetMetadata(HttpRequestMetadataKey.Host)).isNull()
+        Truth.assertThat(req.tryGetMetadata(HttpRequestMetadataKey.Method)).isNull()
+        Truth.assertThat(req.tryGetMetadata(HttpRequestMetadataKey.Path)).isNull()
+        Truth.assertThat(req.tryGetMetadata(HttpRequestMetadataKey.Url)).isEqualTo("null")
+
+        Mockito.`when`(uriRequest.requestLine).thenReturn(null)
+        Mockito.`when`(uriRequest.uri).thenReturn(null)
+        Mockito.`when`(clientContext.targetHost).thenReturn(null)
+
+        req = ApacheHttpRequestInfo(uriRequest, clientContext)
+
+        Truth.assertThat(req.tryGetMetadata(HttpRequestMetadataKey.Host)).isNull()
+        Truth.assertThat(req.tryGetMetadata(HttpRequestMetadataKey.Method)).isNull()
+        Truth.assertThat(req.tryGetMetadata(HttpRequestMetadataKey.Path)).isNull()
+        Truth.assertThat(req.tryGetMetadata(HttpRequestMetadataKey.Url)).isEqualTo("null")
+    }
+
+    @Test
+    fun `no exception when attributes in non-HttpUriRequest are null`() {
+
+        val requestLine = mock(RequestLine::class.java)
+
+        Mockito.`when`(nonUriRequest.requestLine).thenReturn(requestLine)
+        Mockito.`when`(requestLine.method).thenReturn(null)
+        Mockito.`when`(requestLine.uri).thenReturn(null)
+
+        val req = ApacheHttpRequestInfo(nonUriRequest, nonClientContext)
+
+        Truth.assertThat(req.tryGetMetadata(HttpRequestMetadataKey.Host)).isNull()
+        Truth.assertThat(req.tryGetMetadata(HttpRequestMetadataKey.Method)).isNull()
+        Truth.assertThat(req.tryGetMetadata(HttpRequestMetadataKey.Path)).isNull()
+        Truth.assertThat(req.tryGetMetadata(HttpRequestMetadataKey.Url)).isNull()
+
+        Mockito.`when`(nonUriRequest.requestLine).thenReturn(null)
+
+        Truth.assertThat(req.tryGetMetadata(HttpRequestMetadataKey.Host)).isNull()
+        Truth.assertThat(req.tryGetMetadata(HttpRequestMetadataKey.Method)).isNull()
+        Truth.assertThat(req.tryGetMetadata(HttpRequestMetadataKey.Path)).isNull()
+        Truth.assertThat(req.tryGetMetadata(HttpRequestMetadataKey.Url)).isNull()
+    }
+}

jvm/apache-httpclient/src/test/resources/logging.properties

@@ -1,2 +1,2 @@
-handlers = org.slf4j.bridge.SLF4JBridgeHandler
+handlers=org.slf4j.bridge.SLF4JBridgeHandler
 .level=INFO

jvm/apache-httpclient/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker

@@ -0,0 +1 @@
+mock-maker-inline