-
Notifications
You must be signed in to change notification settings - Fork 81
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
Fix missing spans in gorillamux instrumentation #86
Fix missing spans in gorillamux instrumentation #86
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exciting!
Just a note, I'd been holding off on any further changes here to help get #92 across the line first with minimal merge conflicts there. Some of these changes are in #82 now so I'll wait on that one as well and then there may be nothing left on this one. If anyone thinks this approach should change let me know. |
This is ready for re-review. For reference, running "scope": {
"name": "github.com/gorilla/mux"
},
...
"name": "/users/foo",
"parentSpanId": "",
"spanId": "a965bd2e4dec00c4",
"status": {},
"traceId": "5a5dc1cc0de7798f1d349248975d35db"
...
"scope": {
"name": "net/http"
},
...
"name": "/users/foo",
"parentSpanId": "",
"spanId": "ad51aa0cebcf8d6b",
"status": {},
"traceId": "a4a1761ce09da7ca6cc54d7d4d25611b" |
Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
Co-authored-by: Mike Goldsmith <goldsmith.mike@gmail.com>
Which problem is this PR solving?
Short description of the changes
The first commit updates the expected output of a trace for nethttp instrumentation, removing the empty second span. The next set of commits adjusts the http server instrumentation, ultimately just adjusting one of the changes in #34 that used an array of two function names forServeHttp
and keeping only the newer"net/http.HandlerFunc.ServeHTTP"
.I'm not really sure what the trace for gorillamux should look like; the same info exists for two spans, but one is named for net/http and is for gorillamux. This doesn't address that. The trace was already emitting two spans, with the gorillamux span being empty - and this PR populates the gorillamux span.
Note: This has been updated as several of the changes made were ported into #82 . Since then, the gorilla/mux spans were empty. This PR still has the effect of populating the gorilla/mux spans.