Demote Paraswap Errors to warn! + success/failure metrics (#1248)

* add single order solver metrics

solver/src/main.rs

@@ -444,6 +444,7 @@ async fn main() {
         args.shared.paraswap_partner,
         client.clone(),
         native_token_price_estimation_amount,
+        metrics.clone(),
     )
     .expect("failure creating solvers");
     let liquidity_collector = LiquidityCollector {

solver/src/metrics.rs

@@ -20,13 +20,15 @@ use strum::VariantNames;
 /// The maximum time between the completion of two run loops. If exceeded the service will be considered unhealthy.
 const MAX_RUNLOOP_DURATION: Duration = Duration::from_secs(7 * 60);
 
-pub trait SolverMetrics {
+pub trait SolverMetrics: Send + Sync {
     fn orders_fetched(&self, orders: &[LimitOrder]);
     fn liquidity_fetched(&self, liquidity: &[Liquidity]);
     fn settlement_computed(&self, solver_type: &str, start: Instant);
     fn order_settled(&self, order: &Order, solver: &'static str);
     fn settlement_simulation_succeeded(&self, solver: &'static str);
     fn settlement_simulation_failed_on_latest(&self, solver: &'static str);
+    fn single_order_solver_succeeded(&self, solver: &'static str);
+    fn single_order_solver_failed(&self, solver: &'static str);
     fn settlement_simulation_failed(&self, solver: &'static str);
     fn settlement_submitted(&self, successful: bool, solver: &'static str);
     fn orders_matched_but_liquidity(&self, count: usize);
@@ -42,6 +44,7 @@ pub struct Metrics {
     liquidity: IntGaugeVec,
     settlement_simulations: IntCounterVec,
     settlement_submissions: IntCounterVec,
+    single_order_solver_runs: IntCounterVec,
     matched_but_liquidity: IntCounter,
     matched_but_unsettled_orders: IntCounter,
     transport_requests: HistogramVec,
@@ -96,6 +99,12 @@ impl Metrics {
         )?;
         registry.register(Box::new(settlement_submissions.clone()))?;
 
+        let single_order_solver_runs = IntCounterVec::new(
+            Opts::new("single_order_solver", "Success/Failure counts"),
+            &["result", "solver_type"],
+        )?;
+
+        registry.register(Box::new(single_order_solver_runs.clone()))?;
         let matched_but_liquidity = IntCounter::new(
             "orders_matched_liquidity",
             "Counter for the number of orders for which at least one solver computed an execution which was from a known liquidity provider",
@@ -134,6 +143,7 @@ impl Metrics {
             liquidity,
             settlement_simulations,
             settlement_submissions,
+            single_order_solver_runs,
             matched_but_liquidity,
             matched_but_unsettled_orders,
             transport_requests,
@@ -198,6 +208,18 @@ impl SolverMetrics for Metrics {
             .inc()
     }
 
+    fn single_order_solver_succeeded(&self, solver: &'static str) {
+        self.single_order_solver_runs
+            .with_label_values(&["success", solver])
+            .inc()
+    }
+
+    fn single_order_solver_failed(&self, solver: &'static str) {
+        self.single_order_solver_runs
+            .with_label_values(&["failure", solver])
+            .inc()
+    }
+
     fn settlement_simulation_failed(&self, solver: &'static str) {
         self.settlement_simulations
             .with_label_values(&["failure", solver])
@@ -273,6 +295,8 @@ impl SolverMetrics for NoopMetrics {
     fn order_settled(&self, _: &Order, _: &'static str) {}
     fn settlement_simulation_succeeded(&self, _: &'static str) {}
     fn settlement_simulation_failed_on_latest(&self, _: &'static str) {}
+    fn single_order_solver_succeeded(&self, _: &'static str) {}
+    fn single_order_solver_failed(&self, _: &'static str) {}
     fn settlement_simulation_failed(&self, _: &'static str) {}
     fn settlement_submitted(&self, _: bool, _: &'static str) {}
     fn orders_matched_but_liquidity(&self, _: usize) {}

solver/src/solver.rs

@@ -1,3 +1,4 @@
+use crate::metrics::SolverMetrics;
 use crate::{
     liquidity::{LimitOrder, Liquidity},
     settlement::Settlement,
@@ -60,7 +61,7 @@ pub trait Solver: 'static {
 /// A batch auction for a solver to produce a settlement for.
 #[derive(Clone, Debug)]
 pub struct Auction {
-    /// An ID that idetifies a batch within a `Driver` isntance.
+    /// An ID that identifies a batch within a `Driver` instance.
     ///
     /// Note that this ID is not unique across multiple instances of drivers,
     /// in particular it cannot be used to uniquely identify batches across
@@ -152,6 +153,7 @@ pub fn create(
     paraswap_partner: Option<String>,
     client: Client,
     native_token_amount_to_estimate_prices_with: U256,
+    solver_metrics: Arc<dyn SolverMetrics>,
 ) -> Result<Solvers> {
     // Tiny helper function to help out with type inference. Otherwise, all
     // `Box::new(...)` expressions would have to be cast `as Box<dyn Solver>`.
@@ -211,16 +213,17 @@ pub fn create(
                     },
                 )),
                 SolverType::OneInch => {
-                    let one_inch_solver: SingleOrderSolver<_> =
+                    let one_inch_solver: SingleOrderSolver<_> = SingleOrderSolver::new(
                         OneInchSolver::with_disabled_protocols(
                             account,
                             web3.clone(),
                             settlement_contract.clone(),
                             chain_id,
                             disabled_one_inch_protocols.clone(),
                             client.clone(),
-                        )?
-                        .into();
+                        )?,
+                        solver_metrics.clone(),
+                    );
                     // We only want to use 1Inch for high value orders
                     shared(SellVolumeFilteringSolver::new(
                         Box::new(one_inch_solver),
@@ -236,18 +239,24 @@ pub fn create(
                         client.clone(),
                     )
                     .unwrap();
-                    shared(SingleOrderSolver::from(zeroex_solver))
+                    shared(SingleOrderSolver::new(
+                        zeroex_solver,
+                        solver_metrics.clone(),
+                    ))
                 }
-                SolverType::Paraswap => shared(SingleOrderSolver::from(ParaswapSolver::new(
-                    account,
-                    web3.clone(),
-                    settlement_contract.clone(),
-                    token_info_fetcher.clone(),
-                    paraswap_slippage_bps,
-                    disabled_paraswap_dexs.clone(),
-                    client.clone(),
-                    paraswap_partner.clone(),
-                ))),
+                SolverType::Paraswap => shared(SingleOrderSolver::new(
+                    ParaswapSolver::new(
+                        account,
+                        web3.clone(),
+                        settlement_contract.clone(),
+                        token_info_fetcher.clone(),
+                        paraswap_slippage_bps,
+                        disabled_paraswap_dexs.clone(),
+                        client.clone(),
+                        paraswap_partner.clone(),
+                    ),
+                    solver_metrics.clone(),
+                )),
             };
 
             if let Ok(solver) = &solver {

solver/src/solver/oneinch_solver/api.rs

@@ -141,7 +141,6 @@ impl From<SwapResponseError> for SettlementError {
         SettlementError {
             inner: anyhow!(error.message),
             retryable: matches!(error.status_code, 500),
-            should_alert: true,
         }
     }
 }

solver/src/solver/paraswap_solver.rs

@@ -81,15 +81,6 @@ impl From<ParaswapResponseError> for SettlementError {
                     | ParaswapResponseError::ServerBusy
                     | ParaswapResponseError::Send(_),
             ),
-            should_alert: !matches!(
-                err,
-                ParaswapResponseError::PriceChange
-                    | ParaswapResponseError::BuildingTransaction(_)
-                    | ParaswapResponseError::ComputePrice(_)
-                    | ParaswapResponseError::InsufficientLiquidity
-                    | ParaswapResponseError::TooMuchSlippageOnQuote
-                    | ParaswapResponseError::ServerBusy,
-            ),
         }
     }
 }

solver/src/solver/single_order_solver.rs

@@ -1,3 +1,4 @@
+use crate::metrics::SolverMetrics;
 use crate::{
     liquidity::LimitOrder,
     settlement::Settlement,
@@ -6,6 +7,7 @@ use crate::{
 use anyhow::{Error, Result};
 use ethcontract::Account;
 use rand::prelude::SliceRandom;
+use std::sync::Arc;
 use std::{collections::VecDeque, time::Duration};
 
 #[cfg_attr(test, mockall::automock)]
@@ -28,11 +30,12 @@ pub trait SingleOrderSolving {
 
 pub struct SingleOrderSolver<I> {
     inner: I,
+    metrics: Arc<dyn SolverMetrics>,
 }
 
-impl<I: SingleOrderSolving> From<I> for SingleOrderSolver<I> {
-    fn from(inner: I) -> Self {
-        Self { inner }
+impl<I: SingleOrderSolving> SingleOrderSolver<I> {
+    pub fn new(inner: I, metrics: Arc<dyn SolverMetrics>) -> Self {
+        Self { inner, metrics }
     }
 }
 
@@ -54,16 +57,19 @@ impl<I: SingleOrderSolving + Send + Sync + 'static> Solver for SingleOrderSolver
         let settle = async {
             while let Some(order) = orders.pop_front() {
                 match self.inner.try_settle_order(order.clone()).await {
-                    Ok(settlement) => settlements.extend(settlement),
+                    Ok(settlement) => {
+                        self.metrics
+                            .single_order_solver_succeeded(self.inner.name());
+                        settlements.extend(settlement)
+                    }
                     Err(err) => {
                         let name = self.inner.name();
+                        self.metrics.single_order_solver_failed(name);
                         if err.retryable {
-                            tracing::warn!("Solver {} retryable error: {:?}", name, &err);
+                            tracing::warn!("Solver {} retryable error: {:?}", name, &err.inner);
                             orders.push_back(order);
-                        } else if err.should_alert {
-                            tracing::error!("Solver {} hard error: {:?}", name, &err);
                         } else {
-                            tracing::warn!("Solver {} soft error: {:?}", name, &err);
+                            tracing::warn!("Solver {} error: {:?}", name, &err.inner);
                         }
                     }
                 }
@@ -88,16 +94,13 @@ impl<I: SingleOrderSolving + Send + Sync + 'static> Solver for SingleOrderSolver
 pub struct SettlementError {
     pub inner: anyhow::Error,
     pub retryable: bool,
-    // Whether or not this error should be logged as an error
-    pub should_alert: bool,
 }
 
 impl From<anyhow::Error> for SettlementError {
     fn from(err: Error) -> Self {
         SettlementError {
             inner: err,
             retryable: false,
-            should_alert: true,
         }
     }
 }
@@ -106,6 +109,7 @@ impl From<anyhow::Error> for SettlementError {
 mod tests {
     use super::*;
     use crate::liquidity::tests::CapturingSettlementHandler;
+    use crate::metrics::NoopMetrics;
     use anyhow::anyhow;
     use std::sync::Arc;
 
@@ -116,8 +120,10 @@ mod tests {
             .expect_try_settle_order()
             .times(2)
             .returning(|_| Ok(Some(Settlement::new(Default::default()))));
+        inner.expect_name().returning(|| "Mock Solver");
 
-        let solver: SingleOrderSolver<_> = inner.into();
+        let solver: SingleOrderSolver<_> =
+            SingleOrderSolver::new(inner, Arc::new(NoopMetrics::default()));
         let handler = Arc::new(CapturingSettlementHandler::default());
         let order = LimitOrder {
             id: Default::default(),
@@ -166,7 +172,6 @@ mod tests {
                     0 => Err(SettlementError {
                         inner: anyhow!(""),
                         retryable: true,
-                        should_alert: true,
                     }),
                     1 => Ok(None),
                     _ => unreachable!(),
@@ -175,7 +180,8 @@ mod tests {
                 result
             });
 
-        let solver: SingleOrderSolver<_> = inner.into();
+        let solver: SingleOrderSolver<_> =
+            SingleOrderSolver::new(inner, Arc::new(NoopMetrics::default()));
         let handler = Arc::new(CapturingSettlementHandler::default());
         let order = LimitOrder {
             id: Default::default(),
@@ -206,11 +212,11 @@ mod tests {
             Err(SettlementError {
                 inner: anyhow!(""),
                 retryable: false,
-                should_alert: true,
             })
         });
 
-        let solver: SingleOrderSolver<_> = inner.into();
+        let solver: SingleOrderSolver<_> =
+            SingleOrderSolver::new(inner, Arc::new(NoopMetrics::default()));
         let handler = Arc::new(CapturingSettlementHandler::default());
         let order = LimitOrder {
             id: Default::default(),

solver/src/solver/zeroex_solver.rs

@@ -133,7 +133,6 @@ impl From<ZeroExResponseError> for SettlementError {
         SettlementError {
             inner: anyhow!("0x Response Error {:?}", err),
             retryable: matches!(err, ZeroExResponseError::ServerError(_)),
-            should_alert: true,
         }
     }
 }