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

Move failed account takeover logging to redshift, include error #24817

Merged
merged 3 commits into from Sep 17, 2018

Conversation

maddiedierker
Copy link
Contributor

@maddiedierker maddiedierker commented Sep 14, 2018

Builds off of Redshift logging in #24720.

Currently, when an account takeover fails due to an attempt to takeover an account with progress, we log the failure to Honeybadger (example). We originally thought that this wasn't ever really happening, but realized we were wrong. This meant lots of Honeybadger notifications for this use case.

Instead, we should be logging this to Redshift, leveraging the current logging in place. This adds a new event to Redshift:

Study: user-soft-delete-audit
Events: cancelled-silent-account-takeover / cancelled-oauth-account-takeover

  • user_id: user ID of account being taken over
  • data_int: user ID of the account taking over user_id's account
  • data_string: OAuth provider involved in the account takeover
  • data_json:
    • user_type
    • error (currently always "Attempted takeover for account with progress.")

@maddiedierker
Copy link
Contributor Author

Copy link
Contributor

@islemaster islemaster left a comment

Choose a reason for hiding this comment

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

👍 Looks good. It'll be interesting to see if we can eliminate this case with later refactoring!

Copy link
Contributor

@ewjordan ewjordan left a comment

Choose a reason for hiding this comment

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

LGTM, once the CircleCI failures are resolved - looks like there are some related ones, where we used to test for events going to Honeybadger. Also some PD-related failures, which I doubt are related.

@maddiedierker maddiedierker merged commit c0173ca into staging Sep 17, 2018
@maddiedierker maddiedierker deleted the move-hb-logging-to-redshift branch September 17, 2018 18:54
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.

None yet

3 participants