Measure bloom filter AppGossip hit rate (#4085)

Signed-off-by: Yacov Manevich <yacov.manevich@avalabs.org>
Co-authored-by: Stephen Buttolph <stephen@avalabs.org>

Author: yacovm

network/p2p/gossip/gossip.go

@@ -101,6 +101,7 @@ type Metrics struct {
 	tracking                *prometheus.GaugeVec
 	trackingLifetimeAverage prometheus.Gauge
 	topValidators           *prometheus.GaugeVec
+	bloomFilterHitRate      prometheus.Histogram
 }
 
 // NewMetrics returns a common set of metrics
@@ -109,6 +110,15 @@ func NewMetrics(
 	namespace string,
 ) (Metrics, error) {
 	m := Metrics{
+		bloomFilterHitRate: prometheus.NewHistogram(prometheus.HistogramOpts{
+			Namespace: namespace,
+			Name:      "bloomfilter_hit_rate",
+			Help:      "Hit rate (%) of the bloom filter sent by pull gossip",
+			// Buckets are (-∞, 0], (0, 25%], (25%, 50%], (50%, 75%], (75%, ∞).
+			// 0% is placed into its own bucket so that useless bloom filters
+			// can be inspected individually.
+			Buckets: prometheus.LinearBuckets(0, 25, 4),
+		}),
 		count: prometheus.NewCounterVec(
 			prometheus.CounterOpts{
 				Namespace: namespace,
@@ -148,6 +158,7 @@ func NewMetrics(
 		),
 	}
 	err := errors.Join(
+		metrics.Register(m.bloomFilterHitRate),
 		metrics.Register(m.count),
 		metrics.Register(m.bytes),
 		metrics.Register(m.tracking),

network/p2p/gossip/gossip_test.go

@@ -57,6 +57,7 @@ func TestGossiperGossip(t *testing.T) {
 		responder              []*testTx // what the peer we're requesting gossip from has
 		expectedPossibleValues []*testTx // possible values we can have
 		expectedLen            int
+		expectedHitRate        float64
 	}{
 		{
 			name: "no gossip - no one knows anything",
@@ -75,13 +76,15 @@ func TestGossiperGossip(t *testing.T) {
 			responder:              []*testTx{{id: ids.ID{0}}},
 			expectedPossibleValues: []*testTx{{id: ids.ID{0}}},
 			expectedLen:            1,
+			expectedHitRate:        100,
 		},
 		{
 			name:                   "gossip - requester knows nothing",
 			targetResponseSize:     1024,
 			responder:              []*testTx{{id: ids.ID{0}}},
 			expectedPossibleValues: []*testTx{{id: ids.ID{0}}},
 			expectedLen:            1,
+			expectedHitRate:        0,
 		},
 		{
 			name:                   "gossip - requester knows less than responder",
@@ -90,13 +93,15 @@ func TestGossiperGossip(t *testing.T) {
 			responder:              []*testTx{{id: ids.ID{0}}, {id: ids.ID{1}}},
 			expectedPossibleValues: []*testTx{{id: ids.ID{0}}, {id: ids.ID{1}}},
 			expectedLen:            2,
+			expectedHitRate:        50,
 		},
 		{
 			name:                   "gossip - target response size exceeded",
 			targetResponseSize:     32,
 			responder:              []*testTx{{id: ids.ID{0}}, {id: ids.ID{1}}, {id: ids.ID{2}}},
 			expectedPossibleValues: []*testTx{{id: ids.ID{0}}, {id: ids.ID{1}}, {id: ids.ID{2}}},
 			expectedLen:            2,
+			expectedHitRate:        0,
 		},
 	}
 
@@ -123,6 +128,12 @@ func TestGossiperGossip(t *testing.T) {
 
 			metrics, err := NewMetrics(prometheus.NewRegistry(), "")
 			require.NoError(err)
+
+			testHistogram := &testHistogram{
+				Histogram: metrics.bloomFilterHitRate,
+			}
+			metrics.bloomFilterHitRate = testHistogram
+
 			marshaller := testMarshaller{}
 			handler := NewHandler[*testTx](
 				logging.NoLog{},
@@ -175,6 +186,8 @@ func TestGossiperGossip(t *testing.T) {
 
 			require.Len(requestSet.txs, tt.expectedLen)
 			require.Subset(tt.expectedPossibleValues, maps.Values(requestSet.txs))
+			require.Equal(len(tt.responder) > 0, testHistogram.observed)
+			require.InDelta(tt.expectedHitRate, testHistogram.observedVal, 0)
 
 			// we should not receive anything that we already had before we
 			// requested the gossip
@@ -611,3 +624,15 @@ type testValidatorSet struct {
 func (t testValidatorSet) Has(_ context.Context, nodeID ids.NodeID) bool {
 	return t.validators.Contains(nodeID)
 }
+
+type testHistogram struct {
+	prometheus.Histogram
+	observedVal float64
+	observed    bool
+}
+
+func (t *testHistogram) Observe(value float64) {
+	t.Histogram.Observe(value)
+	t.observedVal = value
+	t.observed = true
+}

network/p2p/gossip/handler.go

@@ -50,13 +50,19 @@ func (h Handler[T]) AppRequest(_ context.Context, _ ids.NodeID, _ time.Time, req
 		return nil, p2p.ErrUnexpected
 	}
 
-	responseSize := 0
-	gossipBytes := make([][]byte, 0)
+	var (
+		hits         float64
+		total        float64
+		responseSize int
+		gossipBytes  [][]byte
+	)
 	h.set.Iterate(func(gossipable T) bool {
+		total++
 		gossipID := gossipable.GossipID()
 
 		// filter out what the requesting peer already knows about
 		if bloom.Contains(filter, gossipID[:], salt[:]) {
+			hits++
 			return true
 		}
 
@@ -77,6 +83,11 @@ func (h Handler[T]) AppRequest(_ context.Context, _ ids.NodeID, _ time.Time, req
 		return nil, p2p.ErrUnexpected
 	}
 
+	if total > 0 {
+		hitRate := float64(hits) / float64(total)
+		h.metrics.bloomFilterHitRate.Observe(100 * hitRate)
+	}
+
 	if err := h.metrics.observeMessage(sentPullLabels, len(gossipBytes), responseSize); err != nil {
 		return nil, p2p.ErrUnexpected
 	}