warp: refactor warp handler stats (#944)

* warp: refactor warp handler stats

* warp: update handler stats var name to signatureRequestDuration

---------

Co-authored-by: Ceyhun Onur <ceyhun.onur@avalabs.org>

Author: aaronbuchwald

plugin/evm/network_handler.go

@@ -16,7 +16,6 @@ import (
 	"github.com/ava-labs/subnet-evm/trie"
 	"github.com/ava-labs/subnet-evm/warp"
 	warpHandlers "github.com/ava-labs/subnet-evm/warp/handlers"
-	warpStats "github.com/ava-labs/subnet-evm/warp/handlers/stats"
 )
 
 var _ message.RequestHandler = &networkHandler{}
@@ -25,7 +24,7 @@ type networkHandler struct {
 	stateTrieLeafsRequestHandler *syncHandlers.LeafsRequestHandler
 	blockRequestHandler          *syncHandlers.BlockRequestHandler
 	codeRequestHandler           *syncHandlers.CodeRequestHandler
-	signatureRequestHandler      warpHandlers.SignatureRequestHandler
+	signatureRequestHandler      *warpHandlers.SignatureRequestHandler
 }
 
 // newNetworkHandler constructs the handler for serving network requests.
@@ -41,7 +40,7 @@ func newNetworkHandler(
 		stateTrieLeafsRequestHandler: syncHandlers.NewLeafsRequestHandler(evmTrieDB, provider, networkCodec, syncStats),
 		blockRequestHandler:          syncHandlers.NewBlockRequestHandler(provider, networkCodec, syncStats),
 		codeRequestHandler:           syncHandlers.NewCodeRequestHandler(diskDB, networkCodec, syncStats),
-		signatureRequestHandler:      warpHandlers.NewSignatureRequestHandler(warpBackend, networkCodec, warpStats.NewStats()),
+		signatureRequestHandler:      warpHandlers.NewSignatureRequestHandler(warpBackend, networkCodec),
 	}
 }
 

warp/handlers/signature_request.go

@@ -12,28 +12,21 @@ import (
 	"github.com/ava-labs/avalanchego/utils/crypto/bls"
 	"github.com/ava-labs/subnet-evm/plugin/evm/message"
 	"github.com/ava-labs/subnet-evm/warp"
-	"github.com/ava-labs/subnet-evm/warp/handlers/stats"
 	"github.com/ethereum/go-ethereum/log"
 )
 
-// SignatureRequestHandler is a peer.RequestHandler for message.SignatureRequest
-// serving requested BLS signature data
-type SignatureRequestHandler interface {
-	OnSignatureRequest(ctx context.Context, nodeID ids.NodeID, requestID uint32, signatureRequest message.SignatureRequest) ([]byte, error)
-}
-
-// signatureRequestHandler implements the SignatureRequestHandler interface
-type signatureRequestHandler struct {
+// SignatureRequestHandler serves warp signature requests. It is a peer.RequestHandler for message.SignatureRequest.
+type SignatureRequestHandler struct {
 	backend warp.Backend
 	codec   codec.Manager
-	stats   stats.SignatureRequestHandlerStats
+	stats   *handlerStats
 }
 
-func NewSignatureRequestHandler(backend warp.Backend, codec codec.Manager, stats stats.SignatureRequestHandlerStats) SignatureRequestHandler {
-	return &signatureRequestHandler{
+func NewSignatureRequestHandler(backend warp.Backend, codec codec.Manager) *SignatureRequestHandler {
+	return &SignatureRequestHandler{
 		backend: backend,
 		codec:   codec,
-		stats:   stats,
+		stats:   newStats(),
 	}
 }
 
@@ -42,7 +35,7 @@ func NewSignatureRequestHandler(backend warp.Backend, codec codec.Manager, stats
 // Expects returned errors to be treated as FATAL
 // Returns empty response if signature is not found
 // Assumes ctx is active
-func (s *signatureRequestHandler) OnSignatureRequest(ctx context.Context, nodeID ids.NodeID, requestID uint32, signatureRequest message.SignatureRequest) ([]byte, error) {
+func (s *SignatureRequestHandler) OnSignatureRequest(ctx context.Context, nodeID ids.NodeID, requestID uint32, signatureRequest message.SignatureRequest) ([]byte, error) {
 	startTime := time.Now()
 	s.stats.IncSignatureRequest()
 

warp/handlers/signature_request_test.go

@@ -15,7 +15,6 @@ import (
 	avalancheWarp "github.com/ava-labs/avalanchego/vms/platformvm/warp"
 	"github.com/ava-labs/subnet-evm/plugin/evm/message"
 	"github.com/ava-labs/subnet-evm/warp"
-	"github.com/ava-labs/subnet-evm/warp/handlers/stats"
 	"github.com/stretchr/testify/require"
 )
 
@@ -38,24 +37,22 @@ func TestSignatureHandler(t *testing.T) {
 	unknownMessageID := ids.GenerateTestID()
 
 	emptySignature := [bls.SignatureLen]byte{}
-	mockHandlerStats := &stats.MockSignatureRequestHandlerStats{}
-	signatureRequestHandler := NewSignatureRequestHandler(backend, message.Codec, mockHandlerStats)
 
 	tests := map[string]struct {
 		setup       func() (request message.SignatureRequest, expectedResponse []byte)
-		verifyStats func(t *testing.T, stats *stats.MockSignatureRequestHandlerStats)
+		verifyStats func(t *testing.T, stats *handlerStats)
 	}{
 		"normal": {
 			setup: func() (request message.SignatureRequest, expectedResponse []byte) {
 				return message.SignatureRequest{
 					MessageID: messageID,
 				}, signature[:]
 			},
-			verifyStats: func(t *testing.T, stats *stats.MockSignatureRequestHandlerStats) {
-				require.EqualValues(t, 1, mockHandlerStats.SignatureRequestCount)
-				require.EqualValues(t, 1, mockHandlerStats.SignatureRequestHit)
-				require.EqualValues(t, 0, mockHandlerStats.SignatureRequestMiss)
-				require.Greater(t, mockHandlerStats.SignatureRequestDuration, time.Duration(0))
+			verifyStats: func(t *testing.T, stats *handlerStats) {
+				require.EqualValues(t, 1, stats.signatureRequest.Count())
+				require.EqualValues(t, 1, stats.signatureHit.Count())
+				require.EqualValues(t, 0, stats.signatureMiss.Count())
+				require.Greater(t, stats.signatureRequestDuration.Value(), time.Duration(0))
 			},
 		},
 		"unknown": {
@@ -64,27 +61,28 @@ func TestSignatureHandler(t *testing.T) {
 					MessageID: unknownMessageID,
 				}, emptySignature[:]
 			},
-			verifyStats: func(t *testing.T, stats *stats.MockSignatureRequestHandlerStats) {
-				require.EqualValues(t, 1, mockHandlerStats.SignatureRequestCount)
-				require.EqualValues(t, 1, mockHandlerStats.SignatureRequestMiss)
-				require.EqualValues(t, 0, mockHandlerStats.SignatureRequestHit)
-				require.Greater(t, mockHandlerStats.SignatureRequestDuration, time.Duration(0))
+			verifyStats: func(t *testing.T, stats *handlerStats) {
+				require.EqualValues(t, 1, stats.signatureRequest.Count())
+				require.EqualValues(t, 0, stats.signatureHit.Count())
+				require.EqualValues(t, 1, stats.signatureMiss.Count())
+				require.Greater(t, stats.signatureRequestDuration.Value(), time.Duration(0))
 			},
 		},
 	}
 
 	for name, test := range tests {
-		// Reset stats before each test
-		mockHandlerStats.Reset()
-
 		t.Run(name, func(t *testing.T) {
+			handler := NewSignatureRequestHandler(backend, message.Codec)
+			handler.stats.Clear()
+
 			request, expectedResponse := test.setup()
-			responseBytes, err := signatureRequestHandler.OnSignatureRequest(context.Background(), ids.GenerateTestNodeID(), 1, request)
+			responseBytes, err := handler.OnSignatureRequest(context.Background(), ids.GenerateTestNodeID(), 1, request)
 			require.NoError(t, err)
 
+			test.verifyStats(t, handler.stats)
+
 			// If the expected response is empty, assert that the handler returns an empty response and return early.
 			if len(expectedResponse) == 0 {
-				test.verifyStats(t, mockHandlerStats)
 				require.Len(t, responseBytes, 0, "expected response to be empty")
 				return
 			}
@@ -93,7 +91,6 @@ func TestSignatureHandler(t *testing.T) {
 			require.NoError(t, err, "error unmarshalling SignatureResponse")
 
 			require.Equal(t, expectedResponse, response.Signature[:])
-			test.verifyStats(t, mockHandlerStats)
 		})
 	}
 }

warp/handlers/stats.go

@@ -0,0 +1,40 @@
+// (c) 2023, Ava Labs, Inc. All rights reserved.
+// See the file LICENSE for licensing terms.
+
+package handlers
+
+import (
+	"time"
+
+	"github.com/ava-labs/subnet-evm/metrics"
+)
+
+type handlerStats struct {
+	// SignatureRequestHandler metrics
+	signatureRequest         metrics.Counter
+	signatureHit             metrics.Counter
+	signatureMiss            metrics.Counter
+	signatureRequestDuration metrics.Gauge
+}
+
+func newStats() *handlerStats {
+	return &handlerStats{
+		signatureRequest:         metrics.GetOrRegisterCounter("signature_request_count", nil),
+		signatureHit:             metrics.GetOrRegisterCounter("signature_request_hit", nil),
+		signatureMiss:            metrics.GetOrRegisterCounter("signature_request_miss", nil),
+		signatureRequestDuration: metrics.GetOrRegisterGauge("signature_request_duration", nil),
+	}
+}
+
+func (h *handlerStats) IncSignatureRequest() { h.signatureRequest.Inc(1) }
+func (h *handlerStats) IncSignatureHit()     { h.signatureHit.Inc(1) }
+func (h *handlerStats) IncSignatureMiss()    { h.signatureMiss.Inc(1) }
+func (h *handlerStats) UpdateSignatureRequestTime(duration time.Duration) {
+	h.signatureRequestDuration.Inc(int64(duration))
+}
+func (h *handlerStats) Clear() {
+	h.signatureRequest.Clear()
+	h.signatureHit.Clear()
+	h.signatureMiss.Clear()
+	h.signatureRequestDuration.Update(0)
+}