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.

Sign up for GitHub

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

Jump to bottom

Cherry-pick #11032 to 7.0: Add default timeouts to metricbeat HTTP helpers #11319

Merged
merged 6 commits into from
Mar 27, 2019
Merged

Cherry-pick #11032 to 7.0: Add default timeouts to metricbeat HTTP helpers #11319

merged 6 commits into from
Mar 27, 2019

Conversation

jsoriano
Copy link
Member

@jsoriano jsoriano commented Mar 19, 2019 •
edited
Loading

Cherry-pick of PR #11032, #11353, #11345 and #11437 to 7.0 branch. Original message:

Set a default request timeout of 10 seconds to metricbeat HTTP helpers
so there are less chances of leaking established connections as reported
in #11014.

Add a connection timeout that defaults to two seconds so connections
fail faster in case of network connectivity problems.

@jsoriano jsoriano requested a review from a team as a code owner March 19, 2019 17:28
@jsoriano
Add default timeouts to metricbeat HTTP helpers (elastic#11032)
edc28cb 
Set a default request timeout of 10 seconds to metricbeat HTTP helpers
so there are less chances of leaking established connections.

Add a connection timeout that defaults to two seconds so connections
fail fast in case of network connectivity problems.

(cherry picked from commit 1cdc88b)
@jsoriano
Copy link
Member Author

Umm, this error happened again:

2019/03/19 18:35:24 httptest.Server blocked in Close after 5 seconds, waiting for connections:
  *net.TCPConn 0xc42000e100 127.0.0.1:50464 in state active

It is a problem on the test, but let's wait to fix it before backporting.

@jsoriano
Copy link
Member Author

Something else to add to this backport: #11353

@jsoriano
Fix authentication in HTTP helper (elastic#11353)
eb4d918 
After the refactor done in elastic#11032, baseData was not being passed to the
HTTP helper, so authentication was not working.

Added a test to avoid regressions here.

Fix elastic#11351
@jsoriano
Try to avoid flakiness on http helper timeout tests (elastic#11345)
33e356b 
Block the response only during a time or till the conext is done.
Close body if a request object is returned.
@jsoriano
Copy link
Member Author

Added cherry picks of #11353 and #11345

@jsoriano
Copy link
Member Author

The test failed again in jenkins 🤦‍♂️

12:22:54 command [go test -race -tags integration -cover -coverprofile /tmp/gotestcover-116289405 github.com/elastic/beats/metricbeat/helper]: exit status 1
12:22:54 --- FAIL: TestTimeout (0.01s)
12:22:54 	assertions.go:256: 
12:22:54 			Error Trace:	http_test.go:165
12:22:54 			            				asm_amd64.s:2361
12:22:54 			Error:      	An error is expected but got nil.
12:22:54 			Test:       	TestTimeout
12:22:54 FAIL
12:22:54 coverage: 45.6% of statements

@andrewvc andrewvc mentioned this pull request Mar 22, 2019
Closed
@jsoriano
Copy link
Member Author

#11437 included here too

@jsoriano
Copy link
Member Author

jenkins, test this again

@jsoriano jsoriano merged commit 0a86ced into elastic:7.0 Mar 27, 2019
@jsoriano jsoriano deleted the backport_11032_7.0 branch March 27, 2019 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ruflin ruflin ruflin approved these changes

Assignees
No one assigned
Labels
backport review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jsoriano @ruflin