Skip to content

Commit 2371595

Browse files
test(metricbeat): fix flaky NTP module unit test TestFetchOffset_Success (#49533)
CapturingReporterV2 was not thread-safe, so concurrent appends from metricsets like NTP (multiple servers) could drop events.
1 parent f6662a9 commit 2371595

1 file changed

Lines changed: 21 additions & 9 deletions

File tree

metricbeat/mb/testing/modules.go

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ package testing
5858

5959
import (
6060
"context"
61+
"sync"
6162
"testing"
6263
"time"
6364

@@ -199,31 +200,42 @@ func NewReportingMetricSetV2WithContext(t testing.TB, config interface{}) mb.Rep
199200
return reportingMetricSet
200201
}
201202

202-
// CapturingReporterV2 is a reporter used for testing which stores all events and errors
203+
// CapturingReporterV2 is a reporter used for testing which stores all events and errors.
204+
// It is safe for concurrent use by multiple goroutines (e.g. metricsets that fetch from
205+
// multiple servers in parallel).
203206
type CapturingReporterV2 struct {
207+
mu sync.Mutex
204208
events []mb.Event
205209
errs []error
206210
}
207211

208212
// Event is used to report an event
209213
func (r *CapturingReporterV2) Event(event mb.Event) bool {
214+
r.mu.Lock()
215+
defer r.mu.Unlock()
210216
r.events = append(r.events, event)
211217
return true
212218
}
213219

214220
// Error is used to report an error
215221
func (r *CapturingReporterV2) Error(err error) bool {
222+
r.mu.Lock()
223+
defer r.mu.Unlock()
216224
r.errs = append(r.errs, err)
217225
return true
218226
}
219227

220228
// GetEvents returns all reported events
221229
func (r *CapturingReporterV2) GetEvents() []mb.Event {
230+
r.mu.Lock()
231+
defer r.mu.Unlock()
222232
return r.events
223233
}
224234

225235
// GetErrors returns all reported errors
226236
func (r *CapturingReporterV2) GetErrors() []error {
237+
r.mu.Lock()
238+
defer r.mu.Unlock()
227239
return r.errs
228240
}
229241

@@ -232,7 +244,7 @@ func (r *CapturingReporterV2) GetErrors() []error {
232244
func ReportingFetchV2(metricSet mb.ReportingMetricSetV2) ([]mb.Event, []error) {
233245
r := &CapturingReporterV2{}
234246
metricSet.Fetch(r)
235-
return r.events, r.errs
247+
return r.GetEvents(), r.GetErrors()
236248
}
237249

238250
// ReportingFetchV2Error runs the given reporting metricset and returns all of the
@@ -241,9 +253,9 @@ func ReportingFetchV2Error(metricSet mb.ReportingMetricSetV2Error) ([]mb.Event,
241253
r := &CapturingReporterV2{}
242254
err := metricSet.Fetch(r)
243255
if err != nil {
244-
r.errs = append(r.errs, err)
256+
r.Error(err)
245257
}
246-
return r.events, r.errs
258+
return r.GetEvents(), r.GetErrors()
247259
}
248260

249261
// PeriodicReportingFetchV2Error runs the given metricset and returns
@@ -263,11 +275,11 @@ func PeriodicReportingFetchV2Error(metricSet mb.ReportingMetricSetV2Error, perio
263275
// Fetch the metrics and store them in the
264276
// reporter.
265277
if err := metricSet.Fetch(r); err != nil {
266-
r.errs = append(r.errs, err)
278+
r.Error(err)
267279
return err
268280
}
269281

270-
if len(r.events) > 0 {
282+
if len(r.GetEvents()) > 0 {
271283
// We have metrics, stop the periodic
272284
// and return the metrics.
273285
cancel()
@@ -278,7 +290,7 @@ func PeriodicReportingFetchV2Error(metricSet mb.ReportingMetricSetV2Error, perio
278290
return nil
279291
})
280292

281-
return r.events, r.errs
293+
return r.GetEvents(), r.GetErrors()
282294
}
283295

284296
// ReportingFetchV2WithContext runs the given reporting metricset and returns all of the
@@ -287,9 +299,9 @@ func ReportingFetchV2WithContext(metricSet mb.ReportingMetricSetV2WithContext) (
287299
r := &CapturingReporterV2{}
288300
err := metricSet.Fetch(context.Background(), r)
289301
if err != nil {
290-
r.errs = append(r.errs, err)
302+
r.Error(err)
291303
}
292-
return r.events, r.errs
304+
return r.GetEvents(), r.GetErrors()
293305
}
294306

295307
// NewPushMetricSetV2 instantiates a new PushMetricSetV2 using the given

0 commit comments

Comments
 (0)