From 60ac4b94695b84106412e4e4c969b3b6dd94ed64 Mon Sep 17 00:00:00 2001 From: Prajjwal Kumar Date: Fri, 8 Nov 2024 17:29:44 +0530 Subject: [PATCH] Refactor(core): interpolate success_based_routing config params with their specific values (#6448) Co-authored-by: hyperswitch-bot[bot] <148525504+hyperswitch-bot[bot]@users.noreply.github.com> --- crates/api_models/src/routing.rs | 5 +- crates/external_services/src/grpc_client.rs | 3 - .../src/grpc_client/dynamic_routing.rs | 38 ++------ crates/router/src/core/errors.rs | 2 + crates/router/src/core/payments.rs | 46 ++++++++- .../payments/operations/payment_response.rs | 36 ++++++- crates/router/src/core/payments/routing.rs | 10 ++ crates/router/src/core/routing/helpers.rs | 94 ++++++++++++++++++- 8 files changed, 188 insertions(+), 46 deletions(-) diff --git a/crates/api_models/src/routing.rs b/crates/api_models/src/routing.rs index 47d75b2e8357..389af3dab7bf 100644 --- a/crates/api_models/src/routing.rs +++ b/crates/api_models/src/routing.rs @@ -615,8 +615,11 @@ impl Default for SuccessBasedRoutingConfig { pub enum SuccessBasedRoutingConfigParams { PaymentMethod, PaymentMethodType, - Currency, AuthenticationType, + Currency, + Country, + CardNetwork, + CardBin, } #[derive(serde::Serialize, serde::Deserialize, Debug, Clone, ToSchema)] diff --git a/crates/external_services/src/grpc_client.rs b/crates/external_services/src/grpc_client.rs index 5afd30245519..e7b229a80708 100644 --- a/crates/external_services/src/grpc_client.rs +++ b/crates/external_services/src/grpc_client.rs @@ -5,7 +5,6 @@ use std::{fmt::Debug, sync::Arc}; #[cfg(feature = "dynamic_routing")] use dynamic_routing::{DynamicRoutingClientConfig, RoutingStrategy}; -use router_env::logger; use serde; /// Struct contains all the gRPC Clients @@ -38,8 +37,6 @@ impl GrpcClientSettings { .await .expect("Failed to establish a connection with the Dynamic Routing Server"); - logger::info!("Connection established with gRPC Server"); - Arc::new(GrpcClients { #[cfg(feature = "dynamic_routing")] dynamic_routing: dynamic_routing_connection, diff --git a/crates/external_services/src/grpc_client/dynamic_routing.rs b/crates/external_services/src/grpc_client/dynamic_routing.rs index 343dd80e9516..0546d05ba7c9 100644 --- a/crates/external_services/src/grpc_client/dynamic_routing.rs +++ b/crates/external_services/src/grpc_client/dynamic_routing.rs @@ -9,6 +9,7 @@ use error_stack::ResultExt; use http_body_util::combinators::UnsyncBoxBody; use hyper::body::Bytes; use hyper_util::client::legacy::connect::HttpConnector; +use router_env::logger; use serde; use success_rate::{ success_rate_calculator_client::SuccessRateCalculatorClient, CalSuccessRateConfig, @@ -80,6 +81,7 @@ impl DynamicRoutingClientConfig { let success_rate_client = match self { Self::Enabled { host, port } => { let uri = format!("http://{}:{}", host, port).parse::()?; + logger::info!("Connection established with dynamic routing gRPC Server"); Some(SuccessRateCalculatorClient::with_origin(client, uri)) } Self::Disabled => None, @@ -98,6 +100,7 @@ pub trait SuccessBasedDynamicRouting: dyn_clone::DynClone + Send + Sync { &self, id: String, success_rate_based_config: SuccessBasedRoutingConfig, + params: String, label_input: Vec, ) -> DynamicRoutingResult; /// To update the success rate with the given label @@ -105,6 +108,7 @@ pub trait SuccessBasedDynamicRouting: dyn_clone::DynClone + Send + Sync { &self, id: String, success_rate_based_config: SuccessBasedRoutingConfig, + params: String, response: Vec, ) -> DynamicRoutingResult; } @@ -115,24 +119,9 @@ impl SuccessBasedDynamicRouting for SuccessRateCalculatorClient { &self, id: String, success_rate_based_config: SuccessBasedRoutingConfig, + params: String, label_input: Vec, ) -> DynamicRoutingResult { - let params = success_rate_based_config - .params - .map(|vec| { - vec.into_iter().fold(String::new(), |mut acc_str, params| { - if !acc_str.is_empty() { - acc_str.push(':') - } - acc_str.push_str(params.to_string().as_str()); - acc_str - }) - }) - .get_required_value("params") - .change_context(DynamicRoutingError::MissingRequiredField { - field: "params".to_string(), - })?; - let labels = label_input .into_iter() .map(|conn_choice| conn_choice.to_string()) @@ -167,6 +156,7 @@ impl SuccessBasedDynamicRouting for SuccessRateCalculatorClient { &self, id: String, success_rate_based_config: SuccessBasedRoutingConfig, + params: String, label_input: Vec, ) -> DynamicRoutingResult { let config = success_rate_based_config @@ -182,22 +172,6 @@ impl SuccessBasedDynamicRouting for SuccessRateCalculatorClient { }) .collect(); - let params = success_rate_based_config - .params - .map(|vec| { - vec.into_iter().fold(String::new(), |mut acc_str, params| { - if !acc_str.is_empty() { - acc_str.push(':') - } - acc_str.push_str(params.to_string().as_str()); - acc_str - }) - }) - .get_required_value("params") - .change_context(DynamicRoutingError::MissingRequiredField { - field: "params".to_string(), - })?; - let request = tonic::Request::new(UpdateSuccessRateWindowRequest { id, params, diff --git a/crates/router/src/core/errors.rs b/crates/router/src/core/errors.rs index d095b471e2dd..96321d097946 100644 --- a/crates/router/src/core/errors.rs +++ b/crates/router/src/core/errors.rs @@ -330,6 +330,8 @@ pub enum RoutingError { MetadataParsingError, #[error("Unable to retrieve success based routing config")] SuccessBasedRoutingConfigError, + #[error("Params not found in success based routing config")] + SuccessBasedRoutingParamsNotFoundError, #[error("Unable to calculate success based routing config from dynamic routing service")] SuccessRateCalculationError, #[error("Success rate client from dynamic routing gRPC service not initialized")] diff --git a/crates/router/src/core/payments.rs b/crates/router/src/core/payments.rs index 5d75001b118a..4ad00df9c247 100644 --- a/crates/router/src/core/payments.rs +++ b/crates/router/src/core/payments.rs @@ -72,6 +72,8 @@ use super::{ #[cfg(feature = "frm")] use crate::core::fraud_check as frm_core; #[cfg(all(feature = "v1", feature = "dynamic_routing"))] +use crate::core::routing::helpers as routing_helpers; +#[cfg(all(feature = "v1", feature = "dynamic_routing"))] use crate::types::api::convert_connector_data_to_routable_connectors; use crate::{ configs::settings::{ApplePayPreDecryptFlow, PaymentMethodTypeTokenFilter}, @@ -5580,10 +5582,46 @@ where #[cfg(all(feature = "v1", feature = "dynamic_routing"))] let connectors = { if business_profile.dynamic_routing_algorithm.is_some() { - routing::perform_success_based_routing(state, connectors.clone(), business_profile) - .await - .map_err(|e| logger::error!(success_rate_routing_error=?e)) - .unwrap_or(connectors) + let success_based_routing_config_params_interpolator = + routing_helpers::SuccessBasedRoutingConfigParamsInterpolator::new( + payment_data.get_payment_attempt().payment_method, + payment_data.get_payment_attempt().payment_method_type, + payment_data.get_payment_attempt().authentication_type, + payment_data.get_payment_attempt().currency, + payment_data + .get_billing_address() + .and_then(|address| address.address) + .and_then(|address| address.country), + payment_data + .get_payment_attempt() + .payment_method_data + .as_ref() + .and_then(|data| data.as_object()) + .and_then(|card| card.get("card")) + .and_then(|data| data.as_object()) + .and_then(|card| card.get("card_network")) + .and_then(|network| network.as_str()) + .map(|network| network.to_string()), + payment_data + .get_payment_attempt() + .payment_method_data + .as_ref() + .and_then(|data| data.as_object()) + .and_then(|card| card.get("card")) + .and_then(|data| data.as_object()) + .and_then(|card| card.get("card_isin")) + .and_then(|card_isin| card_isin.as_str()) + .map(|card_isin| card_isin.to_string()), + ); + routing::perform_success_based_routing( + state, + connectors.clone(), + business_profile, + success_based_routing_config_params_interpolator, + ) + .await + .map_err(|e| logger::error!(success_rate_routing_error=?e)) + .unwrap_or(connectors) } else { connectors } diff --git a/crates/router/src/core/payments/operations/payment_response.rs b/crates/router/src/core/payments/operations/payment_response.rs index 0234b97032c7..f0381dbf8220 100644 --- a/crates/router/src/core/payments/operations/payment_response.rs +++ b/crates/router/src/core/payments/operations/payment_response.rs @@ -21,7 +21,7 @@ use tracing_futures::Instrument; use super::{Operation, OperationSessionSetters, PostUpdateTracker}; #[cfg(all(feature = "v1", feature = "dynamic_routing"))] -use crate::core::routing::helpers::push_metrics_for_success_based_routing; +use crate::core::routing::helpers as routing_helpers; use crate::{ connector::utils::PaymentResponseRouterData, consts, @@ -1968,13 +1968,44 @@ async fn payment_response_update_tracker( let state = state.clone(); let business_profile = business_profile.clone(); let payment_attempt = payment_attempt.clone(); + let success_based_routing_config_params_interpolator = + routing_helpers::SuccessBasedRoutingConfigParamsInterpolator::new( + payment_attempt.payment_method, + payment_attempt.payment_method_type, + payment_attempt.authentication_type, + payment_attempt.currency, + payment_data + .address + .get_payment_billing() + .and_then(|address| address.clone().address) + .and_then(|address| address.country), + payment_attempt + .payment_method_data + .as_ref() + .and_then(|data| data.as_object()) + .and_then(|card| card.get("card")) + .and_then(|data| data.as_object()) + .and_then(|card| card.get("card_network")) + .and_then(|network| network.as_str()) + .map(|network| network.to_string()), + payment_attempt + .payment_method_data + .as_ref() + .and_then(|data| data.as_object()) + .and_then(|card| card.get("card")) + .and_then(|data| data.as_object()) + .and_then(|card| card.get("card_isin")) + .and_then(|card_isin| card_isin.as_str()) + .map(|card_isin| card_isin.to_string()), + ); tokio::spawn( async move { - push_metrics_for_success_based_routing( + routing_helpers::push_metrics_with_update_window_for_success_based_routing( &state, &payment_attempt, routable_connectors, &business_profile, + success_based_routing_config_params_interpolator, ) .await .map_err(|e| logger::error!(dynamic_routing_metrics_error=?e)) @@ -1984,6 +2015,7 @@ async fn payment_response_update_tracker( ); } } + payment_data.payment_intent = payment_intent; payment_data.payment_attempt = payment_attempt; router_data.payment_method_status.and_then(|status| { diff --git a/crates/router/src/core/payments/routing.rs b/crates/router/src/core/payments/routing.rs index 40721e9b4c31..28321529ef2d 100644 --- a/crates/router/src/core/payments/routing.rs +++ b/crates/router/src/core/payments/routing.rs @@ -1240,6 +1240,7 @@ pub async fn perform_success_based_routing( state: &SessionState, routable_connectors: Vec, business_profile: &domain::Profile, + success_based_routing_config_params_interpolator: routing::helpers::SuccessBasedRoutingConfigParamsInterpolator, ) -> RoutingResult> { let success_based_dynamic_routing_algo_ref: api_routing::DynamicRoutingAlgorithmRef = business_profile @@ -1293,6 +1294,14 @@ pub async fn perform_success_based_routing( .change_context(errors::RoutingError::SuccessBasedRoutingConfigError) .attach_printable("unable to fetch success_rate based dynamic routing configs")?; + let success_based_routing_config_params = success_based_routing_config_params_interpolator + .get_string_val( + success_based_routing_configs + .params + .as_ref() + .ok_or(errors::RoutingError::SuccessBasedRoutingParamsNotFoundError)?, + ); + let tenant_business_profile_id = routing::helpers::generate_tenant_business_profile_id( &state.tenant.redis_key_prefix, business_profile.get_id().get_string_repr(), @@ -1302,6 +1311,7 @@ pub async fn perform_success_based_routing( .calculate_success_rate( tenant_business_profile_id, success_based_routing_configs, + success_based_routing_config_params, routable_connectors, ) .await diff --git a/crates/router/src/core/routing/helpers.rs b/crates/router/src/core/routing/helpers.rs index 31cd4234714e..0250d00d1bbc 100644 --- a/crates/router/src/core/routing/helpers.rs +++ b/crates/router/src/core/routing/helpers.rs @@ -640,11 +640,12 @@ pub async fn fetch_success_based_routing_configs( /// metrics for success based dynamic routing #[cfg(all(feature = "v1", feature = "dynamic_routing"))] #[instrument(skip_all)] -pub async fn push_metrics_for_success_based_routing( +pub async fn push_metrics_with_update_window_for_success_based_routing( state: &SessionState, payment_attempt: &storage::PaymentAttempt, routable_connectors: Vec, business_profile: &domain::Profile, + success_based_routing_config_params_interpolator: SuccessBasedRoutingConfigParamsInterpolator, ) -> RouterResult<()> { let success_based_dynamic_routing_algo_ref: routing_types::DynamicRoutingAlgorithmRef = business_profile @@ -697,10 +698,20 @@ pub async fn push_metrics_for_success_based_routing( business_profile.get_id().get_string_repr(), ); + let success_based_routing_config_params = success_based_routing_config_params_interpolator + .get_string_val( + success_based_routing_configs + .params + .as_ref() + .ok_or(errors::RoutingError::SuccessBasedRoutingParamsNotFoundError) + .change_context(errors::ApiErrorResponse::InternalServerError)?, + ); + let success_based_connectors = client .calculate_success_rate( tenant_business_profile_id.clone(), success_based_routing_configs.clone(), + success_based_routing_config_params.clone(), routable_connectors.clone(), ) .await @@ -725,9 +736,10 @@ pub async fn push_metrics_for_success_based_routing( let (first_success_based_connector, merchant_connector_id) = first_success_based_connector_label .split_once(':') .ok_or(errors::ApiErrorResponse::InternalServerError) - .attach_printable( - "unable to split connector_name and mca_id from the first connector obtained from dynamic routing service", - )?; + .attach_printable(format!( + "unable to split connector_name and mca_id from the first connector {:?} obtained from dynamic routing service", + first_success_based_connector_label + ))?; let outcome = get_success_based_metrics_outcome_for_payment( &payment_status_attribute, @@ -802,6 +814,7 @@ pub async fn push_metrics_for_success_based_routing( .update_success_rate( tenant_business_profile_id, success_based_routing_configs, + success_based_routing_config_params, vec![routing_types::RoutableConnectorChoiceWithStatus::new( routing_types::RoutableConnectorChoice { choice_kind: api_models::routing::RoutableChoiceKind::FullStruct, @@ -956,3 +969,76 @@ pub async fn default_success_based_routing_setup( ); Ok(ApplicationResponse::Json(new_record)) } + +pub struct SuccessBasedRoutingConfigParamsInterpolator { + pub payment_method: Option, + pub payment_method_type: Option, + pub authentication_type: Option, + pub currency: Option, + pub country: Option, + pub card_network: Option, + pub card_bin: Option, +} + +impl SuccessBasedRoutingConfigParamsInterpolator { + pub fn new( + payment_method: Option, + payment_method_type: Option, + authentication_type: Option, + currency: Option, + country: Option, + card_network: Option, + card_bin: Option, + ) -> Self { + Self { + payment_method, + payment_method_type, + authentication_type, + currency, + country, + card_network, + card_bin, + } + } + + pub fn get_string_val( + &self, + params: &Vec, + ) -> String { + let mut parts: Vec = Vec::new(); + for param in params { + let val = match param { + routing_types::SuccessBasedRoutingConfigParams::PaymentMethod => self + .payment_method + .as_ref() + .map_or(String::new(), |pm| pm.to_string()), + routing_types::SuccessBasedRoutingConfigParams::PaymentMethodType => self + .payment_method_type + .as_ref() + .map_or(String::new(), |pmt| pmt.to_string()), + routing_types::SuccessBasedRoutingConfigParams::AuthenticationType => self + .authentication_type + .as_ref() + .map_or(String::new(), |at| at.to_string()), + routing_types::SuccessBasedRoutingConfigParams::Currency => self + .currency + .as_ref() + .map_or(String::new(), |cur| cur.to_string()), + routing_types::SuccessBasedRoutingConfigParams::Country => self + .country + .as_ref() + .map_or(String::new(), |cn| cn.to_string()), + routing_types::SuccessBasedRoutingConfigParams::CardNetwork => { + self.card_network.clone().unwrap_or_default() + } + routing_types::SuccessBasedRoutingConfigParams::CardBin => { + self.card_bin.clone().unwrap_or_default() + } + }; + if !val.is_empty() { + parts.push(val); + } + } + parts.join(":") + } +}