diff --git a/api/transport/propagation.go b/api/transport/propagation.go index 7c27ba106..ed3c45bba 100644 --- a/api/transport/propagation.go +++ b/api/transport/propagation.go @@ -37,6 +37,11 @@ type CreateOpenTracingSpan struct { ExtraTags opentracing.Tags } +const ( + //TracingTagStatusCode is the span tag key for the YAPRC status code. + TracingTagStatusCode = "rpc.yarpc.status_code" +) + // Do creates a new context that has a reference to the started span. // This should be called before a Outbound makes a call func (c *CreateOpenTracingSpan) Do( diff --git a/tracing.go b/tracing.go index 5f9ee3e07..df84c0c8e 100644 --- a/tracing.go +++ b/tracing.go @@ -26,8 +26,14 @@ import ( opentracing "github.com/opentracing/opentracing-go" ) +const ( + //TracingComponentName helps determine the attribution of a span. + TracingComponentName = "yarpc" +) + // OpentracingTags are tags with YARPC metadata. var OpentracingTags = opentracing.Tags{ "yarpc.version": Version, "go.version": runtime.Version(), + "component": TracingComponentName, } diff --git a/transport/http/handler.go b/transport/http/handler.go index 58969c0e2..3a3424dfa 100644 --- a/transport/http/handler.go +++ b/transport/http/handler.go @@ -122,7 +122,13 @@ func (h handler) callHandler(responseWriter *responseWriter, req *http.Request, treq.Headers = treq.Headers.With(header, value) } } + ctx := req.Context() + ctx, cancel, parseTTLErr := parseTTL(ctx, treq, popHeader(req.Header, TTLMSHeader)) + // parseTTLErr != nil is a problem only if the request is unary. + defer cancel() + ctx, span := h.createSpan(ctx, req, treq, start) if err := transport.ValidateRequest(treq); err != nil { + UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) return err } defer func() { @@ -133,22 +139,18 @@ func (h handler) callHandler(responseWriter *responseWriter, req *http.Request, } }() - ctx := req.Context() - ctx, cancel, parseTTLErr := parseTTL(ctx, treq, popHeader(req.Header, TTLMSHeader)) - // parseTTLErr != nil is a problem only if the request is unary. - defer cancel() - ctx, span := h.createSpan(ctx, req, treq, start) - spec, err := h.router.Choose(ctx, treq) if err != nil { - updateSpanWithErr(span, err) + UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) return err } if parseTTLErr != nil { + UpdateSpanWithErrAndCode(span, parseTTLErr, yarpcerrors.FromError(parseTTLErr).Code()) return parseTTLErr } if err := transport.ValidateRequestContext(ctx); err != nil { + UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) return err } switch spec.Type() { @@ -171,7 +173,7 @@ func (h handler) callHandler(responseWriter *responseWriter, req *http.Request, err = yarpcerrors.Newf(yarpcerrors.CodeUnimplemented, "transport http does not handle %s handlers", spec.Type().String()) } - updateSpanWithErr(span, err) + UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) return err } @@ -185,6 +187,7 @@ func handleOnewayRequest( // returning from the request var buff bytes.Buffer if _, err := iopool.Copy(&buff, treq.Body); err != nil { + UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) return err } treq.Body = &buff @@ -203,7 +206,7 @@ func handleOnewayRequest( Handler: onewayHandler, Logger: logger, }) - updateSpanWithErr(span, err) + UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) }() return nil } @@ -211,8 +214,20 @@ func handleOnewayRequest( func updateSpanWithErr(span opentracing.Span, err error) { if err != nil { span.SetTag("error", true) - span.LogFields(opentracinglog.String("event", err.Error())) + span.LogFields( + opentracinglog.String("event", "error"), + opentracinglog.String("message", err.Error()), + ) + } +} + +// UpdateSpanWithErrAndCode sets the error tag with errcode on a span, if an error is given. +// Returns the given error +func UpdateSpanWithErrAndCode(span opentracing.Span, err error, errCode yarpcerrors.Code) { + if err != nil { + span.SetTag(transport.TracingTagStatusCode, errCode) } + updateSpanWithErr(span, err) } func (h handler) createSpan(ctx context.Context, req *http.Request, treq *transport.Request, start time.Time) (context.Context, opentracing.Span) { diff --git a/transport/http/outbound.go b/transport/http/outbound.go index bb34e04ac..260762c75 100644 --- a/transport/http/outbound.go +++ b/transport/http/outbound.go @@ -34,7 +34,6 @@ import ( "github.com/opentracing/opentracing-go" "github.com/opentracing/opentracing-go/ext" - opentracinglog "github.com/opentracing/opentracing-go/log" "go.uber.org/yarpc" "go.uber.org/yarpc/api/peer" "go.uber.org/yarpc/api/transport" @@ -304,22 +303,27 @@ func (o *Outbound) call(ctx context.Context, treq *transport.Request) (*transpor return nil, err } ctx, hreq, span, err := o.withOpentracingSpan(ctx, hreq, treq, start) + if span != nil { + defer span.Finish() + } if err != nil { + UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) return nil, err } - defer span.Finish() hreq = o.withCoreHeaders(hreq, treq, ttl) hreq = hreq.WithContext(ctx) response, err := o.roundTrip(hreq, treq, start, o.client) - if err != nil { - span.SetTag("error", true) - span.LogFields(opentracinglog.String("event", err.Error())) - return nil, err + if err != nil || (response.StatusCode >= 400 && response.StatusCode < 600) { + // If there's an error or the status code indicates an error, update the span + UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) + if err != nil { + return nil, err + } } - span.SetTag("http.status_code", response.StatusCode) + span.SetTag("http.response.status_code", response.StatusCode) // Service name match validation, return yarpcerrors.CodeInternal error if not match if match, resSvcName := checkServiceMatch(treq.Service, response.Header); !match {