[Heartbeat] Refactor browser summary as a job wrapper (#33819)

* Refactor browser summary as a job wrapper * Fix unit tests * Update heartbeat/monitors/wrappers/wrappers.go Co-authored-by: Andrew Cholakian <[email protected]> * Update changelog Co-authored-by: Andrew Cholakian <[email protected]>
elastic · Jun 1, 2023 · 2d8e5c8 · 2d8e5c8
1 parent dc94ef0
commit 2d8e5c8
Show file tree

Hide file tree

Showing 5 changed files with 78 additions and 56 deletions.
diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc
@@ -71,6 +71,7 @@ https://github.com/elastic/beats/compare/v8.2.0\...main[Check the HEAD diff]
 - Fix broken zip URL monitors. NOTE: Zip URL Monitors will be removed in version 8.7 and replaced with project monitors. {pull}33723[33723]
 - Fix bug where states.duration_ms was incorrect type. {pull}33563[33563]
 - Fix handling of long UDP messages in UDP input. {issue}33836[33836] {pull}33837[33837]
+- Fix browser monitor summary reporting as up when monitor is down. {issue}33374[33374] {pull}33819[33819]
 
 
 *Auditbeat*

diff --git a/heartbeat/monitors/wrappers/wrappers.go b/heartbeat/monitors/wrappers/wrappers.go
@@ -59,7 +59,7 @@ func WrapLightweight(js []jobs.Job, stdMonFields stdfields.StdMonitorFields, mst
 			addMonitorTimespan(stdMonFields),
 			addServiceName(stdMonFields),
 			addMonitorMeta(stdMonFields, len(js) > 1),
-			addMonitorStatus(false),
+			addMonitorStatus(nil),
 			addMonitorErr,
 			addMonitorDuration,
 		),
@@ -82,8 +82,9 @@ func WrapBrowser(js []jobs.Job, stdMonFields stdfields.StdMonitorFields, mst *mo
 		addMonitorTimespan(stdMonFields),
 		addServiceName(stdMonFields),
 		addMonitorMeta(stdMonFields, false),
-		addMonitorStatus(true),
+		addMonitorStatus(byEventType("heartbeat/summary")),
 		addMonitorErr,
+		addBrowserSummary(stdMonFields, byEventType("heartbeat/summary")),
 		addMonitorState(stdMonFields, mst),
 		logJourneySummaries,
 	)
@@ -221,24 +222,19 @@ func timespan(started time.Time, sched *schedule.Schedule, timeout time.Duration
 // by the original Job will be set as a field. The original error will not be
 // passed through as a return value. Errors may still be present but only if there
 // is an actual error wrapping the error.
-func addMonitorStatus(summaryOnly bool) jobs.JobWrapper {
+func addMonitorStatus(match EventMatcher) jobs.JobWrapper {
 	return func(origJob jobs.Job) jobs.Job {
 		return func(event *beat.Event) ([]jobs.Job, error) {
 			cont, err := origJob(event)
 
-			if summaryOnly {
-				hasSummary, _ := event.Fields.HasKey("summary.up")
-				if !hasSummary {
-					return cont, err
-				}
+			if match == nil || match(event) {
+				eventext.MergeEventFields(event, mapstr.M{
+					"monitor": mapstr.M{
+						"status": look.Status(err),
+					},
+				})
 			}
 
-			eventext.MergeEventFields(event, mapstr.M{
-				"monitor": mapstr.M{
-					"status": look.Status(err),
-				},
-			})
-
 			return cont, err
 		}
 	}
@@ -381,3 +377,47 @@ func makeAddSummary() jobs.JobWrapper {
 		}
 	}
 }
+
+type EventMatcher func(event *beat.Event) bool
+
+func addBrowserSummary(sf stdfields.StdMonitorFields, match EventMatcher) jobs.JobWrapper {
+	return func(job jobs.Job) jobs.Job {
+		return func(event *beat.Event) ([]jobs.Job, error) {
+			cont, jobErr := job(event)
+
+			if match != nil && !match(event) {
+				return cont, jobErr
+			}
+
+			status, err := event.GetValue("monitor.status")
+			if err != nil {
+				return nil, fmt.Errorf("could not wrap summary for '%s', no status assigned: %w", sf.ID, err)
+			}
+
+			up, down := 1, 0
+			if monitorstate.StateStatus(status.(string)) == monitorstate.StatusDown {
+				up, down = 0, 1
+			}
+
+			eventext.MergeEventFields(event, mapstr.M{
+				"summary": mapstr.M{
+					"up":   up,
+					"down": down,
+				},
+			})
+
+			return cont, jobErr
+		}
+	}
+}
+
+func byEventType(t string) func(event *beat.Event) bool {
+	return func(event *beat.Event) bool {
+		eventType, err := event.Fields.GetValue("event.type")
+		if err != nil {
+			return false
+		}
+
+		return eventType == t
+	}
+}
diff --git a/heartbeat/monitors/wrappers/wrappers_test.go b/heartbeat/monitors/wrappers/wrappers_test.go
@@ -571,14 +571,10 @@ func makeProjectBrowserJob(t *testing.T, u string, summary bool, projectErr erro
 			},
 		})
 		if summary {
-			sumFields := mapstr.M{"up": 0, "down": 0}
-			if projectErr == nil {
-				sumFields["up"] = 1
-			} else {
-				sumFields["down"] = 1
-			}
 			eventext.MergeEventFields(event, mapstr.M{
-				"summary": sumFields,
+				"event": mapstr.M{
+					"type": "heartbeat/summary",
+				},
 			})
 		}
 		return nil, projectErr
@@ -652,6 +648,9 @@ func TestProjectBrowserJob(t *testing.T) {
 					lookslike.MustCompile(map[string]interface{}{
 						"monitor": map[string]interface{}{"status": "up"},
 						"summary": map[string]interface{}{"up": 1, "down": 0},
+						"event": map[string]interface{}{
+							"type": "heartbeat/summary",
+						},
 					}),
 				))},
 		nil,
@@ -673,6 +672,9 @@ func TestProjectBrowserJob(t *testing.T) {
 							"type":    isdef.IsString,
 							"message": "testerr",
 						},
+						"event": map[string]interface{}{
+							"type": "heartbeat/summary",
+						},
 					}),
 				))},
 		nil,

diff --git a/x-pack/heartbeat/monitors/browser/synthexec/enrich.go b/x-pack/heartbeat/monitors/browser/synthexec/enrich.go
@@ -113,7 +113,6 @@ func (je *journeyEnricher) enrichSynthEvent(event *beat.Event, se *SynthEvent) e
 	var jobErr error
 	if se.Error != nil {
 		jobErr = stepError(se.Error)
-		je.errorCount++
 		if je.error == nil {
 			je.error = jobErr
 		}
@@ -172,15 +171,6 @@ func (je *journeyEnricher) enrichSynthEvent(event *beat.Event, se *SynthEvent) e
 }
 
 func (je *journeyEnricher) createSummary(event *beat.Event) error {
-	var up, down int
-	if je.errorCount > 0 {
-		up = 0
-		down = 1
-	} else {
-		up = 1
-		down = 0
-	}
-
 	// In case of syntax errors or incorrect runner options, the Synthetics
 	// runner would exit immediately with exitCode 1 and we do not set the duration
 	// to inform the journey never ran
@@ -203,10 +193,6 @@ func (je *journeyEnricher) createSummary(event *beat.Event) error {
 			"type":    "heartbeat/summary",
 			"journey": je.journey,
 		},
-		"summary": mapstr.M{
-			"up":   up,
-			"down": down,
-		},
 	})
 
 	eventext.SetMeta(event, wrappers.META_STEP_COUNT, je.stepCount)

diff --git a/x-pack/heartbeat/monitors/browser/synthexec/enrich_test.go b/x-pack/heartbeat/monitors/browser/synthexec/enrich_test.go
@@ -233,9 +233,8 @@ func TestEnrichSynthEvent(t *testing.T) {
 			true,
 			func(t *testing.T, e *beat.Event, je *journeyEnricher) {
 				v := lookslike.MustCompile(mapstr.M{
-					"summary": map[string]int{
-						"up":   0,
-						"down": 1,
+					"event": map[string]string{
+						"type": "heartbeat/summary",
 					},
 				})
 				testslike.Test(t, v, e.Fields)
@@ -252,9 +251,8 @@ func TestEnrichSynthEvent(t *testing.T) {
 			true,
 			func(t *testing.T, e *beat.Event, je *journeyEnricher) {
 				v := lookslike.MustCompile(mapstr.M{
-					"summary": map[string]int{
-						"up":   1,
-						"down": 0,
+					"event": map[string]string{
+						"type": "heartbeat/summary",
 					},
 				})
 				testslike.Test(t, v, e.Fields)
@@ -266,9 +264,8 @@ func TestEnrichSynthEvent(t *testing.T) {
 			false,
 			func(t *testing.T, e *beat.Event, je *journeyEnricher) {
 				v := lookslike.MustCompile(mapstr.M{
-					"summary": map[string]int{
-						"up":   1,
-						"down": 0,
+					"event": map[string]string{
+						"type": "heartbeat/summary",
 					},
 				})
 				testslike.Test(t, v, e.Fields)
@@ -482,9 +479,8 @@ func TestCreateSummaryEvent(t *testing.T) {
 		},
 		expected: mapstr.M{
 			"monitor.duration.us": int64(10),
-			"summary": mapstr.M{
-				"down": 0,
-				"up":   1,
+			"event": mapstr.M{
+				"type": "heartbeat/summary",
 			},
 		},
 		wantErr: false,
@@ -500,9 +496,8 @@ func TestCreateSummaryEvent(t *testing.T) {
 		},
 		expected: mapstr.M{
 			"monitor.duration.us": int64(10),
-			"summary": mapstr.M{
-				"down": 1,
-				"up":   0,
+			"event": mapstr.M{
+				"type": "heartbeat/summary",
 			},
 		},
 		wantErr: true,
@@ -518,9 +513,8 @@ func TestCreateSummaryEvent(t *testing.T) {
 		},
 		expected: mapstr.M{
 			"monitor.duration.us": int64(10),
-			"summary": mapstr.M{
-				"down": 0,
-				"up":   1,
+			"event": mapstr.M{
+				"type": "heartbeat/summary",
 			},
 		},
 		wantErr: true,
@@ -534,9 +528,8 @@ func TestCreateSummaryEvent(t *testing.T) {
 			streamEnricher:  newStreamEnricher(stdfields.StdMonitorFields{}),
 		},
 		expected: mapstr.M{
-			"summary": mapstr.M{
-				"down": 1,
-				"up":   0,
+			"event": mapstr.M{
+				"type": "heartbeat/summary",
 			},
 		},
 		wantErr: true,