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

Add attestation event logging to FC API clients #4481

Merged
merged 8 commits into from
Jan 27, 2025

Conversation

mats-stripe
Copy link
Collaborator

@mats-stripe mats-stripe commented Jan 20, 2025

Summary

Adds logging for various attestation-related events in our Financial Connections API clients (both legacy and async clients). Most of the event logging logic has been consolidated in FinancialConnectionsAPIClientLogger. As part of this, I also had to make a few secondary changes to the API clients;

  1. Only apply attestation-related parameters in the initial /synchronize call.
  2. Pass the corresponding pane and api through the consumer session lookup and sign up APIs for logging purposes.

Motivation

https://docs.google.com/document/d/1joKz5UZHLVazmecfMHbq6gB6n4wj5u8To6AtqYgq_tc/edit?tab=t.0#bookmark=kix.oi6g969wwrz0

Testing

N/a

Changelog

N/a

@mats-stripe mats-stripe requested review from a team as code owners January 20, 2025 16:46
@mats-stripe mats-stripe force-pushed the mats/attestion_event_logging branch from d8d06bc to 2193645 Compare January 20, 2025 17:15
}
}

var parameters: [String: Any] {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@carlosmuvi-stripe let me know which other parameters we should include for each event

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should be good with these + the default parameters sent on all requests

@mats-stripe mats-stripe force-pushed the mats/fc_mobile_lookup_endpoint branch from ac88f3b to dba58e6 Compare January 20, 2025 18:27
@mats-stripe mats-stripe force-pushed the mats/attestion_event_logging branch from 2193645 to 187a2b2 Compare January 20, 2025 18:29
@@ -174,6 +174,7 @@ extension FinancialConnectionsAnalyticsClient {
additionalParameters["single_account"] = manifest.singleAccount
additionalParameters["allow_manual_entry"] = manifest.allowManualEntry
additionalParameters["account_holder_id"] = manifest.accountholderToken
additionalParameters["app_verification_enabled"] = manifest.appVerificationEnabled
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Includes manifest.appVerificationEnabled as a common field, passed along to most logs we emit.

@mats-stripe mats-stripe force-pushed the mats/fc_mobile_lookup_endpoint branch from 45040b1 to 89bc90c Compare January 22, 2025 14:35
@mats-stripe mats-stripe force-pushed the mats/attestion_event_logging branch from 6a46135 to 27b77d0 Compare January 22, 2025 15:16
Base automatically changed from mats/fc_mobile_lookup_endpoint to master January 22, 2025 15:27
@mats-stripe mats-stripe force-pushed the mats/attestion_event_logging branch from 27b77d0 to 4801101 Compare January 22, 2025 15:43
@mats-stripe mats-stripe force-pushed the mats/attestion_event_logging branch from 4801101 to 5f5d685 Compare January 23, 2025 20:25
@mats-stripe mats-stripe merged commit 1582825 into master Jan 27, 2025
6 checks passed
@mats-stripe mats-stripe deleted the mats/attestion_event_logging branch January 27, 2025 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants