feat(observability): add typed connector request/response fields - #2036
feat(observability): add typed connector request/response fields#2036AmitsinghTanwar007 wants to merge 12 commits into
Conversation
| } | ||
| } | ||
|
|
||
| pub fn generate_mandate_revoke_response( |
There was a problem hiding this comment.
why was these functions added in this file instead of type.rs
not related to this pr though, but how did we missed
There was a problem hiding this comment.
Don't know but i think in future we need to write clippy rules so that the structure is maintained
| [server] | ||
| host = "0.0.0.0" | ||
| port = 8000 | ||
| port = 8003 |
There was a problem hiding this comment.
revert these changes.
| fn get_typed_connector_request( | ||
| &self, | ||
| req: &RouterDataV2<$flow, $resource_common_data, $request, $response>, | ||
| ) -> Option<String> | ||
| { | ||
| let bridge = self.[< $flow:snake >]; | ||
| let input_data = [< $connector RouterData >] { | ||
| connector: self.to_owned(), | ||
| router_data: req.clone() | ||
| }; | ||
| bridge | ||
| .request_body(input_data) | ||
| .ok() | ||
| .and_then(|request| crate::connectors::macros::serialize_typed_connector_payload( | ||
| &request, | ||
| "typed_connector_request", | ||
| )) | ||
| } |
There was a problem hiding this comment.
[S1] The typed request is reconstructed, not captured — it can differ from what was actually sent.
This re-invokes bridge.request_body(req.clone()) a second time, independently of get_request_body. For connectors with per-call non-determinism the two diverge:
airwallex/transformers.rs:1264mints a freshuuid::Uuid::new_v4()per call, with a comment stating reuse is rejected asduplicate_request.nuvei/transformers.rs:82-86derives a checksum fromdate_time::now().
The observability record will show a request_id/timestamp/signature that was never on the wire — misleading for exactly the debugging use case this PR exists for.
It also doubles request-transformation cost, including a full RouterDataV2 clone, on the payment hot path.
Capture the typed value once inside get_request_body and thread it through instead.
| .masked_serialize_inner() | ||
| .map(|(v, _)| v) | ||
| .unwrap_or_else(|| match request { | ||
| RequestContent::FormData(_) => json!({"request_type": "FORM_DATA"}), |
There was a problem hiding this comment.
for FormData and RawByte, can we still pass the another parameter of typed struct similar to other types
There was a problem hiding this comment.
I will handle this change in later PR as this may require change in L3 layer which i need to go through,
already the pr has many changes.
| } | ||
|
|
||
| let error_response = match body.status_code { | ||
| let (error_response, typed_response) = match body.status_code { |
There was a problem hiding this comment.
for 4xx or 5xx, since we added the new field inside ConnectorError proto, so should adding the typed_response field fiedl inside ErrorResponse struct be sufficient?
Here we setting inside resource_common_data, and taking back to set ErrorResponse
|
|
||
| let response_body = bridge.response(response_bytes, res.status_code)?; | ||
| event_builder.map(|i| i.set_connector_response(&response_body)); | ||
| // Serialize once: masked Value for event logging, String for typed_connector_response |
There was a problem hiding this comment.
bro, actually for event also we need string type only., we are doing stringy at downstream and that dont handle all cases like array, float etc
So we can actually send string only to events as well, so no need of this divergence
(Both request and response)
| /// logging via `set_connector_response`) and the stringified form (for | ||
| /// `typed_connector_response`). This avoids calling `masked_serialize` twice. | ||
| pub(crate) fn masked_serialize_connector_response<T: serde::Serialize>( | ||
| payload: &T, |
There was a problem hiding this comment.
this fucntion is similar to from_masked_optional that we have as impl for MaskedSerdeValue type?
| crate::connectors::macros::masked_serialize_connector_response(&response_body); | ||
| if let Some(evt) = event_builder { | ||
| if let Some((ref value, _)) = serialized { | ||
| evt.set_connector_response(value); |
There was a problem hiding this comment.
also this set_connector_response, already does MaskedSerdeValue::from_masked_optional
| /// | ||
| /// Replaces the 12-line boilerplate block in every manual `handle_response_v2`. | ||
| #[macro_export] | ||
| macro_rules! set_typed_response { |
There was a problem hiding this comment.
name is set_typed_response
But we do more than that
we can name better
| Ok(()) | ||
| } | ||
|
|
||
| pub(crate) fn serialize_typed_connector_payload<T: serde::Serialize>( |
There was a problem hiding this comment.
it is similar to from_masked_optional that we have.
It comes out of box if we use typed MaskedSerdeValue instead of serde::json, so mistaken unmasked value also not possible
|
|
||
| match self { | ||
| Self::ConnectorErrorResponse(error_response) => match error_response.status_code { | ||
| Self::ConnectorErrorResponse { error_response, .. } => match error_response.status_code |
There was a problem hiding this comment.
having raw_connector_response field inside error_response struct would be cleaner?
…serialization - Add typed_connector_response field directly to ErrorResponse struct - Serialize typed connector payload inside build_error_response, eliminating double-parsing on the error path - Remove get_typed_connector_error_response from ConnectorCommon trait - Remove resource_common_data round-trip for typed error responses in service.rs - Add typed_connector_request/response fields to proto ConnectorError message - Update all ~90 connector build_error_response impls and ~77 transformer ErrorResponse initializers
f7bcea5 to
f929177
Compare
…dSerdeValue for typed payload serialization
…uest into ErrorResponse Simplify ConnectorErrorResponse from struct variant with 5 fields to a single-field tuple variant ConnectorErrorResponse(Box<ErrorResponse>). All raw/typed connector fields now live inside ErrorResponse, matching the typed_connector_response pattern from the previous commit.
…th request content get_request_body now returns ConnectorRequestData which pairs RequestContent with an Option<MaskedSerdeValue> typed payload. This ensures the typed connector request is serialized before conversion to FormData/RawBytes (which would otherwise lose the typed information), and propagates it through all build_request_v2 paths including manual overrides.
Auto-applied by CI: - cargo +nightly fmt --all - make -C sdk generate (if applicable) - make docs (if applicable) This commit was automatically generated by GitHub Actions.
No longer needed — ConnectorRequestData guarantees typed request serialization at the macro level, and response/error typed serialization is handled by the macro-generated code.
Auto-applied by CI: - cargo +nightly fmt --all - make -C sdk generate (if applicable) - make docs (if applicable) This commit was automatically generated by GitHub Actions.
28ba1d5 to
76f7169
Compare
| }) | ||
| .map_err(|e| { | ||
| e.change_context($crate::ConnectorError::ResponseHandlingFailed { | ||
| context: Default::default(), |
There was a problem hiding this comment.
can we update default context
There was a problem hiding this comment.
also why map_Err? already RouterDataV2::try_from has Report ConnectorError?
Summary
Adds
typed_connector_requestandtyped_connector_responsefields across the entire connector service stack to provide structured, domain-typed observability data separate from raw wire-format payloads.Problem
Currently, only
raw_connector_request/raw_connector_responseexist — these carry the literal HTTP bytes sent/received. For debugging and analytics, we also need the typed representation (the Rust domain struct, masked and serialized) so consumers can reason about connector data in a connector-agnostic way without parsing raw JSON/XML from different providers.What changed
Proto definitions (
payment.proto,frm.proto):typed_connector_requestandtyped_connector_responseoptional fields to all response messages (Authorize, Get, Void, Capture, Refund, CreateOrder, SetupRecurring, MandateRevoke, FRM pre/post risk check, etc.)Domain types (
connector_types.rs,frm_types.rs,surcharge_types.rs,payouts_types.rs,merchant_authentication_flow_data.rs):typed_connector_request/typed_connector_responsefields to all flow data structs (PaymentFlowData,RefundFlowData,DisputeFlowData,FrmFlowData,SurchargeFlowData,VerifyWebhookSourceFlowData,RefreshPaymentMethodFlowData,PayoutFlowData)RawConnectorRequestResponsetrait withget/set_typed_connector_requestandget/set_typed_connector_responsemethodsConnector integration trait (
connector_integration_v2.rs):get_typed_connector_request()default method returningNonebuild_request_v2()viaset_typed_connector_request()on the request builderRequest infrastructure (
request.rs):typed_connector_requestfield toRequestandRequestBuilderset_typed_connector_request()builder methodConnector macros (
macros.rs):serialize_typed_connector_payload()utility — masked serialization of typed structsexpand_fn_get_typed_connector_request!macro — generatesget_typed_connector_request()for each flowexpand_fn_handle_response!to capture and settyped_connector_responseon the flow datamacro_connector_implementation!variantsConnector-specific (
bamboraapac/transformers.rs):Serializederive to request structs that were missing it (required for typed serialization)Service layer (
service.rs):typed_connector_requeston flow data from the request object after connector callgRPC response generators (
types.rs,frm/types.rs,payments.rs):generate_*_response()functions now extract and pass typed fields to proto responsesServer layer (
events.rs):typed_connector_request/response: NonetoVerifyWebhookSourceFlowDataconstructionKey design principle: raw ≠ typed
Test plan
cargo checkpasses across all cratesNoneby default for connectors not using the macro system