Require poll metrics to be registered (#2260)

ava-labs · Nov 3, 2023 · cec1cd1 · cec1cd1
1 parent e4cb2cd
commit cec1cd1
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 24 deletions.
diff --git a/snow/consensus/snowman/poll/set.go b/snow/consensus/snowman/poll/set.go
@@ -4,6 +4,7 @@
 package poll
 
 import (
+	"errors"
 	"fmt"
 	"strings"
 	"time"
@@ -19,6 +20,11 @@ import (
 	"github.com/ava-labs/avalanchego/utils/metric"
 )
 
+var (
+	errFailedPollsMetric         = errors.New("failed to register polls metric")
+	errFailedPollDurationMetrics = errors.New("failed to register poll_duration metrics")
+)
+
 type pollHolder interface {
 	GetPoll() Poll
 	StartTime() time.Time
@@ -52,16 +58,14 @@ func NewSet(
 	log logging.Logger,
 	namespace string,
 	reg prometheus.Registerer,
-) Set {
+) (Set, error) {
 	numPolls := prometheus.NewGauge(prometheus.GaugeOpts{
 		Namespace: namespace,
 		Name:      "polls",
 		Help:      "Number of pending network polls",
 	})
 	if err := reg.Register(numPolls); err != nil {
-		log.Error("failed to register polls statistics",
-			zap.Error(err),
-		)
+		return nil, fmt.Errorf("%w: %w", errFailedPollsMetric, err)
 	}
 
 	durPolls, err := metric.NewAverager(
@@ -71,9 +75,7 @@ func NewSet(
 		reg,
 	)
 	if err != nil {
-		log.Error("failed to register poll_duration statistics",
-			zap.Error(err),
-		)
+		return nil, fmt.Errorf("%w: %w", errFailedPollDurationMetrics, err)
 	}
 
 	return &set{
@@ -82,7 +84,7 @@ func NewSet(
 		durPolls: durPolls,
 		factory:  factory,
 		polls:    linkedhashmap.New[uint32, pollHolder](),
-	}
+	}, nil
 }
 
 // Add to the current set of polls

diff --git a/snow/consensus/snowman/poll/set_test.go b/snow/consensus/snowman/poll/set_test.go
@@ -28,7 +28,7 @@ var (
 	vdr5 = ids.NodeID{5}
 )
 
-func TestNewSetErrorOnMetrics(t *testing.T) {
+func TestNewSetErrorOnPollsMetrics(t *testing.T) {
 	require := require.New(t)
 
 	factory := NewEarlyTermNoTraversalFactory(1, 1)
@@ -37,13 +37,29 @@ func TestNewSetErrorOnMetrics(t *testing.T) {
 	registerer := prometheus.NewRegistry()
 
 	require.NoError(registerer.Register(prometheus.NewCounter(prometheus.CounterOpts{
-		Name: "polls",
+		Namespace: namespace,
+		Name:      "polls",
 	})))
+
+	_, err := NewSet(factory, log, namespace, registerer)
+	require.ErrorIs(err, errFailedPollsMetric)
+}
+
+func TestNewSetErrorOnPollDurationMetrics(t *testing.T) {
+	require := require.New(t)
+
+	factory := NewEarlyTermNoTraversalFactory(1, 1)
+	log := logging.NoLog{}
+	namespace := ""
+	registerer := prometheus.NewRegistry()
+
 	require.NoError(registerer.Register(prometheus.NewCounter(prometheus.CounterOpts{
-		Name: "poll_duration",
+		Namespace: namespace,
+		Name:      "poll_duration_count",
 	})))
 
-	require.NotNil(NewSet(factory, log, namespace, registerer))
+	_, err := NewSet(factory, log, namespace, registerer)
+	require.ErrorIs(err, errFailedPollDurationMetrics)
 }
 
 func TestCreateAndFinishPollOutOfOrder_NewerFinishesFirst(t *testing.T) {
@@ -56,7 +72,8 @@ func TestCreateAndFinishPollOutOfOrder_NewerFinishesFirst(t *testing.T) {
 	log := logging.NoLog{}
 	namespace := ""
 	registerer := prometheus.NewRegistry()
-	s := NewSet(factory, log, namespace, registerer)
+	s, err := NewSet(factory, log, namespace, registerer)
+	require.NoError(err)
 
 	// create two polls for the two blocks
 	vdrBag := bag.Of(vdrs...)
@@ -92,7 +109,8 @@ func TestCreateAndFinishPollOutOfOrder_OlderFinishesFirst(t *testing.T) {
 	log := logging.NoLog{}
 	namespace := ""
 	registerer := prometheus.NewRegistry()
-	s := NewSet(factory, log, namespace, registerer)
+	s, err := NewSet(factory, log, namespace, registerer)
+	require.NoError(err)
 
 	// create two polls for the two blocks
 	vdrBag := bag.Of(vdrs...)
@@ -128,7 +146,8 @@ func TestCreateAndFinishPollOutOfOrder_UnfinishedPollsGaps(t *testing.T) {
 	log := logging.NoLog{}
 	namespace := ""
 	registerer := prometheus.NewRegistry()
-	s := NewSet(factory, log, namespace, registerer)
+	s, err := NewSet(factory, log, namespace, registerer)
+	require.NoError(err)
 
 	// create three polls for the two blocks
 	vdrBag := bag.Of(vdrs...)
@@ -172,7 +191,8 @@ func TestCreateAndFinishSuccessfulPoll(t *testing.T) {
 	log := logging.NoLog{}
 	namespace := ""
 	registerer := prometheus.NewRegistry()
-	s := NewSet(factory, log, namespace, registerer)
+	s, err := NewSet(factory, log, namespace, registerer)
+	require.NoError(err)
 
 	require.Zero(s.Len())
 
@@ -204,7 +224,8 @@ func TestCreateAndFinishFailedPoll(t *testing.T) {
 	log := logging.NoLog{}
 	namespace := ""
 	registerer := prometheus.NewRegistry()
-	s := NewSet(factory, log, namespace, registerer)
+	s, err := NewSet(factory, log, namespace, registerer)
+	require.NoError(err)
 
 	require.Zero(s.Len())
 
@@ -233,7 +254,8 @@ func TestSetString(t *testing.T) {
 	log := logging.NoLog{}
 	namespace := ""
 	registerer := prometheus.NewRegistry()
-	s := NewSet(factory, log, namespace, registerer)
+	s, err := NewSet(factory, log, namespace, registerer)
+	require.NoError(err)
 
 	expected := `current polls: (Size = 1)
     RequestID 0:

diff --git a/snow/engine/snowman/transitive.go b/snow/engine/snowman/transitive.go
@@ -116,6 +116,16 @@ func newTransitive(config Config) (*Transitive, error) {
 		config.Params.AlphaPreference,
 		config.Params.AlphaConfidence,
 	)
+	polls, err := poll.NewSet(
+		factory,
+		config.Ctx.Log,
+		"",
+		config.Ctx.Registerer,
+	)
+	if err != nil {
+		return nil, err
+	}
+
 	t := &Transitive{
 		Config:                      config,
 		StateSummaryFrontierHandler: common.NewNoOpStateSummaryFrontierHandler(config.Ctx.Log),
@@ -129,12 +139,7 @@ func newTransitive(config Config) (*Transitive, error) {
 		nonVerifieds:                ancestor.NewTree(),
 		nonVerifiedCache:            nonVerifiedCache,
 		acceptedFrontiers:           acceptedFrontiers,
-		polls: poll.NewSet(
-			factory,
-			config.Ctx.Log,
-			"",
-			config.Ctx.Registerer,
-		),
+		polls:                       polls,
 	}
 
 	return t, t.metrics.Initialize("", config.Ctx.Registerer)