chore: update API metrics bucket and endpoint names (#893)

* chore: update API metrics bucket and endpoint names * fix: typo & tests * fix: lint * chore: add websocket total counter * fix: txs endpoints names & ws subject * fix: unit tests * chore: standardise naming convention * chore: add websocke duration & http code * chore: add websocke duration & http code * fix: lint issues * fix: sync issues with metrics * chore: update websocket durations bucket * fix: PR comments - use sync.Once
vechain · Nov 29, 2024 · 45ab2bd · 45ab2bd
1 parent 198e537
commit 45ab2bd
Show file tree

Hide file tree

Showing 10 changed files with 56 additions and 44 deletions.
diff --git a/api/accounts/accounts.go b/api/accounts/accounts.go
@@ -358,27 +358,27 @@ func (a *Accounts) Mount(root *mux.Router, pathPrefix string) {
 
 	sub.Path("/*").
 		Methods(http.MethodPost).
-		Name("accounts_call_batch_code").
+		Name("POST /accounts/*").
 		HandlerFunc(utils.WrapHandlerFunc(a.handleCallBatchCode))
 	sub.Path("/{address}").
 		Methods(http.MethodGet).
-		Name("accounts_get_account").
+		Name("GET /accounts/{address}").
 		HandlerFunc(utils.WrapHandlerFunc(a.handleGetAccount))
 	sub.Path("/{address}/code").
 		Methods(http.MethodGet).
-		Name("accounts_get_code").
+		Name("GET /accounts/{address}/code").
 		HandlerFunc(utils.WrapHandlerFunc(a.handleGetCode))
 	sub.Path("/{address}/storage/{key}").
 		Methods("GET").
-		Name("accounts_get_storage").
+		Name("GET /accounts/{address}/storage").
 		HandlerFunc(utils.WrapHandlerFunc(a.handleGetStorage))
 	// These two methods are currently deprecated
 	sub.Path("").
 		Methods(http.MethodPost).
-		Name("accounts_call_contract").
+		Name("POST /accounts").
 		HandlerFunc(utils.WrapHandlerFunc(a.handleCallContract))
 	sub.Path("/{address}").
 		Methods(http.MethodPost).
-		Name("accounts_call_contract_address").
+		Name("POST /accounts/{address}").
 		HandlerFunc(utils.WrapHandlerFunc(a.handleCallContract))
 }
diff --git a/api/blocks/blocks.go b/api/blocks/blocks.go
@@ -95,6 +95,6 @@ func (b *Blocks) Mount(root *mux.Router, pathPrefix string) {
 	sub := root.PathPrefix(pathPrefix).Subrouter()
 	sub.Path("/{revision}").
 		Methods(http.MethodGet).
-		Name("blocks_get_block").
+		Name("GET /blocks/{revision}").
 		HandlerFunc(utils.WrapHandlerFunc(b.handleGetBlock))
 }
diff --git a/api/debug/debug.go b/api/debug/debug.go
@@ -466,14 +466,14 @@ func (d *Debug) Mount(root *mux.Router, pathPrefix string) {
 
 	sub.Path("/tracers").
 		Methods(http.MethodPost).
-		Name("debug_trace_clause").
+		Name("POST /debug/tracers").
 		HandlerFunc(utils.WrapHandlerFunc(d.handleTraceClause))
 	sub.Path("/tracers/call").
 		Methods(http.MethodPost).
-		Name("debug_trace_call").
+		Name("POST /debug/tracers/call").
 		HandlerFunc(utils.WrapHandlerFunc(d.handleTraceCall))
 	sub.Path("/storage-range").
 		Methods(http.MethodPost).
-		Name("debug_trace_storage").
+		Name("POST /debug/storage-range").
 		HandlerFunc(utils.WrapHandlerFunc(d.handleDebugStorage))
 }
diff --git a/api/events/events.go b/api/events/events.go
@@ -84,6 +84,6 @@ func (e *Events) Mount(root *mux.Router, pathPrefix string) {
 
 	sub.Path("").
 		Methods(http.MethodPost).
-		Name("logs_filter_event").
+		Name("POST /logs/event").
 		HandlerFunc(utils.WrapHandlerFunc(e.handleFilter))
 }
diff --git a/api/metrics.go b/api/metrics.go
@@ -19,9 +19,15 @@ import (
 )
 
 var (
+	websocketDurations = []int64{
+		0, 1, 2, 5, 10, 25, 50, 100, 250, 500, 1_000, 2_500, 5_000, 10_000, 25_000,
+		50_000, 100_000, 250_000, 500_000, 1000_000, 2_500_000, 5_000_000, 10_000_000,
+	}
 	metricHTTPReqCounter       = metrics.LazyLoadCounterVec("api_request_count", []string{"name", "code", "method"})
 	metricHTTPReqDuration      = metrics.LazyLoadHistogramVec("api_duration_ms", []string{"name", "code", "method"}, metrics.BucketHTTPReqs)
-	metricActiveWebsocketCount = metrics.LazyLoadGaugeVec("api_active_websocket_count", []string{"subject"})
+	metricWebsocketDuration    = metrics.LazyLoadHistogramVec("api_websocket_duration", []string{"name", "code"}, websocketDurations)
+	metricActiveWebsocketGauge = metrics.LazyLoadGaugeVec("api_active_websocket_gauge", []string{"name"})
+	metricWebsocketCounter     = metrics.LazyLoadCounterVec("api_websocket_counter", []string{"name"})
 )
 
 // metricsResponseWriter is a wrapper around http.ResponseWriter that captures the status code.
@@ -62,32 +68,32 @@ func metricsMiddleware(next http.Handler) http.Handler {
 		var (
 			enabled      = false
 			name         = ""
-			subscription = ""
+			subscription = false
 		)
 
 		// all named route will be recorded
 		if rt != nil && rt.GetName() != "" {
 			enabled = true
 			name = rt.GetName()
 			if strings.HasPrefix(name, "subscriptions") {
-				// example path: /subscriptions/txpool -> subject = txpool
-				paths := strings.Split(r.URL.Path, "/")
-				if len(paths) > 2 {
-					subscription = paths[2]
-				}
+				subscription = true
+				name = "WS " + r.URL.Path
 			}
 		}
 
 		now := time.Now()
 		mrw := newMetricsResponseWriter(w)
-		if subscription != "" {
-			metricActiveWebsocketCount().AddWithLabel(1, map[string]string{"subject": subscription})
+		if subscription {
+			metricActiveWebsocketGauge().AddWithLabel(1, map[string]string{"name": name})
+			metricWebsocketCounter().AddWithLabel(1, map[string]string{"name": name})
 		}
 
 		next.ServeHTTP(mrw, r)
 
-		if subscription != "" {
-			metricActiveWebsocketCount().AddWithLabel(-1, map[string]string{"subject": subscription})
+		if subscription {
+			metricActiveWebsocketGauge().AddWithLabel(-1, map[string]string{"name": name})
+			// record websocket duration in seconds, not MS
+			metricWebsocketDuration().ObserveWithLabels(time.Since(now).Milliseconds()/1000, map[string]string{"name": name, "code": strconv.Itoa(mrw.statusCode)})
 		} else if enabled {
 			metricHTTPReqCounter().AddWithLabel(1, map[string]string{"name": name, "code": strconv.Itoa(mrw.statusCode), "method": r.Method})
 			metricHTTPReqDuration().ObserveWithLabels(time.Since(now).Milliseconds(), map[string]string{"name": name, "code": strconv.Itoa(mrw.statusCode), "method": r.Method})

diff --git a/api/metrics_test.go b/api/metrics_test.go
@@ -77,7 +77,7 @@ func TestMetricsMiddleware(t *testing.T) {
 	assert.Equal(t, "method", labels[1].GetName())
 	assert.Equal(t, "GET", labels[1].GetValue())
 	assert.Equal(t, "name", labels[2].GetName())
-	assert.Equal(t, "accounts_get_account", labels[2].GetValue())
+	assert.Equal(t, "GET /accounts/{address}", labels[2].GetValue())
 
 	labels = m[1].GetLabel()
 	assert.Equal(t, 3, len(labels))
@@ -86,7 +86,7 @@ func TestMetricsMiddleware(t *testing.T) {
 	assert.Equal(t, "method", labels[1].GetName())
 	assert.Equal(t, "GET", labels[1].GetValue())
 	assert.Equal(t, "name", labels[2].GetName())
-	assert.Equal(t, "accounts_get_account", labels[2].GetValue())
+	assert.Equal(t, "GET /accounts/{address}", labels[2].GetValue())
 
 	labels = m[2].GetLabel()
 	assert.Equal(t, 3, len(labels))
@@ -95,7 +95,7 @@ func TestMetricsMiddleware(t *testing.T) {
 	assert.Equal(t, "method", labels[1].GetName())
 	assert.Equal(t, "GET", labels[1].GetValue())
 	assert.Equal(t, "name", labels[2].GetName())
-	assert.Equal(t, "accounts_get_account", labels[2].GetValue())
+	assert.Equal(t, "GET /accounts/{address}", labels[2].GetValue())
 }
 
 func TestWebsocketMetrics(t *testing.T) {
@@ -120,13 +120,13 @@ func TestWebsocketMetrics(t *testing.T) {
 	metrics, err := parser.TextToMetricFamilies(bytes.NewReader(body))
 	assert.Nil(t, err)
 
-	m := metrics["thor_metrics_api_active_websocket_count"].GetMetric()
+	m := metrics["thor_metrics_api_active_websocket_gauge"].GetMetric()
 	assert.Equal(t, 1, len(m), "should be 1 metric entries")
 	assert.Equal(t, float64(1), m[0].GetGauge().GetValue())
 
 	labels := m[0].GetLabel()
-	assert.Equal(t, "subject", labels[0].GetName())
-	assert.Equal(t, "beat", labels[0].GetValue())
+	assert.Equal(t, "name", labels[0].GetName())
+	assert.Equal(t, "WS /subscriptions/beat", labels[0].GetValue())
 
 	// initiate 1 beat subscription, active websocket should be 2
 	conn2, _, err := websocket.DefaultDialer.Dial(u.String(), nil)
@@ -137,7 +137,7 @@ func TestWebsocketMetrics(t *testing.T) {
 	metrics, err = parser.TextToMetricFamilies(bytes.NewReader(body))
 	assert.Nil(t, err)
 
-	m = metrics["thor_metrics_api_active_websocket_count"].GetMetric()
+	m = metrics["thor_metrics_api_active_websocket_gauge"].GetMetric()
 	assert.Equal(t, 1, len(m), "should be 1 metric entries")
 	assert.Equal(t, float64(2), m[0].GetGauge().GetValue())
 
@@ -151,16 +151,16 @@ func TestWebsocketMetrics(t *testing.T) {
 	metrics, err = parser.TextToMetricFamilies(bytes.NewReader(body))
 	assert.Nil(t, err)
 
-	m = metrics["thor_metrics_api_active_websocket_count"].GetMetric()
+	m = metrics["thor_metrics_api_active_websocket_gauge"].GetMetric()
 	assert.Equal(t, 2, len(m), "should be 2 metric entries")
 	// both m[0] and m[1] should have the value of 1
 	assert.Equal(t, float64(2), m[0].GetGauge().GetValue())
 	assert.Equal(t, float64(1), m[1].GetGauge().GetValue())
 
-	// m[1] should have the subject of block
+	// m[1] should have the name of block
 	labels = m[1].GetLabel()
-	assert.Equal(t, "subject", labels[0].GetName())
-	assert.Equal(t, "block", labels[0].GetValue())
+	assert.Equal(t, "name", labels[0].GetName())
+	assert.Equal(t, "WS /subscriptions/block", labels[0].GetValue())
 }
 
 func httpGet(t *testing.T, url string) ([]byte, int) {

diff --git a/api/node/node.go b/api/node/node.go
@@ -35,6 +35,6 @@ func (n *Node) Mount(root *mux.Router, pathPrefix string) {
 
 	sub.Path("/network/peers").
 		Methods(http.MethodGet).
-		Name("node_get_peers").
+		Name("GET /node/network/peers").
 		HandlerFunc(utils.WrapHandlerFunc(n.handleNetwork))
 }
diff --git a/api/transactions/transactions.go b/api/transactions/transactions.go
@@ -218,14 +218,14 @@ func (t *Transactions) Mount(root *mux.Router, pathPrefix string) {
 
 	sub.Path("").
 		Methods(http.MethodPost).
-		Name("transactions_send_tx").
+		Name("POST /transactions").
 		HandlerFunc(utils.WrapHandlerFunc(t.handleSendTransaction))
 	sub.Path("/{id}").
 		Methods(http.MethodGet).
-		Name("transactions_get_tx").
+		Name("GET /transactions/{id}").
 		HandlerFunc(utils.WrapHandlerFunc(t.handleGetTransactionByID))
 	sub.Path("/{id}/receipt").
 		Methods(http.MethodGet).
-		Name("transactions_get_receipt").
+		Name("GET /transactions/{id}/receipt").
 		HandlerFunc(utils.WrapHandlerFunc(t.handleGetTransactionReceiptByID))
 }
diff --git a/api/transfers/transfers.go b/api/transfers/transfers.go
@@ -90,6 +90,6 @@ func (t *Transfers) Mount(root *mux.Router, pathPrefix string) {
 
 	sub.Path("").
 		Methods(http.MethodPost).
-		Name("logs_filter_transfer").
+		Name("POST /logs/transfer").
 		HandlerFunc(utils.WrapHandlerFunc(t.handleFilterTransferLogs))
 }
diff --git a/metrics/telemetry.go b/metrics/telemetry.go
@@ -5,7 +5,10 @@
 
 package metrics
 
-import "net/http"
+import (
+	"net/http"
+	"sync"
+)
 
 // metrics is a singleton service that provides global access to a set of meters
 // it wraps multiple implementations and defaults to a no-op implementation
@@ -30,7 +33,11 @@ func HTTPHandler() http.Handler {
 // Define standard buckets for histograms
 var (
 	Bucket10s      = []int64{0, 500, 1000, 2000, 3000, 4000, 5000, 7500, 10_000}
-	BucketHTTPReqs = []int64{0, 150, 300, 450, 600, 900, 1200, 1500, 3000}
+	BucketHTTPReqs = []int64{
+		0, 1, 2, 5, 10, 20, 30, 50, 75, 100,
+		150, 200, 300, 400, 500, 750, 1000,
+		1500, 2000, 3000, 4000, 5000, 10000,
+	}
 )
 
 // HistogramMeter represents the type of metric that is calculated by aggregating
@@ -96,12 +103,11 @@ func GaugeVec(name string, labels []string) GaugeVecMeter {
 // - it avoid metrics definition to determine the singleton to use (noop vs prometheus)
 func LazyLoad[T any](f func() T) func() T {
 	var result T
-	var loaded bool
+	var once sync.Once
 	return func() T {
-		if !loaded {
+		once.Do(func() {
 			result = f()
-			loaded = true
-		}
+		})
 		return result
 	}
 }