From 61ae75fc7203280a15e47e4c7ffef88cc300af5b Mon Sep 17 00:00:00 2001 From: Daniel Redondo Date: Tue, 3 Mar 2020 13:12:23 +0100 Subject: [PATCH 1/5] Try to extract request body payload when the `GetBody` func is nil --- instrumentation/nethttp/client.go | 28 ++++++++++++++++++---- instrumentation/nethttp/nethttp_test.go | 31 ++++++++++++++----------- 2 files changed, 42 insertions(+), 17 deletions(-) diff --git a/instrumentation/nethttp/client.go b/instrumentation/nethttp/client.go index 75fa6ff1..578a7041 100644 --- a/instrumentation/nethttp/client.go +++ b/instrumentation/nethttp/client.go @@ -1,6 +1,7 @@ package nethttp import ( + "bufio" "bytes" "context" "io" @@ -15,6 +16,7 @@ import ( "github.com/opentracing/opentracing-go/log" "go.undefinedlabs.com/scopeagent/instrumentation" + "go.undefinedlabs.com/scopeagent/reflection" ) type contextKey int @@ -223,16 +225,34 @@ func (t *Transport) doRoundTrip(req *http.Request) (*http.Response, error) { // Gets the request payload func getRequestPayload(req *http.Request, bufferSize int) string { var rqPayload string - if req != nil && req.Body != nil && req.Body != http.NoBody && req.GetBody != nil { - rqBody, rqErr := req.GetBody() + if req != nil && req.Body != nil && req.Body != http.NoBody { + getBody := req.GetBody + if getBody == nil { + if ptr, err := reflection.GetFieldPointerOf(req.Body, "src"); err == nil { + reader := *(*io.Reader)(ptr) + if limReader, ok := reader.(*io.LimitedReader); ok { + if bufReader, ok := limReader.R.(*bufio.Reader); ok { + size := bufReader.Buffered() + if size < bufferSize { + bufferSize = size + } + rqBodyBuffer, err := bufReader.Peek(bufferSize) + if err == nil { + return string(bytes.Runes(rqBodyBuffer)) + } + } + } + } + return "" + } + rqBody, rqErr := getBody() if rqErr == nil { rqBodyBuffer := make([]byte, bufferSize) if len, err := rqBody.Read(rqBodyBuffer); err == nil && len > 0 { if len < bufferSize { rqBodyBuffer = rqBodyBuffer[:len] } - rqRunes := bytes.Runes(rqBodyBuffer) - rqPayload = string(rqRunes) + rqPayload = string(bytes.Runes(rqBodyBuffer)) } } } diff --git a/instrumentation/nethttp/nethttp_test.go b/instrumentation/nethttp/nethttp_test.go index 61cb9de2..ffbedc01 100644 --- a/instrumentation/nethttp/nethttp_test.go +++ b/instrumentation/nethttp/nethttp_test.go @@ -1,6 +1,7 @@ package nethttp import ( + "bytes" "fmt" "net/http" "net/http/httptest" @@ -15,7 +16,7 @@ import ( var r *tracer.InMemorySpanRecorder func TestMain(m *testing.M) { - PatchHttpDefaultClient() + PatchHttpDefaultClient(WithPayloadInstrumentation()) // Test tracer r = tracer.NewInMemoryRecorder() @@ -64,10 +65,10 @@ func TestHttpServer(t *testing.T) { return } }) - server := httptest.NewServer(Middleware(nil)) + server := httptest.NewServer(Middleware(nil, MWPayloadInstrumentation())) url := fmt.Sprintf("%s/hello", server.URL) - req, err := http.NewRequest("GET", url, nil) + req, err := http.NewRequest("POST", url, bytes.NewReader([]byte("Hello world request"))) if err != nil { t.Fatalf("%+v", err) } @@ -87,18 +88,22 @@ func TestHttpServer(t *testing.T) { t.Fatalf("there aren't the right number of spans: %d", len(spans)) } checkTags(t, spans[0].Tags, map[string]string{ - "component": "net/http", - "http.method": "GET", - "http.url": "/hello", - "span.kind": "server", - "http.status_code": "200", + "component": "net/http", + "http.method": "POST", + "http.url": "/hello", + "span.kind": "server", + "http.status_code": "200", + "http.request_payload": "Hello world request", + "http.response_payload": "Hello world", }) checkTags(t, spans[1].Tags, map[string]string{ - "component": "net/http", - "http.method": "GET", - "http.url": url, - "peer.ipv4": "127.0.0.1", - "span.kind": "client", + "component": "net/http", + "http.method": "POST", + "http.url": url, + "peer.ipv4": "127.0.0.1", + "span.kind": "client", + "http.request_payload": "Hello world request", + "http.response_payload": "Hello world", }) } From 70fc0d86c70caac26be4de4c8e63c80b5f380fc4 Mon Sep 17 00:00:00 2001 From: Daniel Redondo Date: Mon, 9 Mar 2020 10:13:01 +0100 Subject: [PATCH 2/5] getRequestPayload refactor --- instrumentation/nethttp/client.go | 45 ++++++++++++++++++------------- 1 file changed, 26 insertions(+), 19 deletions(-) diff --git a/instrumentation/nethttp/client.go b/instrumentation/nethttp/client.go index 578a7041..8d925398 100644 --- a/instrumentation/nethttp/client.go +++ b/instrumentation/nethttp/client.go @@ -226,26 +226,10 @@ func (t *Transport) doRoundTrip(req *http.Request) (*http.Response, error) { func getRequestPayload(req *http.Request, bufferSize int) string { var rqPayload string if req != nil && req.Body != nil && req.Body != http.NoBody { - getBody := req.GetBody - if getBody == nil { - if ptr, err := reflection.GetFieldPointerOf(req.Body, "src"); err == nil { - reader := *(*io.Reader)(ptr) - if limReader, ok := reader.(*io.LimitedReader); ok { - if bufReader, ok := limReader.R.(*bufio.Reader); ok { - size := bufReader.Buffered() - if size < bufferSize { - bufferSize = size - } - rqBodyBuffer, err := bufReader.Peek(bufferSize) - if err == nil { - return string(bytes.Runes(rqBodyBuffer)) - } - } - } - } - return "" + if req.GetBody == nil { + return getBodyPayload(req.Body, bufferSize) } - rqBody, rqErr := getBody() + rqBody, rqErr := req.GetBody() if rqErr == nil { rqBodyBuffer := make([]byte, bufferSize) if len, err := rqBody.Read(rqBodyBuffer); err == nil && len > 0 { @@ -259,6 +243,29 @@ func getRequestPayload(req *http.Request, bufferSize int) string { return rqPayload } +// Gets the payload from a body +func getBodyPayload(body io.ReadCloser, bufferSize int) string { + if body == nil { + return "" + } + if ptr, err := reflection.GetFieldPointerOf(body, "src"); err == nil { + reader := *(*io.Reader)(ptr) + if limReader, ok := reader.(*io.LimitedReader); ok { + if bufReader, ok := limReader.R.(*bufio.Reader); ok { + size := bufReader.Buffered() + if size < bufferSize { + bufferSize = size + } + rqBodyBuffer, err := bufReader.Peek(bufferSize) + if err == nil { + return string(bytes.Runes(rqBodyBuffer)) + } + } + } + } + return "" +} + // Gets the response payload func getResponsePayload(resp *http.Response, bufferSize int) string { var rsPayload string From 7e3786591adfef69e1dcd6867fa67768d45e8172 Mon Sep 17 00:00:00 2001 From: Daniel Redondo Date: Mon, 9 Mar 2020 10:16:05 +0100 Subject: [PATCH 3/5] GetBody comment --- instrumentation/nethttp/client.go | 1 + 1 file changed, 1 insertion(+) diff --git a/instrumentation/nethttp/client.go b/instrumentation/nethttp/client.go index 8d925398..49b41a54 100644 --- a/instrumentation/nethttp/client.go +++ b/instrumentation/nethttp/client.go @@ -227,6 +227,7 @@ func getRequestPayload(req *http.Request, bufferSize int) string { var rqPayload string if req != nil && req.Body != nil && req.Body != http.NoBody { if req.GetBody == nil { + // GetBody is nil in server requests return getBodyPayload(req.Body, bufferSize) } rqBody, rqErr := req.GetBody() From f328b613d7068acb6db56b2cad8486efc5760b49 Mon Sep 17 00:00:00 2001 From: Daniel Redondo Date: Thu, 12 Mar 2020 13:35:42 +0100 Subject: [PATCH 4/5] Algorithm change with a MultiReader --- go.mod | 3 + instrumentation/nethttp/client.go | 108 +++++++++++++++--------------- 2 files changed, 58 insertions(+), 53 deletions(-) diff --git a/go.mod b/go.mod index 4e5c18b3..62141a05 100644 --- a/go.mod +++ b/go.mod @@ -15,10 +15,13 @@ require ( github.com/stretchr/testify v1.5.1 github.com/undefinedlabs/go-mpatch v0.0.0-20200122175732-0044123dbb98 github.com/vmihailenco/msgpack v4.0.4+incompatible + golang.org/x/lint v0.0.0-20190313153728-d0100b6bd8b3 // indirect golang.org/x/net v0.0.0-20200114155413-6afb5195e5aa golang.org/x/sys v0.0.0-20200122134326-e047566fdf82 // indirect + golang.org/x/tools v0.0.0-20190524140312-2c0ae7006135 // indirect google.golang.org/appengine v1.6.5 // indirect google.golang.org/grpc v1.27.1 gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 // indirect gopkg.in/tomb.v2 v2.0.0-20161208151619-d5d1b5820637 + honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc // indirect ) diff --git a/instrumentation/nethttp/client.go b/instrumentation/nethttp/client.go index 49b41a54..1f72ae9c 100644 --- a/instrumentation/nethttp/client.go +++ b/instrumentation/nethttp/client.go @@ -1,7 +1,6 @@ package nethttp import ( - "bufio" "bytes" "context" "io" @@ -16,7 +15,6 @@ import ( "github.com/opentracing/opentracing-go/log" "go.undefinedlabs.com/scopeagent/instrumentation" - "go.undefinedlabs.com/scopeagent/reflection" ) type contextKey int @@ -224,69 +222,73 @@ func (t *Transport) doRoundTrip(req *http.Request) (*http.Response, error) { // Gets the request payload func getRequestPayload(req *http.Request, bufferSize int) string { - var rqPayload string - if req != nil && req.Body != nil && req.Body != http.NoBody { - if req.GetBody == nil { - // GetBody is nil in server requests - return getBodyPayload(req.Body, bufferSize) - } - rqBody, rqErr := req.GetBody() - if rqErr == nil { - rqBodyBuffer := make([]byte, bufferSize) - if len, err := rqBody.Read(rqBodyBuffer); err == nil && len > 0 { - if len < bufferSize { - rqBodyBuffer = rqBodyBuffer[:len] - } - rqPayload = string(bytes.Runes(rqBodyBuffer)) - } + if req == nil || req.Body == nil || req.Body == http.NoBody { + return "" + } + if req.GetBody == nil { + // GetBody is nil in server requests + nBody, payload := getBodyPayload(req.Body, bufferSize) + req.Body = nBody + return payload + } + rqBody, rqErr := req.GetBody() + if rqErr != nil { + return "" + } + rqBodyBuffer := make([]byte, bufferSize) + if ln, err := rqBody.Read(rqBodyBuffer); err == nil && ln > 0 { + if ln < bufferSize { + rqBodyBuffer = rqBodyBuffer[:ln] } + return string(bytes.Runes(rqBodyBuffer)) } - return rqPayload + return "" } // Gets the payload from a body -func getBodyPayload(body io.ReadCloser, bufferSize int) string { +func getBodyPayload(body io.ReadCloser, bufferSize int) (io.ReadCloser, string) { if body == nil { - return "" + return body, "" } - if ptr, err := reflection.GetFieldPointerOf(body, "src"); err == nil { - reader := *(*io.Reader)(ptr) - if limReader, ok := reader.(*io.LimitedReader); ok { - if bufReader, ok := limReader.R.(*bufio.Reader); ok { - size := bufReader.Buffered() - if size < bufferSize { - bufferSize = size - } - rqBodyBuffer, err := bufReader.Peek(bufferSize) - if err == nil { - return string(bytes.Runes(rqBodyBuffer)) - } - } - } + rsBodyBuffer := make([]byte, bufferSize) + ln, _ := body.Read(rsBodyBuffer) + if ln == 0 { + return body, "" } - return "" + if ln < bufferSize { + rsBodyBuffer = rsBodyBuffer[:ln] + } + rsPayload := string(bytes.Runes(rsBodyBuffer)) + rBody := struct { + io.Reader + io.Closer + }{ + io.MultiReader(bytes.NewReader(rsBodyBuffer), body), + body, + } + return rBody, rsPayload } // Gets the response payload func getResponsePayload(resp *http.Response, bufferSize int) string { - var rsPayload string - if resp != nil && resp.Body != nil && resp.Body != http.NoBody { - rsBodyBuffer := make([]byte, bufferSize) - len, _ := resp.Body.Read(rsBodyBuffer) - if len > 0 { - if len < bufferSize { - rsBodyBuffer = rsBodyBuffer[:len] - } - rsRunes := bytes.Runes(rsBodyBuffer) - rsPayload = string(rsRunes) - resp.Body = struct { - io.Reader - io.Closer - }{ - io.MultiReader(bytes.NewReader(rsBodyBuffer), resp.Body), - resp.Body, - } - } + if resp == nil || resp.Body == nil || resp.Body == http.NoBody { + return "" + } + rsBodyBuffer := make([]byte, bufferSize) + ln, _ := resp.Body.Read(rsBodyBuffer) + if ln == 0 { + return "" + } + if ln < bufferSize { + rsBodyBuffer = rsBodyBuffer[:ln] + } + rsPayload := string(bytes.Runes(rsBodyBuffer)) + resp.Body = struct { + io.Reader + io.Closer + }{ + io.MultiReader(bytes.NewReader(rsBodyBuffer), resp.Body), + resp.Body, } return rsPayload } From 1adce8ed519033abae4b0139ed626e81e874e62e Mon Sep 17 00:00:00 2001 From: Daniel Redondo Date: Thu, 12 Mar 2020 13:36:27 +0100 Subject: [PATCH 5/5] revert go.mod --- go.mod | 3 --- 1 file changed, 3 deletions(-) diff --git a/go.mod b/go.mod index 62141a05..4e5c18b3 100644 --- a/go.mod +++ b/go.mod @@ -15,13 +15,10 @@ require ( github.com/stretchr/testify v1.5.1 github.com/undefinedlabs/go-mpatch v0.0.0-20200122175732-0044123dbb98 github.com/vmihailenco/msgpack v4.0.4+incompatible - golang.org/x/lint v0.0.0-20190313153728-d0100b6bd8b3 // indirect golang.org/x/net v0.0.0-20200114155413-6afb5195e5aa golang.org/x/sys v0.0.0-20200122134326-e047566fdf82 // indirect - golang.org/x/tools v0.0.0-20190524140312-2c0ae7006135 // indirect google.golang.org/appengine v1.6.5 // indirect google.golang.org/grpc v1.27.1 gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 // indirect gopkg.in/tomb.v2 v2.0.0-20161208151619-d5d1b5820637 - honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc // indirect )