From cc72e06fe55bc12bc607bb392a7648f733b21e42 Mon Sep 17 00:00:00 2001
From: emilioalvap <emilio.alvarezpineiro@elastic.co>
Date: Wed, 18 Oct 2023 21:53:33 +0200
Subject: [PATCH 01/11] Fix monitor duration calc and remove delay from elapsed
 time

---
 CHANGELOG.next.asciidoc                             |  1 +
 .../monitors/wrappers/summarizer/plugmondur.go      | 13 ++++++++++---
 .../monitors/wrappers/summarizer/summarizer.go      |  2 +-
 x-pack/heartbeat/heartbeat.yml                      | 13 ++++++++-----
 4 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc
index 1fd25b84201..2dd20d09e72 100644
--- a/CHANGELOG.next.asciidoc
+++ b/CHANGELOG.next.asciidoc
@@ -117,6 +117,7 @@ is collected by it.
 
 - Fix panics when parsing dereferencing invalid parsed url. {pull}34702[34702]
 - Fix retries to trigger on a down monitor with no previous state. {pull}36842[36842]
+- Fix monitor duration calculation with retries. {pull}36842[36842]
 
 *Metricbeat*
 
diff --git a/heartbeat/monitors/wrappers/summarizer/plugmondur.go b/heartbeat/monitors/wrappers/summarizer/plugmondur.go
index f677e57693f..171e3870269 100644
--- a/heartbeat/monitors/wrappers/summarizer/plugmondur.go
+++ b/heartbeat/monitors/wrappers/summarizer/plugmondur.go
@@ -33,8 +33,7 @@ type LightweightDurationPlugin struct {
 func (lwdsp *LightweightDurationPlugin) EachEvent(event *beat.Event, _ error) EachEventActions {
 	// Effectively only runs once, on the first event
 	if lwdsp.startedAt == nil {
-		now := time.Now()
-		lwdsp.startedAt = &now
+		lwdsp.setEventStartAt()
 	}
 	return 0
 }
@@ -44,7 +43,15 @@ func (lwdsp *LightweightDurationPlugin) BeforeSummary(event *beat.Event) BeforeS
 	return 0
 }
 
-func (lwdsp *LightweightDurationPlugin) BeforeRetry() {}
+func (lwdsp *LightweightDurationPlugin) BeforeRetry() {
+	// Reset event.startAt
+	lwdsp.startedAt = nil
+}
+
+func (lwdsp *LightweightDurationPlugin) setEventStartAt() {
+	now := time.Now()
+	lwdsp.startedAt = &now
+}
 
 // BrowserDurationPlugin handles the logic for writing the `monitor.duration.us` field
 // for browser monitors.
diff --git a/heartbeat/monitors/wrappers/summarizer/summarizer.go b/heartbeat/monitors/wrappers/summarizer/summarizer.go
index 9c3f1bd8abd..cca3fada663 100644
--- a/heartbeat/monitors/wrappers/summarizer/summarizer.go
+++ b/heartbeat/monitors/wrappers/summarizer/summarizer.go
@@ -145,10 +145,10 @@ func (s *Summarizer) Wrap(j jobs.Job) jobs.Job {
 				//    kibana queries
 				// 2. If the site error is very short 1s gives it a tiny bit of time to recover
 				delayedRootJob := func(event *beat.Event) ([]jobs.Job, error) {
+					time.Sleep(s.retryDelay)
 					for _, p := range s.plugins {
 						p.BeforeRetry()
 					}
-					time.Sleep(s.retryDelay)
 					return s.rootJob(event)
 				}
 
diff --git a/x-pack/heartbeat/heartbeat.yml b/x-pack/heartbeat/heartbeat.yml
index 76876b9c7ff..d11f77e8b72 100644
--- a/x-pack/heartbeat/heartbeat.yml
+++ b/x-pack/heartbeat/heartbeat.yml
@@ -23,7 +23,7 @@ heartbeat.config.monitors:
 heartbeat.monitors:
 - type: http
   # Set enabled to true (or delete the following line) to enable this monitor
-  enabled: false
+  enabled: true
   # ID used to uniquely identify this monitor in Elasticsearch even if the config changes
   id: my-monitor
   # Human readable display name for this service in Uptime UI and elsewhere
@@ -31,7 +31,8 @@ heartbeat.monitors:
   # List of URLs to query
   urls: ["http://localhost:9200"]
   # Configure task schedule
-  schedule: '@every 10s'
+  schedule: '@every 10000s'
+  max_attempts: 2
   # Total test connection and data exchange timeout
   #timeout: 16s
   # Name of corresponding APM service, if Elastic APM is in use for the monitored service.
@@ -98,9 +99,11 @@ setup.kibana:
 # Configure what output to use when sending the data collected by the beat.
 
 # ---------------------------- Elasticsearch Output ----------------------------
-output.elasticsearch:
-  # Array of hosts to connect to.
-  hosts: ["localhost:9200"]
+# output.elasticsearch:
+#   # Array of hosts to connect to.
+#   hosts: ["localhost:9200"]
+output.console:
+  pretty: true
 
   # Protocol - either `http` (default) or `https`.
   #protocol: "https"

From 19cf7ffa8885674fb77c716e6e76d8c24b76b28a Mon Sep 17 00:00:00 2001
From: emilioalvap <emilio.alvarezpineiro@elastic.co>
Date: Wed, 18 Oct 2023 22:57:12 +0200
Subject: [PATCH 02/11] Fix monitor duration calc v2

---
 .../monitors/wrappers/summarizer/plugdrop.go  |  4 ++++
 .../monitors/wrappers/summarizer/plugerr.go   |  6 ++++++
 .../wrappers/summarizer/plugmondur.go         | 20 +++++++++----------
 .../wrappers/summarizer/plugstatestat.go      |  4 ++++
 .../monitors/wrappers/summarizer/plugurl.go   |  2 ++
 .../wrappers/summarizer/summarizer.go         | 17 ++++++++++++++--
 x-pack/heartbeat/heartbeat.yml                |  2 +-
 7 files changed, 42 insertions(+), 13 deletions(-)

diff --git a/heartbeat/monitors/wrappers/summarizer/plugdrop.go b/heartbeat/monitors/wrappers/summarizer/plugdrop.go
index fff6c143bf0..a4ddc61abe7 100644
--- a/heartbeat/monitors/wrappers/summarizer/plugdrop.go
+++ b/heartbeat/monitors/wrappers/summarizer/plugdrop.go
@@ -43,3 +43,7 @@ func (d DropBrowserExtraEvents) BeforeSummary(event *beat.Event) BeforeSummaryAc
 func (d DropBrowserExtraEvents) BeforeRetry() {
 	// noop
 }
+
+func (d DropBrowserExtraEvents) BeforeEachEvent(event *beat.Event) {
+	// noop
+}
diff --git a/heartbeat/monitors/wrappers/summarizer/plugerr.go b/heartbeat/monitors/wrappers/summarizer/plugerr.go
index 1010370f520..83ab6de4f5a 100644
--- a/heartbeat/monitors/wrappers/summarizer/plugerr.go
+++ b/heartbeat/monitors/wrappers/summarizer/plugerr.go
@@ -46,6 +46,8 @@ func NewBrowserErrPlugin() *BrowserErrPlugin {
 	}
 }
 
+func (esp *BrowserErrPlugin) BeforeEachEvent(event *beat.Event) {} // noop
+
 func (esp *BrowserErrPlugin) EachEvent(event *beat.Event, eventErr error) EachEventActions {
 	// track these to determine if the journey
 	// needs an error injected due to incompleteness
@@ -127,6 +129,10 @@ func (esp *LightweightErrPlugin) BeforeRetry() {
 	// noop
 }
 
+func (esp *LightweightErrPlugin) BeforeEachEvent(event *beat.Event) {
+	// noop
+}
+
 // errToFieldVal reflects on the error and returns either an *ecserr.ECSErr if possible, and a look.Reason otherwise
 func errToFieldVal(eventErr error) (errVal interface{}) {
 	var asECS *ecserr.ECSErr
diff --git a/heartbeat/monitors/wrappers/summarizer/plugmondur.go b/heartbeat/monitors/wrappers/summarizer/plugmondur.go
index 171e3870269..78627b8c79d 100644
--- a/heartbeat/monitors/wrappers/summarizer/plugmondur.go
+++ b/heartbeat/monitors/wrappers/summarizer/plugmondur.go
@@ -31,11 +31,15 @@ type LightweightDurationPlugin struct {
 }
 
 func (lwdsp *LightweightDurationPlugin) EachEvent(event *beat.Event, _ error) EachEventActions {
-	// Effectively only runs once, on the first event
+	return 0 // noop
+}
+
+func (lwdsp *LightweightDurationPlugin) BeforeEachEvent(event *beat.Event) {
+	// Effectively capture on the first event, on the first event
 	if lwdsp.startedAt == nil {
-		lwdsp.setEventStartAt()
+		now := time.Now()
+		lwdsp.startedAt = &now
 	}
-	return 0
 }
 
 func (lwdsp *LightweightDurationPlugin) BeforeSummary(event *beat.Event) BeforeSummaryActions {
@@ -44,15 +48,10 @@ func (lwdsp *LightweightDurationPlugin) BeforeSummary(event *beat.Event) BeforeS
 }
 
 func (lwdsp *LightweightDurationPlugin) BeforeRetry() {
-	// Reset event.startAt
+	// Reset event start time
 	lwdsp.startedAt = nil
 }
 
-func (lwdsp *LightweightDurationPlugin) setEventStartAt() {
-	now := time.Now()
-	lwdsp.startedAt = &now
-}
-
 // BrowserDurationPlugin handles the logic for writing the `monitor.duration.us` field
 // for browser monitors.
 type BrowserDurationPlugin struct {
@@ -89,4 +88,5 @@ func (bwdsp *BrowserDurationPlugin) BeforeSummary(event *beat.Event) BeforeSumma
 	return 0
 }
 
-func (bwdsp *BrowserDurationPlugin) BeforeRetry() {}
+func (bwdsp *BrowserDurationPlugin) BeforeRetry()                      {}
+func (bwdsp *BrowserDurationPlugin) BeforeEachEvent(event *beat.Event) {} // noop
diff --git a/heartbeat/monitors/wrappers/summarizer/plugstatestat.go b/heartbeat/monitors/wrappers/summarizer/plugstatestat.go
index 4acfee4dc36..cf7e90af5f3 100644
--- a/heartbeat/monitors/wrappers/summarizer/plugstatestat.go
+++ b/heartbeat/monitors/wrappers/summarizer/plugstatestat.go
@@ -74,6 +74,8 @@ func (ssp *BrowserStateStatusPlugin) BeforeRetry() {
 	ssp.cssp.BeforeRetry()
 }
 
+func (ssp *BrowserStateStatusPlugin) BeforeEachEvent(event *beat.Event) {} //noop
+
 // LightweightStateStatusPlugin encapsulates the writing of the primary fields used by the summary,
 // those being `state.*`, `status.*` , `event.type`, and `monitor.check_group`
 type LightweightStateStatusPlugin struct {
@@ -113,6 +115,8 @@ func (ssp *LightweightStateStatusPlugin) BeforeRetry() {
 	ssp.cssp.BeforeRetry()
 }
 
+func (ssp *LightweightStateStatusPlugin) BeforeEachEvent(event *beat.Event) {} // noop
+
 type commonSSP struct {
 	js           *jobsummary.JobSummary
 	stateTracker *monitorstate.Tracker
diff --git a/heartbeat/monitors/wrappers/summarizer/plugurl.go b/heartbeat/monitors/wrappers/summarizer/plugurl.go
index dc4394aa42a..e47463575a3 100644
--- a/heartbeat/monitors/wrappers/summarizer/plugurl.go
+++ b/heartbeat/monitors/wrappers/summarizer/plugurl.go
@@ -52,3 +52,5 @@ func (busp *BrowserURLPlugin) BeforeSummary(event *beat.Event) BeforeSummaryActi
 func (busp *BrowserURLPlugin) BeforeRetry() {
 	busp.urlFields = nil
 }
+
+func (busp *BrowserURLPlugin) BeforeEachEvent(event *beat.Event) {} //noop
diff --git a/heartbeat/monitors/wrappers/summarizer/summarizer.go b/heartbeat/monitors/wrappers/summarizer/summarizer.go
index cca3fada663..ad0902d45af 100644
--- a/heartbeat/monitors/wrappers/summarizer/summarizer.go
+++ b/heartbeat/monitors/wrappers/summarizer/summarizer.go
@@ -42,6 +42,12 @@ type Summarizer struct {
 	startedAt      time.Time
 }
 
+func (s Summarizer) beforeEachEvent(event *beat.Event) {
+	for _, plugin := range s.plugins {
+		plugin.BeforeEachEvent(event)
+	}
+}
+
 // EachEventActions is a set of options using bitmasks to inform execution after the EachEvent callback
 type EachEventActions uint8
 
@@ -58,6 +64,9 @@ const RetryBeforeSummary = 1
 // in one location. Prior to this code was strewn about a bit more and following it was
 // a bit trickier.
 type SummarizerPlugin interface {
+	// BeforeEachEvent is called on each event, and allows for the mutation of events
+	// before monitor execution
+	BeforeEachEvent(event *beat.Event)
 	// EachEvent is called on each event, and allows for the mutation of events
 	EachEvent(event *beat.Event, err error) EachEventActions
 	// BeforeSummary is run on the final (summary) event for each monitor.
@@ -106,6 +115,10 @@ func (s *Summarizer) setupPlugins() {
 // This adds the state and summary top level fields.
 func (s *Summarizer) Wrap(j jobs.Job) jobs.Job {
 	return func(event *beat.Event) ([]jobs.Job, error) {
+
+		// call BeforeEachEvent for each plugin before running job
+		s.beforeEachEvent(event)
+
 		conts, eventErr := j(event)
 
 		s.mtx.Lock()
@@ -149,10 +162,10 @@ func (s *Summarizer) Wrap(j jobs.Job) jobs.Job {
 					for _, p := range s.plugins {
 						p.BeforeRetry()
 					}
-					return s.rootJob(event)
+					return s.Wrap(s.rootJob)(event)
 				}
 
-				conts = []jobs.Job{delayedRootJob}
+				return []jobs.Job{delayedRootJob}, eventErr
 			}
 		}
 
diff --git a/x-pack/heartbeat/heartbeat.yml b/x-pack/heartbeat/heartbeat.yml
index d11f77e8b72..4e638690721 100644
--- a/x-pack/heartbeat/heartbeat.yml
+++ b/x-pack/heartbeat/heartbeat.yml
@@ -23,7 +23,7 @@ heartbeat.config.monitors:
 heartbeat.monitors:
 - type: http
   # Set enabled to true (or delete the following line) to enable this monitor
-  enabled: true
+  enabled: false
   # ID used to uniquely identify this monitor in Elasticsearch even if the config changes
   id: my-monitor
   # Human readable display name for this service in Uptime UI and elsewhere

From 1942943bd5af65d4ab763def14994e993c251ba8 Mon Sep 17 00:00:00 2001
From: emilioalvap <emilio.alvarezpineiro@elastic.co>
Date: Wed, 18 Oct 2023 23:07:31 +0200
Subject: [PATCH 03/11] Update changelog

---
 CHANGELOG.next.asciidoc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc
index 2dd20d09e72..c64bf673a4c 100644
--- a/CHANGELOG.next.asciidoc
+++ b/CHANGELOG.next.asciidoc
@@ -117,7 +117,7 @@ is collected by it.
 
 - Fix panics when parsing dereferencing invalid parsed url. {pull}34702[34702]
 - Fix retries to trigger on a down monitor with no previous state. {pull}36842[36842]
-- Fix monitor duration calculation with retries. {pull}36842[36842]
+- Fix monitor duration calculation with retries. {pull}36900[36900]
 
 *Metricbeat*
 

From 4a6a8338f5a8490acce4827dd267f4a334932cec Mon Sep 17 00:00:00 2001
From: emilioalvap <emilio.alvarezpineiro@elastic.co>
Date: Wed, 18 Oct 2023 23:09:09 +0200
Subject: [PATCH 04/11] Reset heartbeat.yaml

---
 x-pack/heartbeat/heartbeat.yml | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/x-pack/heartbeat/heartbeat.yml b/x-pack/heartbeat/heartbeat.yml
index 4e638690721..76876b9c7ff 100644
--- a/x-pack/heartbeat/heartbeat.yml
+++ b/x-pack/heartbeat/heartbeat.yml
@@ -31,8 +31,7 @@ heartbeat.monitors:
   # List of URLs to query
   urls: ["http://localhost:9200"]
   # Configure task schedule
-  schedule: '@every 10000s'
-  max_attempts: 2
+  schedule: '@every 10s'
   # Total test connection and data exchange timeout
   #timeout: 16s
   # Name of corresponding APM service, if Elastic APM is in use for the monitored service.
@@ -99,11 +98,9 @@ setup.kibana:
 # Configure what output to use when sending the data collected by the beat.
 
 # ---------------------------- Elasticsearch Output ----------------------------
-# output.elasticsearch:
-#   # Array of hosts to connect to.
-#   hosts: ["localhost:9200"]
-output.console:
-  pretty: true
+output.elasticsearch:
+  # Array of hosts to connect to.
+  hosts: ["localhost:9200"]
 
   # Protocol - either `http` (default) or `https`.
   #protocol: "https"

From ea75973a61f305109664b8e9882d4452664c504d Mon Sep 17 00:00:00 2001
From: emilioalvap <emilio.alvarezpineiro@elastic.co>
Date: Thu, 19 Oct 2023 18:50:53 +0200
Subject: [PATCH 05/11] Add plugin order and monitor duration tests

---
 .../wrappers/summarizer/summarizer_test.go    | 181 ++++++++++++++++++
 1 file changed, 181 insertions(+)

diff --git a/heartbeat/monitors/wrappers/summarizer/summarizer_test.go b/heartbeat/monitors/wrappers/summarizer/summarizer_test.go
index 2a94b3e6f59..9b7f82d4bb3 100644
--- a/heartbeat/monitors/wrappers/summarizer/summarizer_test.go
+++ b/heartbeat/monitors/wrappers/summarizer/summarizer_test.go
@@ -19,11 +19,13 @@ package summarizer
 
 import (
 	"fmt"
+	"sync"
 	"testing"
 	"time"
 
 	"github.com/stretchr/testify/require"
 
+	"github.com/elastic/beats/v7/heartbeat/look"
 	"github.com/elastic/beats/v7/heartbeat/monitors/jobs"
 	"github.com/elastic/beats/v7/heartbeat/monitors/stdfields"
 	"github.com/elastic/beats/v7/heartbeat/monitors/wrappers/monitorstate"
@@ -219,3 +221,182 @@ func TestSummarizer(t *testing.T) {
 		})
 	}
 }
+
+// Test wrapper plugin hook order. Guaranteed order for plugins to be called upon determines
+// what data can be appended to the event at each stage through retries. With this guarentee,
+// plugins just need to ascertain that their invariants apply through hook execution order
+func TestSummarizerPluginOrder(t *testing.T) {
+	t.Parallel()
+
+	// these tests use strings to describe sequences of events
+	tests := []struct {
+		name        string
+		maxAttempts int
+		// the attempt number of the given event
+		expectedOrder []string
+	}{
+		{
+			"one attempt",
+			1,
+			[]string{"bee", "job", "ee", "bs"},
+		},
+		{
+			"two attempts",
+			2,
+			[]string{"bee", "job", "ee", "bs", "br", "bee", "job", "ee", "bs"},
+		},
+	}
+
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			// Monitor setup
+			tracker := monitorstate.NewTracker(monitorstate.NilStateLoader, false)
+			sf := stdfields.StdMonitorFields{ID: "testmon", Name: "testmon", Type: "http", MaxAttempts: uint16(tt.maxAttempts)}
+
+			// Test locals
+			calls := make(chan string, 100)
+			mtx := sync.Mutex{}
+			appendCall := func(event string) {
+				mtx.Lock()
+				defer mtx.Unlock()
+
+				// Append call to global chan
+				calls <- event
+			}
+
+			// We simplify these to always down since hook order should not be
+			// determined by status
+			job := func(event *beat.Event) (j []jobs.Job, retErr error) {
+
+				calls <- "job"
+
+				event.Fields = mapstr.M{
+					"monitor": mapstr.M{
+						"id":     "test",
+						"status": string(monitorstate.StatusDown),
+					},
+				}
+
+				return nil, fmt.Errorf("dummyerr")
+			}
+
+			s := NewSummarizer(job, sf, tracker)
+			// Shorten retry delay to make tests run faster
+			s.retryDelay = 2 * time.Millisecond
+			// Add mock plugin
+			s.plugins = append(s.plugins, &MockPlugin{
+				eachEvent: func(_ *beat.Event, _ error) {
+					appendCall("ee")
+				},
+				beforeSummary: func(_ *beat.Event) {
+					appendCall("bs")
+				},
+				beforeRetry: func() {
+					appendCall("br")
+				},
+				beforeEachEvent: func(_ *beat.Event) {
+					appendCall("bee")
+				},
+			})
+			wrapped := s.Wrap(job)
+
+			_, _ = jobs.ExecJobAndConts(t, wrapped)
+
+			close(calls)
+
+			// gather order
+			rcvdOrder := []string{}
+			for c := range calls {
+				rcvdOrder = append(rcvdOrder, c)
+			}
+
+			require.Equal(t, tt.expectedOrder, rcvdOrder)
+			require.Len(t, rcvdOrder, len(tt.expectedOrder))
+		})
+	}
+}
+
+func TestRetryLightweightMonitorDuration(t *testing.T) {
+	t.Parallel()
+
+	// Monitor setup
+	tracker := monitorstate.NewTracker(monitorstate.NilStateLoader, false)
+	sf := stdfields.StdMonitorFields{ID: "testmon", Name: "testmon", Type: "http", MaxAttempts: uint16(2)}
+
+	// We simplify these to always down since hook order should not be
+	// determined by status
+	job := func(event *beat.Event) (j []jobs.Job, retErr error) {
+		time.Sleep(1 * time.Second)
+
+		event.Fields = mapstr.M{
+			"monitor": mapstr.M{
+				"id":     "test",
+				"status": string(monitorstate.StatusDown),
+			},
+		}
+
+		return nil, fmt.Errorf("dummyerr")
+	}
+
+	var retryStart time.Time
+
+	s := NewSummarizer(job, sf, tracker)
+	// Shorten retry delay to make tests run faster
+	s.retryDelay = 2 * time.Millisecond
+	// Add mock plugin
+	s.plugins = append(s.plugins, &MockPlugin{
+		beforeRetry: func() {
+			retryStart = time.Now()
+		},
+		eachEvent:       func(_ *beat.Event, _ error) {},
+		beforeSummary:   func(_ *beat.Event) {},
+		beforeEachEvent: func(_ *beat.Event) {},
+	})
+	wrapped := s.Wrap(job)
+
+	events, _ := jobs.ExecJobAndConts(t, wrapped)
+
+	retryElapsed := time.Since(retryStart)
+	require.False(t, retryStart.IsZero())
+	var rcvdDuration interface{}
+	for _, event := range events {
+		summaryIface, _ := event.GetValue("summary")
+		summary := summaryIface.(*jobsummary.JobSummary)
+
+		if summary.FinalAttempt {
+			rcvdDuration, _ = event.GetValue("monitor.duration.us")
+		}
+	}
+	require.Greater(t, rcvdDuration, int64(0))
+	require.GreaterOrEqual(t, look.RTTMS(retryElapsed), rcvdDuration)
+}
+
+type MockPlugin struct {
+	eachEvent       func(e *beat.Event, err error)
+	beforeSummary   func(e *beat.Event)
+	beforeRetry     func()
+	beforeEachEvent func(e *beat.Event)
+}
+
+func (mp *MockPlugin) EachEvent(e *beat.Event, err error) EachEventActions {
+	mp.eachEvent(e, err)
+
+	return 0
+}
+
+func (mp *MockPlugin) BeforeSummary(e *beat.Event) BeforeSummaryActions {
+	mp.beforeSummary(e)
+
+	return 0
+}
+
+func (mp *MockPlugin) BeforeRetry() {
+	mp.beforeRetry()
+}
+
+func (mp *MockPlugin) BeforeEachEvent(e *beat.Event) {
+	mp.beforeEachEvent(e)
+}

From 078e1ef667e6536d7bced0058391d2fac82aaa92 Mon Sep 17 00:00:00 2001
From: emilioalvap <emilio.alvarezpineiro@elastic.co>
Date: Thu, 19 Oct 2023 18:54:50 +0200
Subject: [PATCH 06/11] Fix comment

---
 heartbeat/monitors/wrappers/summarizer/summarizer_test.go | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/heartbeat/monitors/wrappers/summarizer/summarizer_test.go b/heartbeat/monitors/wrappers/summarizer/summarizer_test.go
index 9b7f82d4bb3..d3f6d3740e3 100644
--- a/heartbeat/monitors/wrappers/summarizer/summarizer_test.go
+++ b/heartbeat/monitors/wrappers/summarizer/summarizer_test.go
@@ -326,8 +326,7 @@ func TestRetryLightweightMonitorDuration(t *testing.T) {
 	tracker := monitorstate.NewTracker(monitorstate.NilStateLoader, false)
 	sf := stdfields.StdMonitorFields{ID: "testmon", Name: "testmon", Type: "http", MaxAttempts: uint16(2)}
 
-	// We simplify these to always down since hook order should not be
-	// determined by status
+	// We simplify these to always down
 	job := func(event *beat.Event) (j []jobs.Job, retErr error) {
 		time.Sleep(1 * time.Second)
 

From b804eb8b1d7e6ae32884f925a517e13515498a59 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emilio=20Alvarez=20Pi=C3=B1eiro?=
 <95703246+emilioalvap@users.noreply.github.com>
Date: Mon, 23 Oct 2023 15:55:55 +0200
Subject: [PATCH 07/11] Update typo

---
 heartbeat/monitors/wrappers/summarizer/summarizer_test.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/heartbeat/monitors/wrappers/summarizer/summarizer_test.go b/heartbeat/monitors/wrappers/summarizer/summarizer_test.go
index d3f6d3740e3..04821e8fd37 100644
--- a/heartbeat/monitors/wrappers/summarizer/summarizer_test.go
+++ b/heartbeat/monitors/wrappers/summarizer/summarizer_test.go
@@ -223,7 +223,7 @@ func TestSummarizer(t *testing.T) {
 }
 
 // Test wrapper plugin hook order. Guaranteed order for plugins to be called upon determines
-// what data can be appended to the event at each stage through retries. With this guarentee,
+// what data can be appended to the event at each stage through retries. With this guarantee,
 // plugins just need to ascertain that their invariants apply through hook execution order
 func TestSummarizerPluginOrder(t *testing.T) {
 	t.Parallel()

From d3ec9edf5f867e3fec26784a9eeb5ecf7c9e3892 Mon Sep 17 00:00:00 2001
From: emilioalvap <emilio.alvarezpineiro@elastic.co>
Date: Tue, 24 Oct 2023 17:15:28 +0200
Subject: [PATCH 08/11] Update feedback

---
 heartbeat/monitors/wrappers/summarizer/summarizer_test.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/heartbeat/monitors/wrappers/summarizer/summarizer_test.go b/heartbeat/monitors/wrappers/summarizer/summarizer_test.go
index 04821e8fd37..b113dd4adcf 100644
--- a/heartbeat/monitors/wrappers/summarizer/summarizer_test.go
+++ b/heartbeat/monitors/wrappers/summarizer/summarizer_test.go
@@ -328,8 +328,6 @@ func TestRetryLightweightMonitorDuration(t *testing.T) {
 
 	// We simplify these to always down
 	job := func(event *beat.Event) (j []jobs.Job, retErr error) {
-		time.Sleep(1 * time.Second)
-
 		event.Fields = mapstr.M{
 			"monitor": mapstr.M{
 				"id":     "test",
@@ -370,6 +368,8 @@ func TestRetryLightweightMonitorDuration(t *testing.T) {
 		}
 	}
 	require.Greater(t, rcvdDuration, int64(0))
+	// Ensures monitor duration only takes into account the last attempt execution time
+	// by comparing it to the time spent after last retry started (retryElapsed)
 	require.GreaterOrEqual(t, look.RTTMS(retryElapsed), rcvdDuration)
 }
 

From df28aa730bb27ebf7f14677c7bdf316a079fe2fc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emilio=20Alvarez=20Pi=C3=B1eiro?=
 <95703246+emilioalvap@users.noreply.github.com>
Date: Tue, 24 Oct 2023 17:17:04 +0200
Subject: [PATCH 09/11] Apply suggestions from code review

---
 heartbeat/monitors/wrappers/summarizer/plugmondur.go      | 2 +-
 heartbeat/monitors/wrappers/summarizer/summarizer_test.go | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/heartbeat/monitors/wrappers/summarizer/plugmondur.go b/heartbeat/monitors/wrappers/summarizer/plugmondur.go
index 78627b8c79d..d71cc96ff2c 100644
--- a/heartbeat/monitors/wrappers/summarizer/plugmondur.go
+++ b/heartbeat/monitors/wrappers/summarizer/plugmondur.go
@@ -35,7 +35,7 @@ func (lwdsp *LightweightDurationPlugin) EachEvent(event *beat.Event, _ error) Ea
 }
 
 func (lwdsp *LightweightDurationPlugin) BeforeEachEvent(event *beat.Event) {
-	// Effectively capture on the first event, on the first event
+	// Effectively capture on the first event
 	if lwdsp.startedAt == nil {
 		now := time.Now()
 		lwdsp.startedAt = &now
diff --git a/heartbeat/monitors/wrappers/summarizer/summarizer_test.go b/heartbeat/monitors/wrappers/summarizer/summarizer_test.go
index b113dd4adcf..c2d5d37e3d9 100644
--- a/heartbeat/monitors/wrappers/summarizer/summarizer_test.go
+++ b/heartbeat/monitors/wrappers/summarizer/summarizer_test.go
@@ -232,7 +232,6 @@ func TestSummarizerPluginOrder(t *testing.T) {
 	tests := []struct {
 		name        string
 		maxAttempts int
-		// the attempt number of the given event
 		expectedOrder []string
 	}{
 		{

From 577a312c90cf78a85f49926f2bd733340b8d4651 Mon Sep 17 00:00:00 2001
From: emilioalvap <emilio.alvarezpineiro@elastic.co>
Date: Tue, 24 Oct 2023 17:30:22 +0200
Subject: [PATCH 10/11] Fix format

---
 heartbeat/monitors/wrappers/summarizer/summarizer_test.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/heartbeat/monitors/wrappers/summarizer/summarizer_test.go b/heartbeat/monitors/wrappers/summarizer/summarizer_test.go
index c2d5d37e3d9..7d74428ea22 100644
--- a/heartbeat/monitors/wrappers/summarizer/summarizer_test.go
+++ b/heartbeat/monitors/wrappers/summarizer/summarizer_test.go
@@ -230,8 +230,8 @@ func TestSummarizerPluginOrder(t *testing.T) {
 
 	// these tests use strings to describe sequences of events
 	tests := []struct {
-		name        string
-		maxAttempts int
+		name          string
+		maxAttempts   int
 		expectedOrder []string
 	}{
 		{

From b1a24b562952a5215fc6f483c5e8b51479e11b0e Mon Sep 17 00:00:00 2001
From: emilioalvap <emilio.alvarezpineiro@elastic.co>
Date: Tue, 24 Oct 2023 17:58:28 +0200
Subject: [PATCH 11/11] Fix windows time precision in test

---
 heartbeat/monitors/wrappers/summarizer/summarizer_test.go | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/heartbeat/monitors/wrappers/summarizer/summarizer_test.go b/heartbeat/monitors/wrappers/summarizer/summarizer_test.go
index 7d74428ea22..e579a649c8e 100644
--- a/heartbeat/monitors/wrappers/summarizer/summarizer_test.go
+++ b/heartbeat/monitors/wrappers/summarizer/summarizer_test.go
@@ -327,6 +327,10 @@ func TestRetryLightweightMonitorDuration(t *testing.T) {
 
 	// We simplify these to always down
 	job := func(event *beat.Event) (j []jobs.Job, retErr error) {
+
+		// some platforms don't have enough precision to track immediate monitors time
+		time.Sleep(100 * time.Millisecond)
+
 		event.Fields = mapstr.M{
 			"monitor": mapstr.M{
 				"id":     "test",