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

refactor(dynamic_routing): add info logs to log the grpc request and response #6962

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
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
10 changes: 10 additions & 0 deletions crates/external_services/src/grpc_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,3 +128,13 @@ impl<T> AddHeaders for tonic::Request<T> {
});
}
}

#[cfg(feature = "dynamic_routing")]
pub(crate) fn create_grpc_request<T: Debug>(message: T, headers: GrpcHeaders) -> tonic::Request<T> {
let mut request = tonic::Request::new(message);
request.add_headers_to_grpc_request(headers);
Comment on lines +132 to +135
Copy link
Member

Choose a reason for hiding this comment

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

How do the error logs thrown by add_headers_to_grpc_request() look like? Do they have the file and line number information that helps identify where exactly it was called, or should we consider adding #[track_caller] to create_grpc_request() as well?


logger::info!(dynamic_routing_request=?request);

request
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ pub use elimination_rate::{
LabelWithBucketName, UpdateEliminationBucketRequest, UpdateEliminationBucketResponse,
};
use error_stack::ResultExt;
use router_env::{instrument, logger, tracing};
#[allow(
missing_docs,
unused_qualifications,
Expand All @@ -21,7 +22,7 @@ pub mod elimination_rate {
}

use super::{Client, DynamicRoutingError, DynamicRoutingResult};
use crate::grpc_client::{AddHeaders, GrpcHeaders};
use crate::grpc_client::{self, GrpcHeaders};

/// The trait Elimination Based Routing would have the functions required to support performance, calculation and invalidation bucket
#[async_trait::async_trait]
Expand Down Expand Up @@ -54,6 +55,7 @@ pub trait EliminationBasedRouting: dyn_clone::DynClone + Send + Sync {

#[async_trait::async_trait]
impl EliminationBasedRouting for EliminationAnalyserClient<Client> {
#[instrument(skip_all)]
async fn perform_elimination_routing(
&self,
id: String,
Expand All @@ -69,14 +71,15 @@ impl EliminationBasedRouting for EliminationAnalyserClient<Client> {

let config = configs.map(ForeignTryFrom::foreign_try_from).transpose()?;

let mut request = tonic::Request::new(EliminationRequest {
id,
params,
labels,
config,
});

request.add_headers_to_grpc_request(headers);
let request = grpc_client::create_grpc_request(
EliminationRequest {
id,
params,
labels,
config,
},
headers,
);

let response = self
.clone()
Expand All @@ -87,9 +90,12 @@ impl EliminationBasedRouting for EliminationAnalyserClient<Client> {
))?
.into_inner();

logger::info!(dynamic_routing_response=?response);

Ok(response)
}

#[instrument(skip_all)]
async fn update_elimination_bucket_config(
&self,
id: String,
Expand All @@ -110,14 +116,15 @@ impl EliminationBasedRouting for EliminationAnalyserClient<Client> {
})
.collect::<Vec<_>>();

let mut request = tonic::Request::new(UpdateEliminationBucketRequest {
id,
params,
labels_with_bucket_name,
config,
});

request.add_headers_to_grpc_request(headers);
let request = grpc_client::create_grpc_request(
UpdateEliminationBucketRequest {
id,
params,
labels_with_bucket_name,
config,
},
headers,
);

let response = self
.clone()
Expand All @@ -127,16 +134,19 @@ impl EliminationBasedRouting for EliminationAnalyserClient<Client> {
"Failed to update the elimination bucket".to_string(),
))?
.into_inner();

logger::info!(dynamic_routing_response=?response);

Ok(response)
}

#[instrument(skip_all)]
async fn invalidate_elimination_bucket(
&self,
id: String,
headers: GrpcHeaders,
) -> DynamicRoutingResult<InvalidateBucketResponse> {
let mut request = tonic::Request::new(InvalidateBucketRequest { id });

request.add_headers_to_grpc_request(headers);
let request = grpc_client::create_grpc_request(InvalidateBucketRequest { id }, headers);

let response = self
.clone()
Expand All @@ -146,6 +156,9 @@ impl EliminationBasedRouting for EliminationAnalyserClient<Client> {
"Failed to invalidate the elimination bucket".to_string(),
))?
.into_inner();

logger::info!(dynamic_routing_response=?response);

Ok(response)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use api_models::routing::{
};
use common_utils::{ext_traits::OptionExt, transformers::ForeignTryFrom};
use error_stack::ResultExt;
use router_env::{instrument, logger, tracing};
pub use success_rate::{
success_rate_calculator_client::SuccessRateCalculatorClient, CalSuccessRateConfig,
CalSuccessRateRequest, CalSuccessRateResponse,
Expand All @@ -15,13 +16,14 @@ pub use success_rate::{
missing_docs,
unused_qualifications,
clippy::unwrap_used,
clippy::as_conversions
clippy::as_conversions,
clippy::use_self
)]
pub mod success_rate {
tonic::include_proto!("success_rate");
}
use super::{Client, DynamicRoutingError, DynamicRoutingResult};
use crate::grpc_client::{AddHeaders, GrpcHeaders};
use crate::grpc_client::{self, GrpcHeaders};
/// The trait Success Based Dynamic Routing would have the functions required to support the calculation and updation window
#[async_trait::async_trait]
pub trait SuccessBasedDynamicRouting: dyn_clone::DynClone + Send + Sync {
Expand Down Expand Up @@ -53,6 +55,7 @@ pub trait SuccessBasedDynamicRouting: dyn_clone::DynClone + Send + Sync {

#[async_trait::async_trait]
impl SuccessBasedDynamicRouting for SuccessRateCalculatorClient<Client> {
#[instrument(skip_all)]
async fn calculate_success_rate(
&self,
id: String,
Expand All @@ -71,14 +74,15 @@ impl SuccessBasedDynamicRouting for SuccessRateCalculatorClient<Client> {
.map(ForeignTryFrom::foreign_try_from)
.transpose()?;

let mut request = tonic::Request::new(CalSuccessRateRequest {
id,
params,
labels,
config,
});

request.add_headers_to_grpc_request(headers);
let request = grpc_client::create_grpc_request(
CalSuccessRateRequest {
id,
params,
labels,
config,
},
headers,
);

let response = self
.clone()
Expand All @@ -89,9 +93,12 @@ impl SuccessBasedDynamicRouting for SuccessRateCalculatorClient<Client> {
))?
.into_inner();

logger::info!(dynamic_routing_response=?response);

Ok(response)
}

#[instrument(skip_all)]
async fn update_success_rate(
&self,
id: String,
Expand All @@ -113,14 +120,15 @@ impl SuccessBasedDynamicRouting for SuccessRateCalculatorClient<Client> {
})
.collect();

let mut request = tonic::Request::new(UpdateSuccessRateWindowRequest {
id,
params,
labels_with_status,
config,
});

request.add_headers_to_grpc_request(headers);
let request = grpc_client::create_grpc_request(
UpdateSuccessRateWindowRequest {
id,
params,
labels_with_status,
config,
},
headers,
);

let response = self
.clone()
Expand All @@ -131,16 +139,18 @@ impl SuccessBasedDynamicRouting for SuccessRateCalculatorClient<Client> {
))?
.into_inner();

logger::info!(dynamic_routing_response=?response);

Ok(response)
}

#[instrument(skip_all)]
async fn invalidate_success_rate_routing_keys(
&self,
id: String,
headers: GrpcHeaders,
) -> DynamicRoutingResult<InvalidateWindowsResponse> {
let mut request = tonic::Request::new(InvalidateWindowsRequest { id });

request.add_headers_to_grpc_request(headers);
let request = grpc_client::create_grpc_request(InvalidateWindowsRequest { id }, headers);

let response = self
.clone()
Expand All @@ -150,6 +160,9 @@ impl SuccessBasedDynamicRouting for SuccessRateCalculatorClient<Client> {
"Failed to invalidate the success rate routing keys".to_string(),
))?
.into_inner();

logger::info!(dynamic_routing_response=?response);

Ok(response)
}
}
Expand Down
3 changes: 3 additions & 0 deletions crates/router/src/core/payments/routing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ use rand::{
distributions::{self, Distribution},
SeedableRng,
};
#[cfg(all(feature = "v1", feature = "dynamic_routing"))]
use router_env::{instrument, tracing};
use rustc_hash::FxHashMap;
use storage_impl::redis::cache::{CacheKey, CGRAPH_CACHE, ROUTING_CACHE};

Expand Down Expand Up @@ -1281,6 +1283,7 @@ pub fn make_dsl_input_for_surcharge(

/// success based dynamic routing
#[cfg(all(feature = "v1", feature = "dynamic_routing"))]
#[instrument(skip_all)]
pub async fn perform_success_based_routing(
state: &SessionState,
routable_connectors: Vec<api_routing::RoutableConnectorChoice>,
Expand Down
12 changes: 10 additions & 2 deletions proto/success_rate.proto
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,11 @@ message CurrentBlockThreshold {
}

message UpdateSuccessRateWindowResponse {
string message = 1;
enum UpdationStatus {
WINDOW_UPDATION_SUCCEEDED = 0;
WINDOW_UPDATION_FAILED = 1;
}
UpdationStatus status = 1;
}

// API-3 types
Expand All @@ -64,5 +68,9 @@ message InvalidateWindowsRequest {
}

message InvalidateWindowsResponse {
string message = 1;
enum InvalidationStatus {
Chethan-rao marked this conversation as resolved.
Show resolved Hide resolved
WINDOW_INVALIDATION_SUCCEEDED = 0;
WINDOW_INVALIDATION_FAILED = 1;
}
InvalidationStatus status = 1;
}
Loading