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

Add WithIdleConnTimeout HTTP client option #308

Merged
merged 1 commit into from Jun 9, 2021

Conversation

austince
Copy link
Contributor

@austince austince commented Jun 8, 2021

This change enables HTTP long-polling SD mechanisms to configure the idle connection timeout so that they can use this config lib to create HTTP clients.

For example, the consul_sd manually creates a http.Transport:
https://github.com/prometheus/prometheus/blob/7bc11dcb06640ce22b4e15eb52b2c065f97cf79a/discovery/consul/consul.go#L191-L198

Signed-off-by: austin ce <austin.cawley@gmail.com>
@austince austince force-pushed the feat/with-idle-conn-timeout branch from aed2fd9 to 5018d4d Compare June 8, 2021 18:38
@austince
Copy link
Contributor Author

austince commented Jun 8, 2021

@roidelapluie could you take a look at this when you get a chance?

@roidelapluie
Copy link
Member

To be clear, do you plan to update consul_sd after this is merged? :)

I am asking because while we can accept some small contributions, the goal is not that we start maintaining a fully generic http client with all the options possible.

@austince
Copy link
Contributor Author

austince commented Jun 8, 2021

To be clear, do you plan to update consul_sd after this is merged? :)

I'd be happy to :) and I'll use it in the updated kuma_sd as well prometheus/prometheus#8844

@austince
Copy link
Contributor Author

austince commented Jun 9, 2021

@roidelapluie do you think this is an acceptable addition, since it has an immediate use case that will not continue to grow?

On a higher note, do you imagine all SDs in Prometheus that need an HTTP client should use this library?

@roidelapluie
Copy link
Member

Yes, for kuma_sd ofc it's good

@roidelapluie roidelapluie merged commit 8281fb2 into prometheus:main Jun 9, 2021
@austince austince deleted the feat/with-idle-conn-timeout branch June 9, 2021 14:55
@austince
Copy link
Contributor Author

austince commented Jun 9, 2021

Great, thank you!

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

Successfully merging this pull request may close these issues.

None yet

2 participants