From ff87c8b075a94401b9badf68d1ee2785f4f78698 Mon Sep 17 00:00:00 2001 From: Jean Arnaud Date: Thu, 9 Nov 2023 18:42:53 +0100 Subject: [PATCH 01/12] Extract HTTP client --- ...d_reader_test.go => cached_reader_test.go} | 0 .../tokendata/observability/http_client.go | 28 +++++++ .../ccip/tokendata/observability/usdc.go | 71 ++++++++++++++++ .../ccip/tokendata/observability/usdc_test.go | 84 +++++++++++++++++++ .../ccip/tokendata/usdc/http_client.go | 33 ++++++++ .../ocr2/plugins/ccip/tokendata/usdc/usdc.go | 26 +++--- 6 files changed, 228 insertions(+), 14 deletions(-) rename core/services/ocr2/plugins/ccip/tokendata/{chached_reader_test.go => cached_reader_test.go} (100%) create mode 100644 core/services/ocr2/plugins/ccip/tokendata/observability/http_client.go create mode 100644 core/services/ocr2/plugins/ccip/tokendata/observability/usdc.go create mode 100644 core/services/ocr2/plugins/ccip/tokendata/observability/usdc_test.go create mode 100644 core/services/ocr2/plugins/ccip/tokendata/usdc/http_client.go diff --git a/core/services/ocr2/plugins/ccip/tokendata/chached_reader_test.go b/core/services/ocr2/plugins/ccip/tokendata/cached_reader_test.go similarity index 100% rename from core/services/ocr2/plugins/ccip/tokendata/chached_reader_test.go rename to core/services/ocr2/plugins/ccip/tokendata/cached_reader_test.go diff --git a/core/services/ocr2/plugins/ccip/tokendata/observability/http_client.go b/core/services/ocr2/plugins/ccip/tokendata/observability/http_client.go new file mode 100644 index 0000000000..537726b2e7 --- /dev/null +++ b/core/services/ocr2/plugins/ccip/tokendata/observability/http_client.go @@ -0,0 +1,28 @@ +package observability + +import ( + "context" + + "github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ccip/tokendata/usdc" +) + +type ObservedIHttpClient struct { + usdc.IHttpClient + metric metricDetails +} + +func NewObservedIHttpClient(origin usdc.IHttpClient, pluginName string) *ObservedIHttpClient { + return &ObservedIHttpClient{ + IHttpClient: origin, + metric: metricDetails{ + histogram: usdcHistogram, + pluginName: pluginName, + }, + } +} + +func (o *ObservedIHttpClient) Get(ctx context.Context, url string) ([]byte, error) { + return withObservedContract(o.metric, "Get", func() ([]byte, error) { + return o.IHttpClient.Get(ctx, url) + }) +} diff --git a/core/services/ocr2/plugins/ccip/tokendata/observability/usdc.go b/core/services/ocr2/plugins/ccip/tokendata/observability/usdc.go new file mode 100644 index 0000000000..3fd33cb33c --- /dev/null +++ b/core/services/ocr2/plugins/ccip/tokendata/observability/usdc.go @@ -0,0 +1,71 @@ +package observability + +import ( + "context" + "time" + + "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/promauto" + + "github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ccip/internal" + "github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ccip/tokendata/usdc" +) + +var ( + latencyBuckets = []float64{ + float64(10 * time.Millisecond), + float64(25 * time.Millisecond), + float64(50 * time.Millisecond), + float64(75 * time.Millisecond), + float64(100 * time.Millisecond), + float64(250 * time.Millisecond), + float64(500 * time.Millisecond), + float64(750 * time.Millisecond), + float64(1 * time.Second), + float64(2 * time.Second), + } + labels = []string{"function", "success"} + usdcHistogram = promauto.NewHistogramVec(prometheus.HistogramOpts{ + Name: "ccip_usdc_reader_request_total", + Help: "Latency of calls to the USDC reader", + Buckets: latencyBuckets, + }, labels) +) + +type metricDetails struct { + histogram *prometheus.HistogramVec + pluginName string +} + +type ObservedUSDCTokenDataReader struct { + usdc.TokenDataReader + metric metricDetails +} + +func NewObservedUSDCTokenDataReader(origin usdc.TokenDataReader, pluginName string) *ObservedUSDCTokenDataReader { + return &ObservedUSDCTokenDataReader{ + TokenDataReader: *usdc.NewUSDCTokenDataReaderWithHttpClient(origin, NewObservedIHttpClient(&usdc.HttpClient{}, pluginName)), + metric: metricDetails{ + histogram: usdcHistogram, + pluginName: pluginName, + }, + } +} + +func (o *ObservedUSDCTokenDataReader) ReadTokenData(ctx context.Context, msg internal.EVM2EVMOnRampCCIPSendRequestedWithMeta) ([]byte, error) { + return withObservedContract(o.metric, "ReadTokenData", func() ([]byte, error) { + return o.TokenDataReader.ReadTokenData(ctx, msg) + }) +} + +func withObservedContract[T any](metric metricDetails, function string, contract func() (T, error)) (T, error) { + contractExecutionStarted := time.Now() + value, err := contract() + metric.histogram. + WithLabelValues( + metric.pluginName, + function, + ). + Observe(float64(time.Since(contractExecutionStarted))) + return value, err +} diff --git a/core/services/ocr2/plugins/ccip/tokendata/observability/usdc_test.go b/core/services/ocr2/plugins/ccip/tokendata/observability/usdc_test.go new file mode 100644 index 0000000000..b84e0d4eba --- /dev/null +++ b/core/services/ocr2/plugins/ccip/tokendata/observability/usdc_test.go @@ -0,0 +1,84 @@ +package observability + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "net/url" + "testing" + + "github.com/ethereum/go-ethereum/common/hexutil" + "github.com/prometheus/client_golang/prometheus" + io_prometheus_client "github.com/prometheus/client_model/go" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + + "github.com/smartcontractkit/chainlink/v2/core/logger" + "github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ccip/internal" + "github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ccip/internal/ccipdata/mocks" + "github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ccip/tokendata/usdc" +) + +// TODO avoid duplicating types from usdc.go +type attestationResponse struct { + Status string `json:"status"` + Attestation string `json:"attestation"` +} + +func TestUSDCMonitoring(t *testing.T) { + + lggr := logger.TestLogger(t) + usdcReader := mocks.NewUSDCReader(t) + msgBody := []byte{0xb0, 0xd1} + usdcReader.On("GetLastUSDCMessagePriorToLogIndexInTx", mock.Anything, mock.Anything, mock.Anything).Return(msgBody, nil) + + // Create a fake USDC server. + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + response := attestationResponse{ + Status: "complete", + Attestation: "720502893578a89a8a87982982ef781c18b193", + } + responseBytes, err := json.Marshal(response) + _, err = w.Write(responseBytes) + require.NoError(t, err) + })) + defer server.Close() + attestationURI, err := url.ParseRequestURI(server.URL) + require.NoError(t, err) + + // Service with mock http client. + usdcService := usdc.NewUSDCTokenDataReader(lggr, usdcReader, attestationURI) + observedService := NewObservedUSDCTokenDataReader(*usdcService, "plugin") + require.NotNil(t, observedService) + msgAndAttestation, err := observedService.ReadTokenData(context.Background(), internal.EVM2EVMOnRampCCIPSendRequestedWithMeta{}) + require.NoError(t, err) + require.NotNil(t, msgAndAttestation) + expectedMessageAndAttestation := "0x0000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000000000000000000800000000000000000000000000000000000000000000000000000000000000002b0d10000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000013720502893578a89a8a87982982ef781c18b19300000000000000000000000000" + require.Equal(t, expectedMessageAndAttestation, hexutil.Encode(msgAndAttestation)) + + // Check that the metrics are updated. + histogram := usdcHistogram + assert.Equal(t, 0, counterFromHistogramByLabels(t, histogram, "plugin", "XYZMethod")) + assert.Equal(t, 1, counterFromHistogramByLabels(t, histogram, "plugin", "ReadTokenData")) + assert.Equal(t, 1, counterFromHistogramByLabels(t, histogram, "plugin", "Get")) + +} + +func counterFromHistogramByLabels(t *testing.T, histogramVec *prometheus.HistogramVec, labels ...string) int { + observer, err := histogramVec.GetMetricWithLabelValues(labels...) + require.NoError(t, err) + + metricCh := make(chan prometheus.Metric, 1) + observer.(prometheus.Histogram).Collect(metricCh) + close(metricCh) + + metric := <-metricCh + pb := &io_prometheus_client.Metric{} + err = metric.Write(pb) + require.NoError(t, err) + + return int(pb.GetHistogram().GetSampleCount()) +} diff --git a/core/services/ocr2/plugins/ccip/tokendata/usdc/http_client.go b/core/services/ocr2/plugins/ccip/tokendata/usdc/http_client.go new file mode 100644 index 0000000000..5e9f126db2 --- /dev/null +++ b/core/services/ocr2/plugins/ccip/tokendata/usdc/http_client.go @@ -0,0 +1,33 @@ +package usdc + +import ( + "context" + "io" + "net/http" +) + +type IHttpClient interface { + // Get issue a GET request to the given url and return the response body. + Get(ctx context.Context, url string) ([]byte, error) +} + +type HttpClient struct { +} + +func (s *HttpClient) Get(ctx context.Context, url string) ([]byte, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) + if err != nil { + return nil, err + } + req.Header.Add("accept", "application/json") + res, err := http.DefaultClient.Do(req) + if err != nil { + return nil, err + } + defer res.Body.Close() + body, err := io.ReadAll(res.Body) + if err != nil { + return nil, err + } + return body, nil +} diff --git a/core/services/ocr2/plugins/ccip/tokendata/usdc/usdc.go b/core/services/ocr2/plugins/ccip/tokendata/usdc/usdc.go index 8f36886c4c..d203b6f015 100644 --- a/core/services/ocr2/plugins/ccip/tokendata/usdc/usdc.go +++ b/core/services/ocr2/plugins/ccip/tokendata/usdc/usdc.go @@ -5,8 +5,6 @@ import ( "encoding/hex" "encoding/json" "fmt" - "io" - "net/http" "net/url" "strings" @@ -65,6 +63,7 @@ func (m messageAndAttestation) Validate() error { type TokenDataReader struct { lggr logger.Logger usdcReader ccipdata.USDCReader + httpClient IHttpClient attestationApi *url.URL } @@ -79,10 +78,20 @@ func NewUSDCTokenDataReader(lggr logger.Logger, usdcReader ccipdata.USDCReader, return &TokenDataReader{ lggr: lggr, usdcReader: usdcReader, + httpClient: &HttpClient{}, attestationApi: usdcAttestationApi, } } +func NewUSDCTokenDataReaderWithHttpClient(origin TokenDataReader, httpClient IHttpClient) *TokenDataReader { + return &TokenDataReader{ + lggr: origin.lggr, + usdcReader: origin.usdcReader, + httpClient: httpClient, + attestationApi: origin.attestationApi, + } +} + func (s *TokenDataReader) ReadTokenData(ctx context.Context, msg internal.EVM2EVMOnRampCCIPSendRequestedWithMeta) (messageAndAttestation []byte, err error) { messageBody, err := s.getUSDCMessageBody(ctx, msg) if err != nil { @@ -135,21 +144,10 @@ func (s *TokenDataReader) getUSDCMessageBody(ctx context.Context, msg internal.E func (s *TokenDataReader) callAttestationApi(ctx context.Context, usdcMessageHash [32]byte) (attestationResponse, error) { fullAttestationUrl := fmt.Sprintf("%s/%s/%s/0x%x", s.attestationApi, apiVersion, attestationPath, usdcMessageHash) - req, err := http.NewRequestWithContext(ctx, "GET", fullAttestationUrl, nil) + body, err := s.httpClient.Get(ctx, fullAttestationUrl) if err != nil { return attestationResponse{}, err } - req.Header.Add("accept", "application/json") - res, err := http.DefaultClient.Do(req) - if err != nil { - return attestationResponse{}, err - } - defer res.Body.Close() - body, err := io.ReadAll(res.Body) - if err != nil { - return attestationResponse{}, err - } - var response attestationResponse err = json.Unmarshal(body, &response) if err != nil { From bbf63245242aee9aa71b82432da5f0810c00d3a7 Mon Sep 17 00:00:00 2001 From: Jean Arnaud Date: Thu, 9 Nov 2023 19:15:32 +0100 Subject: [PATCH 02/12] Remove commented code --- .../ocr2/plugins/ccip/tokendata/usdc/usdc.go | 29 ------------------- 1 file changed, 29 deletions(-) diff --git a/core/services/ocr2/plugins/ccip/tokendata/usdc/usdc.go b/core/services/ocr2/plugins/ccip/tokendata/usdc/usdc.go index e250ff319e..58492be81e 100644 --- a/core/services/ocr2/plugins/ccip/tokendata/usdc/usdc.go +++ b/core/services/ocr2/plugins/ccip/tokendata/usdc/usdc.go @@ -154,34 +154,7 @@ func (s *TokenDataReader) getUSDCMessageBody(ctx context.Context, msg internal.E func (s *TokenDataReader) callAttestationApi(ctx context.Context, usdcMessageHash [32]byte) (attestationResponse, error) { fullAttestationUrl := fmt.Sprintf("%s/%s/%s/0x%x", s.attestationApi, apiVersion, attestationPath, usdcMessageHash) - - //// Use a timeout to guard against attestation API hanging, causing observation timeout and failing to make any progress. - //timeoutCtx, cancel := context.WithTimeout(ctx, s.attestationApiTimeout) - //defer cancel() - //req, err := http.NewRequestWithContext(timeoutCtx, "GET", fullAttestationUrl, nil) - // - //if err != nil { - // return attestationResponse{}, err - //} - //req.Header.Add("accept", "application/json") - //res, err := http.DefaultClient.Do(req) - //if err != nil { - // if errors.Is(err, context.DeadlineExceeded) { - // return attestationResponse{}, tokendata.ErrTimeout - // } - // return attestationResponse{}, err - //} - //defer res.Body.Close() - // - //// Explicitly signal if the API is being rate limited - //if res.StatusCode == http.StatusTooManyRequests { - // return attestationResponse{}, tokendata.ErrRateLimit - //} - // - //body, err := io.ReadAll(res.Body) - body, err := s.httpClient.GetWithTimeout(ctx, fullAttestationUrl, s.attestationApiTimeout) - if err != nil { return attestationResponse{}, err } @@ -190,11 +163,9 @@ func (s *TokenDataReader) callAttestationApi(ctx context.Context, usdcMessageHas if err != nil { return attestationResponse{}, err } - if response.Status == "" { return attestationResponse{}, fmt.Errorf("invalid attestation response: %s", string(body)) } - return response, nil } From ef02ce6ca25bd25f5177f9d56126ed01706c6435 Mon Sep 17 00:00:00 2001 From: Jean Arnaud Date: Thu, 9 Nov 2023 19:17:22 +0100 Subject: [PATCH 03/12] Verify JSON marshalling --- .../ocr2/plugins/ccip/tokendata/observability/usdc_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/core/services/ocr2/plugins/ccip/tokendata/observability/usdc_test.go b/core/services/ocr2/plugins/ccip/tokendata/observability/usdc_test.go index ca8f4bb7df..b14191ec61 100644 --- a/core/services/ocr2/plugins/ccip/tokendata/observability/usdc_test.go +++ b/core/services/ocr2/plugins/ccip/tokendata/observability/usdc_test.go @@ -42,6 +42,7 @@ func TestUSDCMonitoring(t *testing.T) { Attestation: "720502893578a89a8a87982982ef781c18b193", } responseBytes, err := json.Marshal(response) + require.NoError(t, err) _, err = w.Write(responseBytes) require.NoError(t, err) })) From 5494de03a1dae9e2beea720d14145877fcb0c8d9 Mon Sep 17 00:00:00 2001 From: Jean Arnaud Date: Fri, 10 Nov 2023 15:39:51 +0100 Subject: [PATCH 04/12] Test rate limited error --- .../ccip/tokendata/observability/usdc.go | 4 +- .../ccip/tokendata/observability/usdc_test.go | 144 +++++++++++++----- .../ccip/tokendata/usdc/http_client.go | 1 - 3 files changed, 113 insertions(+), 36 deletions(-) diff --git a/core/services/ocr2/plugins/ccip/tokendata/observability/usdc.go b/core/services/ocr2/plugins/ccip/tokendata/observability/usdc.go index 3fd33cb33c..856fe83e5b 100644 --- a/core/services/ocr2/plugins/ccip/tokendata/observability/usdc.go +++ b/core/services/ocr2/plugins/ccip/tokendata/observability/usdc.go @@ -2,6 +2,7 @@ package observability import ( "context" + "strconv" "time" "github.com/prometheus/client_golang/prometheus" @@ -24,7 +25,7 @@ var ( float64(1 * time.Second), float64(2 * time.Second), } - labels = []string{"function", "success"} + labels = []string{"plugin", "function", "success"} usdcHistogram = promauto.NewHistogramVec(prometheus.HistogramOpts{ Name: "ccip_usdc_reader_request_total", Help: "Latency of calls to the USDC reader", @@ -65,6 +66,7 @@ func withObservedContract[T any](metric metricDetails, function string, contract WithLabelValues( metric.pluginName, function, + strconv.FormatBool(err == nil), ). Observe(float64(time.Since(contractExecutionStarted))) return value, err diff --git a/core/services/ocr2/plugins/ccip/tokendata/observability/usdc_test.go b/core/services/ocr2/plugins/ccip/tokendata/observability/usdc_test.go index b14191ec61..6fec603c72 100644 --- a/core/services/ocr2/plugins/ccip/tokendata/observability/usdc_test.go +++ b/core/services/ocr2/plugins/ccip/tokendata/observability/usdc_test.go @@ -6,10 +6,11 @@ import ( "net/http" "net/http/httptest" "net/url" + "strconv" "testing" - "github.com/ethereum/go-ethereum/common/hexutil" "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/promauto" io_prometheus_client "github.com/prometheus/client_model/go" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" @@ -21,52 +22,104 @@ import ( "github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ccip/tokendata/usdc" ) -// TODO avoid duplicating types from usdc.go -type attestationResponse struct { - Status string `json:"status"` - Attestation string `json:"attestation"` +var ( + pluginName = "testplugin" +) + +type expected struct { + pluginName string + function string + success bool + count int } func TestUSDCMonitoring(t *testing.T) { - lggr := logger.TestLogger(t) - usdcReader := mocks.NewUSDCReader(t) - msgBody := []byte{0xb0, 0xd1} - usdcReader.On("GetLastUSDCMessagePriorToLogIndexInTx", mock.Anything, mock.Anything, mock.Anything).Return(msgBody, nil) + tests := []struct { + name string + server *httptest.Server + requests int + expected []expected + }{ + { + name: "success", + server: newSuccessServer(t), + requests: 5, + expected: []expected{ + {pluginName, "ReadTokenData", true, 5}, + {pluginName, "ReadTokenData", false, 0}, + {pluginName, "GetWithTimeout", true, 5}, + {pluginName, "GetWithTimeout", false, 0}, + }, + }, + { + name: "rate_limited", + server: newRateLimitedServer(), + requests: 26, + expected: []expected{ + {pluginName, "ReadTokenData", true, 0}, + {pluginName, "ReadTokenData", false, 26}, + {pluginName, "GetWithTimeout", true, 0}, + {pluginName, "GetWithTimeout", false, 26}, + }, + }, + } - // Create a fake USDC server. - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(http.StatusOK) - response := attestationResponse{ - Status: "complete", - Attestation: "720502893578a89a8a87982982ef781c18b193", - } - responseBytes, err := json.Marshal(response) - require.NoError(t, err) - _, err = w.Write(responseBytes) - require.NoError(t, err) - })) + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + testMonitoring(t, test.name, test.server, test.requests, test.expected, logger.TestLogger(t)) + }) + } + +} + +func testMonitoring(t *testing.T, name string, server *httptest.Server, requests int, expected []expected, log logger.Logger) { + server.Start() defer server.Close() attestationURI, err := url.ParseRequestURI(server.URL) require.NoError(t, err) + // Define test histogram (avoid side effects from other tests if using the real usdcHistogram). + histogram := promauto.NewHistogramVec(prometheus.HistogramOpts{ + Name: "test_histogram_" + name, + Help: "Latency of calls to the USDC reader", + Buckets: latencyBuckets, + }, []string{"plugin", "function", "success"}) + + // Mock USDC reader. + usdcReader := mocks.NewUSDCReader(t) + msgBody := []byte{0xb0, 0xd1} + usdcReader.On("GetLastUSDCMessagePriorToLogIndexInTx", mock.Anything, mock.Anything, mock.Anything).Return(msgBody, nil) + // Service with mock http client. - usdcService := usdc.NewUSDCTokenDataReader(lggr, usdcReader, attestationURI, 10) - observedService := NewObservedUSDCTokenDataReader(*usdcService, "plugin") + usdcService := usdc.NewUSDCTokenDataReader(log, usdcReader, attestationURI, 0) + observedHttpClient := &ObservedIHttpClient{ + IHttpClient: &usdc.HttpClient{}, + metric: metricDetails{histogram, pluginName}, + } + observedService := &ObservedUSDCTokenDataReader{ + TokenDataReader: *usdc.NewUSDCTokenDataReaderWithHttpClient(*usdcService, observedHttpClient), + metric: metricDetails{histogram, pluginName}, + } require.NotNil(t, observedService) - msgAndAttestation, err := observedService.ReadTokenData(context.Background(), internal.EVM2EVMOnRampCCIPSendRequestedWithMeta{}) - require.NoError(t, err) - require.NotNil(t, msgAndAttestation) - expectedMessageAndAttestation := "0x0000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000000000000000000800000000000000000000000000000000000000000000000000000000000000002b0d10000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000013720502893578a89a8a87982982ef781c18b19300000000000000000000000000" - require.Equal(t, expectedMessageAndAttestation, hexutil.Encode(msgAndAttestation)) - // Check that the metrics are updated. - histogram := usdcHistogram - assert.Equal(t, 0, counterFromHistogramByLabels(t, histogram, "plugin", "XYZMethod")) - assert.Equal(t, 1, counterFromHistogramByLabels(t, histogram, "plugin", "ReadTokenData")) - assert.Equal(t, 0, counterFromHistogramByLabels(t, histogram, "plugin", "Get")) - assert.Equal(t, 1, counterFromHistogramByLabels(t, histogram, "plugin", "GetWithTimeout")) + for i := 0; i < requests; i++ { + //msgAndAttestation, err := observedService.ReadTokenData(context.Background(), internal.EVM2EVMOnRampCCIPSendRequestedWithMeta{}) + _, _ = observedService.ReadTokenData(context.Background(), internal.EVM2EVMOnRampCCIPSendRequestedWithMeta{}) + //require.NoError(t, err) + //require.NotNil(t, msgAndAttestation) + //expectedMessageAndAttestation := "0x0000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000000000000000000800000000000000000000000000000000000000000000000000000000000000002b0d10000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000013720502893578a89a8a87982982ef781c18b19300000000000000000000000000" + //require.Equal(t, expectedMessageAndAttestation, hexutil.Encode(msgAndAttestation)) + } + // Check that the metrics are updated. + //histogram := usdcHistogram + assert.Equal(t, 0, counterFromHistogramByLabels(t, histogram, pluginName, "XYZMethod", "true")) + assert.Equal(t, 0, counterFromHistogramByLabels(t, histogram, pluginName, "Get", "true")) + assert.Equal(t, 0, counterFromHistogramByLabels(t, histogram, pluginName, "Get", "false")) + for _, e := range expected { + assert.Equal(t, e.count, counterFromHistogramByLabels(t, histogram, e.pluginName, e.function, strconv.FormatBool(e.success))) + } } func counterFromHistogramByLabels(t *testing.T, histogramVec *prometheus.HistogramVec, labels ...string) int { @@ -84,3 +137,26 @@ func counterFromHistogramByLabels(t *testing.T, histogramVec *prometheus.Histogr return int(pb.GetHistogram().GetSampleCount()) } + +func newSuccessServer(t *testing.T) *httptest.Server { + return httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + response := struct { + Status string `json:"status"` + Attestation string `json:"attestation"` + }{ + Status: "complete", + Attestation: "720502893578a89a8a87982982ef781c18b193", + } + responseBytes, err := json.Marshal(response) + require.NoError(t, err) + _, err = w.Write(responseBytes) + require.NoError(t, err) + })) +} + +func newRateLimitedServer() *httptest.Server { + return httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusTooManyRequests) + })) +} diff --git a/core/services/ocr2/plugins/ccip/tokendata/usdc/http_client.go b/core/services/ocr2/plugins/ccip/tokendata/usdc/http_client.go index 99f952aad9..a3590ae02b 100644 --- a/core/services/ocr2/plugins/ccip/tokendata/usdc/http_client.go +++ b/core/services/ocr2/plugins/ccip/tokendata/usdc/http_client.go @@ -39,7 +39,6 @@ func (s *HttpClient) GetWithTimeout(ctx context.Context, url string, timeout tim timeoutCtx, cancel := context.WithTimeout(ctx, timeout) defer cancel() req, err := http.NewRequestWithContext(timeoutCtx, "GET", url, nil) - if err != nil { return nil, err } From 588cfe5dbbd1b61cee9c3fb63f9644c55b79f476 Mon Sep 17 00:00:00 2001 From: Jean Arnaud Date: Fri, 10 Nov 2023 16:13:54 +0100 Subject: [PATCH 05/12] Separate histograms and labels for client and reader --- .../tokendata/observability/http_client.go | 25 ++++-- .../ccip/tokendata/observability/usdc.go | 16 ++-- .../ccip/tokendata/observability/usdc_test.go | 79 ++++++++++++------- .../ccip/tokendata/usdc/http_client.go | 26 +++--- .../ocr2/plugins/ccip/tokendata/usdc/usdc.go | 2 +- 5 files changed, 94 insertions(+), 54 deletions(-) diff --git a/core/services/ocr2/plugins/ccip/tokendata/observability/http_client.go b/core/services/ocr2/plugins/ccip/tokendata/observability/http_client.go index 2867f9a99f..66b3cab518 100644 --- a/core/services/ocr2/plugins/ccip/tokendata/observability/http_client.go +++ b/core/services/ocr2/plugins/ccip/tokendata/observability/http_client.go @@ -2,6 +2,7 @@ package observability import ( "context" + "strconv" "time" "github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ccip/tokendata/usdc" @@ -16,20 +17,34 @@ func NewObservedIHttpClient(origin usdc.IHttpClient, pluginName string) *Observe return &ObservedIHttpClient{ IHttpClient: origin, metric: metricDetails{ - histogram: usdcHistogram, + histogram: usdcClientHistogram, pluginName: pluginName, }, } } -func (o *ObservedIHttpClient) Get(ctx context.Context, url string) ([]byte, error) { - return withObservedContract(o.metric, "Get", func() ([]byte, error) { +func (o *ObservedIHttpClient) Get(ctx context.Context, url string) ([]byte, int, error) { + return withObservedHttpClient(o.metric, "Get", func() ([]byte, int, error) { return o.IHttpClient.Get(ctx, url) }) } -func (o *ObservedIHttpClient) GetWithTimeout(ctx context.Context, url string, timeout time.Duration) ([]byte, error) { - return withObservedContract(o.metric, "GetWithTimeout", func() ([]byte, error) { +func (o *ObservedIHttpClient) GetWithTimeout(ctx context.Context, url string, timeout time.Duration) ([]byte, int, error) { + return withObservedHttpClient(o.metric, "GetWithTimeout", func() ([]byte, int, error) { return o.IHttpClient.GetWithTimeout(ctx, url, timeout) }) } + +func withObservedHttpClient[T any](metric metricDetails, function string, contract func() (T, int, error)) (T, int, error) { + contractExecutionStarted := time.Now() + value, status, err := contract() + metric.histogram. + WithLabelValues( + metric.pluginName, + function, + strconv.FormatInt(int64(status), 10), + strconv.FormatBool(err == nil), + ). + Observe(float64(time.Since(contractExecutionStarted))) + return value, status, err +} diff --git a/core/services/ocr2/plugins/ccip/tokendata/observability/usdc.go b/core/services/ocr2/plugins/ccip/tokendata/observability/usdc.go index 856fe83e5b..f0516f5a46 100644 --- a/core/services/ocr2/plugins/ccip/tokendata/observability/usdc.go +++ b/core/services/ocr2/plugins/ccip/tokendata/observability/usdc.go @@ -25,12 +25,16 @@ var ( float64(1 * time.Second), float64(2 * time.Second), } - labels = []string{"plugin", "function", "success"} - usdcHistogram = promauto.NewHistogramVec(prometheus.HistogramOpts{ + usdcReaderHistogram = promauto.NewHistogramVec(prometheus.HistogramOpts{ Name: "ccip_usdc_reader_request_total", Help: "Latency of calls to the USDC reader", Buckets: latencyBuckets, - }, labels) + }, []string{"plugin", "function", "success"}) + usdcClientHistogram = promauto.NewHistogramVec(prometheus.HistogramOpts{ + Name: "ccip_usdc_client_request_total", + Help: "Latency of calls to the USDC client", + Buckets: latencyBuckets, + }, []string{"plugin", "function", "status", "success"}) ) type metricDetails struct { @@ -47,19 +51,19 @@ func NewObservedUSDCTokenDataReader(origin usdc.TokenDataReader, pluginName stri return &ObservedUSDCTokenDataReader{ TokenDataReader: *usdc.NewUSDCTokenDataReaderWithHttpClient(origin, NewObservedIHttpClient(&usdc.HttpClient{}, pluginName)), metric: metricDetails{ - histogram: usdcHistogram, + histogram: usdcReaderHistogram, pluginName: pluginName, }, } } func (o *ObservedUSDCTokenDataReader) ReadTokenData(ctx context.Context, msg internal.EVM2EVMOnRampCCIPSendRequestedWithMeta) ([]byte, error) { - return withObservedContract(o.metric, "ReadTokenData", func() ([]byte, error) { + return withObservedReader(o.metric, "ReadTokenData", func() ([]byte, error) { return o.TokenDataReader.ReadTokenData(ctx, msg) }) } -func withObservedContract[T any](metric metricDetails, function string, contract func() (T, error)) (T, error) { +func withObservedReader[T any](metric metricDetails, function string, contract func() (T, error)) (T, error) { contractExecutionStarted := time.Now() value, err := contract() metric.histogram. diff --git a/core/services/ocr2/plugins/ccip/tokendata/observability/usdc_test.go b/core/services/ocr2/plugins/ccip/tokendata/observability/usdc_test.go index 6fec603c72..9ddfe8dc45 100644 --- a/core/services/ocr2/plugins/ccip/tokendata/observability/usdc_test.go +++ b/core/services/ocr2/plugins/ccip/tokendata/observability/usdc_test.go @@ -6,7 +6,6 @@ import ( "net/http" "net/http/httptest" "net/url" - "strconv" "testing" "github.com/prometheus/client_golang/prometheus" @@ -26,65 +25,83 @@ var ( pluginName = "testplugin" ) -type expected struct { +type expectedClient struct { pluginName string function string - success bool + status string + result string + count int +} + +type expectedReader struct { + pluginName string + function string + result string count int } func TestUSDCMonitoring(t *testing.T) { tests := []struct { - name string - server *httptest.Server - requests int - expected []expected + name string + server *httptest.Server + requests int + expectedClientHistogram []expectedClient + expectedReaderHistogram []expectedReader }{ { name: "success", server: newSuccessServer(t), requests: 5, - expected: []expected{ - {pluginName, "ReadTokenData", true, 5}, - {pluginName, "ReadTokenData", false, 0}, - {pluginName, "GetWithTimeout", true, 5}, - {pluginName, "GetWithTimeout", false, 0}, + expectedClientHistogram: []expectedClient{ + {pluginName, "GetWithTimeout", "200", "true", 5}, + {pluginName, "GetWithTimeout", "429", "false", 0}, + }, + expectedReaderHistogram: []expectedReader{ + {pluginName, "ReadTokenData", "true", 5}, + {pluginName, "ReadTokenData", "false", 0}, }, }, { name: "rate_limited", server: newRateLimitedServer(), requests: 26, - expected: []expected{ - {pluginName, "ReadTokenData", true, 0}, - {pluginName, "ReadTokenData", false, 26}, - {pluginName, "GetWithTimeout", true, 0}, - {pluginName, "GetWithTimeout", false, 26}, + expectedClientHistogram: []expectedClient{ + {pluginName, "GetWithTimeout", "200", "true", 0}, + {pluginName, "GetWithTimeout", "429", "false", 26}, + }, + expectedReaderHistogram: []expectedReader{ + {pluginName, "ReadTokenData", "true", 0}, + {pluginName, "ReadTokenData", "false", 26}, }, }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - testMonitoring(t, test.name, test.server, test.requests, test.expected, logger.TestLogger(t)) + testMonitoring(t, test.name, test.server, test.requests, test.expectedClientHistogram, test.expectedReaderHistogram, logger.TestLogger(t)) }) } } -func testMonitoring(t *testing.T, name string, server *httptest.Server, requests int, expected []expected, log logger.Logger) { +func testMonitoring(t *testing.T, name string, server *httptest.Server, requests int, expectedClient []expectedClient, expectedReader []expectedReader, log logger.Logger) { server.Start() defer server.Close() attestationURI, err := url.ParseRequestURI(server.URL) require.NoError(t, err) // Define test histogram (avoid side effects from other tests if using the real usdcHistogram). - histogram := promauto.NewHistogramVec(prometheus.HistogramOpts{ - Name: "test_histogram_" + name, + readerHistogram := promauto.NewHistogramVec(prometheus.HistogramOpts{ + Name: "test_reader_histogram_" + name, Help: "Latency of calls to the USDC reader", Buckets: latencyBuckets, }, []string{"plugin", "function", "success"}) + clientHistogram := promauto.NewHistogramVec(prometheus.HistogramOpts{ + Name: "test_client_histogram_" + name, + Help: "Latency of calls to the USDC client", + Buckets: latencyBuckets, + }, []string{"plugin", "function", "status", "success"}) // Mock USDC reader. usdcReader := mocks.NewUSDCReader(t) @@ -95,17 +112,17 @@ func testMonitoring(t *testing.T, name string, server *httptest.Server, requests usdcService := usdc.NewUSDCTokenDataReader(log, usdcReader, attestationURI, 0) observedHttpClient := &ObservedIHttpClient{ IHttpClient: &usdc.HttpClient{}, - metric: metricDetails{histogram, pluginName}, + metric: metricDetails{clientHistogram, pluginName}, } observedService := &ObservedUSDCTokenDataReader{ TokenDataReader: *usdc.NewUSDCTokenDataReaderWithHttpClient(*usdcService, observedHttpClient), - metric: metricDetails{histogram, pluginName}, + metric: metricDetails{readerHistogram, pluginName}, } require.NotNil(t, observedService) for i := 0; i < requests; i++ { - //msgAndAttestation, err := observedService.ReadTokenData(context.Background(), internal.EVM2EVMOnRampCCIPSendRequestedWithMeta{}) _, _ = observedService.ReadTokenData(context.Background(), internal.EVM2EVMOnRampCCIPSendRequestedWithMeta{}) + //msgAndAttestation, err := observedService.ReadTokenData(context.Background(), internal.EVM2EVMOnRampCCIPSendRequestedWithMeta{}) //require.NoError(t, err) //require.NotNil(t, msgAndAttestation) //expectedMessageAndAttestation := "0x0000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000000000000000000800000000000000000000000000000000000000000000000000000000000000002b0d10000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000013720502893578a89a8a87982982ef781c18b19300000000000000000000000000" @@ -113,12 +130,14 @@ func testMonitoring(t *testing.T, name string, server *httptest.Server, requests } // Check that the metrics are updated. - //histogram := usdcHistogram - assert.Equal(t, 0, counterFromHistogramByLabels(t, histogram, pluginName, "XYZMethod", "true")) - assert.Equal(t, 0, counterFromHistogramByLabels(t, histogram, pluginName, "Get", "true")) - assert.Equal(t, 0, counterFromHistogramByLabels(t, histogram, pluginName, "Get", "false")) - for _, e := range expected { - assert.Equal(t, e.count, counterFromHistogramByLabels(t, histogram, e.pluginName, e.function, strconv.FormatBool(e.success))) + assert.Equal(t, 0, counterFromHistogramByLabels(t, readerHistogram, pluginName, "XYZMethod", "true")) + assert.Equal(t, 0, counterFromHistogramByLabels(t, clientHistogram, pluginName, "Get", "200", "true")) + assert.Equal(t, 0, counterFromHistogramByLabels(t, clientHistogram, pluginName, "Get", "429", "false")) + for _, e := range expectedClient { + assert.Equal(t, e.count, counterFromHistogramByLabels(t, clientHistogram, e.pluginName, e.function, e.status, e.result)) + } + for _, e := range expectedReader { + assert.Equal(t, e.count, counterFromHistogramByLabels(t, readerHistogram, e.pluginName, e.function, e.result)) } } diff --git a/core/services/ocr2/plugins/ccip/tokendata/usdc/http_client.go b/core/services/ocr2/plugins/ccip/tokendata/usdc/http_client.go index a3590ae02b..7bf9e8cb14 100644 --- a/core/services/ocr2/plugins/ccip/tokendata/usdc/http_client.go +++ b/core/services/ocr2/plugins/ccip/tokendata/usdc/http_client.go @@ -13,49 +13,51 @@ import ( type IHttpClient interface { // Get issue a GET request to the given url and return the response body. - Get(ctx context.Context, url string) ([]byte, error) - GetWithTimeout(ctx context.Context, url string, timeout time.Duration) ([]byte, error) + Get(ctx context.Context, url string) ([]byte, int, error) + GetWithTimeout(ctx context.Context, url string, timeout time.Duration) ([]byte, int, error) } type HttpClient struct { } -func (s *HttpClient) Get(ctx context.Context, url string) ([]byte, error) { +func (s *HttpClient) Get(ctx context.Context, url string) ([]byte, int, error) { req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) if err != nil { - return nil, err + return nil, http.StatusBadRequest, err } req.Header.Add("accept", "application/json") res, err := http.DefaultClient.Do(req) if err != nil { - return nil, err + return nil, res.StatusCode, err } defer res.Body.Close() - return io.ReadAll(res.Body) + body, err := io.ReadAll(res.Body) + return body, res.StatusCode, err } -func (s *HttpClient) GetWithTimeout(ctx context.Context, url string, timeout time.Duration) ([]byte, error) { +func (s *HttpClient) GetWithTimeout(ctx context.Context, url string, timeout time.Duration) ([]byte, int, error) { // Use a timeout to guard against attestation API hanging, causing observation timeout and failing to make any progress. timeoutCtx, cancel := context.WithTimeout(ctx, timeout) defer cancel() req, err := http.NewRequestWithContext(timeoutCtx, "GET", url, nil) if err != nil { - return nil, err + return nil, http.StatusBadRequest, err } req.Header.Add("accept", "application/json") res, err := http.DefaultClient.Do(req) if err != nil { if errors.Is(err, context.DeadlineExceeded) { - return nil, tokendata.ErrTimeout + return nil, res.StatusCode, tokendata.ErrTimeout } - return nil, err + return nil, res.StatusCode, err } defer res.Body.Close() // Explicitly signal if the API is being rate limited if res.StatusCode == http.StatusTooManyRequests { - return nil, tokendata.ErrRateLimit + return nil, res.StatusCode, tokendata.ErrRateLimit } - return io.ReadAll(res.Body) + body, err := io.ReadAll(res.Body) + return body, res.StatusCode, err } diff --git a/core/services/ocr2/plugins/ccip/tokendata/usdc/usdc.go b/core/services/ocr2/plugins/ccip/tokendata/usdc/usdc.go index 58492be81e..338efb6b0b 100644 --- a/core/services/ocr2/plugins/ccip/tokendata/usdc/usdc.go +++ b/core/services/ocr2/plugins/ccip/tokendata/usdc/usdc.go @@ -154,7 +154,7 @@ func (s *TokenDataReader) getUSDCMessageBody(ctx context.Context, msg internal.E func (s *TokenDataReader) callAttestationApi(ctx context.Context, usdcMessageHash [32]byte) (attestationResponse, error) { fullAttestationUrl := fmt.Sprintf("%s/%s/%s/0x%x", s.attestationApi, apiVersion, attestationPath, usdcMessageHash) - body, err := s.httpClient.GetWithTimeout(ctx, fullAttestationUrl, s.attestationApiTimeout) + body, _, err := s.httpClient.GetWithTimeout(ctx, fullAttestationUrl, s.attestationApiTimeout) if err != nil { return attestationResponse{}, err } From a2cd68766f9290e9d7cc969b518309ea042d7949 Mon Sep 17 00:00:00 2001 From: Jean Arnaud Date: Fri, 10 Nov 2023 16:34:56 +0100 Subject: [PATCH 06/12] Fix return status --- core/services/ocr2/plugins/ccip/tokendata/usdc/http_client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/services/ocr2/plugins/ccip/tokendata/usdc/http_client.go b/core/services/ocr2/plugins/ccip/tokendata/usdc/http_client.go index 7bf9e8cb14..0a55288891 100644 --- a/core/services/ocr2/plugins/ccip/tokendata/usdc/http_client.go +++ b/core/services/ocr2/plugins/ccip/tokendata/usdc/http_client.go @@ -47,7 +47,7 @@ func (s *HttpClient) GetWithTimeout(ctx context.Context, url string, timeout tim res, err := http.DefaultClient.Do(req) if err != nil { if errors.Is(err, context.DeadlineExceeded) { - return nil, res.StatusCode, tokendata.ErrTimeout + return nil, http.StatusRequestTimeout, tokendata.ErrTimeout } return nil, res.StatusCode, err } From 5d616ae669342a96bbe7558b0fd97238a0e14f60 Mon Sep 17 00:00:00 2001 From: Jean Arnaud Date: Fri, 10 Nov 2023 16:54:31 +0100 Subject: [PATCH 07/12] Doc --- core/services/ocr2/plugins/ccip/tokendata/usdc/http_client.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/services/ocr2/plugins/ccip/tokendata/usdc/http_client.go b/core/services/ocr2/plugins/ccip/tokendata/usdc/http_client.go index 0a55288891..d7b947de59 100644 --- a/core/services/ocr2/plugins/ccip/tokendata/usdc/http_client.go +++ b/core/services/ocr2/plugins/ccip/tokendata/usdc/http_client.go @@ -12,8 +12,9 @@ import ( ) type IHttpClient interface { - // Get issue a GET request to the given url and return the response body. + // Get issue a GET request to the given url and return the response body and status code. Get(ctx context.Context, url string) ([]byte, int, error) + // GetWithTimeout issue a GET request to the given url and return the response body and status code. GetWithTimeout(ctx context.Context, url string, timeout time.Duration) ([]byte, int, error) } From 05f4e0b7b7f56d82bae260a7b56f982938d8a2ef Mon Sep 17 00:00:00 2001 From: Jean Arnaud Date: Mon, 13 Nov 2023 14:06:30 +0100 Subject: [PATCH 08/12] Remove observed token data reader --- .../tokendata/observability/http_client.go | 43 +++++++--- .../ccip/tokendata/observability/usdc.go | 77 ----------------- .../ccip/tokendata/observability/usdc_test.go | 84 ++++++------------- .../ccip/tokendata/usdc/http_client.go | 21 +---- .../ocr2/plugins/ccip/tokendata/usdc/usdc.go | 2 +- 5 files changed, 59 insertions(+), 168 deletions(-) delete mode 100644 core/services/ocr2/plugins/ccip/tokendata/observability/usdc.go diff --git a/core/services/ocr2/plugins/ccip/tokendata/observability/http_client.go b/core/services/ocr2/plugins/ccip/tokendata/observability/http_client.go index 66b3cab518..c48098b699 100644 --- a/core/services/ocr2/plugins/ccip/tokendata/observability/http_client.go +++ b/core/services/ocr2/plugins/ccip/tokendata/observability/http_client.go @@ -5,9 +5,37 @@ import ( "strconv" "time" + "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/promauto" + "github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ccip/tokendata/usdc" ) +var ( + latencyBuckets = []float64{ + float64(10 * time.Millisecond), + float64(25 * time.Millisecond), + float64(50 * time.Millisecond), + float64(75 * time.Millisecond), + float64(100 * time.Millisecond), + float64(250 * time.Millisecond), + float64(500 * time.Millisecond), + float64(750 * time.Millisecond), + float64(1 * time.Second), + float64(2 * time.Second), + } + usdcClientHistogram = promauto.NewHistogramVec(prometheus.HistogramOpts{ + Name: "ccip_usdc_client_request_total", + Help: "Latency of calls to the USDC client", + Buckets: latencyBuckets, + }, []string{"plugin", "status", "success"}) +) + +type metricDetails struct { + histogram *prometheus.HistogramVec + pluginName string +} + type ObservedIHttpClient struct { usdc.IHttpClient metric metricDetails @@ -23,25 +51,18 @@ func NewObservedIHttpClient(origin usdc.IHttpClient, pluginName string) *Observe } } -func (o *ObservedIHttpClient) Get(ctx context.Context, url string) ([]byte, int, error) { - return withObservedHttpClient(o.metric, "Get", func() ([]byte, int, error) { - return o.IHttpClient.Get(ctx, url) - }) -} - -func (o *ObservedIHttpClient) GetWithTimeout(ctx context.Context, url string, timeout time.Duration) ([]byte, int, error) { - return withObservedHttpClient(o.metric, "GetWithTimeout", func() ([]byte, int, error) { - return o.IHttpClient.GetWithTimeout(ctx, url, timeout) +func (o *ObservedIHttpClient) Get(ctx context.Context, url string, timeout time.Duration) ([]byte, int, error) { + return withObservedHttpClient(o.metric, func() ([]byte, int, error) { + return o.IHttpClient.Get(ctx, url, timeout) }) } -func withObservedHttpClient[T any](metric metricDetails, function string, contract func() (T, int, error)) (T, int, error) { +func withObservedHttpClient[T any](metric metricDetails, contract func() (T, int, error)) (T, int, error) { contractExecutionStarted := time.Now() value, status, err := contract() metric.histogram. WithLabelValues( metric.pluginName, - function, strconv.FormatInt(int64(status), 10), strconv.FormatBool(err == nil), ). diff --git a/core/services/ocr2/plugins/ccip/tokendata/observability/usdc.go b/core/services/ocr2/plugins/ccip/tokendata/observability/usdc.go deleted file mode 100644 index f0516f5a46..0000000000 --- a/core/services/ocr2/plugins/ccip/tokendata/observability/usdc.go +++ /dev/null @@ -1,77 +0,0 @@ -package observability - -import ( - "context" - "strconv" - "time" - - "github.com/prometheus/client_golang/prometheus" - "github.com/prometheus/client_golang/prometheus/promauto" - - "github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ccip/internal" - "github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ccip/tokendata/usdc" -) - -var ( - latencyBuckets = []float64{ - float64(10 * time.Millisecond), - float64(25 * time.Millisecond), - float64(50 * time.Millisecond), - float64(75 * time.Millisecond), - float64(100 * time.Millisecond), - float64(250 * time.Millisecond), - float64(500 * time.Millisecond), - float64(750 * time.Millisecond), - float64(1 * time.Second), - float64(2 * time.Second), - } - usdcReaderHistogram = promauto.NewHistogramVec(prometheus.HistogramOpts{ - Name: "ccip_usdc_reader_request_total", - Help: "Latency of calls to the USDC reader", - Buckets: latencyBuckets, - }, []string{"plugin", "function", "success"}) - usdcClientHistogram = promauto.NewHistogramVec(prometheus.HistogramOpts{ - Name: "ccip_usdc_client_request_total", - Help: "Latency of calls to the USDC client", - Buckets: latencyBuckets, - }, []string{"plugin", "function", "status", "success"}) -) - -type metricDetails struct { - histogram *prometheus.HistogramVec - pluginName string -} - -type ObservedUSDCTokenDataReader struct { - usdc.TokenDataReader - metric metricDetails -} - -func NewObservedUSDCTokenDataReader(origin usdc.TokenDataReader, pluginName string) *ObservedUSDCTokenDataReader { - return &ObservedUSDCTokenDataReader{ - TokenDataReader: *usdc.NewUSDCTokenDataReaderWithHttpClient(origin, NewObservedIHttpClient(&usdc.HttpClient{}, pluginName)), - metric: metricDetails{ - histogram: usdcReaderHistogram, - pluginName: pluginName, - }, - } -} - -func (o *ObservedUSDCTokenDataReader) ReadTokenData(ctx context.Context, msg internal.EVM2EVMOnRampCCIPSendRequestedWithMeta) ([]byte, error) { - return withObservedReader(o.metric, "ReadTokenData", func() ([]byte, error) { - return o.TokenDataReader.ReadTokenData(ctx, msg) - }) -} - -func withObservedReader[T any](metric metricDetails, function string, contract func() (T, error)) (T, error) { - contractExecutionStarted := time.Now() - value, err := contract() - metric.histogram. - WithLabelValues( - metric.pluginName, - function, - strconv.FormatBool(err == nil), - ). - Observe(float64(time.Since(contractExecutionStarted))) - return value, err -} diff --git a/core/services/ocr2/plugins/ccip/tokendata/observability/usdc_test.go b/core/services/ocr2/plugins/ccip/tokendata/observability/usdc_test.go index 9ddfe8dc45..9c4cc0ffc2 100644 --- a/core/services/ocr2/plugins/ccip/tokendata/observability/usdc_test.go +++ b/core/services/ocr2/plugins/ccip/tokendata/observability/usdc_test.go @@ -25,119 +25,83 @@ var ( pluginName = "testplugin" ) -type expectedClient struct { +type expected struct { pluginName string - function string status string result string count int } -type expectedReader struct { - pluginName string - function string - result string - count int -} - -func TestUSDCMonitoring(t *testing.T) { +func TestUSDCClientMonitoring(t *testing.T) { tests := []struct { - name string - server *httptest.Server - requests int - expectedClientHistogram []expectedClient - expectedReaderHistogram []expectedReader + name string + server *httptest.Server + requests int + expected []expected }{ { name: "success", server: newSuccessServer(t), requests: 5, - expectedClientHistogram: []expectedClient{ - {pluginName, "GetWithTimeout", "200", "true", 5}, - {pluginName, "GetWithTimeout", "429", "false", 0}, - }, - expectedReaderHistogram: []expectedReader{ - {pluginName, "ReadTokenData", "true", 5}, - {pluginName, "ReadTokenData", "false", 0}, + expected: []expected{ + {pluginName, "200", "true", 5}, + {pluginName, "429", "false", 0}, }, }, { name: "rate_limited", server: newRateLimitedServer(), requests: 26, - expectedClientHistogram: []expectedClient{ - {pluginName, "GetWithTimeout", "200", "true", 0}, - {pluginName, "GetWithTimeout", "429", "false", 26}, - }, - expectedReaderHistogram: []expectedReader{ - {pluginName, "ReadTokenData", "true", 0}, - {pluginName, "ReadTokenData", "false", 26}, + expected: []expected{ + {pluginName, "200", "true", 0}, + {pluginName, "429", "false", 26}, }, }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - testMonitoring(t, test.name, test.server, test.requests, test.expectedClientHistogram, test.expectedReaderHistogram, logger.TestLogger(t)) + testMonitoring(t, test.name, test.server, test.requests, test.expected, logger.TestLogger(t)) }) } } -func testMonitoring(t *testing.T, name string, server *httptest.Server, requests int, expectedClient []expectedClient, expectedReader []expectedReader, log logger.Logger) { +func testMonitoring(t *testing.T, name string, server *httptest.Server, requests int, expected []expected, log logger.Logger) { server.Start() defer server.Close() attestationURI, err := url.ParseRequestURI(server.URL) require.NoError(t, err) // Define test histogram (avoid side effects from other tests if using the real usdcHistogram). - readerHistogram := promauto.NewHistogramVec(prometheus.HistogramOpts{ - Name: "test_reader_histogram_" + name, - Help: "Latency of calls to the USDC reader", - Buckets: latencyBuckets, - }, []string{"plugin", "function", "success"}) - clientHistogram := promauto.NewHistogramVec(prometheus.HistogramOpts{ + histogram := promauto.NewHistogramVec(prometheus.HistogramOpts{ Name: "test_client_histogram_" + name, Help: "Latency of calls to the USDC client", Buckets: latencyBuckets, - }, []string{"plugin", "function", "status", "success"}) + }, []string{"plugin", "status", "success"}) // Mock USDC reader. usdcReader := mocks.NewUSDCReader(t) msgBody := []byte{0xb0, 0xd1} usdcReader.On("GetLastUSDCMessagePriorToLogIndexInTx", mock.Anything, mock.Anything, mock.Anything).Return(msgBody, nil) - // Service with mock http client. + // Service with monitored http client. usdcService := usdc.NewUSDCTokenDataReader(log, usdcReader, attestationURI, 0) observedHttpClient := &ObservedIHttpClient{ IHttpClient: &usdc.HttpClient{}, - metric: metricDetails{clientHistogram, pluginName}, + metric: metricDetails{histogram, pluginName}, } - observedService := &ObservedUSDCTokenDataReader{ - TokenDataReader: *usdc.NewUSDCTokenDataReaderWithHttpClient(*usdcService, observedHttpClient), - metric: metricDetails{readerHistogram, pluginName}, - } - require.NotNil(t, observedService) + tokenDataReader := *usdc.NewUSDCTokenDataReaderWithHttpClient(*usdcService, observedHttpClient) + require.NotNil(t, tokenDataReader) for i := 0; i < requests; i++ { - _, _ = observedService.ReadTokenData(context.Background(), internal.EVM2EVMOnRampCCIPSendRequestedWithMeta{}) - //msgAndAttestation, err := observedService.ReadTokenData(context.Background(), internal.EVM2EVMOnRampCCIPSendRequestedWithMeta{}) - //require.NoError(t, err) - //require.NotNil(t, msgAndAttestation) - //expectedMessageAndAttestation := "0x0000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000000000000000000800000000000000000000000000000000000000000000000000000000000000002b0d10000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000013720502893578a89a8a87982982ef781c18b19300000000000000000000000000" - //require.Equal(t, expectedMessageAndAttestation, hexutil.Encode(msgAndAttestation)) + _, _ = tokenDataReader.ReadTokenData(context.Background(), internal.EVM2EVMOnRampCCIPSendRequestedWithMeta{}) } - // Check that the metrics are updated. - assert.Equal(t, 0, counterFromHistogramByLabels(t, readerHistogram, pluginName, "XYZMethod", "true")) - assert.Equal(t, 0, counterFromHistogramByLabels(t, clientHistogram, pluginName, "Get", "200", "true")) - assert.Equal(t, 0, counterFromHistogramByLabels(t, clientHistogram, pluginName, "Get", "429", "false")) - for _, e := range expectedClient { - assert.Equal(t, e.count, counterFromHistogramByLabels(t, clientHistogram, e.pluginName, e.function, e.status, e.result)) - } - for _, e := range expectedReader { - assert.Equal(t, e.count, counterFromHistogramByLabels(t, readerHistogram, e.pluginName, e.function, e.result)) + // Check that the metrics are updated as expected. + for _, e := range expected { + assert.Equal(t, e.count, counterFromHistogramByLabels(t, histogram, e.pluginName, e.status, e.result)) } } diff --git a/core/services/ocr2/plugins/ccip/tokendata/usdc/http_client.go b/core/services/ocr2/plugins/ccip/tokendata/usdc/http_client.go index d7b947de59..92cbc9aaef 100644 --- a/core/services/ocr2/plugins/ccip/tokendata/usdc/http_client.go +++ b/core/services/ocr2/plugins/ccip/tokendata/usdc/http_client.go @@ -13,30 +13,13 @@ import ( type IHttpClient interface { // Get issue a GET request to the given url and return the response body and status code. - Get(ctx context.Context, url string) ([]byte, int, error) - // GetWithTimeout issue a GET request to the given url and return the response body and status code. - GetWithTimeout(ctx context.Context, url string, timeout time.Duration) ([]byte, int, error) + Get(ctx context.Context, url string, timeout time.Duration) ([]byte, int, error) } type HttpClient struct { } -func (s *HttpClient) Get(ctx context.Context, url string) ([]byte, int, error) { - req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) - if err != nil { - return nil, http.StatusBadRequest, err - } - req.Header.Add("accept", "application/json") - res, err := http.DefaultClient.Do(req) - if err != nil { - return nil, res.StatusCode, err - } - defer res.Body.Close() - body, err := io.ReadAll(res.Body) - return body, res.StatusCode, err -} - -func (s *HttpClient) GetWithTimeout(ctx context.Context, url string, timeout time.Duration) ([]byte, int, error) { +func (s *HttpClient) Get(ctx context.Context, url string, timeout time.Duration) ([]byte, int, error) { // Use a timeout to guard against attestation API hanging, causing observation timeout and failing to make any progress. timeoutCtx, cancel := context.WithTimeout(ctx, timeout) defer cancel() diff --git a/core/services/ocr2/plugins/ccip/tokendata/usdc/usdc.go b/core/services/ocr2/plugins/ccip/tokendata/usdc/usdc.go index 338efb6b0b..d90d621f25 100644 --- a/core/services/ocr2/plugins/ccip/tokendata/usdc/usdc.go +++ b/core/services/ocr2/plugins/ccip/tokendata/usdc/usdc.go @@ -154,7 +154,7 @@ func (s *TokenDataReader) getUSDCMessageBody(ctx context.Context, msg internal.E func (s *TokenDataReader) callAttestationApi(ctx context.Context, usdcMessageHash [32]byte) (attestationResponse, error) { fullAttestationUrl := fmt.Sprintf("%s/%s/%s/0x%x", s.attestationApi, apiVersion, attestationPath, usdcMessageHash) - body, _, err := s.httpClient.GetWithTimeout(ctx, fullAttestationUrl, s.attestationApiTimeout) + body, _, err := s.httpClient.Get(ctx, fullAttestationUrl, s.attestationApiTimeout) if err != nil { return attestationResponse{}, err } From 95364aaf7968e61b469ed50ad9c48d8f79c275e3 Mon Sep 17 00:00:00 2001 From: Jean Arnaud Date: Tue, 14 Nov 2023 11:28:06 +0100 Subject: [PATCH 09/12] Refactor tokendata packages --- .../tokendata/{usdc => http}/http_client.go | 2 +- .../observed_http_client.go} | 43 +++++++++++-------- .../ccip/tokendata/observability/usdc.go | 1 + .../ccip/tokendata/observability/usdc_test.go | 35 ++++++--------- .../ocr2/plugins/ccip/tokendata/usdc/usdc.go | 8 ++-- 5 files changed, 45 insertions(+), 44 deletions(-) rename core/services/ocr2/plugins/ccip/tokendata/{usdc => http}/http_client.go (99%) rename core/services/ocr2/plugins/ccip/tokendata/{observability/http_client.go => http/observed_http_client.go} (60%) create mode 100644 core/services/ocr2/plugins/ccip/tokendata/observability/usdc.go diff --git a/core/services/ocr2/plugins/ccip/tokendata/usdc/http_client.go b/core/services/ocr2/plugins/ccip/tokendata/http/http_client.go similarity index 99% rename from core/services/ocr2/plugins/ccip/tokendata/usdc/http_client.go rename to core/services/ocr2/plugins/ccip/tokendata/http/http_client.go index 92cbc9aaef..3a00f81c0c 100644 --- a/core/services/ocr2/plugins/ccip/tokendata/usdc/http_client.go +++ b/core/services/ocr2/plugins/ccip/tokendata/http/http_client.go @@ -1,4 +1,4 @@ -package usdc +package http import ( "context" diff --git a/core/services/ocr2/plugins/ccip/tokendata/observability/http_client.go b/core/services/ocr2/plugins/ccip/tokendata/http/observed_http_client.go similarity index 60% rename from core/services/ocr2/plugins/ccip/tokendata/observability/http_client.go rename to core/services/ocr2/plugins/ccip/tokendata/http/observed_http_client.go index c48098b699..6f14d76e00 100644 --- a/core/services/ocr2/plugins/ccip/tokendata/observability/http_client.go +++ b/core/services/ocr2/plugins/ccip/tokendata/http/observed_http_client.go @@ -1,4 +1,4 @@ -package observability +package http import ( "context" @@ -7,12 +7,10 @@ import ( "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" - - "github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ccip/tokendata/usdc" ) var ( - latencyBuckets = []float64{ + UsdcLatencyBuckets = []float64{ float64(10 * time.Millisecond), float64(25 * time.Millisecond), float64(50 * time.Millisecond), @@ -23,31 +21,41 @@ var ( float64(750 * time.Millisecond), float64(1 * time.Second), float64(2 * time.Second), + float64(3 * time.Second), + float64(4 * time.Second), + float64(5 * time.Second), } usdcClientHistogram = promauto.NewHistogramVec(prometheus.HistogramOpts{ Name: "ccip_usdc_client_request_total", Help: "Latency of calls to the USDC client", - Buckets: latencyBuckets, - }, []string{"plugin", "status", "success"}) + Buckets: UsdcLatencyBuckets, + }, []string{"status", "success"}) ) -type metricDetails struct { - histogram *prometheus.HistogramVec - pluginName string +type MetricDetails struct { + histogram *prometheus.HistogramVec } type ObservedIHttpClient struct { - usdc.IHttpClient - metric metricDetails + IHttpClient + metric MetricDetails +} + +func NewMetricDetails(histogram *prometheus.HistogramVec) MetricDetails { + return MetricDetails{ + histogram: histogram, + } +} + +// NewObservedIHttpClient Create a new ObservedIHttpClient with the USDC client metric. +func NewObservedIHttpClient(origin IHttpClient) *ObservedIHttpClient { + return NewObservedIHttpClientWithMetric(origin, NewMetricDetails(usdcClientHistogram)) } -func NewObservedIHttpClient(origin usdc.IHttpClient, pluginName string) *ObservedIHttpClient { +func NewObservedIHttpClientWithMetric(origin IHttpClient, metric MetricDetails) *ObservedIHttpClient { return &ObservedIHttpClient{ IHttpClient: origin, - metric: metricDetails{ - histogram: usdcClientHistogram, - pluginName: pluginName, - }, + metric: metric, } } @@ -57,12 +65,11 @@ func (o *ObservedIHttpClient) Get(ctx context.Context, url string, timeout time. }) } -func withObservedHttpClient[T any](metric metricDetails, contract func() (T, int, error)) (T, int, error) { +func withObservedHttpClient[T any](metric MetricDetails, contract func() (T, int, error)) (T, int, error) { contractExecutionStarted := time.Now() value, status, err := contract() metric.histogram. WithLabelValues( - metric.pluginName, strconv.FormatInt(int64(status), 10), strconv.FormatBool(err == nil), ). diff --git a/core/services/ocr2/plugins/ccip/tokendata/observability/usdc.go b/core/services/ocr2/plugins/ccip/tokendata/observability/usdc.go new file mode 100644 index 0000000000..8dd9c1a990 --- /dev/null +++ b/core/services/ocr2/plugins/ccip/tokendata/observability/usdc.go @@ -0,0 +1 @@ +package observability diff --git a/core/services/ocr2/plugins/ccip/tokendata/observability/usdc_test.go b/core/services/ocr2/plugins/ccip/tokendata/observability/usdc_test.go index 9c4cc0ffc2..ea0c1792d9 100644 --- a/core/services/ocr2/plugins/ccip/tokendata/observability/usdc_test.go +++ b/core/services/ocr2/plugins/ccip/tokendata/observability/usdc_test.go @@ -18,18 +18,14 @@ import ( "github.com/smartcontractkit/chainlink/v2/core/logger" "github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ccip/internal" "github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ccip/internal/ccipdata/mocks" + http2 "github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ccip/tokendata/http" "github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ccip/tokendata/usdc" ) -var ( - pluginName = "testplugin" -) - type expected struct { - pluginName string - status string - result string - count int + status string + result string + count int } func TestUSDCClientMonitoring(t *testing.T) { @@ -45,8 +41,8 @@ func TestUSDCClientMonitoring(t *testing.T) { server: newSuccessServer(t), requests: 5, expected: []expected{ - {pluginName, "200", "true", 5}, - {pluginName, "429", "false", 0}, + {"200", "true", 5}, + {"429", "false", 0}, }, }, { @@ -54,8 +50,8 @@ func TestUSDCClientMonitoring(t *testing.T) { server: newRateLimitedServer(), requests: 26, expected: []expected{ - {pluginName, "200", "true", 0}, - {pluginName, "429", "false", 26}, + {"200", "true", 0}, + {"429", "false", 26}, }, }, } @@ -78,8 +74,8 @@ func testMonitoring(t *testing.T, name string, server *httptest.Server, requests histogram := promauto.NewHistogramVec(prometheus.HistogramOpts{ Name: "test_client_histogram_" + name, Help: "Latency of calls to the USDC client", - Buckets: latencyBuckets, - }, []string{"plugin", "status", "success"}) + Buckets: http2.UsdcLatencyBuckets, + }, []string{"status", "success"}) // Mock USDC reader. usdcReader := mocks.NewUSDCReader(t) @@ -87,12 +83,9 @@ func testMonitoring(t *testing.T, name string, server *httptest.Server, requests usdcReader.On("GetLastUSDCMessagePriorToLogIndexInTx", mock.Anything, mock.Anything, mock.Anything).Return(msgBody, nil) // Service with monitored http client. - usdcService := usdc.NewUSDCTokenDataReader(log, usdcReader, attestationURI, 0) - observedHttpClient := &ObservedIHttpClient{ - IHttpClient: &usdc.HttpClient{}, - metric: metricDetails{histogram, pluginName}, - } - tokenDataReader := *usdc.NewUSDCTokenDataReaderWithHttpClient(*usdcService, observedHttpClient) + observedHttpClient := http2.NewObservedIHttpClientWithMetric(&http2.HttpClient{}, http2.NewMetricDetails(histogram)) + tokenDataReaderDefault := usdc.NewUSDCTokenDataReader(log, usdcReader, attestationURI, 0) + tokenDataReader := usdc.NewUSDCTokenDataReaderWithHttpClient(*tokenDataReaderDefault, observedHttpClient) require.NotNil(t, tokenDataReader) for i := 0; i < requests; i++ { @@ -101,7 +94,7 @@ func testMonitoring(t *testing.T, name string, server *httptest.Server, requests // Check that the metrics are updated as expected. for _, e := range expected { - assert.Equal(t, e.count, counterFromHistogramByLabels(t, histogram, e.pluginName, e.status, e.result)) + assert.Equal(t, e.count, counterFromHistogramByLabels(t, histogram, e.status, e.result)) } } diff --git a/core/services/ocr2/plugins/ccip/tokendata/usdc/usdc.go b/core/services/ocr2/plugins/ccip/tokendata/usdc/usdc.go index d90d621f25..7ef4833be6 100644 --- a/core/services/ocr2/plugins/ccip/tokendata/usdc/usdc.go +++ b/core/services/ocr2/plugins/ccip/tokendata/usdc/usdc.go @@ -17,6 +17,7 @@ import ( "github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ccip/internal" "github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ccip/internal/ccipdata" "github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ccip/tokendata" + "github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ccip/tokendata/http" "github.com/smartcontractkit/chainlink/v2/core/services/pg" "github.com/smartcontractkit/chainlink/v2/core/utils" ) @@ -65,7 +66,7 @@ func (m messageAndAttestation) Validate() error { type TokenDataReader struct { lggr logger.Logger usdcReader ccipdata.USDCReader - httpClient IHttpClient + httpClient http.IHttpClient attestationApi *url.URL attestationApiTimeout time.Duration } @@ -82,17 +83,16 @@ func NewUSDCTokenDataReader(lggr logger.Logger, usdcReader ccipdata.USDCReader, if usdcAttestationApiTimeoutSeconds == 0 { timeout = defaultAttestationTimeout } - return &TokenDataReader{ lggr: lggr, usdcReader: usdcReader, - httpClient: &HttpClient{}, + httpClient: http.NewObservedIHttpClient(&http.HttpClient{}), attestationApi: usdcAttestationApi, attestationApiTimeout: timeout, } } -func NewUSDCTokenDataReaderWithHttpClient(origin TokenDataReader, httpClient IHttpClient) *TokenDataReader { +func NewUSDCTokenDataReaderWithHttpClient(origin TokenDataReader, httpClient http.IHttpClient) *TokenDataReader { return &TokenDataReader{ lggr: origin.lggr, usdcReader: origin.usdcReader, From c27da983ec004561daf181e67ff74311ed760fa0 Mon Sep 17 00:00:00 2001 From: Jean Arnaud Date: Tue, 14 Nov 2023 11:40:36 +0100 Subject: [PATCH 10/12] Move test next to monitored client --- .../plugins/ccip/tokendata/http/observed_http_client.go | 4 ++-- .../usdc_test.go => http/usdc_client_test.go} | 7 +++---- .../ocr2/plugins/ccip/tokendata/observability/usdc.go | 1 - 3 files changed, 5 insertions(+), 7 deletions(-) rename core/services/ocr2/plugins/ccip/tokendata/{observability/usdc_test.go => http/usdc_client_test.go} (93%) delete mode 100644 core/services/ocr2/plugins/ccip/tokendata/observability/usdc.go diff --git a/core/services/ocr2/plugins/ccip/tokendata/http/observed_http_client.go b/core/services/ocr2/plugins/ccip/tokendata/http/observed_http_client.go index 6f14d76e00..6bed5fee92 100644 --- a/core/services/ocr2/plugins/ccip/tokendata/http/observed_http_client.go +++ b/core/services/ocr2/plugins/ccip/tokendata/http/observed_http_client.go @@ -10,7 +10,7 @@ import ( ) var ( - UsdcLatencyBuckets = []float64{ + usdcLatencyBuckets = []float64{ float64(10 * time.Millisecond), float64(25 * time.Millisecond), float64(50 * time.Millisecond), @@ -28,7 +28,7 @@ var ( usdcClientHistogram = promauto.NewHistogramVec(prometheus.HistogramOpts{ Name: "ccip_usdc_client_request_total", Help: "Latency of calls to the USDC client", - Buckets: UsdcLatencyBuckets, + Buckets: usdcLatencyBuckets, }, []string{"status", "success"}) ) diff --git a/core/services/ocr2/plugins/ccip/tokendata/observability/usdc_test.go b/core/services/ocr2/plugins/ccip/tokendata/http/usdc_client_test.go similarity index 93% rename from core/services/ocr2/plugins/ccip/tokendata/observability/usdc_test.go rename to core/services/ocr2/plugins/ccip/tokendata/http/usdc_client_test.go index ea0c1792d9..c0cf67c38f 100644 --- a/core/services/ocr2/plugins/ccip/tokendata/observability/usdc_test.go +++ b/core/services/ocr2/plugins/ccip/tokendata/http/usdc_client_test.go @@ -1,4 +1,4 @@ -package observability +package http import ( "context" @@ -18,7 +18,6 @@ import ( "github.com/smartcontractkit/chainlink/v2/core/logger" "github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ccip/internal" "github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ccip/internal/ccipdata/mocks" - http2 "github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ccip/tokendata/http" "github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ccip/tokendata/usdc" ) @@ -74,7 +73,7 @@ func testMonitoring(t *testing.T, name string, server *httptest.Server, requests histogram := promauto.NewHistogramVec(prometheus.HistogramOpts{ Name: "test_client_histogram_" + name, Help: "Latency of calls to the USDC client", - Buckets: http2.UsdcLatencyBuckets, + Buckets: usdcLatencyBuckets, }, []string{"status", "success"}) // Mock USDC reader. @@ -83,7 +82,7 @@ func testMonitoring(t *testing.T, name string, server *httptest.Server, requests usdcReader.On("GetLastUSDCMessagePriorToLogIndexInTx", mock.Anything, mock.Anything, mock.Anything).Return(msgBody, nil) // Service with monitored http client. - observedHttpClient := http2.NewObservedIHttpClientWithMetric(&http2.HttpClient{}, http2.NewMetricDetails(histogram)) + observedHttpClient := NewObservedIHttpClientWithMetric(&HttpClient{}, NewMetricDetails(histogram)) tokenDataReaderDefault := usdc.NewUSDCTokenDataReader(log, usdcReader, attestationURI, 0) tokenDataReader := usdc.NewUSDCTokenDataReaderWithHttpClient(*tokenDataReaderDefault, observedHttpClient) require.NotNil(t, tokenDataReader) diff --git a/core/services/ocr2/plugins/ccip/tokendata/observability/usdc.go b/core/services/ocr2/plugins/ccip/tokendata/observability/usdc.go deleted file mode 100644 index 8dd9c1a990..0000000000 --- a/core/services/ocr2/plugins/ccip/tokendata/observability/usdc.go +++ /dev/null @@ -1 +0,0 @@ -package observability From 05d6cab4c646de686879c70af3430c616dd13b2c Mon Sep 17 00:00:00 2001 From: Jean Arnaud Date: Tue, 14 Nov 2023 12:03:39 +0100 Subject: [PATCH 11/12] Use test histogram --- .../{http => observability}/usdc_client_test.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) rename core/services/ocr2/plugins/ccip/tokendata/{http => observability}/usdc_client_test.go (90%) diff --git a/core/services/ocr2/plugins/ccip/tokendata/http/usdc_client_test.go b/core/services/ocr2/plugins/ccip/tokendata/observability/usdc_client_test.go similarity index 90% rename from core/services/ocr2/plugins/ccip/tokendata/http/usdc_client_test.go rename to core/services/ocr2/plugins/ccip/tokendata/observability/usdc_client_test.go index c0cf67c38f..2975cc4ccf 100644 --- a/core/services/ocr2/plugins/ccip/tokendata/http/usdc_client_test.go +++ b/core/services/ocr2/plugins/ccip/tokendata/observability/usdc_client_test.go @@ -1,4 +1,4 @@ -package http +package observability import ( "context" @@ -7,6 +7,7 @@ import ( "net/http/httptest" "net/url" "testing" + "time" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" @@ -18,6 +19,7 @@ import ( "github.com/smartcontractkit/chainlink/v2/core/logger" "github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ccip/internal" "github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ccip/internal/ccipdata/mocks" + http2 "github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ccip/tokendata/http" "github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ccip/tokendata/usdc" ) @@ -72,8 +74,8 @@ func testMonitoring(t *testing.T, name string, server *httptest.Server, requests // Define test histogram (avoid side effects from other tests if using the real usdcHistogram). histogram := promauto.NewHistogramVec(prometheus.HistogramOpts{ Name: "test_client_histogram_" + name, - Help: "Latency of calls to the USDC client", - Buckets: usdcLatencyBuckets, + Help: "Latency of calls to the USDC mock client", + Buckets: []float64{float64(250 * time.Millisecond), float64(1 * time.Second), float64(5 * time.Second)}, }, []string{"status", "success"}) // Mock USDC reader. @@ -82,7 +84,7 @@ func testMonitoring(t *testing.T, name string, server *httptest.Server, requests usdcReader.On("GetLastUSDCMessagePriorToLogIndexInTx", mock.Anything, mock.Anything, mock.Anything).Return(msgBody, nil) // Service with monitored http client. - observedHttpClient := NewObservedIHttpClientWithMetric(&HttpClient{}, NewMetricDetails(histogram)) + observedHttpClient := http2.NewObservedIHttpClientWithMetric(&http2.HttpClient{}, http2.NewMetricDetails(histogram)) tokenDataReaderDefault := usdc.NewUSDCTokenDataReader(log, usdcReader, attestationURI, 0) tokenDataReader := usdc.NewUSDCTokenDataReaderWithHttpClient(*tokenDataReaderDefault, observedHttpClient) require.NotNil(t, tokenDataReader) From fe701e3a7a3e5bc7c628ab8a418c15ad63bc373d Mon Sep 17 00:00:00 2001 From: Jean Arnaud Date: Tue, 14 Nov 2023 12:27:08 +0100 Subject: [PATCH 12/12] Remove metric details --- .../tokendata/http/observed_http_client.go | 24 ++++++------------- .../observability/usdc_client_test.go | 2 +- 2 files changed, 8 insertions(+), 18 deletions(-) diff --git a/core/services/ocr2/plugins/ccip/tokendata/http/observed_http_client.go b/core/services/ocr2/plugins/ccip/tokendata/http/observed_http_client.go index 6bed5fee92..fa49406b10 100644 --- a/core/services/ocr2/plugins/ccip/tokendata/http/observed_http_client.go +++ b/core/services/ocr2/plugins/ccip/tokendata/http/observed_http_client.go @@ -32,43 +32,33 @@ var ( }, []string{"status", "success"}) ) -type MetricDetails struct { - histogram *prometheus.HistogramVec -} - type ObservedIHttpClient struct { IHttpClient - metric MetricDetails -} - -func NewMetricDetails(histogram *prometheus.HistogramVec) MetricDetails { - return MetricDetails{ - histogram: histogram, - } + histogram *prometheus.HistogramVec } // NewObservedIHttpClient Create a new ObservedIHttpClient with the USDC client metric. func NewObservedIHttpClient(origin IHttpClient) *ObservedIHttpClient { - return NewObservedIHttpClientWithMetric(origin, NewMetricDetails(usdcClientHistogram)) + return NewObservedIHttpClientWithMetric(origin, usdcClientHistogram) } -func NewObservedIHttpClientWithMetric(origin IHttpClient, metric MetricDetails) *ObservedIHttpClient { +func NewObservedIHttpClientWithMetric(origin IHttpClient, histogram *prometheus.HistogramVec) *ObservedIHttpClient { return &ObservedIHttpClient{ IHttpClient: origin, - metric: metric, + histogram: histogram, } } func (o *ObservedIHttpClient) Get(ctx context.Context, url string, timeout time.Duration) ([]byte, int, error) { - return withObservedHttpClient(o.metric, func() ([]byte, int, error) { + return withObservedHttpClient(o.histogram, func() ([]byte, int, error) { return o.IHttpClient.Get(ctx, url, timeout) }) } -func withObservedHttpClient[T any](metric MetricDetails, contract func() (T, int, error)) (T, int, error) { +func withObservedHttpClient[T any](histogram *prometheus.HistogramVec, contract func() (T, int, error)) (T, int, error) { contractExecutionStarted := time.Now() value, status, err := contract() - metric.histogram. + histogram. WithLabelValues( strconv.FormatInt(int64(status), 10), strconv.FormatBool(err == nil), diff --git a/core/services/ocr2/plugins/ccip/tokendata/observability/usdc_client_test.go b/core/services/ocr2/plugins/ccip/tokendata/observability/usdc_client_test.go index 2975cc4ccf..9a0bb8c98a 100644 --- a/core/services/ocr2/plugins/ccip/tokendata/observability/usdc_client_test.go +++ b/core/services/ocr2/plugins/ccip/tokendata/observability/usdc_client_test.go @@ -84,7 +84,7 @@ func testMonitoring(t *testing.T, name string, server *httptest.Server, requests usdcReader.On("GetLastUSDCMessagePriorToLogIndexInTx", mock.Anything, mock.Anything, mock.Anything).Return(msgBody, nil) // Service with monitored http client. - observedHttpClient := http2.NewObservedIHttpClientWithMetric(&http2.HttpClient{}, http2.NewMetricDetails(histogram)) + observedHttpClient := http2.NewObservedIHttpClientWithMetric(&http2.HttpClient{}, histogram) tokenDataReaderDefault := usdc.NewUSDCTokenDataReader(log, usdcReader, attestationURI, 0) tokenDataReader := usdc.NewUSDCTokenDataReaderWithHttpClient(*tokenDataReaderDefault, observedHttpClient) require.NotNil(t, tokenDataReader)