Skip to content
This repository has been archived by the owner on Oct 3, 2023. It is now read-only.

instrumentation-http: Parse URL instead of relying on pathname #169

Merged
merged 5 commits into from
Nov 16, 2018

Conversation

whs
Copy link
Contributor

@whs whs commented Oct 30, 2018

instrumentation-http rely on options.pathname for outgoing request to name the span. However, several libraries are passing options directly to http.request which save it as is.

Affected libraries include:

To reproduce, simply setup a tracer and fire an outgoing request with request. The trace will have the name of GET undefined.

This PR ensure that we parse every option for pathname instead of relying that it exists.

Copy link

@OsvaldoRosado OsvaldoRosado left a comment

Choose a reason for hiding this comment

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

Marking as request changes to remove the "review required". Looks good to me other than the comment I left to ensure compatibility with existing options.pathname when missing options.path.

@whs
Copy link
Contributor Author

whs commented Nov 16, 2018

Fixed, please check

@isaikevych isaikevych merged commit d4b3d80 into census-instrumentation:master Nov 16, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants