-
Notifications
You must be signed in to change notification settings - Fork 435
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
{profiler,ddtrace/tracer}: add UDS client options #788
Conversation
Allow the overriding the HTTP client and transport. As a convenience, provide an option to create a custom client and transport that accesses a Unix Domain Socket. Modified upload tests to exercise both default and custom HTTP clients.
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.
This looks good to me, left a few nits.
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.
This LGTM, but I'd love it if we could clean up / sort out the tests a bit to make them easier to figure out.
None of the functions are clear on what they do and the multiple return values (e.g. io.Closer, string, testServerWaiter
) are not clarified. Perhaps named return variables will help here.
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.
It's all good but I find the test complexity and abstraction a bit overkill. We need to severely simplify this.
29a3885
to
dd3b1ac
Compare
Background
Some customers need to override networking over which the profiler sends data.
Description