Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

contrib: fix span start option races #2418

Merged
merged 23 commits into from
Dec 12, 2023
Merged

contrib: fix span start option races #2418

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
8f24d7b
contrib/*: fix multiple spanOpts potential append() data race
eliottness Dec 7, 2023
2ef8ddc
fix start span option races
Dec 7, 2023
5e8ff05
fix echo span tag ordering
Dec 7, 2023
1b1eba1
fix chi options
Dec 7, 2023
260f6d8
refactor to make copies more explicit and add helper method
katiehockman Dec 8, 2023
9778ffe
move OptionsCopy to a new contrib-wide internal package
katiehockman Dec 8, 2023
f24ebd3
add a unit test for OptionsCopy
katiehockman Dec 8, 2023
694bf1f
rename options.OptionsCopy to options.Copy to avoid repetitive naming
katiehockman Dec 8, 2023
4fe86a7
fix additional races in other packages
katiehockman Dec 8, 2023
346aa7c
gofmt on roundtripper.go
katiehockman Dec 8, 2023
64fd168
clarify godoc for options.Copy
katiehockman Dec 8, 2023
5bc5a22
small cleanups to avoid unnecessary changes
katiehockman Dec 8, 2023
bcb9b19
revert changes to fiber.go and roundtripper.go
katiehockman Dec 8, 2023
ddf67cd
avoid opts change side effects in goji.go and httptreemux.go
katiehockman Dec 8, 2023
7b0ae77
fix httptreemux.go based on https://go.dev/play/p/bcPus1ofHPd
katiehockman Dec 8, 2023
0b9c2db
remove all changes that aren't strictly necessary to fix the race
katiehockman Dec 11, 2023
bb7a30d
remove unecessary changes in httptreemux.go
katiehockman Dec 11, 2023
4b0e472
fix TraceAndServe in contrib/net/http
katiehockman Dec 11, 2023
492981b
Merge branch 'main' into eliott.bouhana/fix-span-opts-race
katiehockman Dec 11, 2023
01dd225
allocate enough space in the destination slice to make the fully copy
katiehockman Dec 11, 2023
1ab2a4c
Merge branch 'eliott.bouhana/fix-span-opts-race' of github.com:DataDo…
katiehockman Dec 11, 2023
3c089f2
remove unnecessary changes in goji.go
katiehockman Dec 12, 2023
d9b7e10
Merge branch 'main' into eliott.bouhana/fix-span-opts-race
katiehockman Dec 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion contrib/database/sql/conn.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"math"
"time"

"gopkg.in/DataDog/dd-trace-go.v1/contrib/internal/options"
"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"
Expand Down Expand Up @@ -285,7 +286,8 @@ func (tp *traceParams) tryTrace(ctx context.Context, qtype QueryType, query stri
return
}
dbSystem, _ := normalizeDBSystem(tp.driverName)
opts := append(spanOpts,
opts := options.Copy(spanOpts...)
opts = append(opts,
tracer.ServiceName(tp.cfg.serviceName),
tracer.SpanType(ext.SpanTypeSQL),
tracer.StartTime(startTime),
Expand Down
6 changes: 4 additions & 2 deletions contrib/database/sql/sql.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,8 +236,10 @@ func Open(driverName, dataSourceName string, opts ...Option) (*sql.DB, error) {
return nil, err
}
// since we're not using the dsnConnector, we need to register the dsn manually in the config
opts = append(opts, WithDSN(dataSourceName))
return OpenDB(connector, opts...), nil
knusbaum marked this conversation as resolved.
Show resolved Hide resolved
optsCopy := make([]Option, len(opts))
copy(optsCopy, opts) // avoid modifying the provided opts, so make a copy instead, and use this
optsCopy = append(optsCopy, WithDSN(dataSourceName))
return OpenDB(connector, optsCopy...), nil
}
return OpenDB(&dsnConnector{dsn: dataSourceName, driver: d}, opts...), nil
}
Expand Down
4 changes: 3 additions & 1 deletion contrib/gin-gonic/gin/gintrace.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"math"

"gopkg.in/DataDog/dd-trace-go.v1/contrib/internal/httptrace"
"gopkg.in/DataDog/dd-trace-go.v1/contrib/internal/options"
"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/appsec"
Expand Down Expand Up @@ -44,7 +45,8 @@ func Middleware(service string, opts ...Option) gin.HandlerFunc {
if cfg.ignoreRequest(c) {
return
}
opts := append(spanOpts, tracer.ResourceName(cfg.resourceNamer(c)))
opts := options.Copy(spanOpts...) // opts must be a copy of cfg.spanOpts, locally scoped, to avoid races.
opts = append(opts, tracer.ResourceName(cfg.resourceNamer(c)))
if !math.IsNaN(cfg.analyticsRate) {
opts = append(opts, tracer.Tag(ext.EventSampleRate, cfg.analyticsRate))
}
Expand Down
3 changes: 2 additions & 1 deletion contrib/go-chi/chi.v5/chi.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"net/http"

"gopkg.in/DataDog/dd-trace-go.v1/contrib/internal/httptrace"
"gopkg.in/DataDog/dd-trace-go.v1/contrib/internal/options"
"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/appsec"
Expand Down Expand Up @@ -46,7 +47,7 @@ func Middleware(opts ...Option) func(next http.Handler) http.Handler {
next.ServeHTTP(w, r)
return
}
opts := spanOpts
katiehockman marked this conversation as resolved.
Show resolved Hide resolved
opts := options.Copy(spanOpts...) // opts must be a copy of spanOpts, locally scoped, to avoid races.
if !math.IsNaN(cfg.analyticsRate) {
opts = append(opts, tracer.Tag(ext.EventSampleRate, cfg.analyticsRate))
}
Expand Down
54 changes: 54 additions & 0 deletions contrib/go-chi/chi.v5/chi_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"net/http/httptest"
"strconv"
"strings"
"sync"
"testing"

pappsec "gopkg.in/DataDog/dd-trace-go.v1/appsec"
Expand Down Expand Up @@ -609,3 +610,56 @@ func TestUnknownResourceName(t *testing.T) {
require.Equal(t, "service-name", spans[0].Tag(ext.ServiceName))
require.Equal(t, "GET unknown", spans[0].Tag(ext.ResourceName))
}

// Highly concurrent test running many goroutines to try to uncover concurrency
// issues such as deadlocks, data races, etc.
func TestConcurrency(t *testing.T) {
mt := mocktracer.Start()
defer mt.Stop()

expectedCap := 10
opts := make([]Option, 0, expectedCap)
opts = append(opts, []Option{
WithServiceName("foobar"),
WithSpanOptions(tracer.Tag("tag1", "value1")),
}...)
expectedLen := 2

router := chi.NewRouter()
require.Len(t, opts, expectedLen)
require.True(t, cap(opts) == expectedCap)

router.Use(Middleware(opts...))
router.Get("/user/{id}", func(w http.ResponseWriter, r *http.Request) {
_, ok := tracer.SpanFromContext(r.Context())
require.True(t, ok)
})

// Create a bunch of goroutines that will all try to use the same router using our middleware
nbReqGoroutines := 1000
var startBarrier, finishBarrier sync.WaitGroup
startBarrier.Add(1)
finishBarrier.Add(nbReqGoroutines)

for n := 0; n < nbReqGoroutines; n++ {
go func() {
startBarrier.Wait()
defer finishBarrier.Done()

for i := 0; i < 100; i++ {
r := httptest.NewRequest("GET", "/user/123", nil)
w := httptest.NewRecorder()
router.ServeHTTP(w, r)
}
}()
}

startBarrier.Done()
finishBarrier.Wait()

// Side effects on opts is not the main purpose of this test, but it's worth checking just in case.
require.Len(t, opts, expectedLen)
require.True(t, cap(opts) == expectedCap)
// All the others config data are internal to the closures in Middleware and cannot be tested.
// Running this test with -race is the best chance to find a concurrency issue.
}
3 changes: 2 additions & 1 deletion contrib/go-chi/chi/chi.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"net/http"

"gopkg.in/DataDog/dd-trace-go.v1/contrib/internal/httptrace"
"gopkg.in/DataDog/dd-trace-go.v1/contrib/internal/options"
"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/appsec"
Expand Down Expand Up @@ -46,7 +47,7 @@ func Middleware(opts ...Option) func(next http.Handler) http.Handler {
next.ServeHTTP(w, r)
return
}
opts := spanOpts
opts := options.Copy(spanOpts...) // opts must be a copy of spanOpts, locally scoped, to avoid races.
if !math.IsNaN(cfg.analyticsRate) {
opts = append(opts, tracer.Tag(ext.EventSampleRate, cfg.analyticsRate))
}
Expand Down
4 changes: 2 additions & 2 deletions contrib/google.golang.org/grpc.v12/grpc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"strings"

"gopkg.in/DataDog/dd-trace-go.v1/contrib/google.golang.org/internal/grpcutil"
"gopkg.in/DataDog/dd-trace-go.v1/contrib/internal/options"
"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"
Expand Down Expand Up @@ -72,8 +73,7 @@ func startServerSpanFromContext(ctx context.Context, method string, cfg *interce
}
// copy opts in case the caller reuses the slice in parallel
// we will add the items in extraOpts
optsLocal := make([]tracer.StartSpanOption, len(cfg.spanOpts), len(cfg.spanOpts)+len(extraOpts))
copy(optsLocal, cfg.spanOpts)
optsLocal := options.Copy(cfg.spanOpts...)
knusbaum marked this conversation as resolved.
Show resolved Hide resolved
optsLocal = append(optsLocal, extraOpts...)
md, _ := metadata.FromContext(ctx) // nil is ok
if sctx, err := tracer.Extract(grpcutil.MDCarrier(md)); err == nil {
Expand Down
2 changes: 1 addition & 1 deletion contrib/google.golang.org/grpc/stats_client.go
View file
Open in desktop
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
9 changes: 4 additions & 5 deletions contrib/gorilla/mux/mux.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ import (
"net/http"

httptraceinternal "gopkg.in/DataDog/dd-trace-go.v1/contrib/internal/httptrace"
"gopkg.in/DataDog/dd-trace-go.v1/contrib/internal/options"
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"
"gopkg.in/DataDog/dd-trace-go.v1/internal/log"
Expand Down Expand Up @@ -96,18 +96,17 @@ func (r *Router) ServeHTTP(w http.ResponseWriter, req *http.Request) {
return
}
var (
match mux.RouteMatch
spanopts []ddtrace.StartSpanOption
route string
match mux.RouteMatch
route string
)
spanopts := options.Copy(r.config.spanOpts...)
// get the resource associated to this request
if r.Match(req, &match) && match.Route != nil {
if h, err := match.Route.GetHostTemplate(); err == nil {
spanopts = append(spanopts, tracer.Tag("mux.host", h))
}
route, _ = match.Route.GetPathTemplate()
}
spanopts = append(spanopts, r.config.spanOpts...)
katiehockman marked this conversation as resolved.
Show resolved Hide resolved
spanopts = append(spanopts, httptraceinternal.HeaderTagsFromRequest(req, r.config.headerTags))
resource := r.config.resourceNamer(r, req)
httptrace.TraceAndServe(r.Router, w, req, &httptrace.ServeConfig{
Expand Down
17 changes: 17 additions & 0 deletions contrib/internal/options/options.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Unless explicitly stated otherwise all files in this repository are licensed
// under the Apache License Version 2.0.
// This product includes software developed at Datadog (https://www.datadoghq.com/).
// Copyright 2023 Datadog, Inc.

package options
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I was reading some contribs changed in this PR, I think we could go further to help thanks to some ImmutableOptions wrapper type.

For example avoiding:

func Middleware(opts ...Options) Handler {
    opts = options.Copy(opts) // problem avoided here
    opts = append(opts, foo)

    return func(w http.ResponseWriter, r *http.Request) {
        opts = append(opts, headersTags(r)) // problem not avoided here
    }
}

And doing instead:

func Middleware(opts ...Options) Handler {
    localOpts = options.Copy(opts) // problem avoided here
    localOpts = append(localOpts, foo)
    opts := options.ImmutableOptions(localOpts)
    return func(w http.ResponseWriter, r *http.Request) {
        opts = append(opts, headersTags(r)) // problem not avoided here
    }
}

Or having a full wrapper type Options with an Append method so it can do more safety checks?

My point is: how can we further improve this API to avoid future mistakes again and maybe add this ability to block any further uses of append once we know we are done and anything coming later is in a request handler, from concurrent goroutines.

cc @knusbaum

Copy link
Contributor

@knusbaum knusbaum Dec 11, 2023 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Abstracting the mutations is potentially a good idea, but we would have to be careful to do it in an efficient way so that every Append doesn't cause a copy.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should consider it, but let's do so in a follow-up PR. I'd like to keep this PR as minimal as possible given the already large blast radius of changes.


import "gopkg.in/DataDog/dd-trace-go.v1/ddtrace"

// Copy should be used any time existing options are copied into
// a new locally scoped set of options. This is to avoid data races and
// accidental side effects.
func Copy(opts ...ddtrace.StartSpanOption) []ddtrace.StartSpanOption {
dup := make([]ddtrace.StartSpanOption, len(opts))
copy(dup, opts)
return dup
}
37 changes: 37 additions & 0 deletions contrib/internal/options/options_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// Unless explicitly stated otherwise all files in this repository are licensed
// under the Apache License Version 2.0.
// This product includes software developed at Datadog (https://www.datadoghq.com/).
// Copyright 2023 Datadog, Inc.

package options

import (
"testing"

"github.com/stretchr/testify/assert"
"gopkg.in/DataDog/dd-trace-go.v1/ddtrace"
"gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer"
)

func TestStringSliceModify(t *testing.T) {
t.Run("modify-original", func(t *testing.T) {
opts := []ddtrace.StartSpanOption{tracer.Tag("mytag", "myvalue")}
optsCopy := Copy(opts...)
opts[0] = tracer.ResourceName("somethingelse")
cfg := new(ddtrace.StartSpanConfig)
for _, fn := range optsCopy {
fn(cfg)
}
assert.Equal(t, "myvalue", cfg.Tags["mytag"])
})
t.Run("modify-copy", func(t *testing.T) {
opts := []ddtrace.StartSpanOption{tracer.Tag("mytag", "myvalue")}
optsCopy := Copy(opts...)
optsCopy[0] = tracer.ResourceName("somethingelse")
cfg := new(ddtrace.StartSpanConfig)
for _, fn := range opts {
fn(cfg)
}
assert.Equal(t, "myvalue", cfg.Tags["mytag"])
})
}
4 changes: 3 additions & 1 deletion contrib/julienschmidt/httprouter/httprouter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"strings"

httptraceinternal "gopkg.in/DataDog/dd-trace-go.v1/contrib/internal/httptrace"
"gopkg.in/DataDog/dd-trace-go.v1/contrib/internal/options"
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"
Expand Down Expand Up @@ -61,7 +62,8 @@ 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))
spanOpts := options.Copy(r.config.spanOpts...) // spanOpts must be a copy of r.config.spanOpts, locally scoped, to avoid races.
spanOpts = append(spanOpts, httptraceinternal.HeaderTagsFromRequest(req, r.config.headerTags))

httptrace.TraceAndServe(r.Router, w, req, &httptrace.ServeConfig{
Service: r.config.serviceName,
Expand Down
8 changes: 5 additions & 3 deletions contrib/k8s.io/client-go/kubernetes/kubernetes.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ const (
)

// WrapRoundTripperFunc creates a new WrapTransport function using the given set of
// RountripperOption. It is useful when desiring to enable Trace Analytics or setting
// RoundTripperOption. It is useful when desiring to enable Trace Analytics or setting
// up a RoundTripperAfterFunc.
func WrapRoundTripperFunc(opts ...httptrace.RoundTripperOption) func(http.RoundTripper) http.RoundTripper {
return func(rt http.RoundTripper) http.RoundTripper {
Expand All @@ -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) {
localOpts := make([]httptrace.RoundTripperOption, len(opts))
copy(localOpts, opts) // make a copy of the opts, to avoid data races and side effects.
localOpts = append(localOpts, httptrace.WithBefore(func(req *http.Request, span ddtrace.Span) {
knusbaum marked this conversation as resolved.
Show resolved Hide resolved
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, localOpts...)
}

// RequestToResource parses a Kubernetes request and extracts a resource name from it.
Expand Down
10 changes: 7 additions & 3 deletions contrib/labstack/echo.v4/echotrace.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"strconv"

"gopkg.in/DataDog/dd-trace-go.v1/contrib/internal/httptrace"
"gopkg.in/DataDog/dd-trace-go.v1/contrib/internal/options"
"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"
Expand Down Expand Up @@ -61,12 +62,15 @@ 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 := options.Copy(spanOpts...) // opts must be a copy of spanOpts, locally scoped, to avoid races.
if !math.IsNaN(cfg.analyticsRate) {
opts = append(opts, tracer.Tag(ext.EventSampleRate, cfg.analyticsRate))
}
opts = append(opts, httptrace.HeaderTagsFromRequest(request, cfg.headerTags))
opts = append(opts,
tracer.ResourceName(resource),
tracer.Tag(ext.HTTPRoute, route),
httptrace.HeaderTagsFromRequest(request, cfg.headerTags))

var finishOpts []tracer.FinishOption
if cfg.noDebugStack {
finishOpts = []tracer.FinishOption{tracer.NoDebugStack()}
Expand Down
11 changes: 7 additions & 4 deletions contrib/labstack/echo/echotrace.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"strconv"

"gopkg.in/DataDog/dd-trace-go.v1/contrib/internal/httptrace"
"gopkg.in/DataDog/dd-trace-go.v1/contrib/internal/options"
"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"
Expand Down Expand Up @@ -50,20 +51,22 @@ 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 := options.Copy(spanOpts...) // opts must be a copy of spanOpts, locally scoped, to avoid races.
if !math.IsNaN(cfg.analyticsRate) {
opts = append(opts, tracer.Tag(ext.EventSampleRate, cfg.analyticsRate))
}
opts = append(opts, httptrace.HeaderTagsFromRequest(request, cfg.headerTags))
opts = append(opts,
tracer.ResourceName(resource),
httptrace.HeaderTagsFromRequest(request, cfg.headerTags))
// TODO: Should this also have an `http.route` tag like the v4 library does?

var finishOpts []tracer.FinishOption
if cfg.noDebugStack {
finishOpts = []tracer.FinishOption{tracer.NoDebugStack()}
}

span, ctx := httptrace.StartRequestSpan(request, opts...)
defer func() {
//httptrace.FinishRequestSpan(span, c.Response().Status, finishOpts...)
span.Finish(finishOpts...)
}()

Expand Down
3 changes: 2 additions & 1 deletion contrib/net/http/trace.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"net/http"

"gopkg.in/DataDog/dd-trace-go.v1/contrib/internal/httptrace"
"gopkg.in/DataDog/dd-trace-go.v1/contrib/internal/options"
"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"
Expand Down Expand Up @@ -53,7 +54,7 @@ func TraceAndServe(h http.Handler, w http.ResponseWriter, r *http.Request, cfg *
if cfg == nil {
cfg = new(ServeConfig)
}
opts := cfg.SpanOpts
opts := options.Copy(cfg.SpanOpts...) // make a copy of cfg.SpanOpts to avoid races
if cfg.Service != "" {
opts = append(opts, tracer.ServiceName(cfg.Service))
}
Expand Down