From 45ab2bde4c6af9b8e06453c54b02663e95232f0a Mon Sep 17 00:00:00 2001 From: Darren Kelly <107671032+darrenvechain@users.noreply.github.com> Date: Fri, 29 Nov 2024 09:35:32 +0000 Subject: [PATCH] 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 --- api/accounts/accounts.go | 12 ++++++------ api/blocks/blocks.go | 2 +- api/debug/debug.go | 6 +++--- api/events/events.go | 2 +- api/metrics.go | 28 +++++++++++++++++----------- api/metrics_test.go | 22 +++++++++++----------- api/node/node.go | 2 +- api/transactions/transactions.go | 6 +++--- api/transfers/transfers.go | 2 +- metrics/telemetry.go | 18 ++++++++++++------ 10 files changed, 56 insertions(+), 44 deletions(-) diff --git a/api/accounts/accounts.go b/api/accounts/accounts.go index 4bd940709..54058a160 100644 --- 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 index bddb3ac12..ff86e02e6 100644 --- 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 index e84a88d57..5ff54f1dc 100644 --- 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 index 40dff7b09..669c47b03 100644 --- 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 index 9fd5c3d94..57631be71 100644 --- 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,7 +68,7 @@ func metricsMiddleware(next http.Handler) http.Handler { var ( enabled = false name = "" - subscription = "" + subscription = false ) // all named route will be recorded @@ -70,24 +76,24 @@ func metricsMiddleware(next http.Handler) http.Handler { 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 index 7cb1794e4..4e4b6daaf 100644 --- 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 index 11c1ce7e1..21c69dcdf 100644 --- 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 index af32cb6da..3cf2e4d65 100644 --- 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 index cad4ee6b3..25d2e2599 100644 --- 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 index 9ab3bd633..1d1ee96f2 100644 --- 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 } }