diff --git a/CHANGELOG.md b/CHANGELOG.md index b7e40c409..df71e9c96 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/context.go b/context.go index 48753bdd1..1531a09b7 100644 --- a/context.go +++ b/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. diff --git a/model/marshal.go b/model/marshal.go index b4201f9b7..5b3b90c67 100644 --- a/model/marshal.go +++ b/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) diff --git a/model/marshal_fastjson.go b/model/marshal_fastjson.go index a83c53b87..17e51a312 100644 --- a/model/marshal_fastjson.go +++ b/model/marshal_fastjson.go @@ -243,10 +243,27 @@ func (v *Span) MarshalFastJSON(w *fastjson.Writer) { func (v *SpanContext) MarshalFastJSON(w *fastjson.Writer) { w.RawByte('{') + first := true if v.Database != nil { - w.RawString("\"db\":") + const prefix = ",\"db\":" + if first { + first = false + w.RawString(prefix[1:]) + } else { + w.RawString(prefix) + } v.Database.MarshalFastJSON(w) } + if v.HTTP != nil { + const prefix = ",\"http\":" + if first { + first = false + w.RawString(prefix[1:]) + } else { + w.RawString(prefix) + } + v.HTTP.MarshalFastJSON(w) + } w.RawByte('}') } diff --git a/model/marshal_test.go b/model/marshal_test.go index 2741cbefc..7da50bae8 100644 --- a/model/marshal_test.go +++ b/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"), + }, + }, }}, } } diff --git a/model/model.go b/model/model.go index 67fa5716e..e97c5371d 100644 --- a/model/model.go +++ b/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". diff --git a/module/apmhttp/client.go b/module/apmhttp/client.go index 4e60d8d26..a50400722 100644 --- a/module/apmhttp/client.go +++ b/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() diff --git a/module/apmhttp/client_test.go b/module/apmhttp/client_test.go index 132196d4e..ae1f7bb97 100644 --- a/module/apmhttp/client_test.go +++ b/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) diff --git a/spancontext.go b/spancontext.go index b0aefb3b3..66935b772 100644 --- a/spancontext.go +++ b/spancontext.go @@ -1,6 +1,8 @@ package elasticapm import ( + "net/http" + "github.com/elastic/apm-agent-go/model" ) @@ -8,6 +10,7 @@ import ( 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 +}