diff --git a/crates/shared/src/price_estimation/factory.rs b/crates/shared/src/price_estimation/factory.rs index 011a0f6742..37a2fe5f70 100644 --- a/crates/shared/src/price_estimation/factory.rs +++ b/crates/shared/src/price_estimation/factory.rs @@ -189,25 +189,35 @@ impl<'a> PriceEstimatorFactory<'a> { let estimator = self.get_estimator(driver)?.native.clone(); Ok(( driver.name.clone(), - Arc::new(NativePriceEstimator::new( - Arc::new(self.sanitized(estimator)), - self.network.native_token, - native_token_price_estimation_amount, + Arc::new(InstrumentedPriceEstimator::new( + NativePriceEstimator::new( + Arc::new(self.sanitized(estimator)), + self.network.native_token, + native_token_price_estimation_amount, + ), + driver.name.to_string(), + )), + )) + } + NativePriceEstimatorSource::OneInchSpotPriceApi => { + let name = "OneInchSpotPriceApi".to_string(); + Ok(( + name.clone(), + Arc::new(InstrumentedPriceEstimator::new( + native::OneInch::new( + self.components.http_factory.create(), + self.args.one_inch_url.clone(), + self.args.one_inch_api_key.clone(), + self.network.chain_id, + self.network.block_stream.clone(), + self.components.tokens.clone(), + ), + name, )), )) } - NativePriceEstimatorSource::OneInchSpotPriceApi => Ok(( - "OneInchSpotPriceApi".into(), - Arc::new(native::OneInch::new( - self.components.http_factory.create(), - self.args.one_inch_url.clone(), - self.args.one_inch_api_key.clone(), - self.network.chain_id, - self.network.block_stream.clone(), - self.components.tokens.clone(), - )), - )), NativePriceEstimatorSource::CoinGecko => { + let name = "CoinGecko".to_string(); let coin_gecko = native::CoinGecko::new( self.components.http_factory.create(), self.args.coin_gecko.coin_gecko_url.clone(), @@ -240,12 +250,15 @@ impl<'a> PriceEstimatorFactory<'a> { .unwrap(), }; - Arc::new(BufferedRequest::with_config(coin_gecko, configuration)) + Arc::new(InstrumentedPriceEstimator::new( + BufferedRequest::with_config(coin_gecko, configuration), + name.clone() + "Buffered", + )) } else { - Arc::new(coin_gecko) + Arc::new(InstrumentedPriceEstimator::new(coin_gecko, name.clone())) }; - Ok(("CoinGecko".into(), coin_gecko)) + Ok((name, coin_gecko)) } } } @@ -395,12 +408,9 @@ impl PriceEstimatorCreating for ExternalPriceEstimator { } } -fn instrument( - estimator: impl PriceEstimating, +fn instrument( + estimator: T, name: impl Into, -) -> Arc { - Arc::new(InstrumentedPriceEstimator::new( - Box::new(estimator), - name.into(), - )) +) -> Arc> { + Arc::new(InstrumentedPriceEstimator::new(estimator, name.into())) } diff --git a/crates/shared/src/price_estimation/instrumented.rs b/crates/shared/src/price_estimation/instrumented.rs index d60c8f732d..5842748b25 100644 --- a/crates/shared/src/price_estimation/instrumented.rs +++ b/crates/shared/src/price_estimation/instrumented.rs @@ -1,21 +1,28 @@ use { - crate::price_estimation::{PriceEstimating, PriceEstimationError, Query}, + crate::price_estimation::{ + native::{NativePriceEstimateResult, NativePriceEstimating}, + PriceEstimating, + PriceEstimationError, + Query, + }, + ethcontract::jsonrpc::futures_util::future::BoxFuture, futures::future::FutureExt, + primitive_types::H160, prometheus::{HistogramVec, IntCounterVec}, std::{sync::Arc, time::Instant}, tracing::Instrument, }; /// An instrumented price estimator. -pub struct InstrumentedPriceEstimator { - inner: Box, +pub struct InstrumentedPriceEstimator { + inner: T, name: String, metrics: &'static Metrics, } -impl InstrumentedPriceEstimator { +impl InstrumentedPriceEstimator { /// Wraps an existing price estimator in an instrumented one. - pub fn new(inner: Box, name: String) -> Self { + pub fn new(inner: T, name: String) -> Self { let metrics = Metrics::instance(observe::metrics::get_storage_registry()).unwrap(); for result in ["success", "failure"] { metrics @@ -29,9 +36,21 @@ impl InstrumentedPriceEstimator { metrics, } } + + /// Determines the result of a price estimate as either "success" or + /// "failure". + fn estimate_result(&self, estimate: Result<&B, &PriceEstimationError>) -> &str { + // Count as a successful request if the answer is ok (no error) or if the error + // is No Liquidity + if estimate.is_ok() || matches!(estimate, Err(PriceEstimationError::NoLiquidity)) { + "success" + } else { + "failure" + } + } } -impl PriceEstimating for InstrumentedPriceEstimator { +impl PriceEstimating for InstrumentedPriceEstimator { fn estimate( &self, query: Arc, @@ -43,9 +62,7 @@ impl PriceEstimating for InstrumentedPriceEstimator { .price_estimation_times .with_label_values(&[self.name.as_str()]) .observe(start.elapsed().as_secs_f64()); - - let success = !matches!(&estimate, Err(PriceEstimationError::EstimatorInternal(_))); - let result = if success { "success" } else { "failure" }; + let result = self.estimate_result(estimate.as_ref()); self.metrics .price_estimates .with_label_values(&[self.name.as_str(), result]) @@ -58,6 +75,28 @@ impl PriceEstimating for InstrumentedPriceEstimator { } } +impl NativePriceEstimating for InstrumentedPriceEstimator { + fn estimate_native_price(&self, token: H160) -> BoxFuture<'_, NativePriceEstimateResult> { + async move { + let start = Instant::now(); + let estimate = self.inner.estimate_native_price(token).await; + self.metrics + .price_estimation_times + .with_label_values(&[self.name.as_str()]) + .observe(start.elapsed().as_secs_f64()); + let result = self.estimate_result(estimate.as_ref()); + self.metrics + .price_estimates + .with_label_values(&[self.name.as_str(), result]) + .inc(); + + estimate + } + .instrument(tracing::info_span!("native estimator", name = &self.name,)) + .boxed() + } +} + #[derive(prometheus_metric_storage::MetricStorage)] struct Metrics { /// price estimates @@ -117,7 +156,7 @@ mod tests { async { Err(PriceEstimationError::EstimatorInternal(anyhow!(""))) }.boxed() }); - let instrumented = InstrumentedPriceEstimator::new(Box::new(estimator), "foo".to_string()); + let instrumented = InstrumentedPriceEstimator::new(estimator, "foo".to_string()); let _ = instrumented.estimate(queries[0].clone()).await; let _ = instrumented.estimate(queries[1].clone()).await; diff --git a/crates/shared/src/price_estimation/trade_finder.rs b/crates/shared/src/price_estimation/trade_finder.rs index c4efe13e52..41af0e6be2 100644 --- a/crates/shared/src/price_estimation/trade_finder.rs +++ b/crates/shared/src/price_estimation/trade_finder.rs @@ -23,6 +23,7 @@ use { /// A `TradeFinding`-based price estimator with request sharing and rate /// limiting. +#[derive(Clone)] pub struct TradeEstimator { inner: Arc, sharing: RequestSharing, BoxFuture<'static, Result>>, @@ -104,16 +105,6 @@ impl Inner { } } -impl Clone for TradeEstimator { - fn clone(&self) -> Self { - Self { - inner: self.inner.clone(), - sharing: self.sharing.clone(), - rate_limiter: self.rate_limiter.clone(), - } - } -} - impl PriceEstimating for TradeEstimator { fn estimate(&self, query: Arc) -> futures::future::BoxFuture<'_, PriceEstimateResult> { self.estimate(query).boxed()