Merge pull request #172 from axw/apmhttp-client-http-url

 module/apmhttp: set client request URL

CHANGELOG.md

@@ -16,6 +16,7 @@
  - module/apmsql: fix a bug preventing errors from being captured (#160)
  - Introduce `Tracer.StartTransactionOptions`, drop variadic args from `Tracer.StartTransaction` (#165)
  - module/apmgorm: introduce GORM instrumentation module (#169, #170)
+ - module/apmhttp: record outgoing request URLs in span context (#172)
 
 ## [v0.4.0](https://github.com/elastic/apm-agent-go/releases/tag/v0.4.0)
 

context.go

@@ -77,10 +77,9 @@ func (c *Context) SetTag(key, value string) {
 
 // SetHTTPRequest sets details of the HTTP request in the context.
 //
-// This function may be used for either clients or servers. For
-// server-side requests, various proxy forwarding headers are taken
-// into account to reconstruct the URL, and determining the client
-// address.
+// This function relates to server-side requests. Various proxy
+// forwarding headers are taken into account to reconstruct the URL,
+// and determining the client address.
 //
 // If the request URL contains user info, it will be removed and
 // excluded from the URL's "full" field.

model/marshal.go

@@ -42,6 +42,61 @@ func (t *Time) UnmarshalJSON(data []byte) error {
 	return nil
 }
 
+// UnmarshalJSON unmarshals the JSON data into v.
+func (v *HTTPSpanContext) UnmarshalJSON(data []byte) error {
+	var httpSpanContext struct {
+		URL string
+	}
+	if err := json.Unmarshal(data, &httpSpanContext); err != nil {
+		return err
+	}
+	u, err := url.Parse(httpSpanContext.URL)
+	if err != nil {
+		return err
+	}
+	v.URL = u
+	return nil
+}
+
+// MarshalFastJSON writes the JSON representation of v to w.
+func (v *HTTPSpanContext) MarshalFastJSON(w *fastjson.Writer) {
+	w.RawByte('{')
+	beforeURL := w.Size()
+	w.RawString(`"url":"`)
+	if v.marshalURL(w) {
+		w.RawByte('"')
+	} else {
+		w.Rewind(beforeURL)
+	}
+	w.RawByte('}')
+}
+
+func (v *HTTPSpanContext) marshalURL(w *fastjson.Writer) bool {
+	if v.URL.Scheme != "" {
+		if !marshalScheme(w, v.URL.Scheme) {
+			return false
+		}
+		w.RawString("://")
+	} else {
+		w.RawString("http://")
+	}
+	w.StringContents(v.URL.Host)
+	if v.URL.Path == "" {
+		w.RawByte('/')
+	} else {
+		w.StringContents(v.URL.Path)
+	}
+	if v.URL.RawQuery != "" {
+		w.RawByte('?')
+		w.StringContents(v.URL.RawQuery)
+	}
+	if v.URL.Fragment != "" {
+		w.RawByte('#')
+		w.StringContents(v.URL.Fragment)
+	}
+	return true
+}
+
 // MarshalFastJSON writes the JSON representation of v to w.
 func (v *URL) MarshalFastJSON(w *fastjson.Writer) {
 	w.RawByte('{')
@@ -118,7 +173,7 @@ func (v *URL) MarshalFastJSON(w *fastjson.Writer) {
 	if schemeEnd != -1 && v.Hostname != "" && v.Path != "" {
 		before := w.Size()
 		w.RawString(",\"full\":")
-		if !v.marshalFullURL(w, schemeBegin, schemeEnd) {
+		if !v.marshalFullURL(w, w.Bytes()[schemeBegin:schemeEnd]) {
 			w.Rewind(before)
 		}
 	}
@@ -150,10 +205,10 @@ func marshalScheme(w *fastjson.Writer, scheme string) bool {
 	return true
 }
 
-func (v *URL) marshalFullURL(w *fastjson.Writer, schemeBegin, schemeEnd int) bool {
+func (v *URL) marshalFullURL(w *fastjson.Writer, scheme []byte) bool {
 	w.RawByte('"')
 	before := w.Size()
-	w.RawBytes(w.Bytes()[schemeBegin:schemeEnd])
+	w.RawBytes(scheme)
 	w.RawString("://")
 	if strings.IndexByte(v.Hostname, ':') == -1 {
 		w.StringContents(v.Hostname)

model/marshal_test.go

@@ -101,6 +101,17 @@ func TestMarshalTransaction(t *testing.T) {
 					},
 				},
 			},
+			map[string]interface{}{
+				"name":     "GET testing.invalid:8000",
+				"start":    float64(3),
+				"duration": float64(4),
+				"type":     "ext.http",
+				"context": map[string]interface{}{
+					"http": map[string]interface{}{
+						"url": "http://testing.invalid:8000/path?query#fragment",
+					},
+				},
+			},
 		},
 	}
 
@@ -583,6 +594,16 @@ func fakeTransaction() model.Transaction {
 					User:      "barb",
 				},
 			},
+		}, {
+			Name:     "GET testing.invalid:8000",
+			Start:    3,
+			Duration: 4,
+			Type:     "ext.http",
+			Context: &model.SpanContext{
+				HTTP: &model.HTTPSpanContext{
+					URL: mustParseURL("http://testing.invalid:8000/path?query#fragment"),
+				},
+			},
 		}},
 	}
 }

model/model.go

@@ -208,6 +208,9 @@ type SpanContext struct {
 	// Database holds contextual information for database
 	// operation spans.
 	Database *DatabaseSpanContext `json:"db,omitempty"`
+
+	// HTTP holds contextual information for HTTP client request spans.
+	HTTP *HTTPSpanContext `json:"http,omitempty"`
 }
 
 // DatabaseSpanContext holds contextual information for database
@@ -228,6 +231,12 @@ type DatabaseSpanContext struct {
 	User string `json:"user,omitempty"`
 }
 
+// HTTPSpanContext holds contextual information for HTTP client request spans.
+type HTTPSpanContext struct {
+	// URL is the request URL.
+	URL *url.URL
+}
+
 // Context holds contextual information relating to a transaction or error.
 type Context struct {
 	// Request holds details of the HTTP request relating to the
@@ -459,7 +468,8 @@ type RequestSocket struct {
 	RemoteAddress string `json:"remote_address,omitempty"`
 }
 
-// URL represents a request URL.
+// URL represents a server-side (transaction) request URL,
+// broken down into its constituent parts.
 type URL struct {
 	// Full is the full URL, e.g.
 	// "https://example.com:443/search/?q=elasticsearch#top".

module/apmhttp/client.go

@@ -82,6 +82,7 @@ func (r *roundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
 	name := r.requestName(req)
 	spanType := "ext.http"
 	span := tx.StartSpan(name, spanType, elasticapm.SpanFromContext(ctx))
+	span.Context.SetHTTPRequest(req)
 	defer span.End()
 	if !span.Dropped() {
 		traceContext = span.TraceContext()

module/apmhttp/client_test.go

@@ -5,6 +5,7 @@ import (
 	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
+	"net/url"
 	"os"
 	"testing"
 
@@ -36,10 +37,18 @@ func TestClient(t *testing.T) {
 	server := httptest.NewServer(mux)
 	defer server.Close()
 
+	serverURL, err := url.Parse(server.URL)
+	require.NoError(t, err)
+	serverURL.Path = "/foo"
+
+	// Add user info to the URL; it should be stripped off.
+	requestURL := *serverURL
+	requestURL.User = url.UserPassword("root", "hunter2")
+
 	tx := tracer.StartTransaction("name", "type")
 	ctx := elasticapm.ContextWithTransaction(context.Background(), tx)
 	client := apmhttp.WrapClient(http.DefaultClient)
-	resp, err := ctxhttp.Get(ctx, client, server.URL+"/foo")
+	resp, err := ctxhttp.Get(ctx, client, requestURL.String())
 	assert.NoError(t, err)
 	resp.Body.Close()
 	assert.Equal(t, http.StatusTeapot, resp.StatusCode)
@@ -56,7 +65,12 @@ func TestClient(t *testing.T) {
 	span := transaction.Spans[0]
 	assert.Equal(t, "GET "+server.Listener.Addr().String(), span.Name)
 	assert.Equal(t, "ext.http", span.Type)
-	assert.Nil(t, span.Context)
+	assert.Equal(t, &model.SpanContext{
+		HTTP: &model.HTTPSpanContext{
+			// Note no user info included in server.URL.
+			URL: serverURL,
+		},
+	}, span.Context)
 }
 
 func TestClientTraceparentHeader(t *testing.T) {
@@ -93,13 +107,9 @@ func TestClientTraceparentHeader(t *testing.T) {
 	transaction := transactions[0]
 	require.Len(t, transaction.Spans, 1)
 
-	span := transaction.Spans[0]
-	assert.Equal(t, "GET "+server.Listener.Addr().String(), span.Name)
-	assert.Equal(t, "ext.http", span.Type)
-	assert.Nil(t, span.Context)
-
 	clientTraceContext, err := apmhttp.ParseTraceparentHeader(string(responseBody))
 	assert.NoError(t, err)
+	span := transaction.Spans[0]
 	assert.Equal(t, span.TraceID, model.TraceID(clientTraceContext.Trace))
 	assert.Equal(t, span.ID, model.SpanID(clientTraceContext.Span))
 	assert.Equal(t, transaction.ID.SpanID, span.ParentID)

spancontext.go

@@ -1,13 +1,16 @@
 package elasticapm
 
 import (
+	"net/http"
+
 	"github.com/elastic/apm-agent-go/model"
 )
 
 // SpanContext provides methods for setting span context.
 type SpanContext struct {
 	model    model.SpanContext
 	database model.DatabaseSpanContext
+	http     model.HTTPSpanContext
 }
 
 // DatabaseSpanContext holds database span context.
@@ -29,6 +32,7 @@ type DatabaseSpanContext struct {
 func (c *SpanContext) build() *model.SpanContext {
 	switch {
 	case c.model.Database != nil:
+	case c.model.HTTP != nil:
 	default:
 		return nil
 	}
@@ -44,3 +48,12 @@ func (c *SpanContext) SetDatabase(db DatabaseSpanContext) {
 	c.database = model.DatabaseSpanContext(db)
 	c.model.Database = &c.database
 }
+
+// SetHTTPRequest sets the details of the HTTP request in the context.
+//
+// This function relates to client requests. If the request URL contains
+// user info, it will be removed and excluded from the stored URL.
+func (c *SpanContext) SetHTTPRequest(req *http.Request) {
+	c.http.URL = req.URL
+	c.model.HTTP = &c.http
+}