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

Expose httpClient in v3 go-agent #154

Closed
yuanmwang-wf opened this issue Jun 9, 2020 · 3 comments
Closed

Expose httpClient in v3 go-agent #154

yuanmwang-wf opened this issue Jun 9, 2020 · 3 comments

Comments

@yuanmwang-wf
Copy link

yuanmwang-wf commented Jun 9, 2020

We recently ran into problems described in golang/go#33006 where a bad connection could potentially block all outgoing http requests from an application.

We discovered the default client created by http.DefaultClient contains some shared/global state and any library may assume they were only modifying one http client when it unfortunately modifies all http clients in an app, for more of this please see https://github.com/hashicorp/go-cleanhttp.

A workaround for this is to set up custom http.Transport and then pass the transport to a custom http.Client with sane timeouts, however, right now we can only pass a custom transport here but we're not able to pass a custom client to the v3 agent. Is it possible to expose the http client used in v3 agent so we can pass our own custom client?

Aha! Link: https://newrelic.aha.io/features/GO-121

@purple4reina
Copy link
Contributor

@yuanmwang-wf Thank you so much for opening this issue. We really appreciate that you've included the link to golang/go#33006 as well. I believe we have all we need in order to reproduce this and get to a solution for you.

That being said, would you feel happy if instead of exposing the http.Client we instead simply stop using the http.DefaultClient?

Take good care,
Rey

@yuanmwang-wf
Copy link
Author

That being said, would you feel happy if instead of exposing the http.Client we instead simply stop using the http.DefaultClient?

Yes, that works for us.

@purple4reina
Copy link
Contributor

Hi @yuanmwang-wf I've opened a PR with some proposed changes that should address this issue. We'll get a release out shortly, but I wanted to give you access to this in the meantime. #163

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants