Fix span error tagging in GO YARPC/http

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(

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,
 }

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,16 +206,28 @@ func handleOnewayRequest(
 			Handler: onewayHandler,
 			Logger:  logger,
 		})
-		updateSpanWithErr(span, err)
+		UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code())
 	}()
 	return nil
 }
 
 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) {

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 {