Demote Paraswap Errors to warn! + success/failure metrics

solver/src/driver.rs

@@ -119,13 +119,15 @@ impl Driver {
             let metrics = &self.metrics;
             async move {
                 let start_time = Instant::now();
-                let result =
-                    match tokio::time::timeout_at(auction.deadline.into(), solver.solve(auction))
-                        .await
-                    {
-                        Ok(inner) => inner,
-                        Err(_timeout) => Err(anyhow!("solver timed out")),
-                    };
+                let result = match tokio::time::timeout_at(
+                    auction.deadline.into(),
+                    solver.solve(auction, metrics.clone()),
+                )
+                .await
+                {
+                    Ok(inner) => inner,
+                    Err(_timeout) => Err(anyhow!("solver timed out")),
+                };
                 metrics.settlement_computed(solver.name(), start_time);
                 (solver.clone(), result)
             }

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_not_settled(&self, count: usize);
@@ -41,6 +43,7 @@ pub struct Metrics {
     liquidity: IntGaugeVec,
     settlement_simulations: IntCounterVec,
     settlement_submissions: IntCounterVec,
+    single_order_solver_runs: IntCounterVec,
     matched_but_unsettled_orders: IntCounter,
     transport_requests: HistogramVec,
     pool_cache_hits: IntCounter,
@@ -94,6 +97,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_unsettled_orders = IntCounter::new(
             "orders_matched_not_settled",
             "Counter for the number of orders for which at least one solver computed an execution which was not chosen in this run-loop",
@@ -126,6 +135,7 @@ impl Metrics {
             liquidity,
             settlement_simulations,
             settlement_submissions,
+            single_order_solver_runs,
             matched_but_unsettled_orders,
             transport_requests,
             pool_cache_hits,
@@ -189,6 +199,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])
@@ -260,6 +282,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_not_settled(&self, _: usize) {}

solver/src/solver.rs

@@ -1,3 +1,4 @@
+use crate::metrics::SolverMetrics;
 use crate::{
     liquidity::{LimitOrder, Liquidity},
     settlement::Settlement,
@@ -47,7 +48,11 @@ pub trait Solver: 'static {
     /// order) so that they can be merged by the driver at its leisure.
     ///
     /// id identifies this instance of solving by the driver in which it invokes all solvers.
-    async fn solve(&self, auction: Auction) -> Result<Vec<Settlement>>;
+    async fn solve(
+        &self,
+        auction: Auction,
+        metrics: Arc<dyn SolverMetrics>,
+    ) -> Result<Vec<Settlement>>;
 
     /// Returns solver's account that should be used to submit settlements.
     fn account(&self) -> &Account;
@@ -307,7 +312,11 @@ impl SellVolumeFilteringSolver {
 
 #[async_trait::async_trait]
 impl Solver for SellVolumeFilteringSolver {
-    async fn solve(&self, mut auction: Auction) -> Result<Vec<Settlement>> {
+    async fn solve(
+        &self,
+        mut auction: Auction,
+        metrics: Arc<dyn SolverMetrics>,
+    ) -> Result<Vec<Settlement>> {
         let original_length = auction.orders.len();
         auction.orders = self
             .filter_orders(auction.orders, &auction.price_estimates)
@@ -316,7 +325,7 @@ impl Solver for SellVolumeFilteringSolver {
             "Filtered {} orders because on insufficient volume",
             original_length - auction.orders.len()
         );
-        self.inner.solve(auction).await
+        self.inner.solve(auction, metrics).await
     }
 
     fn account(&self) -> &Account {
@@ -338,7 +347,7 @@ mod tests {
     pub struct NoopSolver();
     #[async_trait::async_trait]
     impl Solver for NoopSolver {
-        async fn solve(&self, _: Auction) -> Result<Vec<Settlement>> {
+        async fn solve(&self, _: Auction, _: Arc<dyn SolverMetrics>) -> Result<Vec<Settlement>> {
             Ok(Vec::new())
         }
 

solver/src/solver/baseline_solver.rs

@@ -1,3 +1,4 @@
+use crate::metrics::SolverMetrics;
 use crate::{
     liquidity::{
         token_pairs, AmmOrderExecution, ConstantProductOrder, LimitOrder, Liquidity,
@@ -29,6 +30,7 @@ impl Solver for BaselineSolver {
         Auction {
             orders, liquidity, ..
         }: Auction,
+        _: Arc<dyn SolverMetrics>,
     ) -> Result<Vec<Settlement>> {
         Ok(self.solve_(orders, liquidity))
     }

solver/src/solver/http_solver.rs

@@ -3,6 +3,7 @@ pub mod model;
 mod settlement;
 
 use self::{model::*, settlement::SettlementContext};
+use crate::metrics::SolverMetrics;
 use crate::{
     liquidity::{LimitOrder, Liquidity},
     settlement::Settlement,
@@ -559,6 +560,7 @@ impl Solver for HttpSolver {
             deadline,
             price_estimates,
         }: Auction,
+        _: Arc<dyn SolverMetrics>,
     ) -> Result<Vec<Settlement>> {
         if orders.is_empty() {
             return Ok(Vec::new());

solver/src/solver/naive_solver.rs

@@ -1,5 +1,6 @@
 mod multi_order_solver;
 
+use crate::metrics::SolverMetrics;
 use crate::{
     liquidity::{ConstantProductOrder, LimitOrder, Liquidity},
     settlement::Settlement,
@@ -9,6 +10,7 @@ use anyhow::Result;
 use ethcontract::Account;
 use model::TokenPair;
 use std::collections::HashMap;
+use std::sync::Arc;
 
 pub struct NaiveSolver {
     account: Account,
@@ -27,6 +29,7 @@ impl Solver for NaiveSolver {
         Auction {
             orders, liquidity, ..
         }: Auction,
+        _: Arc<dyn SolverMetrics>,
     ) -> Result<Vec<Settlement>> {
         let uniswaps = extract_deepest_amm_liquidity(&liquidity);
         Ok(settle(orders, uniswaps).await)

solver/src/solver/oneinch_solver/api.rs

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

solver/src/solver/paraswap_solver.rs

@@ -81,7 +81,7 @@ impl From<ParaswapResponseError> for SettlementError {
                     | ParaswapResponseError::ServerBusy
                     | ParaswapResponseError::Send(_),
             ),
-            should_alert: !matches!(
+            track_failure: !matches!(
                 err,
                 ParaswapResponseError::PriceChange
                     | ParaswapResponseError::BuildingTransaction(_)

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)]
@@ -45,6 +47,7 @@ impl<I: SingleOrderSolving + Send + Sync + 'static> Solver for SingleOrderSolver
             deadline,
             ..
         }: Auction,
+        metrics: Arc<dyn SolverMetrics>,
     ) -> Result<Vec<Settlement>> {
         // Randomize which orders we start with to prevent us getting stuck on bad orders.
         orders.shuffle(&mut rand::thread_rng());
@@ -54,14 +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) => {
+                        metrics.single_order_solver_succeeded(self.inner.name());
+                        settlements.extend(settlement)
+                    }
                     Err(err) => {
                         let name = self.inner.name();
+                        // TODO - determine if we should record failure on all or just some of these.
                         if err.retryable {
                             tracing::warn!("Solver {} retryable error: {:?}", name, &err);
                             orders.push_back(order);
-                        } else if err.should_alert {
-                            tracing::error!("Solver {} hard error: {:?}", name, &err);
+                        } else if err.track_failure {
+                            tracing::warn!("Solver {} hard error: {:?}", name, &err);
+                            metrics.single_order_solver_failed(name);
                         } else {
                             tracing::warn!("Solver {} soft error: {:?}", name, &err);
                         }
@@ -89,15 +97,15 @@ 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,
+    pub track_failure: bool,
 }
 
 impl From<anyhow::Error> for SettlementError {
     fn from(err: Error) -> Self {
         SettlementError {
             inner: err,
             retryable: false,
-            should_alert: true,
+            track_failure: true,
         }
     }
 }
@@ -106,6 +114,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;
 
@@ -143,10 +152,13 @@ mod tests {
         ];
 
         let settlements = solver
-            .solve(Auction {
-                orders,
-                ..Default::default()
-            })
+            .solve(
+                Auction {
+                    orders,
+                    ..Default::default()
+                },
+                Arc::new(NoopMetrics::default()),
+            )
             .await
             .unwrap();
         assert_eq!(settlements.len(), 2);
@@ -166,7 +178,7 @@ mod tests {
                     0 => Err(SettlementError {
                         inner: anyhow!(""),
                         retryable: true,
-                        should_alert: true,
+                        track_failure: true,
                     }),
                     1 => Ok(None),
                     _ => unreachable!(),
@@ -190,10 +202,13 @@ mod tests {
             is_liquidity_order: false,
         };
         solver
-            .solve(Auction {
-                orders: vec![order],
-                ..Default::default()
-            })
+            .solve(
+                Auction {
+                    orders: vec![order],
+                    ..Default::default()
+                },
+                Arc::new(NoopMetrics::default()),
+            )
             .await
             .unwrap();
     }
@@ -206,7 +221,7 @@ mod tests {
             Err(SettlementError {
                 inner: anyhow!(""),
                 retryable: false,
-                should_alert: true,
+                track_failure: true,
             })
         });
 
@@ -225,10 +240,13 @@ mod tests {
             is_liquidity_order: false,
         };
         solver
-            .solve(Auction {
-                orders: vec![order],
-                ..Default::default()
-            })
+            .solve(
+                Auction {
+                    orders: vec![order],
+                    ..Default::default()
+                },
+                Arc::new(NoopMetrics::default()),
+            )
             .await
             .unwrap();
     }

solver/src/solver/zeroex_solver.rs

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