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.

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

contrib/net/http: add dynamic resource naming #1142

Merged
merged 17 commits into from Apr 28, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
15 changes: 14 additions & 1 deletion contrib/net/http/http.go
Expand Up @@ -57,13 +57,26 @@ func (mux *ServeMux) ServeHTTP(w http.ResponseWriter, r *http.Request) {

// WrapHandler wraps an http.Handler with tracing using the given service and resource.
func WrapHandler(h http.Handler, service, resource string, opts ...Option) http.Handler {
opts = append(opts, WithStaticResourceNamer(resource))
return WrapHandlerWithDynamicResourceName(h, service, opts...)
}

// WrapHandlerWithDynamicResourceName wraps an http.Handler with tracing using the given service and generates a
// resource name from the inbound request. It probably makes more API sense for this to supplant WrapHandler but
// for backcompat we leave that one, with its static naming, alone. Unless maybe we were to allow resource to be nil?
func WrapHandlerWithDynamicResourceName(h http.Handler, service string, opts ...Option) http.Handler {
gbbr marked this conversation as resolved.
Show resolved Hide resolved
cfg := new(config)
defaults(cfg)

// for this case, apply a default of a dynamic namer before we (maybe) overwrite it with the Option supplied.
WithDynamicResourceNamer()(cfg)

for _, fn := range opts {
fn(cfg)
}
log.Debug("contrib/net/http: Wrapping Handler: Service: %s, Resource: %s, %#v", service, resource, cfg)
log.Debug("contrib/net/http: Wrapping Handler: Service: %s, Resource: (Dynamic), %#v", service, cfg)
return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
resource := cfg.resourceNamer(req)
httputil.TraceAndServe(h, &httputil.TraceConfig{
ResponseWriter: w,
Request: req,
Expand Down
26 changes: 26 additions & 0 deletions contrib/net/http/option.go
Expand Up @@ -22,6 +22,7 @@ type config struct {
spanOpts []ddtrace.StartSpanOption
finishOpts []ddtrace.FinishOption
ignoreRequest func(*http.Request) bool
resourceNamer func(*http.Request) string
}

// MuxOption has been deprecated in favor of Option.
Expand All @@ -45,6 +46,9 @@ func defaults(cfg *config) {
cfg.spanOpts = append(cfg.spanOpts, tracer.Tag(ext.EventSampleRate, cfg.analyticsRate))
}
cfg.ignoreRequest = func(_ *http.Request) bool { return false }

// not sure this is the right thing for a default, but it's only used in the dynamic name case
cfg.resourceNamer = func(_ *http.Request) string { return cfg.serviceName }
}

// WithIgnoreRequest holds the function to use for determining if the
Expand Down Expand Up @@ -95,6 +99,28 @@ func WithSpanOptions(opts ...ddtrace.StartSpanOption) Option {
}
}

// WithDynamicResourceNamer automatically populates the name of a resource from the Method and URL.Path of the
// inbound request.
func WithDynamicResourceNamer() Option {
return WithResourceNamer(func(req *http.Request) string {
return req.Method + " " + req.URL.Path
})
}

// WithStaticResourceNamer populates the name of a resource with a static string.
func WithStaticResourceNamer(name string) Option {
return WithResourceNamer(func(req *http.Request) string {
return name
})
}
gbbr marked this conversation as resolved.
Show resolved Hide resolved

// WithResourceNamer populates the name of a resource based on a custom function.
func WithResourceNamer(namer func(req *http.Request) string) Option {
return func(cfg *config) {
cfg.resourceNamer = namer
}
}

// NoDebugStack prevents stack traces from being attached to spans finishing
// with an error. This is useful in situations where errors are frequent and
// performance is critical.
Expand Down