Proof of Concept: Document analytics events via YARD (LG-5590) by zachmargolis · Pull Request #6014 · 18F/identity-idp

zachmargolis · 2022-03-02T01:26:31Z

Adds AnalyticsEvents module where we can put explicitly typed events
Example script that uses YARD to parse these and output JSON
that we could use to power more accessible documentation

Previous work: manual documentation https://github.com/18F/identity-analytics-etl/blob/b42d5ca75bce809e00dcd1aa1bbbedf96339aca7/docs/Managing-analytics-events-in-IDP.md#user-registration-agency-handoff-complete

Next Steps:

Decide if we think this amount of extra typing is acceptable
Decide how we want to display this
- My proposal: run the JSON output script during asset compilation and output the file, then we can either serve it from public, or S3
- Update the LG handbook to have a page that pulls this JSON and renders it (similar to what we did for https://login.gov/help/manage-your-account/international-phone-support/)

Example JSON output:

> ./scripts/document-analytics-events       
{
  "events": [
    {
      "event_name": "Account Reset",
      "description": "Tracks events related to a user requesting to delete their account during the sign in process\n(because they have no other means to sign in).",
      "attributes": [
        {
          "name": "success",
          "types": [
            "Boolean"
          ],
          "description": null
        },
        {
          "name": "event",
          "types": [
            "\"cancel\"",
            "\"delete\"",
            "\"cancel token validation\"",
            "\"granted token validation\"",
            ":notifications"
          ],
          "description": null
        },
        {
          "name": "message_id",
          "types": [
            "String"
          ],
          "description": "Request ID from AWS Pinpoint API"
        },
...

- Adds AnalyticsEvents module where we can put explicitly typed events - Example script that uses YARD to parse these and output JSON that we could use to power more accessible documentation

zachmargolis · 2022-03-02T01:28:09Z

app/services/analytics.rb

  end

  # rubocop:disable Layout/LineLength
-  ACCOUNT_RESET = 'Account Reset'


I picked this one because it was first on the list. It turned out to be super verbose to document because it is essentially 5 different events (it has its own event key 🤦 )

I will take some time and try migrating other events to see how much typing they result in

Maybe this is a good thing as it uncovers a smell where this should have separate event names for those events?

Created a ticket to track splitting it up: https://cm-jira.usa.gov/browse/LG-5910

Does splitting it up mean splitting up each event (like this one would be for Account Reset) into a separate document, or does this happen currently? I think with the number of events we have, the documentation can get rather large.

Not 100% sure what you're asking? The goal is to have a lot of documentation, so we can better explain what the events in our logs mean, so when we query events, we query them confidently.

But yes, this analytics_events.rb file would be expected to get pretty big

So the file analytics_events.rb will have all of the events going forward not just account reset such as all the events in analytics.rb?

Yeah next steps would be:

communicating to the rest of the team that new events should go to analytics_events.rb

Writing tickets to help migrate all the old events

zachmargolis · 2022-03-02T01:29:58Z

app/services/analytics_events.rb

+    track_event(
+      'Account Reset',
+      {
+        success: success,


One of the cons of this approach is each key needs to be written 3 times in this file (YARD, param, and then when it's passed over to track_event), which is very error-prone

Thanks to #6012, we can be sure that if we forget to pass a param, we will error because it's unused

zachmargolis · 2022-03-02T01:31:58Z

app/services/analytics_events.rb

@@ -0,0 +1,55 @@
+module AnalyticsEvents
+  # @identity.idp.event_name Account Reset


👀 custom tag! we can make a form of the script that errors when this is absent as a lint, maybe even try to make sure that the string is present in the method body

changelog: Internal, Documentation, Add documentation for analytics events in events.log

…wlist

zachmargolis · 2022-03-03T02:30:37Z

lib/analytics_events_documenter.rb

+      {
+        event_name: method_object.tag(EVENT_NAME_TAG)&.text,
+        description: method_object.docstring.presence,
+        attributes: attributes,
+      }


💡 One idea is if we added line number + filename + commit SHA, the docs could link to the source in GitHub

I gave this a shot, I don't think it added much to the docs, but we can always revisit and add it later if needed

…ents

…e wraps it

zachmargolis · 2022-03-04T20:01:33Z

app/services/analytics_events.rb

+
+  # @identity.idp.event_name Account Delete visited
+  # When a user visits the page to delete their account
+  def account_delete_visited


Turns out there are a lot of these events with no params, which would make for a super easy migration if we wanted

git grep -E "analytics.track_event\(Analytics::[A-Z_]+\)"

Full list of events without params git grep -E "analytics.track_event\(Analytics::[A-Z_]+\)" | cut -d'(' -f 2 | cut -d')' -f 1 | sort | uniq

Analytics::ACCOUNT_VISIT Analytics::AUTHENTICATION_CONFIRMATION Analytics::AUTHENTICATION_CONFIRMATION_CONTINUE Analytics::AUTHENTICATION_CONFIRMATION_RESET Analytics::BACKUP_CODE_CREATED Analytics::BANNED_USER_REDIRECT Analytics::BANNED_USER_VISITED Analytics::BROKEN_PERSONAL_KEY_REGENERATED Analytics::EMAIL_LANGUAGE_VISITED Analytics::EVENTS_VISIT Analytics::FORGET_ALL_BROWSERS_SUBMITTED Analytics::FORGET_ALL_BROWSERS_VISITED Analytics::IDV_ADDRESS_VISIT Analytics::IDV_COME_BACK_LATER_VISIT Analytics::IDV_FORGOT_PASSWORD Analytics::IDV_FORGOT_PASSWORD_CONFIRMED Analytics::IDV_GPO_ADDRESS_LETTER_REQUESTED Analytics::IDV_GPO_VERIFICATION_VISITED Analytics::IDV_INTRO_VISIT Analytics::IDV_PERSONAL_KEY_SUBMITTED Analytics::IDV_PERSONAL_KEY_VISITED Analytics::IDV_PHONE_CONFIRMATION_OTP_RATE_LIMIT_ATTEMPTS Analytics::IDV_PHONE_CONFIRMATION_OTP_RATE_LIMIT_LOCKED_OUT Analytics::IDV_PHONE_CONFIRMATION_OTP_RATE_LIMIT_SENDS Analytics::IDV_PHONE_CONFIRMATION_OTP_VISIT Analytics::IDV_PHONE_OTP_DELIVERY_SELECTION_VISIT Analytics::IDV_PHONE_RECORD_VISIT Analytics::IDV_REVIEW_COMPLETE Analytics::IDV_REVIEW_VISIT Analytics::MULTI_FACTOR_AUTH_ENTER_TOTP_VISIT Analytics::MULTI_FACTOR_AUTH_MAX_ATTEMPTS Analytics::MULTI_FACTOR_AUTH_MAX_SENDS Analytics::MULTI_FACTOR_AUTH_OPTION_LIST_VISIT Analytics::PASSWORD_MAX_ATTEMPTS Analytics::PASSWORD_RESET_VISIT Analytics::PENDING_ACCOUNT_RESET_CANCELLED Analytics::PENDING_ACCOUNT_RESET_VISITED Analytics::PERSONAL_KEY_REACTIVATION Analytics::PERSONAL_KEY_REACTIVATION_SIGN_IN Analytics::PERSONAL_KEY_REACTIVATION_VISITED Analytics::PHONE_CHANGE_VIEWED Analytics::PROFILE_PERSONAL_KEY_CREATE Analytics::PROFILE_PERSONAL_KEY_VISIT Analytics::PROOFING_ADDRESS_RESULT_MISSING Analytics::PROOFING_DOCUMENT_RESULT_MISSING Analytics::PROOFING_RESOLUTION_RESULT_MISSING Analytics::RULES_OF_USE_VISIT Analytics::SESSION_KEPT_ALIVE Analytics::SESSION_TIMED_OUT Analytics::SESSION_TOTAL_DURATION_TIMEOUT Analytics::SP_HANDOFF_BOUNCED_DETECTED Analytics::SP_HANDOFF_BOUNCED_VISIT Analytics::SP_INACTIVE_VISIT Analytics::TOTP_USER_DISABLED Analytics::USER_REGISTRATION_ENTER_EMAIL_VISIT Analytics::USER_REGISTRATION_PHONE_SETUP_VISIT Analytics::USER_REGISTRATION_PIV_CAC_DISABLED Analytics::USER_REGISTRATION_PIV_CAC_SETUP_VISIT

theabrad

LGTM

gsa-manish

lgtm

Proof of Concept: Document analytics events via YARD (LG-5590)

77c4e9c

- Adds AnalyticsEvents module where we can put explicitly typed events - Example script that uses YARD to parse these and output JSON that we could use to power more accessible documentation

zachmargolis commented Mar 2, 2022

View reviewed changes

zachmargolis added 2 commits March 1, 2022 17:48

optional

78a13b0

Migrate ACCOUNT_DELETE_SUBMITTED

1085051

zachmargolis requested review from gsa-manish, stevegsa and theabrad March 2, 2022 17:49

zachmargolis force-pushed the margolis-analytics-events branch from 2bd4744 to 1085051 Compare March 2, 2022 18:05

zachmargolis added 7 commits March 2, 2022 13:16

Use Makefile to generate the JSON

cbe37c2

Migrate ACCOUNT_DELETE_VISITED

dd75757

fixup ACCOUNT_DELETE_SUBMITTED

a28fc64

Makefile help line

0b54508

Add separate lint and output commands, factor into a class

2d4f2c6

rubocops

b9b5740

Update analytics lint command and fix analytics lint errors

778eeaa

zachmargolis marked this pull request as ready for review March 2, 2022 23:23

zachmargolis added 4 commits March 2, 2022 16:01

Bundle exec stuff

5cfb746

Add changelog

75dd072

changelog: Internal, Documentation, Add documentation for analytics events in events.log

Add additional code coverage

d562814

Switch to dasherized /api/analytics-events endpoint, add to CORS allo…

cdeca15

…wlist

zachmargolis commented Mar 3, 2022

View reviewed changes

zachmargolis mentioned this pull request Mar 3, 2022

Add article that renders IDP analytics events from JSON (LG-5590) GSA-TTS/identity-handbook#218

Merged

zachmargolis added 6 commits March 3, 2022 08:15

Merge remote-tracking branch 'origin/main' into margolis-analytics-ev…

7cb3951

…ents

gitignore

13da56d

eslint ignore doc dir

531b95d

build analytics_events as part of artifact compilation

24ba2bc

Move JSON building to build-post-config

1c8a239

Serve the analytics events data from Rails so that the CORS middlewar…

7377ed0

…e wraps it

zachmargolis added 3 commits March 3, 2022 18:03

Clean up documentation

e872498

Clean up documentation

fb3a914

Migrate ACCOUNT_DELETION event

698e2c7

zachmargolis commented Mar 4, 2022

View reviewed changes

theabrad approved these changes Mar 4, 2022

View reviewed changes

gsa-manish approved these changes Mar 4, 2022

View reviewed changes

zachmargolis merged commit 51ff7b0 into main Mar 4, 2022

zachmargolis deleted the margolis-analytics-events branch March 4, 2022 22:18

mitchellhenke mentioned this pull request Mar 11, 2022

Fix 500 during account reset #6050

Merged

gsa-manish mentioned this pull request Mar 17, 2022

Document analytics events part 5 #6084

Merged

aduth mentioned this pull request May 3, 2022

LG-6201: Add personal confirm step submitted as frontend unprefixed event #6291

Merged

		@@ -0,0 +1,55 @@
		module AnalyticsEvents
		# @identity.idp.event_name Account Reset

Conversation

zachmargolis commented Mar 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zachmargolis Mar 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

theabrad left a comment

Choose a reason for hiding this comment

Uh oh!

gsa-manish left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

zachmargolis commented Mar 2, 2022 •

edited

Loading

zachmargolis Mar 2, 2022 •

edited

Loading