Skip to content

add analytic events for lambda callbacks#4561

Merged
mitchellhenke merged 1 commit intomasterfrom
mitchellhenke/async-lambda-result-analytics
Jan 5, 2021
Merged

add analytic events for lambda callbacks#4561
mitchellhenke merged 1 commit intomasterfrom
mitchellhenke/async-lambda-result-analytics

Conversation

@mitchellhenke
Copy link
Contributor

further debugging for smoke tests

Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

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

LGTM

if dcs
analytics.track_event(
Analytics::LAMBDA_RESULT_ADDRESS_PROOF_RESULT,
result: address_result_parameter,
Copy link
Contributor

Choose a reason for hiding this comment

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

are we trying to log the whole result? or just the result ID?

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to guard this on env so we don't create too many extra log events?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think these could good to have even beyond smoke test issues. I don't think they'd be too numerous since they'd be limited to proofing

We do log full responses, at least for errors, during phone verification from what I can see in cloudwatch. There isn't any PII in the lambda callbacks with the exception of doc auth, which should be explicitly excluding it.

Copy link
Contributor

Choose a reason for hiding this comment

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

makes sense!

@mitchellhenke mitchellhenke merged commit faf0b6e into master Jan 5, 2021
@mitchellhenke mitchellhenke deleted the mitchellhenke/async-lambda-result-analytics branch January 5, 2021 18:45
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.

2 participants