Skip to content

Commit

Permalink
Log errors for expvar query for benchtest (#13778)
Browse files Browse the repository at this point in the history
  • Loading branch information
lahsivjar authored Jul 29, 2024
1 parent 90e03ef commit ea74820
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 10 deletions.
15 changes: 13 additions & 2 deletions systemtest/benchtest/expvar/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import (
"errors"
"sync"
"time"

"go.uber.org/zap"
)

type Metric int
Expand Down Expand Up @@ -71,15 +73,23 @@ type Collector struct {
metrics map[Metric]AggregateStats
watches map[Metric]watchItem
stopped bool

logger *zap.Logger
}

// StartNewCollector creates a new collector and starts
// querying expvar endpoint at the specified interval.
func StartNewCollector(ctx context.Context, serverURL string, period time.Duration) (*Collector, error) {
func StartNewCollector(
ctx context.Context,
serverURL string,
period time.Duration,
logger *zap.Logger,
) (*Collector, error) {
c := &Collector{
metrics: make(map[Metric]AggregateStats),
watches: make(map[Metric]watchItem),
stopped: false,
logger: logger,
}
return c, c.start(ctx, serverURL, period)
}
Expand Down Expand Up @@ -215,7 +225,8 @@ func (c *Collector) start(ctx context.Context, serverURL string, period time.Dur
select {
case <-ctx.Done():
return
case <-errChan:
case err := <-errChan:
c.logger.Warn("failed while querying expvar", zap.Error(err))
return
case m := <-outChan:
c.accumulate(m)
Expand Down
13 changes: 7 additions & 6 deletions systemtest/benchtest/expvar/metrics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,14 @@ import (
"time"

"github.com/stretchr/testify/assert"
"go.uber.org/zap/zaptest"
)

func TestStart(t *testing.T) {
serverURL := setupTestServer(t, 1, make(chan bool, 1))
ctx, cancel := context.WithTimeout(context.Background(), time.Second)
defer cancel()
c, err := StartNewCollector(ctx, serverURL, 10*time.Second)
c, err := StartNewCollector(ctx, serverURL, 10*time.Second, zaptest.NewLogger(t))
assert.NoError(t, err)
assert.Equal(t, int64(1), c.Get(Goroutines).samples)
assert.Equal(t, int64(1), c.Get(Goroutines).First)
Expand All @@ -45,7 +46,7 @@ func TestAggregate(t *testing.T) {
serverURL := setupTestServer(t, samples, done)
ctx, cancel := context.WithTimeout(context.Background(), time.Second)
defer cancel()
c, err := StartNewCollector(ctx, serverURL, 10*time.Millisecond)
c, err := StartNewCollector(ctx, serverURL, 10*time.Millisecond, zaptest.NewLogger(t))
assert.NoError(t, err)
assert.True(t, <-done)
stats := c.Get(Goroutines)
Expand All @@ -61,7 +62,7 @@ func TestWatchMetric(t *testing.T) {
serverURL := setupTestServer(t, 5, make(chan bool, 1))
ctx, cancel := context.WithTimeout(context.Background(), time.Second)
defer cancel()
c, err := StartNewCollector(ctx, serverURL, 10*time.Millisecond)
c, err := StartNewCollector(ctx, serverURL, 10*time.Millisecond, zaptest.NewLogger(t))
assert.NoError(t, err)
watcher, err := c.WatchMetric(Goroutines, int64(5))
assert.NoError(t, err)
Expand All @@ -77,7 +78,7 @@ func TestWatchMetric(t *testing.T) {
func TestWatchMetricFail(t *testing.T) {
serverURL := setupTestServer(t, 1, make(chan bool, 1))
ctx, cancel := context.WithTimeout(context.Background(), time.Second)
c, err := StartNewCollector(ctx, serverURL, 10*time.Millisecond)
c, err := StartNewCollector(ctx, serverURL, 10*time.Millisecond, zaptest.NewLogger(t))
assert.NoError(t, err)
cancel()
<-time.After(100 * time.Millisecond)
Expand All @@ -88,7 +89,7 @@ func TestWatchMetricFail(t *testing.T) {
func TestWatchMetricFalse(t *testing.T) {
serverURL := setupTestServer(t, 1, make(chan bool, 1))
ctx, cancel := context.WithTimeout(context.Background(), time.Second)
c, err := StartNewCollector(ctx, serverURL, 10*time.Millisecond)
c, err := StartNewCollector(ctx, serverURL, 10*time.Millisecond, zaptest.NewLogger(t))
assert.NoError(t, err)
watcher, err := c.WatchMetric(Goroutines, 20)
assert.NoError(t, err)
Expand All @@ -108,7 +109,7 @@ func TestWatchMetricNonBlocking(t *testing.T) {
timeout := 100 * time.Millisecond
ctx, cancel := context.WithTimeout(context.Background(), timeout)
defer cancel()
c, err := StartNewCollector(ctx, serverURL, 10*time.Millisecond)
c, err := StartNewCollector(ctx, serverURL, 10*time.Millisecond, zaptest.NewLogger(t))
assert.NoError(t, err)
watcher, err := c.WatchMetric(Goroutines, int64(5))
assert.NoError(t, err)
Expand Down
3 changes: 2 additions & 1 deletion systemtest/benchtest/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import (
"time"

"go.elastic.co/apm/v2/stacktrace"
"go.uber.org/zap/zaptest"
"golang.org/x/time/rate"

"github.com/elastic/apm-perf/loadgen"
Expand Down Expand Up @@ -65,7 +66,7 @@ func runBenchmark(f BenchmarkFunc) (testing.BenchmarkResult, bool, bool, error)
defer cancel()
var err error
server := loadgencfg.Config.ServerURL.String()
collector, err = expvar.StartNewCollector(ctx, server, 100*time.Millisecond)
collector, err = expvar.StartNewCollector(ctx, server, 100*time.Millisecond, zaptest.NewLogger(b))
if err != nil {
b.Error(err)
failed = b.Failed()
Expand Down
3 changes: 2 additions & 1 deletion systemtest/benchtest/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
"time"

"github.com/stretchr/testify/assert"
"go.uber.org/zap/zaptest"

"github.com/elastic/apm-server/systemtest/benchtest/expvar"
)
Expand Down Expand Up @@ -173,7 +174,7 @@ func TestAddExpvarMetrics(t *testing.T) {
server := getTestServer(t, tt.responseMetrics, tt.memstatsMetrics)
defer server.Close()
ctx, cancel := context.WithTimeout(context.Background(), time.Second)
collector, err := expvar.StartNewCollector(ctx, server.URL, 10*time.Millisecond)
collector, err := expvar.StartNewCollector(ctx, server.URL, 10*time.Millisecond, zaptest.NewLogger(t))
assert.NoError(t, err)
<-time.After(100 * time.Millisecond)
cancel()
Expand Down

0 comments on commit ea74820

Please sign in to comment.