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

fix http plugin url tag and agent requests being traced #883

Merged
merged 1 commit into from
Mar 5, 2020

Conversation

rochdev
Copy link
Member

@rochdev rochdev commented Mar 5, 2020

What does this PR do?

Fix http plugin url tag and agent requests being traced.

Motivation

A bug was reported that the requests to the trace agent were being traced. After investigating the issue, the root cause was that the URL was not formatted properly by the http plugin. This caused the http.url tag to have an incorrect value, and also caused the filter on the agent endpoint to not match the URL.

The reason the tests were passing is that they were picking up the HTTP server span which had the correct tag value. This has also been fixed by disabling the server instrumentation in the client test.

@rochdev rochdev added bug Something isn't working integrations labels Mar 5, 2020
@rochdev rochdev added this to the 0.18.2 milestone Mar 5, 2020
@rochdev rochdev requested a review from ericmustin March 5, 2020 03:10
@rochdev rochdev requested a review from a team as a code owner March 5, 2020 03:10
Copy link
Contributor

@ericmustin ericmustin left a comment

Choose a reason for hiding this comment

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

👍 , lgtm, when i have a sec we should probably also add a test that verifies that requests to trace-agent are not traced. don't want to block this getting merged/released though

@rochdev
Copy link
Member Author

rochdev commented Mar 5, 2020

Unfortunately there is no easy way right now to test that the agent is not called in a specific test, but good point, eventually it would definitely make sense to test the default blacklist.

@rochdev rochdev merged commit 4713260 into master Mar 5, 2020
@rochdev rochdev deleted the fix-http-url branch March 5, 2020 16:52
@rochdev rochdev modified the milestones: 0.18.2, 0.19.0 Mar 5, 2020
vecerek pushed a commit to vecerek/dd-trace-js that referenced this pull request Apr 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working integrations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants