Add analytics event for Twilio errors #1824
Conversation
**Why**: So we can see how often these errors occur and if they're specific to a particular country or area code.
**Why**: To help troubleshoot errors. It's helpful to know if the user successfully went to the account page after 2FA if they signed in directly. Conversely, if they ended up on the account page instead of going back to the SP, that's helpful to know as well.
|
I found some more events we want to track, so I figured I'd add them as additional commits in this PR. Let me know if you prefer me to open a separate PR. |
**Why**: To get metrics on SP vs IdP initiated logouts, and to get a more complete picture of user events when troubleshooting.
monfresh
requested review from
nickbristow,
annalee,
jmhooper,
mugizico and
tbaxter-18f
| @@ -5,7 +5,7 @@ def index | |||
|
|
|||
| result = @logout_form.submit | |||
|
|
|||
| analytics.track_event(Analytics::OPENID_CONNECT_LOGOUT, result.to_h.except(:redirect_uri)) | |||
| analytics.track_event(Analytics::LOGOUT_INITIATED, result.to_h.except(:redirect_uri)) | |||
There was a problem hiding this comment.
Do we want to keep track of whether the logout is OIDC or SAML with an event? I.e. instead of creating a LOGOUT_INITIATED event, creating at SAML_LOGOUT event?
There was a problem hiding this comment.
I think I like that idea, too.
There was a problem hiding this comment.
One event makes it easier to compare OIDC vs SAML. In Kibana, you would query a single event name, and then you can filter on sp_initiated: true vs sp_initiated: false. And then to determine the split between OIDC and SAML, you would further filter on oidc: true vs oidc: false.
There was a problem hiding this comment.
It's really just one logout event, but with different properties each time. This is similar to how we track multi-factor authentication. We use a single event, but the method of authentication is captured as an event property, and it can be SMS, Voice, TOTP, or personal key.
Why: So we can see how often these errors occur and if they're
specific to a particular country or area code.
Hi! Before submitting your PR for review, and/or before merging it, please
go through the following checklist:
For DB changes, check for missing indexes, check to see if the changes
affect other apps (such as the dashboard), make sure the DB columns in the
various environments are properly populated, coordinate with devops, plan
migrations in separate steps.
For route changes, make sure GET requests don't change state or result in
destructive behavior. GET requests should only result in information being
read, not written.
For encryption changes, make sure it is compatible with data that was
encrypted with the old code.
Do not disable Rubocop or Reek offenses unless you are absolutely sure
they are false positives. If you're not sure how to fix the offense, please
ask a teammate.
When reading data, write tests for nil values, empty strings,
and invalid formats.
When calling
redirect_toin a controller, use_url, not_path.When adding user data to the session, use the
user_sessionhelperinstead of the
sessionhelper so the data does not persist beyond the user'ssession.