[Heartbeat] Improved redirect support

CHANGELOG.next.asciidoc

@@ -42,6 +42,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
 *Heartbeat*
 
 - Removed the `add_host_metadata` and `add_cloud_metadata` processors from the default config. These don't fit well with ECS for Heartbeat and were rarely used.
+- Fixed/altered redirect behavior. `max_redirects` now defaults to 0 (no redirects). Following redirects now works across hosts, but some timing fields will not be reported. {pull}14125[14125]
 - Removed `host.name` field that should never have been included. Heartbeat uses `observer.*` fields instead. {pull}14140[14140]
 
 *Journalbeat*

heartbeat/docs/fields.asciidoc

@@ -4963,6 +4963,16 @@ alias to: url.full
 Hash of the full response body. Can be used to group responses with identical hashes.
 
 
+type: keyword
+
+--
+
+*`http.response.redirects`*::
++
+--
+List of redirects followed to arrive at final content. Last item on the list is the URL for which body content is shown.
+
+
 type: keyword
 
 --

heartbeat/docs/heartbeat-options.asciidoc

@@ -388,6 +388,17 @@ Example configuration:
   hosts: ["http://myhost:80"]
 -------------------------------------------------------------------------------
 
+[float]
+[[monitor-http-max-redirects]]
+==== `max_redirects`
+
+The total number of redirections Heartbeat will follow. Defaults to 0, meaning heartbeat will not follow redirects,
+but will report the status of the redirect. If set to a number greater than 0 heartbeat will follow that number of redirects.
+
+When this option is set to a value greater than zero the `monitor.ip` field will no longer be reported, as multiple
+DNS requests across multiple IPs may return multiple IPs. Fine grained network timing data will also not be recorded, as with redirects
+that data will span multiple requests. Specifically the fields `http.rtt.content.us`, `http.rtt.response_header.us`,
+`http.rtt.total.us`, `http.rtt.validate.us`, `http.rtt.write_request.us` and `dns.rtt.us` will be omitted.
 
 [float]
 [[monitor-http-proxy-url]]

heartbeat/hbtest/hbtestutil.go

@@ -73,6 +73,23 @@ func SizedResponseHandler(bytes int) http.HandlerFunc {
 	)
 }
 
+// RedirectHandler redirects the paths at the keys in the redirectingPaths map to the locations in their values.
+// For paths not in the redirectingPaths map it returns a 200 response with the given body.
+func RedirectHandler(redirectingPaths map[string]string, body string) http.HandlerFunc {
+	return http.HandlerFunc(
+		func(w http.ResponseWriter, r *http.Request) {
+			url, _ := url.Parse(r.RequestURI)
+			redirectTarget, isRedirect := redirectingPaths[url.Path]
+			if isRedirect {
+				w.Header().Add("Location", redirectTarget)
+				w.WriteHeader(302)
+			} else {
+				w.WriteHeader(200)
+				io.WriteString(w, body)
+			}
+		})
+}
+
 // ServerPort takes an httptest.Server and returns its port as a uint16.
 func ServerPort(server *httptest.Server) (uint16, error) {
 	u, err := url.Parse(server.URL)

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

@@ -23,6 +23,11 @@
                   type: keyword
                   description: >
                     Hash of the full response body. Can be used to group responses with identical hashes.
+            - name: redirects
+              type: keyword
+              description: >
+                List of redirects followed to arrive at final content. Last item on the list is the URL for which
+                body content is shown.
         - name: rtt
           type: group
           description: >

heartbeat/monitors/active/http/config.go

@@ -92,7 +92,7 @@ type compressionConfig struct {
 
 var defaultConfig = Config{
 	Timeout:      16 * time.Second,
-	MaxRedirects: 10,
+	MaxRedirects: 0,
 	Response: responseConfig{
 		IncludeBody:         "on_error",
 		IncludeBodyMaxBytes: 2048,

heartbeat/monitors/active/http/http.go

@@ -80,7 +80,10 @@ func create(
 	// Determine whether we're using a proxy or not and then use that to figure out how to
 	// run the job
 	var makeJob func(string) (jobs.Job, error)
-	if config.ProxyURL != "" {
+	// In the event that a ProxyURL is present, or redirect support is enabled
+	// we execute DNS resolution requests inline with the request, not running them as a separate job, and not returning
+	// separate DNS rtt data.
+	if config.ProxyURL != "" || config.MaxRedirects > 0 {
 		transport, err := newRoundTripper(&config, tls)
 		if err != nil {
 			return nil, 0, err

heartbeat/monitors/active/http/http_test.go

@@ -107,23 +107,10 @@ func httpBaseChecks(urlStr string) validator.Validator {
 func respondingHTTPChecks(url string, statusCode int) validator.Validator {
 	return lookslike.Compose(
 		httpBaseChecks(url),
+		httpBodyChecks(),
 		lookslike.MustCompile(map[string]interface{}{
 			"http": map[string]interface{}{
-				"response.status_code": statusCode,
-				"response.body.hash":   isdef.IsString,
-				// TODO add this isdef to lookslike in a robust way
-				"response.body.bytes": isdef.Is("an int64 greater than 0", func(path llpath.Path, v interface{}) *llresult.Results {
-					raw, ok := v.(int64)
-					if !ok {
-						return llresult.SimpleResult(path, false, "%s is not an int64", reflect.TypeOf(v))
-					}
-					if raw >= 0 {
-						return llresult.ValidResult(path)
-					}
-
-					return llresult.SimpleResult(path, false, "value %v not >= 0 ", raw)
-
-				}),
+				"response.status_code":   statusCode,
 				"rtt.content.us":         isdef.IsDuration,
 				"rtt.response_header.us": isdef.IsDuration,
 				"rtt.total.us":           isdef.IsDuration,
@@ -134,6 +121,38 @@ func respondingHTTPChecks(url string, statusCode int) validator.Validator {
 	)
 }
 
+func minimalRespondingHTTPChecks(url string, statusCode int) validator.Validator {
+	return lookslike.Compose(
+		httpBaseChecks(url),
+		httpBodyChecks(),
+		lookslike.MustCompile(map[string]interface{}{
+			"http": map[string]interface{}{
+				"response.status_code": statusCode,
+				"rtt.total.us":         isdef.IsDuration,
+			},
+		}),
+	)
+}
+
+func httpBodyChecks() validator.Validator {
+	return lookslike.MustCompile(map[string]interface{}{
+		// TODO add this isdef to lookslike in a robust way
+		"http.response.body.bytes": isdef.Is("an int64 greater than 0", func(path llpath.Path, v interface{}) *llresult.Results {
+			raw, ok := v.(int64)
+			if !ok {
+				return llresult.SimpleResult(path, false, "%s is not an int64", reflect.TypeOf(v))
+			}
+			if raw >= 0 {
+				return llresult.ValidResult(path)
+			}
+
+			return llresult.SimpleResult(path, false, "value %v not >= 0 ", raw)
+
+		}),
+		"http.response.body.hash": isdef.IsString,
+	})
+}
+
 func respondingHTTPBodyChecks(body string) validator.Validator {
 	return lookslike.MustCompile(map[string]interface{}{
 		"http.response.body.content": body,
@@ -443,3 +462,49 @@ func TestUnreachableJob(t *testing.T) {
 		event.Fields,
 	)
 }
+
+func TestRedirect(t *testing.T) {
+	redirectingPaths := map[string]string{
+		"/redirect_one": "/redirect_two",
+		"/redirect_two": "/",
+	}
+	expectedBody := "TargetBody"
+	server := httptest.NewServer(hbtest.RedirectHandler(redirectingPaths, expectedBody))
+	defer server.Close()
+
+	testURL := server.URL + "/redirect_one"
+	configSrc := map[string]interface{}{
+		"urls":                testURL,
+		"timeout":             "1s",
+		"check.response.body": expectedBody,
+		"max_redirects":       10,
+	}
+
+	config, err := common.NewConfigFrom(configSrc)
+	require.NoError(t, err)
+
+	jobs, _, err := create("redirect", config)
+	require.NoError(t, err)
+
+	job := wrappers.WrapCommon(jobs, "test", "", "http")[0]
+
+	event := &beat.Event{}
+	_, err = job(event)
+	require.NoError(t, err)
+
+	testslike.Test(
+		t,
+		lookslike.Strict(lookslike.Compose(
+			hbtest.BaseChecks("", "up", "http"),
+			hbtest.SummaryChecks(1, 0),
+			minimalRespondingHTTPChecks(testURL, 200),
+			lookslike.MustCompile(map[string]interface{}{
+				"http.redirects": []string{
+					server.URL + redirectingPaths["/redirect_one"],
+					server.URL + redirectingPaths["/redirect_two"],
+				},
+			}),
+		)),
+		event.Fields,
+	)
+}

heartbeat/monitors/active/http/task.go

@@ -49,11 +49,14 @@ func newHTTPMonitorHostJob(
 	validator RespCheck,
 ) (jobs.Job, error) {
 
+	// Trace visited URLs when redirects occur
+	var redirects []string
 	client := &http.Client{
-		CheckRedirect: makeCheckRedirect(config.MaxRedirects),
+		CheckRedirect: makeCheckRedirect(config.MaxRedirects, &redirects),
 		Transport:     transport,
 		Timeout:       config.Timeout,
 	}
+
 	request, err := buildRequest(addr, config, enc)
 	if err != nil {
 		return nil, err
@@ -62,7 +65,7 @@ func newHTTPMonitorHostJob(
 	timeout := config.Timeout
 
 	return jobs.MakeSimpleJob(func(event *beat.Event) error {
-		_, _, err := execPing(event, client, request, body, timeout, validator, config.Response)
+		_, _, err := execPing(event, client, request, body, timeout, validator, config.Response, &redirects)
 		return err
 	}), nil
 }
@@ -104,7 +107,7 @@ func createPingFactory(
 ) func(*net.IPAddr) jobs.Job {
 	timeout := config.Timeout
 	isTLS := request.URL.Scheme == "https"
-	checkRedirect := makeCheckRedirect(config.MaxRedirects)
+	checkRedirect := makeCheckRedirect(config.MaxRedirects, nil)
 
 	return monitors.MakePingIPFactory(func(event *beat.Event, ip *net.IPAddr) error {
 		addr := net.JoinHostPort(ip.String(), strconv.Itoa(int(port)))
@@ -153,7 +156,7 @@ func createPingFactory(
 			},
 		}
 
-		_, end, err := execPing(event, client, request, body, timeout, validator, config.Response)
+		_, end, err := execPing(event, client, request, body, timeout, validator, config.Response, nil)
 		cbMutex.Lock()
 		defer cbMutex.Unlock()
 
@@ -211,6 +214,7 @@ func execPing(
 	timeout time.Duration,
 	validator func(*http.Response) error,
 	responseConfig responseConfig,
+	redirects *[]string,
 ) (start, end time.Time, errReason reason.Reason) {
 	ctx, cancel := context.WithTimeout(context.Background(), timeout)
 	defer cancel()
@@ -228,8 +232,12 @@ func execPing(
 		return start, time.Now(), errReason
 	}
 
-	// Add response.status_code
-	eventext.MergeEventFields(event, common.MapStr{"http": common.MapStr{"response": common.MapStr{"status_code": resp.StatusCode}}})
+	// Add response.status_code and redirects
+	response := common.MapStr{"response": common.MapStr{"status_code": resp.StatusCode}}
+	if redirects != nil && len(*redirects) > 0 {
+		response["redirects"] = redirects
+	}
+	eventext.MergeEventFields(event, common.MapStr{"http": response})
 	// Download the body, close the response body, then attach all fields
 	err := handleRespBody(event, resp, responseConfig, errReason)
 	if err != nil {
@@ -298,14 +306,21 @@ func splitHostnamePort(requ *http.Request) (string, uint16, error) {
 	return host, uint16(p), nil
 }
 
-func makeCheckRedirect(max int) func(*http.Request, []*http.Request) error {
+// makeCheckRedirect checks if max redirects are exceeded, also append to the redirects list if we're tracking those.
+// It's kind of ugly to return a result via a pointer argument, but it's the interface the
+// golang HTTP client gives us.
+func makeCheckRedirect(max int, redirects *[]string) func(*http.Request, []*http.Request) error {
 	if max == 0 {
 		return func(_ *http.Request, _ []*http.Request) error {
 			return http.ErrUseLastResponse
 		}
 	}
 
-	return func(_ *http.Request, via []*http.Request) error {
+	return func(r *http.Request, via []*http.Request) error {
+		if redirects != nil {
+			*redirects = append(*redirects, r.URL.String())
+		}
+
 		if max == len(via) {
 			return http.ErrUseLastResponse
 		}

heartbeat/monitors/active/http/task_test.go

@@ -132,7 +132,7 @@ func makeTestHTTPRequest(t *testing.T) *http.Request {
 }
 
 func TestZeroMaxRedirectShouldError(t *testing.T) {
-	checker := makeCheckRedirect(0)
+	checker := makeCheckRedirect(0, nil)
 	req := makeTestHTTPRequest(t)
 
 	res := checker(req, nil)
@@ -141,7 +141,7 @@ func TestZeroMaxRedirectShouldError(t *testing.T) {
 
 func TestNonZeroRedirect(t *testing.T) {
 	limit := 5
-	checker := makeCheckRedirect(limit)
+	checker := makeCheckRedirect(limit, nil)
 
 	var via []*http.Request
 	// Test requests within the limit