From 39e3d383665edad8d5a311ef09b8d7c547890ce3 Mon Sep 17 00:00:00 2001 From: wlmyng <127570466+wlmyng@users.noreply.github.com> Date: Mon, 16 Dec 2024 09:25:24 -0800 Subject: [PATCH] [mvr] add external dns lookup metric to track resolving mvr from non-mainnet graphql (#20633) ## Description To resolve the mvr table on testnet, we consult the mainnet graphql service. We previously observed some slowness around dns lookup, so this metric will help track whether the issue persists. Also added to `sui-graphql-rpc` crate. ## Test plan How did you test the new or updated feature? --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] Indexer: - [ ] JSON-RPC: - [ ] GraphQL: - [ ] CLI: - [ ] Rust SDK: - [ ] REST API: --- .../src/data/move_registry_data_loader.rs | 36 ++++++++++++------- crates/sui-graphql-rpc/src/metrics.rs | 24 +++++++++++++ crates/sui-graphql-rpc/src/server/builder.rs | 5 ++- crates/sui-mvr-graphql-rpc/README.md | 10 ++++++ .../src/data/move_registry_data_loader.rs | 36 ++++++++++++------- crates/sui-mvr-graphql-rpc/src/metrics.rs | 24 +++++++++++++ .../sui-mvr-graphql-rpc/src/server/builder.rs | 5 ++- 7 files changed, 114 insertions(+), 26 deletions(-) diff --git a/crates/sui-graphql-rpc/src/data/move_registry_data_loader.rs b/crates/sui-graphql-rpc/src/data/move_registry_data_loader.rs index ba5eee56a6647..cdf29fc1dd928 100644 --- a/crates/sui-graphql-rpc/src/data/move_registry_data_loader.rs +++ b/crates/sui-graphql-rpc/src/data/move_registry_data_loader.rs @@ -7,6 +7,7 @@ use std::{collections::HashMap, str::FromStr, sync::Arc}; use async_graphql::dataloader::{DataLoader, Loader}; use serde::{Deserialize, Serialize}; +use crate::metrics::Metrics; use crate::{ config::MoveRegistryConfig, error::Error, @@ -26,6 +27,7 @@ const QUERY_FRAGMENT: &str = pub(crate) struct ExternalNamesLoader { client: reqwest::Client, config: MoveRegistryConfig, + metrics: Metrics, } /// Helper types for accessing a shared `DataLoader` instance. @@ -33,10 +35,11 @@ pub(crate) struct ExternalNamesLoader { pub(crate) struct MoveRegistryDataLoader(pub Arc>); impl ExternalNamesLoader { - pub(crate) fn new(config: MoveRegistryConfig) -> Self { + pub(crate) fn new(config: MoveRegistryConfig, metrics: Metrics) -> Self { Self { client: reqwest::Client::new(), config, + metrics, } } @@ -71,9 +74,9 @@ impl ExternalNamesLoader { } impl MoveRegistryDataLoader { - pub(crate) fn new(config: MoveRegistryConfig) -> Self { + pub(crate) fn new(config: MoveRegistryConfig, metrics: Metrics) -> Self { let batch_size = config.page_limit as usize; - let data_loader = DataLoader::new(ExternalNamesLoader::new(config), tokio::spawn) + let data_loader = DataLoader::new(ExternalNamesLoader::new(config, metrics), tokio::spawn) .max_batch_size(batch_size); Self(Arc::new(data_loader)) } @@ -102,15 +105,24 @@ impl Loader for ExternalNamesLoader { variables: serde_json::Value::Null, }; - let res = self - .client - .post(api_url) - .json(&request_body) - .send() - .await - .map_err(|e| { - Error::MoveNameRegistry(MoveRegistryError::FailedToQueryExternalApi(e.to_string())) - })?; + let res = { + let _timer_guard = self + .metrics + .app_metrics + .external_mvr_resolution_latency + .start_timer(); + + self.client + .post(api_url) + .json(&request_body) + .send() + .await + .map_err(|e| { + Error::MoveNameRegistry(MoveRegistryError::FailedToQueryExternalApi( + e.to_string(), + )) + })? + }; if !res.status().is_success() { return Err(Error::MoveNameRegistry( diff --git a/crates/sui-graphql-rpc/src/metrics.rs b/crates/sui-graphql-rpc/src/metrics.rs index 4e4d50e998fd8..155987f9c2c63 100644 --- a/crates/sui-graphql-rpc/src/metrics.rs +++ b/crates/sui-graphql-rpc/src/metrics.rs @@ -43,6 +43,7 @@ const DB_QUERY_COST_BUCKETS: &[f64] = &[ pub(crate) struct Metrics { pub db_metrics: Arc, pub request_metrics: Arc, + pub app_metrics: Arc, } #[derive(Clone)] @@ -81,14 +82,23 @@ pub(crate) struct RequestMetrics { pub inflight_requests: Gauge, } +#[derive(Clone)] +pub(crate) struct AppMetrics { + /// The time it takes for the non-mainnet graphql service to resolve the mvr object from + /// mainnet. + pub external_mvr_resolution_latency: Histogram, +} + impl Metrics { pub(crate) fn new(registry: &Registry) -> Self { let db_metrics = DBMetrics::new(registry); let request_metrics = RequestMetrics::new(registry); + let app_metrics = AppMetrics::new(registry); Self { db_metrics: Arc::new(db_metrics), request_metrics: Arc::new(request_metrics), + app_metrics: Arc::new(app_metrics), } } @@ -253,6 +263,20 @@ impl RequestMetrics { } } +impl AppMetrics { + pub(crate) fn new(registry: &Registry) -> Self { + Self { + external_mvr_resolution_latency: register_histogram_with_registry!( + "external_mvr_resolution_latency", + "The time it takes for the non-mainnet graphql service to resolve the mvr object from mainnet", + LATENCY_SEC_BUCKETS.to_vec(), + registry, + ) + .unwrap(), + } + } +} + /// When an error occurs, GraphQL returns a vector of PathSegments, /// that we can use to retrieve the last node which contains the error. pub(crate) fn query_label_for_error(query: &[PathSegment]) -> String { diff --git a/crates/sui-graphql-rpc/src/server/builder.rs b/crates/sui-graphql-rpc/src/server/builder.rs index 37d28c35f2895..2de8aab2eed7d 100644 --- a/crates/sui-graphql-rpc/src/server/builder.rs +++ b/crates/sui-graphql-rpc/src/server/builder.rs @@ -469,7 +469,10 @@ impl ServerBuilder { .context_data(metrics.clone()) .context_data(config.clone()) .context_data(move_registry_config.clone()) - .context_data(MoveRegistryDataLoader::new(move_registry_config)); + .context_data(MoveRegistryDataLoader::new( + move_registry_config, + metrics.clone(), + )); if config.internal_features.feature_gate { builder = builder.extension(FeatureGate); diff --git a/crates/sui-mvr-graphql-rpc/README.md b/crates/sui-mvr-graphql-rpc/README.md index e66199a128a1f..332c516f261ed 100644 --- a/crates/sui-mvr-graphql-rpc/README.md +++ b/crates/sui-mvr-graphql-rpc/README.md @@ -1,5 +1,15 @@ # sui-mvr-graphql-rpc +## Maintaining parity with sui-graphql-rpc crate +Generates a series of patch files from the named commit to the current HEAD, and rewrites the paths to the destination crate. +``` +# Create patch with stripped paths +git format-patch ..HEAD --stdout -- ./crates/sui-mvr-graphql-rpc | sed 's|crates/sui-mvr-graphql-rpc/|crates/sui-graphql-rpc/|g' > patches.diff + +# Apply the patch +git apply patches.diff +``` + ## Dev setup Note that we use compilation flags to determine the backend for Diesel. If you're using VS Code, make sure to update settings.json with the appropriate features - there should at least be a "pg_backend" (or other backend.) ``` diff --git a/crates/sui-mvr-graphql-rpc/src/data/move_registry_data_loader.rs b/crates/sui-mvr-graphql-rpc/src/data/move_registry_data_loader.rs index ba5eee56a6647..cdf29fc1dd928 100644 --- a/crates/sui-mvr-graphql-rpc/src/data/move_registry_data_loader.rs +++ b/crates/sui-mvr-graphql-rpc/src/data/move_registry_data_loader.rs @@ -7,6 +7,7 @@ use std::{collections::HashMap, str::FromStr, sync::Arc}; use async_graphql::dataloader::{DataLoader, Loader}; use serde::{Deserialize, Serialize}; +use crate::metrics::Metrics; use crate::{ config::MoveRegistryConfig, error::Error, @@ -26,6 +27,7 @@ const QUERY_FRAGMENT: &str = pub(crate) struct ExternalNamesLoader { client: reqwest::Client, config: MoveRegistryConfig, + metrics: Metrics, } /// Helper types for accessing a shared `DataLoader` instance. @@ -33,10 +35,11 @@ pub(crate) struct ExternalNamesLoader { pub(crate) struct MoveRegistryDataLoader(pub Arc>); impl ExternalNamesLoader { - pub(crate) fn new(config: MoveRegistryConfig) -> Self { + pub(crate) fn new(config: MoveRegistryConfig, metrics: Metrics) -> Self { Self { client: reqwest::Client::new(), config, + metrics, } } @@ -71,9 +74,9 @@ impl ExternalNamesLoader { } impl MoveRegistryDataLoader { - pub(crate) fn new(config: MoveRegistryConfig) -> Self { + pub(crate) fn new(config: MoveRegistryConfig, metrics: Metrics) -> Self { let batch_size = config.page_limit as usize; - let data_loader = DataLoader::new(ExternalNamesLoader::new(config), tokio::spawn) + let data_loader = DataLoader::new(ExternalNamesLoader::new(config, metrics), tokio::spawn) .max_batch_size(batch_size); Self(Arc::new(data_loader)) } @@ -102,15 +105,24 @@ impl Loader for ExternalNamesLoader { variables: serde_json::Value::Null, }; - let res = self - .client - .post(api_url) - .json(&request_body) - .send() - .await - .map_err(|e| { - Error::MoveNameRegistry(MoveRegistryError::FailedToQueryExternalApi(e.to_string())) - })?; + let res = { + let _timer_guard = self + .metrics + .app_metrics + .external_mvr_resolution_latency + .start_timer(); + + self.client + .post(api_url) + .json(&request_body) + .send() + .await + .map_err(|e| { + Error::MoveNameRegistry(MoveRegistryError::FailedToQueryExternalApi( + e.to_string(), + )) + })? + }; if !res.status().is_success() { return Err(Error::MoveNameRegistry( diff --git a/crates/sui-mvr-graphql-rpc/src/metrics.rs b/crates/sui-mvr-graphql-rpc/src/metrics.rs index 4e4d50e998fd8..155987f9c2c63 100644 --- a/crates/sui-mvr-graphql-rpc/src/metrics.rs +++ b/crates/sui-mvr-graphql-rpc/src/metrics.rs @@ -43,6 +43,7 @@ const DB_QUERY_COST_BUCKETS: &[f64] = &[ pub(crate) struct Metrics { pub db_metrics: Arc, pub request_metrics: Arc, + pub app_metrics: Arc, } #[derive(Clone)] @@ -81,14 +82,23 @@ pub(crate) struct RequestMetrics { pub inflight_requests: Gauge, } +#[derive(Clone)] +pub(crate) struct AppMetrics { + /// The time it takes for the non-mainnet graphql service to resolve the mvr object from + /// mainnet. + pub external_mvr_resolution_latency: Histogram, +} + impl Metrics { pub(crate) fn new(registry: &Registry) -> Self { let db_metrics = DBMetrics::new(registry); let request_metrics = RequestMetrics::new(registry); + let app_metrics = AppMetrics::new(registry); Self { db_metrics: Arc::new(db_metrics), request_metrics: Arc::new(request_metrics), + app_metrics: Arc::new(app_metrics), } } @@ -253,6 +263,20 @@ impl RequestMetrics { } } +impl AppMetrics { + pub(crate) fn new(registry: &Registry) -> Self { + Self { + external_mvr_resolution_latency: register_histogram_with_registry!( + "external_mvr_resolution_latency", + "The time it takes for the non-mainnet graphql service to resolve the mvr object from mainnet", + LATENCY_SEC_BUCKETS.to_vec(), + registry, + ) + .unwrap(), + } + } +} + /// When an error occurs, GraphQL returns a vector of PathSegments, /// that we can use to retrieve the last node which contains the error. pub(crate) fn query_label_for_error(query: &[PathSegment]) -> String { diff --git a/crates/sui-mvr-graphql-rpc/src/server/builder.rs b/crates/sui-mvr-graphql-rpc/src/server/builder.rs index ab3f7f3965fd7..aab4a0ddb4c0c 100644 --- a/crates/sui-mvr-graphql-rpc/src/server/builder.rs +++ b/crates/sui-mvr-graphql-rpc/src/server/builder.rs @@ -469,7 +469,10 @@ impl ServerBuilder { .context_data(metrics.clone()) .context_data(config.clone()) .context_data(move_registry_config.clone()) - .context_data(MoveRegistryDataLoader::new(move_registry_config)); + .context_data(MoveRegistryDataLoader::new( + move_registry_config, + metrics.clone(), + )); if config.internal_features.feature_gate { builder = builder.extension(FeatureGate);