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/gorilla/mux: added optional middlewares on mux tracer router #825

Merged
merged 10 commits into from
Mar 16, 2021

Conversation

dianashevchenko
Copy link
Contributor

Added middlewares which can be applied to recorder request information as span tags. WithHeaderTags will add request headers(except x-datadog-* prefixed) as span tags and WithQueryParameters will add request query parameters as a single span tag.

Fixes #480

contrib/gorilla/mux/option.go Outdated Show resolved Hide resolved
contrib/gorilla/mux/mux_test.go Outdated Show resolved Hide resolved
contrib/gorilla/mux/mux_test.go Outdated Show resolved Hide resolved
contrib/gorilla/mux/option.go Outdated Show resolved Hide resolved
contrib/gorilla/mux/option.go Outdated Show resolved Hide resolved
contrib/gorilla/mux/option.go Outdated Show resolved Hide resolved
contrib/gorilla/mux/option.go Outdated Show resolved Hide resolved
contrib/gorilla/mux/option.go Outdated Show resolved Hide resolved
contrib/gorilla/mux/option.go Outdated Show resolved Hide resolved
@dianashevchenko dianashevchenko added this to the 1.29.0 milestone Feb 23, 2021
contrib/gorilla/mux/option.go Outdated Show resolved Hide resolved
contrib/gorilla/mux/option.go Outdated Show resolved Hide resolved
@gbbr gbbr modified the milestones: 1.29.0, 1.30.0 Feb 26, 2021
Copy link
Contributor

@gbbr gbbr left a comment

Choose a reason for hiding this comment

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

Thanks for taking the time to do this. httputil is much nicer now :)

contrib/gorilla/mux/mux_test.go Show resolved Hide resolved
contrib/internal/httputil/trace.go Outdated Show resolved Hide resolved
contrib/gorilla/mux/option.go Show resolved Hide resolved
Copy link
Contributor

@gbbr gbbr left a comment

Choose a reason for hiding this comment

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

Really well done. Almost there

contrib/gorilla/mux/option.go Outdated Show resolved Hide resolved
contrib/internal/httputil/trace.go Outdated Show resolved Hide resolved
contrib/internal/httputil/trace.go Outdated Show resolved Hide resolved
contrib/gorilla/mux/option.go Outdated Show resolved Hide resolved
gbbr
gbbr previously approved these changes Mar 5, 2021
Copy link
Contributor

@gbbr gbbr left a comment

Choose a reason for hiding this comment

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

Really nice work Diana. Thank you! Let's wait with the merge until we release and unfreeze.

contrib/internal/httputil/trace_test.go Show resolved Hide resolved
@gbbr
Copy link
Contributor

gbbr commented Mar 10, 2021

@dianashevchenko can you please rebase this on v1 so we can get CI to pass and merge?

Added middlewares which can be applied to recorder request information as span tags. WithHeaderTags will add request headers(except x-datadog-* prefixed) as span tags and WithQueryParameters will add request query parameters as a single span tag.

Fixes #480
Updated docs for the implementation. Renamed struct fields and methods to reflect the functionality better
Refactored code of the TraceAndServe function to use one config parameter, and changed its usage. Added QueryParamTags field that would enable appending query params to the http.url tag. It is currently available only for the gorilla/mux package
Removed sprintf and added doc comment to TraceConfig
}
}

func headerTagsFromRequest(req *http.Request) ddtrace.StartSpanOption {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please move this into gorilla/mux/mux.go where it is used. It's out of place here.

Copy link
Contributor

@gbbr gbbr left a comment

Choose a reason for hiding this comment

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

Thanks!

@gbbr gbbr merged commit 9b576cc into v1 Mar 16, 2021
@gbbr gbbr deleted the shevchenko/add-mux-middleware branch March 16, 2021 09:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

contrib/gorilla/mux: add middleware to include HTTP headers and query string
2 participants