search

syepes / network_exporter

ICMP / Ping & MTR & TCP Port & HTTP Get - Network Prometheus exporter
Apache License 2.0
329 stars 59 forks source link

HTTPGet checks using a proxy do not reuse network connection #31

Closed paebersold closed 1 year ago

paebersold commented 1 year ago

Hello, we have found that using a proxy for HTTPGet requests results in every check generating a new network connection (and old connections are never closed). Our sample config (with names/ips changed)

---
http_get:
 interval: 15s
 timeout: 5s
targets:
  - name: useproxy
    host: https://bob.server.com/ready
    type: HTTPGet
    proxy: http://proxy:3128
  - name: noproxy
    host: http://1.2.3.4:9100/metrics
    type: HTTPGet

For target 'useproxy' ever check generates a new connection so after a period of time we have hundreds for established connections, and we see keepalive traffic (an ACK of length 0) on each connection every interval period.

For target 'noproxy' we see one connection that is kept for the entire duration and see the normal traffic from the http GET every interval.

Initially I thought this may have been an issue in pkg/http/http.go that the function HTTPGetProxy does not do the boilerplate 

defer resp.Body.Close()

as suggested in https://pkg.go.dev/net/http#pkg-overview. However adding this did not change the behaviour. Also HTTPGet works just fine without it.

Setting the Close field on the HTTP Request in HTTPGetProxy ala

req.Close = true

fixes the issue as expected.

However I am still puzzled on the difference in behaviour as the two functions HTTPGetProxy and HTTPGet are almost identical. Is this behaviour seen by others?

For reference the client system is Sles12sp5 linux x86 (4.12.14-122.136) and the proxy is running Apache 2.4.51 and using mod_proxy.

Thanks for your time.

syepes commented 1 year ago

Nice finding, it it strange that it's doing this as it should close the connection as the body is fully read (http.go#L67)

If you find the solution please don't hesitate in submitting a PR :-)

syepes commented 1 year ago

Just pushed a version that should close the connections, give this one a try. https://github.com/syepes/network_exporter/releases/tag/1.6.6

paebersold commented 1 year ago

Yep that was what I had tested out (the req.close). Works as expected (ie no connection reuse, no connections left open). Thanks for putting out release so quick.

syepes commented 1 year ago

No problem and thanks for reporting it