From bb6ec49a66bc9380ff0f5eca44cad381b7dc4368 Mon Sep 17 00:00:00 2001 From: Nishant Joshi Date: Fri, 11 Aug 2023 14:09:00 +0530 Subject: [PATCH] feat(generics): add metrics for database calls (#1901) --- crates/diesel_models/src/lib.rs | 10 ++ crates/diesel_models/src/query/generics.rs | 113 ++++++++++++++---- crates/router/src/types/storage/dispute.rs | 16 +-- .../src/types/storage/payment_intent.rs | 15 ++- crates/router/src/types/storage/refund.rs | 15 ++- crates/router_env/src/lib.rs | 1 + 6 files changed, 125 insertions(+), 45 deletions(-) diff --git a/crates/diesel_models/src/lib.rs b/crates/diesel_models/src/lib.rs index 72eba52117..0d1e517046 100644 --- a/crates/diesel_models/src/lib.rs +++ b/crates/diesel_models/src/lib.rs @@ -98,3 +98,13 @@ pub(crate) mod diesel_impl { } } } + +pub(crate) mod metrics { + use router_env::{counter_metric, global_meter, histogram_metric, metrics_context, once_cell}; + + metrics_context!(CONTEXT); + global_meter!(GLOBAL_METER, "ROUTER_API"); + + counter_metric!(DATABASE_CALLS_COUNT, GLOBAL_METER); + histogram_metric!(DATABASE_CALL_TIME, GLOBAL_METER); +} diff --git a/crates/diesel_models/src/query/generics.rs b/crates/diesel_models/src/query/generics.rs index d9311997af..33956ab457 100644 --- a/crates/diesel_models/src/query/generics.rs +++ b/crates/diesel_models/src/query/generics.rs @@ -22,14 +22,61 @@ use diesel::{ use error_stack::{report, IntoReport, ResultExt}; use router_env::{instrument, logger, tracing}; -use crate::{errors, PgPooledConn, StorageResult}; +use crate::{ + errors::{self}, + PgPooledConn, StorageResult, +}; + +pub mod db_metrics { + use router_env::opentelemetry::KeyValue; + + #[derive(Debug)] + pub enum DatabaseOperation { + FindOne, + Filter, + Update, + Insert, + Delete, + DeleteWithResult, + UpdateWithResults, + UpdateOne, + } + + #[inline] + pub async fn track_database_call(future: Fut, operation: DatabaseOperation) -> U + where + Fut: std::future::Future, + { + let start = std::time::Instant::now(); + let output = future.await; + let time_elapsed = start.elapsed(); + + let table_name = std::any::type_name::().rsplit("::").nth(1); + + let attributes = [ + KeyValue::new("table", table_name.unwrap_or("undefined")), + KeyValue::new("operation", format!("{:?}", operation)), + ]; + + crate::metrics::DATABASE_CALLS_COUNT.add(&crate::metrics::CONTEXT, 1, &attributes); + crate::metrics::DATABASE_CALL_TIME.record( + &crate::metrics::CONTEXT, + time_elapsed.as_secs_f64(), + &attributes, + ); + + output + } +} + +use db_metrics::*; #[instrument(level = "DEBUG", skip_all)] pub async fn generic_insert(conn: &PgPooledConn, values: V) -> StorageResult where - T: HasTable + Table + 'static, + T: HasTable
+ Table + 'static + Debug, V: Debug + Insertable, - ::FromClause: QueryFragment, + ::FromClause: QueryFragment + Debug, >::Values: CanInsertInSingleQuery + QueryFragment + 'static, InsertStatement>::Values>: AsQuery + LoadQuery<'static, PgConnection, R> + Send, @@ -40,7 +87,10 @@ where let query = diesel::insert_into(::table()).values(values); logger::debug!(query = %debug_query::(&query).to_string()); - match query.get_result_async(conn).await.into_report() { + match track_database_call::(query.get_result_async(conn), DatabaseOperation::Insert) + .await + .into_report() + { Ok(value) => Ok(value), Err(err) => match err.current_context() { ConnectionError::Query(DieselError::DatabaseError( @@ -74,8 +124,7 @@ where let query = diesel::update(::table().filter(predicate)).set(values); logger::debug!(query = %debug_query::(&query).to_string()); - query - .execute_async(conn) + track_database_call::(query.execute_async(conn), DatabaseOperation::Update) .await .into_report() .change_context(errors::DatabaseError::Others) @@ -109,7 +158,12 @@ where let query = diesel::update(::table().filter(predicate)).set(values); - match query.to_owned().get_results_async(conn).await { + match track_database_call::( + query.to_owned().get_results_async(conn), + DatabaseOperation::UpdateWithResults, + ) + .await + { Ok(result) => { logger::debug!(query = %debug_query::(&query).to_string()); Ok(result) @@ -195,7 +249,12 @@ where let query = diesel::update(::table().find(id.to_owned())).set(values); - match query.to_owned().get_result_async(conn).await { + match track_database_call::( + query.to_owned().get_result_async(conn), + DatabaseOperation::UpdateOne, + ) + .await + { Ok(result) => { logger::debug!(query = %debug_query::(&query).to_string()); Ok(result) @@ -226,8 +285,7 @@ where let query = diesel::delete(::table().filter(predicate)); logger::debug!(query = %debug_query::(&query).to_string()); - query - .execute_async(conn) + track_database_call::(query.execute_async(conn), DatabaseOperation::Delete) .await .into_report() .change_context(errors::DatabaseError::Others) @@ -261,18 +319,20 @@ where let query = diesel::delete(::table().filter(predicate)); logger::debug!(query = %debug_query::(&query).to_string()); - query - .get_results_async(conn) - .await - .into_report() - .change_context(errors::DatabaseError::Others) - .attach_printable_lazy(|| "Error while deleting") - .and_then(|result| { - result.first().cloned().ok_or_else(|| { - report!(errors::DatabaseError::NotFound) - .attach_printable("Object to be deleted does not exist") - }) + track_database_call::( + query.get_results_async(conn), + DatabaseOperation::DeleteWithResult, + ) + .await + .into_report() + .change_context(errors::DatabaseError::Others) + .attach_printable_lazy(|| "Error while deleting") + .and_then(|result| { + result.first().cloned().ok_or_else(|| { + report!(errors::DatabaseError::NotFound) + .attach_printable("Object to be deleted does not exist") }) + }) } #[instrument(level = "DEBUG", skip_all)] @@ -287,7 +347,10 @@ where let query = ::table().find(id.to_owned()); logger::debug!(query = %debug_query::(&query).to_string()); - match query.first_async(conn).await.into_report() { + match track_database_call::(query.first_async(conn), DatabaseOperation::FindOne) + .await + .into_report() + { Ok(value) => Ok(value), Err(err) => match err.current_context() { ConnectionError::Query(DieselError::NotFound) => { @@ -337,8 +400,7 @@ where let query = ::table().filter(predicate); logger::debug!(query = %debug_query::(&query).to_string()); - query - .get_result_async(conn) + track_database_call::(query.get_result_async(conn), DatabaseOperation::FindOne) .await .into_report() .map_err(|err| match err.current_context() { @@ -410,8 +472,7 @@ where logger::debug!(query = %debug_query::(&query).to_string()); - query - .get_results_async(conn) + track_database_call::(query.get_results_async(conn), DatabaseOperation::Filter) .await .into_report() .change_context(errors::DatabaseError::NotFound) diff --git a/crates/router/src/types/storage/dispute.rs b/crates/router/src/types/storage/dispute.rs index 01b1ea01ca..7b66df6b05 100644 --- a/crates/router/src/types/storage/dispute.rs +++ b/crates/router/src/types/storage/dispute.rs @@ -2,7 +2,7 @@ use async_bb8_diesel::AsyncRunQueryDsl; use common_utils::errors::CustomResult; use diesel::{associations::HasTable, ExpressionMethods, QueryDsl}; pub use diesel_models::dispute::{Dispute, DisputeNew, DisputeUpdate}; -use diesel_models::{errors, schema::dispute::dsl}; +use diesel_models::{errors, query::generics::db_metrics, schema::dispute::dsl}; use error_stack::{IntoReport, ResultExt}; use crate::{connection::PgPooledConn, logger}; @@ -61,11 +61,13 @@ impl DisputeDbExt for Dispute { logger::debug!(query = %diesel::debug_query::(&filter).to_string()); - filter - .get_results_async(conn) - .await - .into_report() - .change_context(errors::DatabaseError::NotFound) - .attach_printable_lazy(|| "Error filtering records by predicate") + db_metrics::track_database_call::<::Table, _, _>( + filter.get_results_async(conn), + db_metrics::DatabaseOperation::Filter, + ) + .await + .into_report() + .change_context(errors::DatabaseError::NotFound) + .attach_printable_lazy(|| "Error filtering records by predicate") } } diff --git a/crates/router/src/types/storage/payment_intent.rs b/crates/router/src/types/storage/payment_intent.rs index 79e5d28562..a78ea8eaaf 100644 --- a/crates/router/src/types/storage/payment_intent.rs +++ b/crates/router/src/types/storage/payment_intent.rs @@ -1,5 +1,6 @@ use async_bb8_diesel::AsyncRunQueryDsl; use diesel::{associations::HasTable, debug_query, pg::Pg, ExpressionMethods, JoinOnDsl, QueryDsl}; +use diesel_models::query::generics::db_metrics; pub use diesel_models::{ errors, payment_attempt::PaymentAttempt, @@ -96,12 +97,14 @@ impl PaymentIntentDbExt for PaymentIntent { crate::logger::debug!(query = %debug_query::(&filter).to_string()); - filter - .get_results_async(conn) - .await - .into_report() - .change_context(errors::DatabaseError::NotFound) - .attach_printable_lazy(|| "Error filtering records by predicate") + db_metrics::track_database_call::<::Table, _, _>( + filter.get_results_async(conn), + db_metrics::DatabaseOperation::Filter, + ) + .await + .into_report() + .change_context(errors::DatabaseError::NotFound) + .attach_printable_lazy(|| "Error filtering records by predicate") } #[instrument(skip(conn))] diff --git a/crates/router/src/types/storage/refund.rs b/crates/router/src/types/storage/refund.rs index 8f3be72342..76084f87bd 100644 --- a/crates/router/src/types/storage/refund.rs +++ b/crates/router/src/types/storage/refund.rs @@ -7,6 +7,7 @@ pub use diesel_models::refund::{ use diesel_models::{ enums::{Currency, RefundStatus}, errors, + query::generics::db_metrics, schema::refund::dsl, }; use error_stack::{IntoReport, ResultExt}; @@ -78,12 +79,14 @@ impl RefundDbExt for Refund { logger::debug!(query = %diesel::debug_query::(&filter).to_string()); - filter - .get_results_async(conn) - .await - .into_report() - .change_context(errors::DatabaseError::NotFound) - .attach_printable_lazy(|| "Error filtering records by predicate") + db_metrics::track_database_call::<::Table, _, _>( + filter.get_results_async(conn), + db_metrics::DatabaseOperation::Filter, + ) + .await + .into_report() + .change_context(errors::DatabaseError::NotFound) + .attach_printable_lazy(|| "Error filtering records by predicate") } async fn filter_by_meta_constraints( diff --git a/crates/router_env/src/lib.rs b/crates/router_env/src/lib.rs index 956c0b91d9..d3612767ff 100644 --- a/crates/router_env/src/lib.rs +++ b/crates/router_env/src/lib.rs @@ -20,6 +20,7 @@ pub mod vergen; // pub use literally; #[doc(inline)] pub use logger::*; +pub use once_cell; pub use opentelemetry; pub use tracing; #[cfg(feature = "actix_web")]