-
Notifications
You must be signed in to change notification settings - Fork 457
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
ddtrace/tracer: fix UDS/http url configuration and error messages #1604
Conversation
This commit cleans up the configuration of the agent URL so that it reflects the actual URL being used by the Tracer. Previously it would be left as localhost:8162 if the tracer was using UDS, and the error messages would mention localhost:8126 even though it was sending to the agent over unix sockets. In addition, it cleans up/fixes a few tests associated with this change.
@knusbaum should we also update the agent URL when |
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.
Looks good, good 👀 spotting those flipped test assertions.
One question about the logic in newConfig
Nice catch, @mackjmr. That should be fixed now. |
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.
Approved! with one optional nit
Hi! Thanks for this @knusbaum, how soon can we get this merged? :D |
This commit cleans up the configuration of the agent URL so that it reflects the actual URL being used by the Tracer. Previously it would be left as localhost:8162 if the tracer was using UDS, and the error messages would mention localhost:8126 even though it was sending to the agent over unix sockets.
In addition, it cleans up/fixes a few tests associated with this change.
What does this PR do?
Motivation
Describe how to test/QA your changes
Reviewer's Checklist
Triage
milestone is set.