[Heartbeat] Capture HTTP Response Bodies #13022

andrewvc · 2019-07-22T19:59:17Z

This PR adds optional support for capturing HTTP response bodies in heartbeat events.

By default it sets the body only on responses that return errors, but can optionally be set to never report the body or report it on all checks.

Fixes elastic/uptime#37

elasticmachine · 2019-07-22T19:59:20Z

Pinging @elastic/uptime

heartbeat/monitors/active/http/respbody.go

andrewvc · 2019-07-24T13:12:50Z

We should store hashes of the bodies with it

heartbeat/monitors/active/http/respbody_test.go

heartbeat/monitors/active/http/config.go

ruflin · 2019-08-22T09:21:09Z

heartbeat/monitors/active/http/config.go

@@ -49,6 +51,11 @@ type Config struct {
 	Check checkConfig `config:"check"`
 }

+type responseConfig struct {
+	IncludeBody         string `config:"include_body"`
+	IncludeBodyMaxBytes int    `config:"include_body_max_bytes"`


Does also not seem to be documented. How does it overlap with the truncate? Probably see it in the code below.

It is in the documentation! I documented the whole response block as one thing here. Glad to change it, but it's in keeping with the style of some other feature's documentation.

Sorry, missed that. Seems my command - F foo failed me.

ruflin · 2019-08-22T09:21:42Z

heartbeat/monitors/active/http/config.go

@@ -103,6 +114,20 @@ var defaultConfig = Config{
 	},
 }

+func (r *responseConfig) Validate() error {
+	switch strings.ToLower(r.IncludeBody) {


Do you really want to do the strings to lower? I would enforce users to write it correctly.

I mean, I don't see one way as being correct or another. Some people like typing in upper case. It's not a big deal to me however.

Agree, just personal preference. Both options ok for me.

heartbeat/monitors/active/http/config.go

ruflin · 2019-08-22T09:27:18Z

heartbeat/monitors/active/http/simple_transp.go

@@ -144,7 +144,7 @@ func (t *SimpleTransport) RoundTrip(req *http.Request) (*http.Response, error) {
 		// finally, return the real error. No need to return a response here
 		return nil, errors.New("http: request timed out while waiting for response")
 	}
-	close(readerDone)
+	//close(readerDone)


That actually should still be in place AFAICT, undid this change.

ruflin · 2019-08-22T09:28:41Z

heartbeat/monitors/active/http/task.go

-		}
-		return start, end, errReason
+	// Add response.status_code
+	eventext.MergeEventFields(event, common.MapStr{"http": common.MapStr{"response": common.MapStr{"status_code": resp.StatusCode}}})


I wonder if you want to do that only after the handleRespBody to not have that also in the timing.

The reason this is first is because handleRespBody can return an error causing an early exit. It would be a little awkward to inject this before the error check/return.

This is pretty fast code to execute, and given that we aren't writing real time code, the effect should be negligible.

ruflin · 2019-08-22T09:29:34Z

heartbeat/monitors/active/http/_meta/fields.yml

+                - name: hash
+                  type: keyword
+                  description: >
+                    Hash of the response body. Can be used to group responses with identical hashes


Is this the hash of the full response body of the partial response body that is returned?

The full body, clarified it in the latest push.

andrewvc · 2019-08-22T19:40:08Z

@ruflin the latest changes should have addressed all your comments. Thanks for the review @ruflin !

andrewvc · 2019-08-26T17:18:15Z

Test failures in filebeat and metricbeat seem unrelated

ruflin

LGTM. Can be merged as is from my perspective but would be great to still get a response to the questions for a potential follow up.

heartbeat/monitors/active/http/_meta/fields.yml

ruflin · 2019-08-27T07:19:16Z

heartbeat/monitors/active/http/config.go

@@ -103,6 +114,20 @@ var defaultConfig = Config{
 	},
 }

+func (r *responseConfig) Validate() error {
+	switch strings.ToLower(r.IncludeBody) {


Agree, just personal preference. Both options ok for me.

ruflin · 2019-08-27T07:23:11Z

heartbeat/monitors/active/http/respbody.go

+			break
+		}
+
+		if readErr != nil {


Sorry, just seeing this now. Shouldn't this line directly be after line 95? I get that if you have an EOF that you also want to get the content first, but if you have an actual error, I assume also the readSize is probably 0 ?

Nope! See https://golang.org/pkg/io/#Reader

Callers should always process the n > 0 bytes returned before considering the error err. Doing so correctly handles I/O errors that happen after reading some bytes and also both of the allowed EOF behaviors.

Thanks for the reminder 👍

Co-Authored-By: Nicolas Ruflin <spam@ruflin.com>

This PR adds optional support for capturing HTTP response bodies in heartbeat events. By default it sets the body only on responses that return errors, but can optionally be set to never report the body or report it on all checks. (cherry picked from commit 1e3e65c)

Updates the go-lookslike library we maintain to 0.3.0 which uses a much improved reflection strategy. This fixed an issue in the last release where Strict() which checked for extra fields wasn't working. As a result some of our tests developed since then didn't assert that the new fields added in #13022 were actually valid. Those tests are fixed in this PR.

Currently, when using an HTTP body validator (either regexp or JSON) will break the storage of HTTP bodies with response.include_body (introduced in #13022). The root cause is that both validation and reading the body for inclusion in the event share the same ReadCloser provided by *http.Response. This patch looks at both validation and body settings to determine how much of the body to read, reads that much, then passes that to the validation and body inclusion code as a []byte. Resolves #13751

Currently, when using an HTTP body validator (either regexp or JSON) will break the storage of HTTP bodies with response.include_body (introduced in elastic#13022). The root cause is that both validation and reading the body for inclusion in the event share the same ReadCloser provided by *http.Response. This patch looks at both validation and body settings to determine how much of the body to read, reads that much, then passes that to the validation and body inclusion code as a []byte. Resolves elastic#13751 (cherry picked from commit 6c6b396)

) Currently, when using an HTTP body validator (either regexp or JSON) will break the storage of HTTP bodies with response.include_body (introduced in #13022). The root cause is that both validation and reading the body for inclusion in the event share the same ReadCloser provided by *http.Response. This patch looks at both validation and body settings to determine how much of the body to read, reads that much, then passes that to the validation and body inclusion code as a []byte. Resolves #13751 (cherry picked from commit 6c6b396)

Currently, when using an HTTP body validator (either regexp or JSON) will break the storage of HTTP bodies with response.include_body (introduced in elastic#13022). The root cause is that both validation and reading the body for inclusion in the event share the same ReadCloser provided by *http.Response. This patch looks at both validation and body settings to determine how much of the body to read, reads that much, then passes that to the validation and body inclusion code as a []byte. Resolves elastic#13751

… (elastic#14310) Currently, when using an HTTP body validator (either regexp or JSON) will break the storage of HTTP bodies with response.include_body (introduced in elastic#13022). The root cause is that both validation and reading the body for inclusion in the event share the same ReadCloser provided by *http.Response. This patch looks at both validation and body settings to determine how much of the body to read, reads that much, then passes that to the validation and body inclusion code as a []byte. Resolves elastic#13751 (cherry picked from commit 46643b0)

andrewvc added 6 commits July 19, 2019 15:57

Checkpoint

7019a77

Checkpoint

9a2a3b3

More checks

9c097f1

Cleanup

598aa42

More cleanup

9173812

Setup for tests

a3b7ad8

andrewvc added enhancement in progress Pull request is currently in progress. Heartbeat Team:obs-ds-hosted-services Label for the Observability Hosted Services team labels Jul 22, 2019

andrewvc requested a review from a team as a code owner July 22, 2019 19:59

andrewvc self-assigned this Jul 22, 2019

houndci-bot reviewed Jul 22, 2019

View reviewed changes

heartbeat/monitors/active/http/respbody.go Outdated Show resolved Hide resolved

andrewvc mentioned this pull request Jul 22, 2019

[Uptime] Allow Heartbeat to capture body text on error elastic/uptime#37

Closed

andrewvc added 6 commits July 26, 2019 15:29

Updates

011dc8f

Better code organization

5ccb8c9

Fix license header

8a2ff61

Fix check

8e6b927

Make update

c96e5e3

Fix respbody

3a91546

andrewvc mentioned this pull request Aug 8, 2019

[META] Show response bodies elastic/uptime#76

Closed

2 tasks

houndci-bot reviewed Aug 20, 2019

View reviewed changes

heartbeat/monitors/active/http/respbody_test.go Outdated Show resolved Hide resolved

houndci-bot reviewed Aug 20, 2019

View reviewed changes

heartbeat/monitors/active/http/respbody_test.go Outdated Show resolved Hide resolved

andrewvc added 5 commits August 21, 2019 07:19

Make hound happy

aff722d

Merge remote-tracking branch 'origin/master' into body-on-error

38dd2e9

Add more tests and clean up code

629dfd2

Always store hash + size, add docs

486cfe4

Add changelog

37b9078

FMT

c6bd5f1

andrewvc requested a review from ruflin August 21, 2019 17:06

andrewvc added review v7.4.0 and removed in progress Pull request is currently in progress. labels Aug 21, 2019

ruflin reviewed Aug 22, 2019

View reviewed changes

andrewvc added 2 commits August 22, 2019 14:35

Incorporate PR feedback

d7f34c8

Clarify docs

84fcf11

Update fields.go

2c2bec4

ruflin approved these changes Aug 27, 2019

View reviewed changes

Update heartbeat/monitors/active/http/_meta/fields.yml

38126f5

Co-Authored-By: Nicolas Ruflin <spam@ruflin.com>

andrewvc merged commit 1e3e65c into elastic:master Aug 27, 2019

andrewvc deleted the body-on-error branch August 27, 2019 17:44

This was referenced Sep 3, 2019

[Heartbeat] HTTP check failures should include body in error #11123

Closed

[libbeat] Update lookslike to 0.3.0 #13498

Merged

andrewvc mentioned this pull request Sep 20, 2019

[Heartbeat] Body on_error does not trigger on validation failures #13751

Closed

andrewvc mentioned this pull request Oct 24, 2019

[Heartbeat] Correctly store HTTP bodies with validation #14223

Merged

andrewvc mentioned this pull request Oct 29, 2019

[Heartbeat] Correctly store HTTP bodies with validation (#14223) #14310

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Heartbeat] Capture HTTP Response Bodies #13022

[Heartbeat] Capture HTTP Response Bodies #13022

andrewvc commented Jul 22, 2019 •

edited

Loading

elasticmachine commented Jul 22, 2019

andrewvc commented Jul 24, 2019

ruflin Aug 22, 2019

andrewvc Aug 22, 2019

ruflin Aug 27, 2019

ruflin Aug 22, 2019

andrewvc Aug 22, 2019

ruflin Aug 27, 2019

ruflin Aug 22, 2019

andrewvc Aug 22, 2019

ruflin Aug 22, 2019

andrewvc Aug 22, 2019

ruflin Aug 22, 2019

andrewvc Aug 22, 2019

andrewvc commented Aug 22, 2019

andrewvc commented Aug 26, 2019

ruflin left a comment

ruflin Aug 27, 2019

ruflin Aug 27, 2019

andrewvc Aug 27, 2019

ruflin Aug 28, 2019

[Heartbeat] Capture HTTP Response Bodies #13022

[Heartbeat] Capture HTTP Response Bodies #13022

Conversation

andrewvc commented Jul 22, 2019 • edited Loading

elasticmachine commented Jul 22, 2019

andrewvc commented Jul 24, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewvc commented Aug 22, 2019

andrewvc commented Aug 26, 2019

ruflin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewvc commented Jul 22, 2019 •

edited

Loading