Add http url to span context (#218)

* Adding HTTP Context Type

* Unit tests

* Handling null URI

apm-agent-core/src/main/java/co/elastic/apm/impl/context/SpanContext.java

@@ -21,6 +21,7 @@
 package co.elastic.apm.impl.context;
 
 import co.elastic.apm.impl.transaction.Db;
+import co.elastic.apm.impl.transaction.Http;
 
 
 /**
@@ -33,20 +34,33 @@ public class SpanContext extends AbstractContext {
      */
     private final Db db = new Db();
 
+    /**
+     * An object containing contextual data for outgoing HTTP spans
+     */
+    private final Http http = new Http();
+
     /**
      * An object containing contextual data for database spans
      */
     public Db getDb() {
         return db;
     }
 
+    /**
+     * An object containing contextual data for outgoing HTTP spans
+     */
+    public Http getHttp() {
+        return http;
+    }
+
     @Override
     public void resetState() {
         super.resetState();
         db.resetState();
+        http.resetState();
     }
 
     public boolean hasContent() {
-        return super.hasContent() || db.hasContent();
+        return super.hasContent() || db.hasContent() || http.hasContent();
     }
 }

apm-agent-core/src/main/java/co/elastic/apm/impl/transaction/Http.java

@@ -0,0 +1,62 @@
+/*-
+ * #%L
+ * Elastic APM Java agent
+ * %%
+ * Copyright (C) 2018 Elastic and contributors
+ * %%
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * 
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ * #L%
+ */
+package co.elastic.apm.impl.transaction;
+
+import co.elastic.apm.objectpool.Recyclable;
+
+import javax.annotation.Nullable;
+
+public class Http implements Recyclable {
+
+    /**
+     * URL used by this HTTP outgoing span
+     */
+    @Nullable
+    private String url;
+
+    /**
+     * URL used for the outgoing HTTP call
+     */
+    @Nullable
+    public String getUrl() {
+        return url;
+    }
+
+    /**
+     * URL used for the outgoing HTTP call
+     */
+    public Http withUrl(@Nullable String url) {
+        this.url = url;
+        return this;
+    }
+
+    @Override
+    public void resetState() {
+        url = null;
+    }
+
+    public boolean hasContent() {
+        return url != null;
+    }
+
+    public void copyFrom(Http other) {
+        url = other.url;
+    }
+}

apm-agent-core/src/main/java/co/elastic/apm/report/serialize/DslJsonSerializer.java

@@ -40,6 +40,7 @@
 import co.elastic.apm.impl.payload.TransactionPayload;
 import co.elastic.apm.impl.stacktrace.StacktraceConfiguration;
 import co.elastic.apm.impl.transaction.Db;
+import co.elastic.apm.impl.transaction.Http;
 import co.elastic.apm.impl.transaction.Id;
 import co.elastic.apm.impl.transaction.Span;
 import co.elastic.apm.impl.transaction.SpanCount;
@@ -568,11 +569,15 @@ private void serializeSpanContext(SpanContext context) {
         writeFieldName("context");
         jw.writeByte(OBJECT_START);
 
-        boolean dbContextWritten = serializeDbContext(context.getDb());
+        // Assuming either DB or HTTP data can be related to a span
+        boolean spanContextWritten = serializeDbContext(context.getDb());
+        if(!spanContextWritten) {
+            spanContextWritten = serializeHttpContext(context.getHttp());
+        }
 
         Map<String, String> tags = context.getTags();
         if (!tags.isEmpty()) {
-            if (dbContextWritten) {
+            if (spanContextWritten) {
                 jw.writeByte(COMMA);
             }
             writeFieldName("tags");
@@ -597,6 +602,17 @@ private boolean serializeDbContext(final Db db) {
         return writeDbElement;
     }
 
+    private boolean serializeHttpContext(final Http http) {
+        boolean writeHttpElement = http.hasContent();
+        if (writeHttpElement) {
+            writeFieldName("http");
+            jw.writeByte(OBJECT_START);
+            writeLastField("url", http.getUrl());
+            jw.writeByte(OBJECT_END);
+        }
+        return writeHttpElement;
+    }
+
     private void serializeSpanCountV1(final SpanCount spanCount) {
         writeFieldName("span_count");
         jw.writeByte(OBJECT_START);

apm-agent-core/src/test/java/co/elastic/apm/TransactionUtils.java

@@ -89,9 +89,10 @@ public static void fillTransaction(Transaction t) {
             .withStatement("SELECT * FROM product_types WHERE user_id=?")
             .withType("sql")
             .withUser("readonly_user");
-        span.getContext().getTags().put("monitored_by", "ACME");
-        span.getContext().getTags().put("framework", "some-framework");
+        span.addTag("monitored_by", "ACME");
+        span.addTag("framework", "some-framework");
         t.addSpan(span);
+
         t.addSpan(new Span(mock(ElasticApmTracer.class))
             .start(t, null, 0, false)
             .withName("GET /api/types")
@@ -104,6 +105,14 @@ public static void fillTransaction(Transaction t) {
             .start(t, null, 0, false)
             .withName("GET /api/types")
             .withType("request"));
+
+        span = new Span(mock(ElasticApmTracer.class))
+            .start(t, null, 0, false)
+            .appendToName("GET ")
+            .appendToName("test.elastic.co")
+            .withType("ext.http.apache-httpclient");
+        span.getContext().getHttp().withUrl("http://test.elastic.co/test-service");
+        t.addSpan(span);
     }
 
 }

apm-agent-core/src/test/java/co/elastic/apm/impl/payload/TransactionPayloadJsonSchemaTest.java

@@ -135,35 +135,39 @@ void testJsonStructure() throws IOException {
 
     private void validateJsonStructure(TransactionPayload payload, boolean useIntakeV2) throws IOException {
         when(coreConfiguration.isDistributedTracingEnabled()).thenReturn(false);
-        DslJsonSerializer serializer = new DslJsonSerializer(coreConfiguration.isDistributedTracingEnabled(), mock(StacktraceConfiguration.class));
 
         List<Span> v1Spans = payload.getTransactions().get(0).getSpans();
         List<Span> v2Spans = payload.getSpans();
         List<Span> spansForUse = (useIntakeV2)? v2Spans: v1Spans;
         assertThat((useIntakeV2)? v1Spans: v2Spans).isEmpty();
 
-        validateDbSpanSchema(payload, serializer, true, useIntakeV2);
+        JsonNode serializedSpans = getSerializedSpans(payload, useIntakeV2);
+        validateDbSpanSchema(serializedSpans, true);
+        validateHttpSpanSchema(serializedSpans);
 
         for (Span span: spansForUse) {
             if (span.getType() != null && span.getType().equals("db.postgresql.query")) {
                 span.getContext().getTags().clear();
-                validateDbSpanSchema(payload, serializer, false, useIntakeV2);
+                validateDbSpanSchema(getSerializedSpans(payload, useIntakeV2), false);
                 break;
             }
         }
     }
 
-    private void validateDbSpanSchema(TransactionPayload payload, DslJsonSerializer serializer, boolean shouldContainTags, boolean useIntakeV2) throws IOException {
+    private JsonNode getSerializedSpans(TransactionPayload payload, boolean useIntakeV2) throws IOException {
+        DslJsonSerializer serializer = new DslJsonSerializer(coreConfiguration.isDistributedTracingEnabled(), mock(StacktraceConfiguration.class));
         final String content = serializer.toJsonString(payload);
         JsonNode node = objectMapper.readTree(content);
 
-        JsonNode v1Spans = node.get("transactions").get(0).get("spans");
-        JsonNode v2Spans = node.get("spans");
-        assertThat((useIntakeV2)? v1Spans: v2Spans).isNull();
+        JsonNode v1SerializedSpans = node.get("transactions").get(0).get("spans");
+        JsonNode v2SerializedSpans = node.get("spans");
+        assertThat((useIntakeV2)? v1SerializedSpans: v2SerializedSpans).isNull();
+        return (useIntakeV2)? v2SerializedSpans: v1SerializedSpans;
+    }
 
+    private void validateDbSpanSchema(JsonNode serializedSpans, boolean shouldContainTags) throws IOException {
         boolean contextOfDbSpanFound = false;
-        JsonNode spansForUse = (useIntakeV2)? v2Spans: v1Spans;
-        for (JsonNode child: spansForUse) {
+        for (JsonNode child: serializedSpans) {
             if(child.get("type").textValue().startsWith("db.")) {
                 contextOfDbSpanFound = true;
                 JsonNode context = child.get("context");
@@ -188,6 +192,22 @@ private void validateDbSpanSchema(TransactionPayload payload, DslJsonSerializer
         assertThat(contextOfDbSpanFound).isTrue();
     }
 
+    private void validateHttpSpanSchema(JsonNode serializedSpans)  {
+        boolean contextOfHttpSpanFound = false;
+        for (JsonNode child: serializedSpans) {
+            if(child.get("type").textValue().startsWith("ext.http.")) {
+                assertThat(child.get("name").textValue()).isEqualTo("GET test.elastic.co");
+                JsonNode context = child.get("context");
+                assertThat(context.get("db")).isNull();
+                contextOfHttpSpanFound = true;
+                JsonNode http = context.get("http");
+                assertThat(http).isNotNull();
+                assertThat(http.get("url").textValue()).isEqualTo("http://test.elastic.co/test-service");
+            }
+        }
+        assertThat(contextOfHttpSpanFound).isTrue();
+    }
+
     private void validate(TransactionPayload payload) throws IOException {
         when(coreConfiguration.isDistributedTracingEnabled()).thenReturn(false);
         DslJsonSerializer serializer = new DslJsonSerializer(coreConfiguration.isDistributedTracingEnabled(), mock(StacktraceConfiguration.class));

apm-agent-plugins/apm-apache-httpclient-plugin/src/main/java/co/elastic/apm/httpclient/ApacheHttpClientInstrumentation.java

@@ -56,7 +56,7 @@ private static void onBeforeExecute(@Advice.Argument(0) HttpRoute route,
             return;
         }
         final AbstractSpan<?> parent = tracer.getActive();
-        span = HttpClientHelper.startHttpClientSpan(parent, request.getMethod(), route.getTargetHost().getHostName(), SPAN_TYPE_APACHE_HTTP_CLIENT);
+        span = HttpClientHelper.startHttpClientSpan(parent, request.getMethod(), request.getURI(), route.getTargetHost().getHostName(), SPAN_TYPE_APACHE_HTTP_CLIENT);
         if (span != null) {
             request.addHeader(TraceContext.TRACE_PARENT_HEADER, span.getTraceContext().getOutgoingTraceParentHeader().toString());
         } else if (!request.containsHeader(TraceContext.TRACE_PARENT_HEADER)) {

apm-agent-plugins/apm-httpclient-core/src/main/java/co/elastic/apm/http/client/HttpClientHelper.java

@@ -24,22 +24,28 @@
 import co.elastic.apm.impl.transaction.Span;
 
 import javax.annotation.Nullable;
+import java.net.URI;
 
 public class HttpClientHelper {
 
     public static final String HTTP_CLIENT_SPAN_TYPE_PREFIX = "ext.http.";
 
     @Nullable
     @VisibleForAdvice
-    public static Span startHttpClientSpan(AbstractSpan<?> parent, String method, String hostName, String spanType) {
+    public static Span startHttpClientSpan(AbstractSpan<?> parent, String method, @Nullable URI uri, String hostName, String spanType) {
+        Span span = null;
         if (!isAlreadyMonitored(parent)) {
-            return parent
+            span = parent
                 .createSpan()
                 .withType(spanType)
                 .appendToName(method).appendToName(" ").appendToName(hostName)
                 .activate();
+
+            if (uri != null) {
+                span.getContext().getHttp().withUrl(uri.toString());
+            }
         }
-        return null;
+        return span;
     }
 
     /*

apm-agent-plugins/apm-httpclient-core/src/test/java/co/elastic/apm/httpclient/AbstractHttpClientInstrumentationTest.java

@@ -25,7 +25,6 @@
 import co.elastic.apm.impl.transaction.Transaction;
 import com.github.tomakehurst.wiremock.core.WireMockConfiguration;
 import com.github.tomakehurst.wiremock.junit.WireMockRule;
-import org.assertj.core.api.Java6Assertions;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
@@ -58,10 +57,12 @@ public final void setUpWiremock() {
 
     @Test
     public void testHttpCall() {
-        performGetWithinTransaction("/");
+        String path = "/";
+        performGetWithinTransaction(path);
 
         assertThat(reporter.getTransactions()).hasSize(1);
         assertThat(reporter.getSpans()).hasSize(1);
+        assertThat(reporter.getSpans().get(0).getContext().getHttp().getUrl()).isEqualTo(getBaseUrl() + path);
 
         final String traceParentHeader = reporter.getFirstSpan().getTraceContext().getOutgoingTraceParentHeader().toString();
         verify(getRequestedFor(urlPathEqualTo("/"))
@@ -70,10 +71,12 @@ public void testHttpCall() {
 
     @Test
     public void testHttpCallRedirect() {
-        performGetWithinTransaction("/redirect");
+        String path = "/redirect";
+        performGetWithinTransaction(path);
 
         assertThat(reporter.getTransactions()).hasSize(1);
         assertThat(reporter.getSpans()).hasSize(1);
+        assertThat(reporter.getSpans().get(0).getContext().getHttp().getUrl()).isEqualTo(getBaseUrl() + path);
 
         final String traceParentHeader = reporter.getFirstSpan().getTraceContext().getOutgoingTraceParentHeader().toString();
         verify(getRequestedFor(urlPathEqualTo("/redirect"))
@@ -84,13 +87,15 @@ public void testHttpCallRedirect() {
 
     @Test
     public void testHttpCallCircularRedirect() {
-        performGetWithinTransaction("/circular-redirect");
+        String path = "/circular-redirect";
+        performGetWithinTransaction(path);
 
-        Java6Assertions.assertThat(reporter.getTransactions()).hasSize(1);
-        Java6Assertions.assertThat(reporter.getSpans()).hasSize(1);
-        Java6Assertions.assertThat(reporter.getErrors()).hasSize(1);
-        Java6Assertions.assertThat(reporter.getFirstError().getException()).isNotNull();
-        Java6Assertions.assertThat(reporter.getFirstError().getException().getClass()).isNotNull();
+        assertThat(reporter.getTransactions()).hasSize(1);
+        assertThat(reporter.getSpans()).hasSize(1);
+        assertThat(reporter.getErrors()).hasSize(1);
+        assertThat(reporter.getFirstError().getException()).isNotNull();
+        assertThat(reporter.getFirstError().getException().getClass()).isNotNull();
+        assertThat(reporter.getSpans().get(0).getContext().getHttp().getUrl()).isEqualTo(getBaseUrl() + path);
 
         final String traceParentHeader = reporter.getFirstSpan().getTraceContext().getOutgoingTraceParentHeader().toString();
         verify(getRequestedFor(urlPathEqualTo("/circular-redirect"))

apm-agent-plugins/apm-spring-resttemplate-plugin/src/main/java/co/elastic/apm/resttemplate/SpringRestTemplateInstrumentation.java

@@ -57,8 +57,8 @@ private static void beforeExecute(@Advice.This ClientHttpRequest request,
             return;
         }
         final AbstractSpan<?> parent = tracer.getActive();
-        span = HttpClientHelper.startHttpClientSpan(parent, Objects.toString(request.getMethod()), request.getURI().getHost(),
-            SPAN_TYPE_SPRING_REST_TEMPLATE);
+        span = HttpClientHelper.startHttpClientSpan(parent, Objects.toString(request.getMethod()), request.getURI(),
+            request.getURI().getHost(), SPAN_TYPE_SPRING_REST_TEMPLATE);
         if (span != null) {
             request.getHeaders().add(TraceContext.TRACE_PARENT_HEADER, span.getTraceContext().getOutgoingTraceParentHeader().toString());
         }