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

feat(events): add type info to outgoing requests & maintain structural & PII type info #2956

Merged
merged 28 commits into from
Dec 13, 2023

Conversation

lsampras
Copy link
Member

@lsampras lsampras commented Nov 22, 2023

Type of Change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring
  • Dependency updates
  • Documentation
  • CI/CD

Description

  • Currently we rely on the send_request function and the Request object to send outgoing api calls
  • This internally relies on assuming that the caller has passed a correct content type & correctly serialized the json payload
  • Instead we now accept a generic request that implements serialize + maskedSerialize s.t we can use it to send requests + log the masked bodies for such requests

Additional Changes

  • This PR modifies the API contract
  • This PR modifies the database schema
  • This PR modifies application configuration/environment variables

Motivation and Context

  • This allows us to be more robust about our request body types and avoid mismatch's
  • The masking capability also allows us to now log the raw request as needed

Future enhancements

  • we can also require the bodies to implement debug fmt for quick logging
  • we can also extract metadata from the request bodies such as payment_id/refund_id/flow type via common traits

How did you test it?

  • make payment calls with confirm=true
  • the underlying connector call would be logged with the following info
image

Checklist

  • I formatted the code cargo +nightly fmt --all
  • I addressed lints thrown by cargo clippy
  • I reviewed the submitted code
  • I added unit tests for my changes where possible
  • I added a CHANGELOG entry if applicable

@lsampras lsampras added A-framework Area: Framework C-feature Category: Feature request or enhancement labels Nov 22, 2023
@lsampras lsampras marked this pull request as ready for review December 2, 2023 22:09
@lsampras lsampras requested review from a team as code owners December 2, 2023 22:09
@lsampras lsampras requested review from a team as code owners December 8, 2023 08:39
crates/common_utils/src/request.rs Outdated Show resolved Hide resolved
crates/router/src/events/connector_api_logs.rs Outdated Show resolved Hide resolved
crates/router/src/services/api.rs Outdated Show resolved Hide resolved
config/docker_compose.toml Show resolved Hide resolved
crates/router/src/connector/cybersource.rs Outdated Show resolved Hide resolved
crates/router/src/connector/cybersource.rs Outdated Show resolved Hide resolved
crates/router/src/connector/powertranz.rs Show resolved Hide resolved
crates/pm_auth/src/types/api.rs Outdated Show resolved Hide resolved
crates/router/src/services/api.rs Outdated Show resolved Hide resolved
crates/router/src/services/api.rs Outdated Show resolved Hide resolved
crates/router/src/services/api.rs Outdated Show resolved Hide resolved
@ArjunKarthik ArjunKarthik added the S-waiting-on-author Status: This PR is incomplete or needs to address review comments label Dec 12, 2023
ArjunKarthik
ArjunKarthik previously approved these changes Dec 12, 2023
NishantJoshi00
NishantJoshi00 previously approved these changes Dec 12, 2023
ThisIsMani
ThisIsMani previously approved these changes Dec 12, 2023
Copy link
Contributor

@ThisIsMani ThisIsMani left a comment

Choose a reason for hiding this comment

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

Reviewed dashboard specific changes.

@lsampras lsampras removed the S-waiting-on-author Status: This PR is incomplete or needs to address review comments label Dec 12, 2023
@Gnanasundari24 Gnanasundari24 added this pull request to the merge queue Dec 12, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 12, 2023
@lsampras lsampras requested a review from a team as a code owner December 12, 2023 14:32
@lsampras lsampras added this pull request to the merge queue Dec 13, 2023
Merged via the queue into main with commit 6e82b0b Dec 13, 2023
10 of 12 checks passed
@lsampras lsampras deleted the connector_events_v3 branch December 13, 2023 06:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-framework Area: Framework C-feature Category: Feature request or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants