Add support for idle_connection_timeout to elasticsearch output

[leehinman]

Proposed commit message

add support for idle_connection_timeout for ES output. This allows connections to be closed if they aren't being used.

Checklist

• My code follows the style guidelines of this project
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
  - [ ] I have made corresponding change to the default configuration files
• I have added tests that prove my fix is effective or that my feature works
• I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Author's Checklist

• [ ]

How to test this PR locally

1. Add idle_connection_timeout to elasticsearch output config

output.elasticsearch:
  idle_connection_timeout: 10s

2. Start beat, and send some events to elasticsearch
3. observer network connection with netstat -an
4. wait timeout time, and observer that network connection is being closed

Related issues

• Relates Allow queue configuration to be specified under the output type #35615

Use cases

Screenshots

Logs

[mergify]

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @leehinman? 🙏.
For such, you'll need to label your PR with:

• The upcoming major version of the Elastic Stack
• The upcoming minor version of the Elastic Stack (if you're not pushing a breaking change)

To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

• backport-v8./d.0 is the label to automatically backport to the 8./d branch. /d is the digit

[elasticmachine]

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

• Start Time: 2023-10-25T16:45:34.408+0000

• Duration: 101 min 39 sec

Test stats 🧪

Test	Results
Failed	0
Passed	28618
Skipped	2015
Total	30633

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

• /test : Re-trigger the build.

• /package : Generate the packages and run the E2E tests.

• /beats-tester : Run the installation tests with beats-tester.

• run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

[cmacknz]

Is the default being 0 actually the behavior from before? There are a lot of layers to follow in elastic-agent-libs, but I see when we create an HTTP round tripper we clone the default transport:

https://github.com/elastic/elastic-agent-libs/blob/9ec47704eb509f0944507fa11eb48a580711bbba/transport/httpcommon/httpcommon.go#L254-L259

func (settings *HTTPTransportSettings) httpRoundTripper(
	tls *tlscommon.TLSConfig,
	dialer, tlsDialer transport.Dialer,
	opts ...TransportOption,
) *http.Transport {
	t := http.DefaultTransport.(*http.Transport).Clone()

The default transport uses a 90 second timeout by default:

https://cs.opensource.google/go/go/+/refs/tags/go1.21.3:src/net/http/transport.go;l=38-54

// DefaultTransport is the default implementation of Transport and is
// used by DefaultClient. It establishes network connections as needed
// and caches them for reuse by subsequent calls. It uses HTTP proxies
// as directed by the environment variables HTTP_PROXY, HTTPS_PROXY
// and NO_PROXY (or the lowercase versions thereof).
var DefaultTransport RoundTripper = &Transport{
	Proxy: ProxyFromEnvironment,
	DialContext: defaultTransportDialContext(&net.Dialer{
		Timeout:   30 * time.Second,
		KeepAlive: 30 * time.Second,
	}),
	ForceAttemptHTTP2:     true,
	MaxIdleConns:          100,
	IdleConnTimeout:       90 * time.Second,
	TLSHandshakeTimeout:   10 * time.Second,
	ExpectContinueTimeout: 1 * time.Second,
}

[cmacknz]

To be clear 0 might be what this configuration was set to, but I am not sure the result IdleConnTimeout was 0, I think it was 90s because IdleConnTimeout is only written if it is not zero which means it uses the underlying Go default:

https://github.com/elastic/elastic-agent-libs/blob/9ec47704eb509f0944507fa11eb48a580711bbba/transport/httpcommon/httpcommon.go#L309-L313

func (opts WithKeepaliveSettings) applyTransport(_ *HTTPTransportSettings, t *http.Transport) {
	t.DisableKeepAlives = opts.Disable
	if opts.IdleConnTimeout != 0 {
		t.IdleConnTimeout = opts.IdleConnTimeout
	}

[leehinman]

We were both wrong. The esleg client sets it to 60s by default.

beats/libbeat/esleg/eslegclient/connection.go

Lines 105 to 107 in 42f2f94

	if s.IdleConnTimeout == 0 {
	s.IdleConnTimeout = 1 * time.Minute
	}

This also matches testing I did with Wireshark where I observed the default at 60s.

[cmacknz]

[mergify]

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b 35615_idle_timeout upstream/35615_idle_timeout
git merge upstream/main
git push upstream 35615_idle_timeout

[ebeahan]

The wording here states zero is no limit. But in the conversation and snippet here, isn't setting idle_connection_timeout to 0 going to set the client to still use a 60s timeout?

[leehinman]

Thanks, should be fixed now.

[andrewvc]

LGTM