Skip to content

Commit

Permalink
contrib/*: fix multiple spanOpts potential append() data race
Browse files Browse the repository at this point in the history
Signed-off-by: Eliott Bouhana <eliott.bouhana@datadoghq.com>
  • Loading branch information
eliottness committed Dec 7, 2023
1 parent 3dc6559 commit 8f24d7b
Show file tree
Hide file tree
Showing 9 changed files with 34 additions and 11 deletions.
5 changes: 4 additions & 1 deletion contrib/gin-gonic/gin/gintrace.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,10 @@ func Middleware(service string, opts ...Option) gin.HandlerFunc {
if cfg.ignoreRequest(c) {
return
}
opts := append(spanOpts, tracer.ResourceName(cfg.resourceNamer(c)))

opts := make([]tracer.StartSpanOption, len(spanOpts), len(spanOpts)+4)
copy(opts, spanOpts)
opts = append(opts, tracer.ResourceName(cfg.resourceNamer(c)))
if !math.IsNaN(cfg.analyticsRate) {
opts = append(opts, tracer.Tag(ext.EventSampleRate, cfg.analyticsRate))
}
Expand Down
6 changes: 5 additions & 1 deletion contrib/go-chi/chi.v5/chi.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ package chi // import "gopkg.in/DataDog/dd-trace-go.v1/contrib/go-chi/chi.v5"

import (
"fmt"
"gopkg.in/DataDog/dd-trace-go.v1/ddtrace"
"math"
"net/http"

Expand Down Expand Up @@ -46,10 +47,13 @@ func Middleware(opts ...Option) func(next http.Handler) http.Handler {
next.ServeHTTP(w, r)
return
}
opts := spanOpts
opts := make([]ddtrace.StartSpanOption, len(spanOpts), len(spanOpts)+2)
copy(opts, cfg.spanOpts)

if !math.IsNaN(cfg.analyticsRate) {
opts = append(opts, tracer.Tag(ext.EventSampleRate, cfg.analyticsRate))
}

opts = append(opts, httptrace.HeaderTagsFromRequest(r, cfg.headerTags))
span, ctx := httptrace.StartRequestSpan(r, opts...)
ww := middleware.NewWrapResponseWriter(w, r.ProtoMajor)
Expand Down
6 changes: 5 additions & 1 deletion contrib/go-chi/chi/chi.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ package chi // import "gopkg.in/DataDog/dd-trace-go.v1/contrib/go-chi/chi"

import (
"fmt"
"gopkg.in/DataDog/dd-trace-go.v1/ddtrace"
"math"
"net/http"

Expand Down Expand Up @@ -46,10 +47,13 @@ func Middleware(opts ...Option) func(next http.Handler) http.Handler {
next.ServeHTTP(w, r)
return
}
opts := spanOpts
opts := make([]ddtrace.StartSpanOption, len(spanOpts), len(spanOpts)+2)
copy(opts, cfg.spanOpts)

if !math.IsNaN(cfg.analyticsRate) {
opts = append(opts, tracer.Tag(ext.EventSampleRate, cfg.analyticsRate))
}

opts = append(opts, httptrace.HeaderTagsFromRequest(r, cfg.headerTags))
span, ctx := httptrace.StartRequestSpan(r, opts...)
ww := middleware.NewWrapResponseWriter(w, r.ProtoMajor)
Expand Down
2 changes: 1 addition & 1 deletion contrib/google.golang.org/grpc/stats_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ import (
"context"
"net"

"google.golang.org/grpc/stats"
"gopkg.in/DataDog/dd-trace-go.v1/ddtrace/ext"
"gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer"
"google.golang.org/grpc/stats"
)

// NewClientStatsHandler returns a gRPC client stats.Handler to trace RPC calls.
Expand Down
7 changes: 5 additions & 2 deletions contrib/julienschmidt/httprouter/httprouter.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
package httprouter // import "gopkg.in/DataDog/dd-trace-go.v1/contrib/julienschmidt/httprouter"

import (
"gopkg.in/DataDog/dd-trace-go.v1/ddtrace"
"math"
"net/http"
"strings"
Expand Down Expand Up @@ -61,12 +62,14 @@ func (r *Router) ServeHTTP(w http.ResponseWriter, req *http.Request) {
route = strings.Replace(route, param.Value, ":"+param.Key, 1)
}
resource := req.Method + " " + route
spanOpts := append(r.config.spanOpts, httptraceinternal.HeaderTagsFromRequest(req, r.config.headerTags))
opts := make([]ddtrace.StartSpanOption, len(r.config.spanOpts), len(r.config.spanOpts)+1)
copy(opts, r.config.spanOpts)
opts = append(opts, httptraceinternal.HeaderTagsFromRequest(req, r.config.headerTags))

httptrace.TraceAndServe(r.Router, w, req, &httptrace.ServeConfig{
Service: r.config.serviceName,
Resource: resource,
SpanOpts: spanOpts,
SpanOpts: opts,
Route: route,
})
}
6 changes: 4 additions & 2 deletions contrib/k8s.io/client-go/kubernetes/kubernetes.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,9 @@ func WrapRoundTripper(rt http.RoundTripper) http.RoundTripper {
}

func wrapRoundTripperWithOptions(rt http.RoundTripper, opts ...httptrace.RoundTripperOption) http.RoundTripper {
opts = append(opts, httptrace.WithBefore(func(req *http.Request, span ddtrace.Span) {
newOpts := make([]httptrace.RoundTripperOption, len(opts), len(opts)+1)
copy(newOpts, opts)
newOpts = append(newOpts, httptrace.WithBefore(func(req *http.Request, span ddtrace.Span) {
span.SetTag(ext.ResourceName, RequestToResource(req.Method, req.URL.Path))
span.SetTag(ext.Component, componentName)
span.SetTag(ext.SpanKind, ext.SpanKindClient)
Expand All @@ -62,7 +64,7 @@ func wrapRoundTripperWithOptions(rt http.RoundTripper, opts ...httptrace.RoundTr
span.SetTag("kubernetes.audit_id", kubeAuditID)
}))
log.Debug("contrib/k8s.io/client-go/kubernetes: Wrapping RoundTripper.")
return httptrace.WrapRoundTripper(rt, opts...)
return httptrace.WrapRoundTripper(rt, newOpts...)
}

// RequestToResource parses a Kubernetes request and extracts a resource name from it.
Expand Down
4 changes: 3 additions & 1 deletion contrib/labstack/echo.v4/echotrace.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,9 @@ func Middleware(opts ...Option) echo.MiddlewareFunc {
request := c.Request()
route := c.Path()
resource := request.Method + " " + route
opts := append(spanOpts, tracer.ResourceName(resource), tracer.Tag(ext.HTTPRoute, route))
opts := make([]ddtrace.StartSpanOption, len(spanOpts), len(spanOpts)+3)
copy(opts, spanOpts)
opts = append(spanOpts, tracer.ResourceName(resource), tracer.Tag(ext.HTTPRoute, route))

if !math.IsNaN(cfg.analyticsRate) {
opts = append(opts, tracer.Tag(ext.EventSampleRate, cfg.analyticsRate))
Expand Down
4 changes: 3 additions & 1 deletion contrib/labstack/echo/echotrace.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,9 @@ func Middleware(opts ...Option) echo.MiddlewareFunc {
return func(c echo.Context) error {
request := c.Request()
resource := request.Method + " " + c.Path()
opts := append(spanOpts, tracer.ResourceName(resource))
opts := make([]ddtrace.StartSpanOption, len(spanOpts), len(spanOpts)+3)
copy(opts, spanOpts)
opts = append(spanOpts, tracer.ResourceName(resource))

if !math.IsNaN(cfg.analyticsRate) {
opts = append(opts, tracer.Tag(ext.EventSampleRate, cfg.analyticsRate))
Expand Down
5 changes: 4 additions & 1 deletion contrib/urfave/negroni/negroni.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ package negroni

import (
"fmt"
"gopkg.in/DataDog/dd-trace-go.v1/ddtrace"
"math"
"net/http"

Expand All @@ -33,7 +34,9 @@ type DatadogMiddleware struct {
}

func (m *DatadogMiddleware) ServeHTTP(w http.ResponseWriter, r *http.Request, next http.HandlerFunc) {
opts := append(
opts := make([]ddtrace.StartSpanOption, len(m.cfg.spanOpts), len(m.cfg.spanOpts)+2)
copy(opts, m.cfg.spanOpts)
opts = append(
m.cfg.spanOpts,
tracer.ServiceName(m.cfg.serviceName),
tracer.ResourceName(m.cfg.resourceNamer(r)),
Expand Down

0 comments on commit 8f24d7b

Please sign in to comment.