fix: avoid cmd/status when journey has already finished

Author: Lucas Fernandes da Costa

x-pack/heartbeat/monitors/browser/synthexec/enrich.go

@@ -76,8 +76,6 @@ func (je *journeyEnricher) enrich(event *beat.Event, se *SynthEvent) error {
 			je.checkGroup = makeUuid()
 			je.journey = se.Journey
 			je.start = event.Timestamp
-		case "cmd/status":
-			fallthrough
 		case "journey/end":
 			je.end = event.Timestamp
 		}
@@ -115,8 +113,13 @@ func (je *journeyEnricher) enrichSynthEvent(event *beat.Event, se *SynthEvent) e
 
 	switch se.Type {
 	case "cmd/status":
-		je.journeyComplete = false
-		return je.createSummary(event)
+		// If a command failed _after_ the journey was complete, as it happens
+		// when an `afterAll` hook fails, for example, we don't wan't to include
+		// a summary in the cmd/status event.
+		if !je.journeyComplete {
+			je.end = event.Timestamp
+			return je.createSummary(event)
+		}
 	case "journey/end":
 		je.journeyComplete = true
 		return je.createSummary(event)

x-pack/heartbeat/monitors/browser/synthexec/enrich_test.go

@@ -21,6 +21,18 @@ import (
 	"github.com/elastic/go-lookslike/testslike"
 )
 
+func makeStepEvent(typ string, ts float64, name string, index int, status string, urlstr string, err *SynthError) *SynthEvent {
+	return &SynthEvent{
+		Type:                 typ,
+		TimestampEpochMicros: 1000 + ts,
+		PackageVersion:       "1.0.0",
+		Step:                 &Step{Name: name, Index: index, Status: status},
+		Error:                err,
+		Payload:              common.MapStr{},
+		URL:                  urlstr,
+	}
+}
+
 func TestJourneyEnricher(t *testing.T) {
 	journey := &Journey{
 		Name: "A Journey Name",
@@ -50,17 +62,6 @@ func TestJourneyEnricher(t *testing.T) {
 		Journey:              journey,
 		Payload:              common.MapStr{},
 	}
-	makeStepEvent := func(typ string, ts float64, name string, index int, status string, urlstr string, err *SynthError) *SynthEvent {
-		return &SynthEvent{
-			Type:                 typ,
-			TimestampEpochMicros: 1000 + ts,
-			PackageVersion:       "1.0.0",
-			Step:                 &Step{Name: name, Index: index, Status: status},
-			Error:                err,
-			Payload:              common.MapStr{},
-			URL:                  urlstr,
-		}
-	}
 	url1 := "http://example.net/url1"
 	url2 := "http://example.net/url2"
 	url3 := "http://example.net/url3"
@@ -213,3 +214,75 @@ func TestEnrichSynthEvent(t *testing.T) {
 		})
 	}
 }
+
+func TestNoSummaryOnAfterHook(t *testing.T) {
+	journey := &Journey{
+		Name: "A journey that fails after completing",
+		Id:   "my-bad-after-all-hook",
+	}
+	journeyStart := &SynthEvent{
+		Type:                 "journey/start",
+		TimestampEpochMicros: 1000,
+		PackageVersion:       "1.0.0",
+		Journey:              journey,
+		Payload:              common.MapStr{},
+	}
+	syntherr := &SynthError{
+		Message: "my-errmsg",
+		Name:    "my-errname",
+		Stack:   "my\nerr\nstack",
+	}
+	journeyEnd := &SynthEvent{
+		Type:                 "journey/end",
+		TimestampEpochMicros: 2000,
+		PackageVersion:       "1.0.0",
+		Journey:              journey,
+		Payload:              common.MapStr{},
+	}
+	cmdStatus := &SynthEvent{
+		Type:                 "cmd/status",
+		Error:                &SynthError{Name: "cmdexit", Message: "cmd err msg"},
+		TimestampEpochMicros: 3000,
+	}
+
+	badStepUrl := "https://example.com/bad-step"
+	synthEvents := []*SynthEvent{
+		journeyStart,
+		makeStepEvent("step/start", 10, "Step1", 1, "", "", nil),
+		makeStepEvent("step/end", 20, "Step1", 1, "failed", badStepUrl, syntherr),
+		journeyEnd,
+		cmdStatus,
+	}
+
+	je := &journeyEnricher{}
+
+	for idx, se := range synthEvents {
+		e := &beat.Event{}
+		t.Run(fmt.Sprintf("event %d", idx), func(t *testing.T) {
+			enrichErr := je.enrich(e, se)
+
+			if se != nil && se.Type == "cmd/status" {
+				t.Run("no summary in cmd/status", func(t *testing.T) {
+					require.NotContains(t, e.Fields, "summary")
+				})
+			}
+
+			// Only the journey/end event should get a summary when
+			// it's emitted before the cmd/status (when an afterX hook fails).
+			if se != nil && se.Type == "journey/end" {
+				require.Equal(t, stepError(syntherr), enrichErr)
+
+				u, _ := url.Parse(badStepUrl)
+				t.Run("summary in journey/end", func(t *testing.T) {
+					v := lookslike.MustCompile(common.MapStr{
+						"synthetics.type":     "heartbeat/summary",
+						"url":                 wrappers.URLFields(u),
+						"monitor.duration.us": int64(journeyEnd.Timestamp().Sub(journeyStart.Timestamp()) / time.Microsecond),
+					})
+
+					testslike.Test(t, v, e.Fields)
+				})
+			}
+		})
+	}
+}