diff --git a/context.go b/context.go index 9ab5e93f8..421af0af0 100644 --- a/context.go +++ b/context.go @@ -226,6 +226,10 @@ func (c *Context) SetHTTPResponseHeaders(h http.Header) { } // SetHTTPStatusCode records the HTTP response status code. +// +// If, when the transaction ends, its Outcome field has not +// been explicitly set, it will be set based on the status code: +// "success" if statusCode < 500, and "failure" otherwise. func (c *Context) SetHTTPStatusCode(statusCode int) { c.response.StatusCode = statusCode c.model.Response = &c.response @@ -254,3 +258,15 @@ func (c *Context) SetUsername(username string) { c.model.User = &c.user } } + +// outcome returns the outcome to assign to the associated transaction, +// based on context (e.g. HTTP status code). +func (c *Context) outcome() string { + if c.response.StatusCode != 0 { + if c.response.StatusCode < 500 { + return "success" + } + return "failure" + } + return "" +} diff --git a/module/apmgrpc/client.go b/module/apmgrpc/client.go index f3c1a38a9..bcbd33991 100644 --- a/module/apmgrpc/client.go +++ b/module/apmgrpc/client.go @@ -22,6 +22,7 @@ package apmgrpc import ( "golang.org/x/net/context" "google.golang.org/grpc" + "google.golang.org/grpc/codes" "google.golang.org/grpc/metadata" "go.elastic.co/apm" @@ -51,7 +52,11 @@ func NewUnaryClientInterceptor(o ...ClientOption) grpc.UnaryClientInterceptor { if span != nil { defer span.End() } - return invoker(ctx, method, req, resp, cc, opts...) + err := invoker(ctx, method, req, resp, cc, opts...) + if span != nil { + setSpanOutcome(span, err) + } + return err } } @@ -95,6 +100,23 @@ func outgoingContextWithTraceContext( return metadata.NewOutgoingContext(ctx, md) } +func setSpanOutcome(span *apm.Span, err error) { + statusCode := statusCodeFromError(err) + + // On the client side, all codes except for OK and Unknown are treated + // as failures by default, and can be overridden. + if span.Outcome == "" { + switch statusCode { + case codes.Unknown: + span.Outcome = "unknown" + case codes.OK: + span.Outcome = "success" + default: + span.Outcome = "failure" + } + } +} + type clientOptions struct { tracer *apm.Tracer } diff --git a/module/apmgrpc/client_test.go b/module/apmgrpc/client_test.go index ab098d7bd..cd1069281 100644 --- a/module/apmgrpc/client_test.go +++ b/module/apmgrpc/client_test.go @@ -26,7 +26,9 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/net/context" + "google.golang.org/grpc/codes" pb "google.golang.org/grpc/examples/helloworld/helloworld" + "google.golang.org/grpc/status" "go.elastic.co/apm" "go.elastic.co/apm/apmtest" @@ -171,3 +173,29 @@ func TestClientTransactionUnsampled(t *testing.T) { assert.Equal(t, clientTransaction.TraceID, serverTransactions[0].TraceID) assert.Equal(t, clientTransaction.ID, serverTransactions[0].ParentID) } + +func TestClientOutcome(t *testing.T) { + s, helloworldServer, addr := newServer(t, apmtest.DiscardTracer) + defer s.GracefulStop() + + conn, client := newClient(t, addr) + defer conn.Close() + + clientTracer := apmtest.NewRecordingTracer() + defer clientTracer.Close() + + _, spans, _ := clientTracer.WithTransaction(func(ctx context.Context) { + client.SayHello(ctx, &pb.HelloRequest{Name: "birita"}) + + helloworldServer.err = status.Errorf(codes.Unknown, "boom") + client.SayHello(ctx, &pb.HelloRequest{Name: "birita"}) + + helloworldServer.err = status.Errorf(codes.NotFound, "boom") + client.SayHello(ctx, &pb.HelloRequest{Name: "birita"}) + }) + require.Len(t, spans, 3) + + assert.Equal(t, "success", spans[0].Outcome) + assert.Equal(t, "unknown", spans[1].Outcome) + assert.Equal(t, "failure", spans[2].Outcome) +} diff --git a/module/apmgrpc/go.mod b/module/apmgrpc/go.mod index c8ad9f9f0..f8fdb0cc9 100644 --- a/module/apmgrpc/go.mod +++ b/module/apmgrpc/go.mod @@ -5,7 +5,7 @@ require ( github.com/stretchr/testify v1.4.0 go.elastic.co/apm v1.8.0 go.elastic.co/apm/module/apmhttp v1.8.0 - golang.org/x/net v0.0.0-20190724013045-ca1201d0de80 + golang.org/x/net v0.0.0-20200226121028-0de0cce0169b google.golang.org/grpc v1.17.0 ) diff --git a/module/apmgrpc/go.sum b/module/apmgrpc/go.sum index af983b634..f5a69094c 100644 --- a/module/apmgrpc/go.sum +++ b/module/apmgrpc/go.sum @@ -47,19 +47,28 @@ github.com/stretchr/testify v1.3.0 h1:TivCn/peBQ7UY8ooIcPgZFpTNSz0Q2U6UrFlUfqbe0 github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.4.0 h1:2E4SXV/wtOkTonXsotYi4li6zVWxYlZuYNCXe9XRJyk= github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= -go.elastic.co/fastjson v1.0.0 h1:ooXV/ABvf+tBul26jcVViPT3sBir0PvXgibYB1IQQzg= -go.elastic.co/fastjson v1.0.0/go.mod h1:PmeUOMMtLHQr9ZS9J9owrAVg0FkaZDRZJEFTTGHtchs= +github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= +go.elastic.co/fastjson v1.1.0 h1:3MrGBWWVIxe/xvsbpghtkFoPciPhOCmjsR/HfwEeQR4= +go.elastic.co/fastjson v1.1.0/go.mod h1:boNGISWMjQsUPy/t6yqt2/1Wx4YNPSe+mZjlyw9vKKI= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= +golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE= +golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= -golang.org/x/net v0.0.0-20190724013045-ca1201d0de80 h1:Ao/3l156eZf2AW5wK8a7/smtodRU+gha3+BeqJ69lRk= -golang.org/x/net v0.0.0-20190724013045-ca1201d0de80/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= +golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= +golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= +golang.org/x/net v0.0.0-20200226121028-0de0cce0169b h1:0mm1VjtFUOIlE1SbDlwjYaDxZVDP2S5ou6y0gSgXHu8= +golang.org/x/net v0.0.0-20200226121028-0de0cce0169b/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4 h1:YUO/7uOKsKeq9UokNS62b8FYywz3ker1l1vDZRCRefw= golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e h1:vcxGaoTs7kV8m5Np9uUNQin4BrLOthgV7252N8V+FwY= +golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= +golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20191025021431-6c3a3bfe00ae/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20191204072324-ce4227a45e2e h1:9vRrk9YW2BTzLP0VCB9ZDjU4cPqkg+IDWL7XgxA1yxQ= golang.org/x/sys v0.0.0-20191204072324-ce4227a45e2e/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= @@ -69,6 +78,11 @@ golang.org/x/text v0.3.2 h1:tW2bmiBqwgJj/UpqtC8EpXEZVYOwU0yG4iWbprSVAcs= golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk= golang.org/x/tools v0.0.0-20180828015842-6cd1fcedba52/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= +golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= +golang.org/x/tools v0.0.0-20200509030707-2212a7e161a5/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= +golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= google.golang.org/appengine v1.1.0/go.mod h1:EbEs0AVv82hx2wNQdGPgUI5lhzA/G0D9YwlJXL52JkM= google.golang.org/genproto v0.0.0-20180817151627-c66870c02cf8 h1:Nw54tB0rB7hY/N0NQvRW8DG4Yk3Q6T9cu9RcFQDu1tc= google.golang.org/genproto v0.0.0-20180817151627-c66870c02cf8/go.mod h1:JiN7NxoALGmiZfu7CAH4rXhgtRTLTxftemlI0sWmxmc= diff --git a/module/apmgrpc/server.go b/module/apmgrpc/server.go index 7fb288c2b..5187ec3d1 100644 --- a/module/apmgrpc/server.go +++ b/module/apmgrpc/server.go @@ -120,17 +120,34 @@ func getIncomingMetadataTraceContext(md metadata.MD, header string) (apm.TraceCo } func setTransactionResult(tx *apm.Transaction, err error) { - if err == nil { - tx.Result = codes.OK.String() - } else { - statusCode := codes.Unknown - if s, ok := status.FromError(err); ok { - statusCode = s.Code() + statusCode := statusCodeFromError(err) + tx.Result = statusCode.String() + + // For gRPC servers, the transaction outcome is generally "success", + // except for codes which are not subject to client interpretation. + if tx.Outcome == "" { + switch statusCode { + case codes.Unknown: + tx.Outcome = "unknown" + case codes.DataLoss, codes.Unavailable, codes.Internal, codes.Unimplemented: + tx.Outcome = "failure" + default: + tx.Outcome = "success" } - tx.Result = statusCode.String() } } +func statusCodeFromError(err error) codes.Code { + if err == nil { + return codes.OK + } + statusCode := codes.Unknown + if s, ok := status.FromError(err); ok { + statusCode = s.Code() + } + return statusCode +} + type serverOptions struct { tracer *apm.Tracer recover bool diff --git a/module/apmgrpc/server_test.go b/module/apmgrpc/server_test.go index 91ec46ebb..9da953510 100644 --- a/module/apmgrpc/server_test.go +++ b/module/apmgrpc/server_test.go @@ -109,6 +109,7 @@ func testServerTransactionHappy(t *testing.T, p testParams) { assert.Equal(t, "/helloworld.Greeter/SayHello", tx.Name) assert.Equal(t, "request", tx.Type) assert.Equal(t, "OK", tx.Result) + assert.Equal(t, "success", tx.Outcome) assert.Equal(t, model.TraceID(traceID), tx.TraceID) assert.Equal(t, model.SpanID(clientSpanID), tx.ParentID) assert.Equal(t, &model.Context{ @@ -137,6 +138,7 @@ func testServerTransactionUnknownError(t *testing.T, p testParams) { assert.Equal(t, "/helloworld.Greeter/SayHello", tx.Name) assert.Equal(t, "request", tx.Type) assert.Equal(t, "Unknown", tx.Result) + assert.Equal(t, "unknown", tx.Outcome) } func testServerTransactionStatusError(t *testing.T, p testParams) { @@ -150,6 +152,7 @@ func testServerTransactionStatusError(t *testing.T, p testParams) { assert.Equal(t, "/helloworld.Greeter/SayHello", tx.Name) assert.Equal(t, "request", tx.Type) assert.Equal(t, "DataLoss", tx.Result) + assert.Equal(t, "failure", tx.Outcome) } func testServerTransactionPanic(t *testing.T, p testParams) { diff --git a/module/apmhttp/client_test.go b/module/apmhttp/client_test.go index 4a28ffe44..ae77d5611 100644 --- a/module/apmhttp/client_test.go +++ b/module/apmhttp/client_test.go @@ -326,6 +326,34 @@ func TestWithClientTrace(t *testing.T) { assert.Equal(t, "Response", spans[2].Name) } +func TestClientOutcome(t *testing.T) { + mux := http.NewServeMux() + mux.Handle("/2xx", http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + w.WriteHeader(200) + })) + mux.Handle("/4xx", http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + w.WriteHeader(400) + })) + mux.Handle("/5xx", http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + w.WriteHeader(500) + })) + srv := httptest.NewServer(mux) + defer srv.Close() + + _, spans, _ := apmtest.WithTransaction(func(ctx context.Context) { + mustGET(ctx, srv.URL+"/2xx") + mustGET(ctx, srv.URL+"/4xx") + mustGET(ctx, srv.URL+"/5xx") + }) + require.Len(t, spans, 3) + + // 200s are considered successful, while 400s and 500s + // are both considered failed by the client. + assert.Equal(t, "success", spans[0].Outcome) + assert.Equal(t, "failure", spans[1].Outcome) + assert.Equal(t, "failure", spans[2].Outcome) +} + func mustGET(ctx context.Context, url string, o ...apmhttp.ClientOption) (statusCode int, responseBody string) { client := apmhttp.WrapClient(http.DefaultClient, o...) resp, err := ctxhttp.Get(ctx, client, url) diff --git a/module/apmhttp/go.mod b/module/apmhttp/go.mod index e496fca8e..6baf79c2f 100644 --- a/module/apmhttp/go.mod +++ b/module/apmhttp/go.mod @@ -4,7 +4,7 @@ require ( github.com/pkg/errors v0.8.1 github.com/stretchr/testify v1.4.0 go.elastic.co/apm v1.8.0 - golang.org/x/net v0.0.0-20190724013045-ca1201d0de80 + golang.org/x/net v0.0.0-20200226121028-0de0cce0169b golang.org/x/text v0.3.2 // indirect ) diff --git a/module/apmhttp/go.sum b/module/apmhttp/go.sum index 197618445..703fa14fc 100644 --- a/module/apmhttp/go.sum +++ b/module/apmhttp/go.sum @@ -37,13 +37,25 @@ github.com/stretchr/testify v1.3.0 h1:TivCn/peBQ7UY8ooIcPgZFpTNSz0Q2U6UrFlUfqbe0 github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.4.0 h1:2E4SXV/wtOkTonXsotYi4li6zVWxYlZuYNCXe9XRJyk= github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= +github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= go.elastic.co/fastjson v1.0.0 h1:ooXV/ABvf+tBul26jcVViPT3sBir0PvXgibYB1IQQzg= go.elastic.co/fastjson v1.0.0/go.mod h1:PmeUOMMtLHQr9ZS9J9owrAVg0FkaZDRZJEFTTGHtchs= +go.elastic.co/fastjson v1.1.0 h1:3MrGBWWVIxe/xvsbpghtkFoPciPhOCmjsR/HfwEeQR4= +go.elastic.co/fastjson v1.1.0/go.mod h1:boNGISWMjQsUPy/t6yqt2/1Wx4YNPSe+mZjlyw9vKKI= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= +golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= +golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= +golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= +golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20190724013045-ca1201d0de80 h1:Ao/3l156eZf2AW5wK8a7/smtodRU+gha3+BeqJ69lRk= golang.org/x/net v0.0.0-20190724013045-ca1201d0de80/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= +golang.org/x/net v0.0.0-20200226121028-0de0cce0169b h1:0mm1VjtFUOIlE1SbDlwjYaDxZVDP2S5ou6y0gSgXHu8= +golang.org/x/net v0.0.0-20200226121028-0de0cce0169b/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= +golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20191025021431-6c3a3bfe00ae/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20191204072324-ce4227a45e2e h1:9vRrk9YW2BTzLP0VCB9ZDjU4cPqkg+IDWL7XgxA1yxQ= golang.org/x/sys v0.0.0-20191204072324-ce4227a45e2e/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= @@ -52,6 +64,11 @@ golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.2 h1:tW2bmiBqwgJj/UpqtC8EpXEZVYOwU0yG4iWbprSVAcs= golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= +golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= +golang.org/x/tools v0.0.0-20200509030707-2212a7e161a5/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= +golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 h1:qIbj1fsPNlZgppZ+VLlY7N33q108Sa+fhmuc+sWQYwY= gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= diff --git a/module/apmhttp/handler_test.go b/module/apmhttp/handler_test.go index b5cbb994e..d9c2c9839 100644 --- a/module/apmhttp/handler_test.go +++ b/module/apmhttp/handler_test.go @@ -107,6 +107,55 @@ func TestHandler(t *testing.T) { }, transaction.Context) } +func TestHandlerOutcome(t *testing.T) { + tracer, transport := transporttest.NewRecorderTracer() + defer tracer.Close() + + mux := http.NewServeMux() + mux.Handle("/2xx", http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + w.WriteHeader(200) + })) + mux.Handle("/4xx", http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + w.WriteHeader(400) + })) + mux.Handle("/5xx", http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + w.WriteHeader(500) + })) + mux.Handle("/panic", http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + panic("eek") + })) + mux.Handle("/override", http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + tx := apm.TransactionFromContext(req.Context()) + tx.Outcome = "unknown" // override status + w.WriteHeader(500) + })) + + srv := httptest.NewServer(apmhttp.Wrap(mux, apmhttp.WithTracer(tracer))) + defer srv.Close() + for _, path := range []string{"/2xx", "/4xx", "/5xx", "/panic", "/override"} { + resp, _ := srv.Client().Get(srv.URL + path) + resp.Body.Close() + } + tracer.Flush(nil) + + payloads := transport.Payloads() + require.Len(t, payloads.Transactions, 5) + + // 200s are obviously considered successful. + // + // Less obvious: 400s are considered successful from the + // server's perspective, as they are _client_ errors. + // + // 500s and panics (implicit 500) are considered failures. + // Outcome can be overridden by the handler, in which case + // the status code will not impact the outcome. + assert.Equal(t, "success", payloads.Transactions[0].Outcome) + assert.Equal(t, "success", payloads.Transactions[1].Outcome) + assert.Equal(t, "failure", payloads.Transactions[2].Outcome) + assert.Equal(t, "failure", payloads.Transactions[3].Outcome) + assert.Equal(t, "unknown", payloads.Transactions[4].Outcome) +} + func TestHandlerCaptureBodyRaw(t *testing.T) { tracer, transport := transporttest.NewRecorderTracer() defer tracer.Close() diff --git a/span.go b/span.go index c34429c7f..1fda02447 100644 --- a/span.go +++ b/span.go @@ -293,6 +293,9 @@ func (s *Span) End() { if s.Duration < 0 { s.Duration = time.Since(s.timestamp) } + if s.Outcome == "" { + s.Outcome = s.Context.outcome() + } if s.dropped() { if s.tx == nil { droppedSpanDataPool.Put(s.SpanData) diff --git a/spancontext.go b/spancontext.go index 180fe1ddc..d80b82f2b 100644 --- a/spancontext.go +++ b/spancontext.go @@ -168,6 +168,10 @@ func (c *SpanContext) SetHTTPRequest(req *http.Request) { } // SetHTTPStatusCode records the HTTP response status code. +// +// If, when the transaction ends, its Outcome field has not +// been explicitly set, it will be set based on the status code: +// "success" if statusCode < 400, and "failure" otherwise. func (c *SpanContext) SetHTTPStatusCode(statusCode int) { c.http.StatusCode = statusCode c.model.HTTP = &c.http @@ -191,3 +195,15 @@ func (c *SpanContext) SetDestinationService(service DestinationServiceSpanContex c.destination.Service = &c.destinationService c.model.Destination = &c.destination } + +// outcome returns the outcome to assign to the associated span, based on +// context (e.g. HTTP status code). +func (c *SpanContext) outcome() string { + if c.http.StatusCode != 0 { + if c.http.StatusCode < 400 { + return "success" + } + return "failure" + } + return "" +} diff --git a/transaction.go b/transaction.go index 4db04e99b..85b2b8738 100644 --- a/transaction.go +++ b/transaction.go @@ -265,6 +265,9 @@ func (tx *Transaction) End() { if tx.Duration < 0 { tx.Duration = time.Since(tx.timestamp) } + if tx.Outcome == "" { + tx.Outcome = tx.Context.outcome() + } tx.enqueue() } else { tx.reset(tx.tracer)