Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Instrument Solver Engine #2129

Merged
merged 8 commits into from
Dec 8, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions crates/solvers/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ hex = "0.4"
hyper = "0.14"
itertools = "0.11"
num = "0.4"
prometheus = { workspace = true }
prometheus-metric-storage = { workspace = true }
Comment on lines +27 to +28
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My plan didn't pan out entirely. 🙁
Maybe we can reexport the types we need from those crates in the observe crate as well in another PR. Then observe should be the only crate that directly depends on them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not able to get this to compile. The proc macro that generates the Metrics struct requires prometheus_metric_storage so even with re-exports I'm getting

error[E0433]: failed to resolve: use of undeclared crate or module prometheus_metric_storage
--> crates/solvers/src/infra/metrics.rs:7:24
|
7 | #[derive(Debug, Clone, metrics::MetricStorage)]
| ^^^^^^^^^^^^^^^^^^^^^^ use of undeclared crate or module prometheus_metric_storage
|
= note: this error originates in the derive macro metrics::MetricStorage (in Nightly builds, run with -Z macro-backtrace for more info)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel free to merge the PR without this. It would just be a nice clean up that can happen at any time and should not delay proper metrics on the solvers crate.
That being said I remember exporting macros requiring a specific macro on top of it and reexporting a macro might be even more magical. 😄 🤷‍♂️

reqwest = "0.11"
serde = "1"
serde_json = "1"
Expand Down
1 change: 1 addition & 0 deletions crates/solvers/src/api/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ impl Api {
shutdown: impl Future<Output = ()> + Send + 'static,
) -> Result<(), hyper::Error> {
let app = axum::Router::new()
.route("/metrics", axum::routing::get(routes::metrics))
.route("/healthz", axum::routing::get(routes::healthz))
.route("/solve", axum::routing::post(routes::solve))
.route("/notify", axum::routing::post(routes::notify))
Expand Down
9 changes: 9 additions & 0 deletions crates/solvers/src/api/routes/metrics.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
use prometheus::Encoder;

pub async fn metrics() -> String {
let registry = observe::metrics::get_registry();
let encoder = prometheus::TextEncoder::new();
let mut buffer = Vec::new();
encoder.encode(&registry.gather(), &mut buffer).unwrap();
String::from_utf8(buffer).unwrap()
fleupold marked this conversation as resolved.
Show resolved Hide resolved
}
3 changes: 2 additions & 1 deletion crates/solvers/src/api/routes/mod.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
use serde::Serialize;

mod healthz;
mod metrics;
mod notify;
mod solve;

pub(super) use {healthz::healthz, notify::notify, solve::solve};
pub(super) use {healthz::healthz, metrics::metrics, notify::notify, solve::solve};

#[derive(Debug, Serialize)]
#[serde(untagged)]
Expand Down
1 change: 1 addition & 0 deletions crates/solvers/src/domain/solver/dex/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ impl Dex {
gas_price: auction::GasPrice,
) -> Option<dex::Swap> {
let dex_err_handler = |err: infra::dex::Error| {
infra::observe::solve_error(&err.format_variant());
match &err {
err @ infra::dex::Error::NotFound => {
if order.partially_fillable {
Expand Down
13 changes: 10 additions & 3 deletions crates/solvers/src/domain/solver/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
use crate::domain::{auction, notification, solution};
use crate::{
domain::{auction, notification, solution},
infra::observe,
};

pub mod baseline;
pub mod dex;
Expand All @@ -19,12 +22,16 @@ impl Solver {
/// returning multiple solutions to later merge multiple non-overlapping
/// solutions to get one big more gas efficient solution.
pub async fn solve(&self, auction: auction::Auction) -> Vec<solution::Solution> {
match self {
observe::solve(&auction);
let deadline = auction.deadline.clone();
let solutions = match self {
Solver::Baseline(solver) => solver.solve(auction).await,
Solver::Naive(solver) => solver.solve(auction).await,
Solver::Legacy(solver) => solver.solve(auction).await,
Solver::Dex(solver) => solver.solve(auction).await,
}
};
observe::solved(&deadline, &solutions);
solutions
}

/// Notifies the solver about important events. Some of those events are
Expand Down
12 changes: 12 additions & 0 deletions crates/solvers/src/infra/dex/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,18 @@ pub enum Error {
Other(Box<dyn std::error::Error + Send + Sync>),
}

impl Error {
/// for instrumentization purposes
pub fn format_variant(&self) -> &'static str {
match self {
Self::OrderNotSupported => "OrderNotSupported",
Self::NotFound => "NotFound",
Self::RateLimited => "RateLimited",
Self::Other(_) => "Other",
}
}
}

impl From<balancer::Error> for Error {
fn from(err: balancer::Error) -> Self {
match err {
Expand Down
1 change: 1 addition & 0 deletions crates/solvers/src/infra/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ pub mod cli;
pub mod config;
pub mod contracts;
pub mod dex;
pub mod observe;
29 changes: 29 additions & 0 deletions crates/solvers/src/infra/observe/metrics.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/// Metrics for the solver engine.
#[derive(Debug, Clone, prometheus_metric_storage::MetricStorage)]
pub struct Metrics {
/// The amount of time this solver engine has for solving.
#[metric(buckets(0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15))]
pub time_limit: prometheus::Histogram,

/// The amount of time this solver engine has left when it finished solving.
#[metric(buckets(0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15))]
pub remaining_time: prometheus::Histogram,

/// Errors that occurred during solving.
#[metric(labels("reason"))]
pub solve_errors: prometheus::IntCounterVec,

/// The number of solutions that were found.
pub solutions: prometheus::IntCounter,
}

/// Setup the metrics registry.
pub fn init() {
observe::metrics::setup_registry_reentrant(Some("solver-engine".to_owned()), None);
}

/// Get the metrics instance.
pub fn get() -> &'static Metrics {
Metrics::instance(observe::metrics::get_storage_registry())
.expect("unexpected error getting metrics instance")
}
34 changes: 34 additions & 0 deletions crates/solvers/src/infra/observe/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
use {
crate::domain::{auction, solution},
chrono::Utc,
};

pub mod metrics;

pub fn solve(auction: &auction::Auction) {
fleupold marked this conversation as resolved.
Show resolved Hide resolved
metrics::get()
.time_limit
.observe(remaining_time(&auction.deadline));
}

pub fn solved(deadline: &auction::Deadline, solutions: &[solution::Solution]) {
metrics::get()
.remaining_time
.observe(remaining_time(deadline));
metrics::get().solutions.inc_by(solutions.len() as u64);
}

pub fn solve_error(reason: &str) {
metrics::get()
.solve_errors
.with_label_values(&[reason])
.inc();
}

fn remaining_time(deadline: &auction::Deadline) -> f64 {
deadline
.0
.signed_duration_since(Utc::now())
.num_milliseconds() as f64
/ 1000.0
}
Loading