contrib: refactor http request span tags

contrib/emicklei/go-restful/restful.go

@@ -10,6 +10,7 @@ import (
 	"math"
 	"strconv"
 
+	httptrace "gopkg.in/DataDog/dd-trace-go.v1/contrib/net/http"
 	"gopkg.in/DataDog/dd-trace-go.v1/ddtrace"
 	"gopkg.in/DataDog/dd-trace-go.v1/ddtrace/ext"
 	"gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer"
@@ -26,29 +27,18 @@ func FilterFunc(configOpts ...Option) restful.FilterFunction {
 	}
 	log.Debug("contrib/emicklei/go-restful: Creating tracing filter: %#v", cfg)
 	return func(req *restful.Request, resp *restful.Response, chain *restful.FilterChain) {
-		opts := []ddtrace.StartSpanOption{
-			tracer.ServiceName(cfg.serviceName),
-			tracer.ResourceName(req.SelectedRoutePath()),
-			tracer.SpanType(ext.SpanTypeWeb),
-			tracer.Tag(ext.HTTPMethod, req.Request.Method),
-			tracer.Tag(ext.HTTPURL, req.Request.URL.Path),
-		}
+		var opt ddtrace.StartSpanOption
 		if !math.IsNaN(cfg.analyticsRate) {
-			opts = append(opts, tracer.Tag(ext.EventSampleRate, cfg.analyticsRate))
-		}
-		if spanctx, err := tracer.Extract(tracer.HTTPHeadersCarrier(req.Request.Header)); err == nil {
-			opts = append(opts, tracer.ChildOf(spanctx))
+			opt = tracer.Tag(ext.EventSampleRate, cfg.analyticsRate)
 		}
-		span, ctx := tracer.StartSpanFromContext(req.Request.Context(), "http.request", opts...)
-		defer span.Finish()
+		span, ctx := httptrace.StartRequestSpan(req.Request, cfg.serviceName, req.SelectedRoutePath(), false, opt)
+		defer func() {
+			httptrace.FinishRequestSpan(span, resp.StatusCode(), tracer.WithError(resp.Error()))
+		}()
 
 		// pass the span through the request context
 		req.Request = req.Request.WithContext(ctx)
-
 		chain.ProcessFilter(req, resp)
-
-		span.SetTag(ext.HTTPCode, strconv.Itoa(resp.StatusCode()))
-		span.SetTag(ext.Error, resp.Error())
 	}
 }
 

contrib/gin-gonic/gin/gintrace.go

@@ -9,9 +9,8 @@ package gin // import "gopkg.in/DataDog/dd-trace-go.v1/contrib/gin-gonic/gin"
 import (
 	"fmt"
 	"math"
-	"net/http"
-	"strconv"
 
+	httptrace "gopkg.in/DataDog/dd-trace-go.v1/contrib/net/http"
 	"gopkg.in/DataDog/dd-trace-go.v1/ddtrace"
 	"gopkg.in/DataDog/dd-trace-go.v1/ddtrace/ext"
 	"gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer"
@@ -34,27 +33,21 @@ func Middleware(service string, opts ...Option) gin.HandlerFunc {
 		if cfg.ignoreRequest(c) {
 			return
 		}
-		resource := cfg.resourceNamer(c)
 		opts := []ddtrace.StartSpanOption{
-			tracer.ServiceName(cfg.serviceName),
-			tracer.ResourceName(resource),
-			tracer.SpanType(ext.SpanTypeWeb),
-			tracer.Tag(ext.HTTPMethod, c.Request.Method),
-			tracer.Tag(ext.HTTPURL, c.Request.URL.Path),
 			tracer.Measured(),
 		}
 		if !math.IsNaN(cfg.analyticsRate) {
 			opts = append(opts, tracer.Tag(ext.EventSampleRate, cfg.analyticsRate))
 		}
-		if spanctx, err := tracer.Extract(tracer.HTTPHeadersCarrier(c.Request.Header)); err == nil {
-			opts = append(opts, tracer.ChildOf(spanctx))
-		}
-		span, ctx := tracer.StartSpanFromContext(c.Request.Context(), "http.request", opts...)
-		defer span.Finish()
+		span, ctx := httptrace.StartRequestSpan(c.Request, cfg.serviceName, cfg.resourceNamer(c), false, opts...)
 
 		// pass the span through the request context
 		c.Request = c.Request.WithContext(ctx)
 
+		defer func() {
+			httptrace.FinishRequestSpan(span, c.Writer.Status())
+		}()
+
 		// Use AppSec if enabled by user
 		if appsecEnabled {
 			afterMiddleware := useAppSec(c, span)
@@ -64,12 +57,6 @@ func Middleware(service string, opts ...Option) gin.HandlerFunc {
 		// serve the request to the next middleware
 		c.Next()
 
-		status := c.Writer.Status()
-		span.SetTag(ext.HTTPCode, strconv.Itoa(status))
-		if status >= 500 && status < 600 {
-			span.SetTag(ext.Error, fmt.Errorf("%d: %s", status, http.StatusText(status)))
-		}
-
 		if len(c.Errors) > 0 {
 			span.SetTag("gin.errors", c.Errors.String())
 		}

contrib/net/http/trace.go

@@ -8,6 +8,7 @@ package http // import "gopkg.in/DataDog/dd-trace-go.v1/contrib/net/http"
 //go:generate sh -c "go run make_responsewriter.go | gofmt > trace_gen.go"
 
 import (
+	"context"
 	"fmt"
 	"net/http"
 	"strconv"
@@ -26,7 +27,7 @@ type ServeConfig struct {
 	Service string
 	// Resource optionally specifies the resource name for this request.
 	Resource string
-	// QueryParams specifies any query parameters that be appended to the resulting "http.url" tag.
+	// QueryParams should be true in order to append the URL query values to the  "http.url" tag.
 	QueryParams bool
 	// RouteParams specifies framework-specific route parameters (e.g. for route /user/:id coming
 	// in as /user/123 we'll have {"id": "123"}). This field is optional and is used for monitoring
@@ -41,20 +42,33 @@ type ServeConfig struct {
 // TraceAndServe serves the handler h using the given ResponseWriter and Request, applying tracing
 // according to the specified config.
 func TraceAndServe(h http.Handler, w http.ResponseWriter, r *http.Request, cfg *ServeConfig) {
-	if cfg == nil {
-		cfg = new(ServeConfig)
+	span, ctx := StartRequestSpan(r, cfg.Service, cfg.Resource, cfg.QueryParams, cfg.SpanOpts...)
+	rw, ddrw := wrapResponseWriter(w)
+	defer func() {
+		FinishRequestSpan(span, ddrw.status, cfg.FinishOpts...)
+	}()
+
+	if appsec.Enabled() {
+		h = httpsec.WrapHandler(h, span, cfg.RouteParams)
 	}
+	h.ServeHTTP(rw, r.WithContext(ctx))
+}
+
+// StartRequestSpan starts an HTTP request span with the standard list of HTTP request span tags. URL query parameters
+// are added to the URL tag when queryParams is true. Any further span start option can be added with opts.
+func StartRequestSpan(r *http.Request, service, resource string, queryParams bool, opts ...ddtrace.StartSpanOption) (tracer.Span, context.Context) {
 	path := r.URL.Path
-	if cfg.QueryParams {
+	if queryParams {
 		path += "?" + r.URL.RawQuery
 	}
-	opts := append([]ddtrace.StartSpanOption{
+	opts = append([]ddtrace.StartSpanOption{
 		tracer.SpanType(ext.SpanTypeWeb),
-		tracer.ServiceName(cfg.Service),
-		tracer.ResourceName(cfg.Resource),
+		tracer.ServiceName(service),
+		tracer.ResourceName(resource),
 		tracer.Tag(ext.HTTPMethod, r.Method),
 		tracer.Tag(ext.HTTPURL, path),
-	}, cfg.SpanOpts...)
+		tracer.Tag(ext.HTTPUserAgent, r.UserAgent()),
+	}, opts...)
 	if r.URL.Host != "" {
 		opts = append([]ddtrace.StartSpanOption{
 			tracer.Tag("http.host", r.URL.Host),
@@ -63,24 +77,23 @@ func TraceAndServe(h http.Handler, w http.ResponseWriter, r *http.Request, cfg *
 	if spanctx, err := tracer.Extract(tracer.HTTPHeadersCarrier(r.Header)); err == nil {
 		opts = append(opts, tracer.ChildOf(spanctx))
 	}
-	span, ctx := tracer.StartSpanFromContext(r.Context(), "http.request", opts...)
-	rw, ddrw := wrapResponseWriter(w)
-	defer func() {
-		if ddrw.status == 0 {
-			span.SetTag(ext.HTTPCode, "200")
-		} else {
-			span.SetTag(ext.HTTPCode, strconv.Itoa(ddrw.status))
-		}
-		if ddrw.status >= 500 && ddrw.status < 600 {
-			span.SetTag(ext.Error, fmt.Errorf("%d: %s", ddrw.status, http.StatusText(ddrw.status)))
-		}
-		span.Finish(cfg.FinishOpts...)
-	}()
+	return tracer.StartSpanFromContext(r.Context(), "http.request", opts...)
+}
 
-	if appsec.Enabled() {
-		h = httpsec.WrapHandler(h, span, cfg.RouteParams)
+// FinishRequestSpan finishes the given HTTP request span with the standard list of HTTP request span tags.
+// Any further span finish option can be added with opts.
+func FinishRequestSpan(s tracer.Span, status int, opts ...tracer.FinishOption) {
+	var statusStr string
+	if status == 0 {
+		statusStr = "200"
+	} else {
+		statusStr = strconv.Itoa(status)
 	}
-	h.ServeHTTP(rw, r.WithContext(ctx))
+	s.SetTag(ext.HTTPCode, statusStr)
+	if status >= 500 && status < 600 {
+		s.SetTag(ext.Error, fmt.Errorf("%d: %s", status, http.StatusText(status)))
+	}
+	s.Finish(opts...)
 }
 
 // responseWriter is a small wrapper around an http response writer that will

contrib/urfave/negroni/negroni.go

@@ -10,11 +10,10 @@ import (
 	"fmt"
 	"math"
 	"net/http"
-	"strconv"
 
 	"github.com/urfave/negroni"
 
-	"gopkg.in/DataDog/dd-trace-go.v1/ddtrace"
+	httptrace "gopkg.in/DataDog/dd-trace-go.v1/contrib/net/http"
 	"gopkg.in/DataDog/dd-trace-go.v1/ddtrace/ext"
 	"gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer"
 	"gopkg.in/DataDog/dd-trace-go.v1/internal/log"
@@ -26,38 +25,28 @@ type DatadogMiddleware struct {
 }
 
 func (m *DatadogMiddleware) ServeHTTP(w http.ResponseWriter, r *http.Request, next http.HandlerFunc) {
-	opts := []ddtrace.StartSpanOption{
-		tracer.SpanType(ext.SpanTypeWeb),
-		tracer.ServiceName(m.cfg.serviceName),
-		tracer.Tag(ext.HTTPMethod, r.Method),
-		tracer.Tag(ext.HTTPURL, r.URL.Path),
-		tracer.Tag(ext.ResourceName, m.cfg.resourceNamer(r)),
-		tracer.Measured(),
-	}
+	opts := append(m.cfg.spanOpts, tracer.Measured())
 	if !math.IsNaN(m.cfg.analyticsRate) {
 		opts = append(opts, tracer.Tag(ext.EventSampleRate, m.cfg.analyticsRate))
 	}
-	if spanctx, err := tracer.Extract(tracer.HTTPHeadersCarrier(r.Header)); err == nil {
-		opts = append(opts, tracer.ChildOf(spanctx))
-	}
-	opts = append(opts, m.cfg.spanOpts...)
-	span, ctx := tracer.StartSpanFromContext(r.Context(), "http.request", opts...)
-	defer span.Finish()
-
-	r = r.WithContext(ctx)
-
-	next(w, r)
-
-	// check if the responseWriter is of type negroni.ResponseWriter
-	responseWriter, ok := w.(negroni.ResponseWriter)
-	if ok {
-		status := responseWriter.Status()
-		span.SetTag(ext.HTTPCode, strconv.Itoa(status))
-		if m.cfg.isStatusError(status) {
-			// mark 5xx server error
-			span.SetTag(ext.Error, fmt.Errorf("%d: %s", status, http.StatusText(status)))
+	span, ctx := httptrace.StartRequestSpan(r, m.cfg.serviceName, m.cfg.resourceNamer(r), false, opts...)
+	defer func() {
+		// check if the responseWriter is of type negroni.ResponseWriter
+		var (
+			status int
+			opts   []tracer.FinishOption
+		)
+		responseWriter, ok := w.(negroni.ResponseWriter)
+		if ok {
+			status = responseWriter.Status()
+			if m.cfg.isStatusError(status) {
+				opts = []tracer.FinishOption{tracer.WithError(fmt.Errorf("%d: %s", status, http.StatusText(status)))}
+			}
 		}
-	}
+		httptrace.FinishRequestSpan(span, status, opts...)
+	}()
+
+	next(w, r.WithContext(ctx))
 }
 
 // Middleware create the negroni middleware that will trace incoming requests

ddtrace/ext/tags.go

@@ -33,6 +33,9 @@ const (
 	// HTTPURL sets the HTTP URL for a span.
 	HTTPURL = "http.url"
 
+	// HTTPUserAgent sets the HTTP user agent tag.
+	HTTPUserAgent = "http.useragent"
+
 	// SpanName is a pseudo-key for setting a span's operation name by means of
 	// a tag. It is mostly here to facilitate vendor-agnostic frameworks like Opentracing
 	// and OpenCensus.