LG-12657: Make Idv::AnalyticsEventsEnhancer opt-out #10230

matthinz · 2024-03-11T21:51:11Z

🎫 Ticket

Link to the relevant ticket:
LG-12657

🛠 Summary of changes

In working on #10216, I realized that the list of events that Idv::AnalyticsEventsEnhancer enhances has gotten a little stale. Specifically, we haven't been adding new events to it.

This PR:

Updates Idv::AnalyticsEventsEnhancer to "enhance" any method that starts with idv_ (note that this is the method name, not the event name, so old-style "IdV:" and new-style "idv_" events will be covered).
Allows "opting out" of enhancement for specific events.
Adds linter support for verifying "enhanced" method arguments are correctly passed to track_event

Initially, the list of "opted out" methods just includes any frontend events that were not previously "opted in". The net change of this PR is that all idv_* events logged by the backend will now have proofing_components added to them (this will be expanded to additional new properties in subsequent PRs).

📜 Testing Plan

Provide a checklist of steps to confirm the changes.

Go through identity verification
Verify that certain events like IdV: doc auth verify submitted now have proofing_components

app/services/idv/analytics_events_enhancer.rb

app/services/analytics_events.rb

app/services/idv/analytics_events_enhancer.rb

zachmargolis · 2024-03-13T02:28:59Z

lib/linters/enhanced_idv_events_linter.rb

+          kwrest = method.arguments.find { |a| a.kwrestarg_type? }
+          return if !kwrest
+
+          corrector.insert_before(kwrest, "#{arg_name}: nil, ")


omg major props, I don't think we have any other of our lints autocorrect

lib/linters/enhanced_idv_events_linter.rb

matthinz · 2024-03-14T17:58:23Z

app/services/analytics_events.rb

@@ -624,7 +624,7 @@ def idv_acuant_sdk_loaded(
    success:,
    use_alternate_sdk:,
    liveness_checking_required:,
-    **_extra
+    **extra


There were a few methods that accepted **extra but didn't pass it on to track_event. I've updated those to pass data properly.

matthinz · 2024-03-14T18:15:08Z

lib/linters/enhanced_idv_events_linter.rb

+module RuboCop
+  module Cop
+    module IdentityIdp
+      class EnhancedIdvEventsLinter < RuboCop::Cop::Base


Honestly, here be dragons. But analytics_events.rb is long enough that automating large edits to it was worth the trouble.

matthinz · 2024-03-14T23:49:57Z

lib/linters/enhanced_idv_events_linter.rb

+              line.is_a?(Parser::Source::Comment) && /@param/.match?(line.text)
+            end
+
+            comment = "# @param [Object] #{arg_name} TODO: Write doc comment"


I wanted this autocorrect (adding doc comment) in place so I could get all the missing @param declarations in analytics_events.rb but would be happy to remove it if we want potentially avoid ending up with a mess of autogenerated "TODO" comments in the future

(if say, a developer had a git precommit hook that automatically ran rubocop -a and committed the results)

matthinz · 2024-03-14T23:54:26Z

app/services/idv/analytics_events_enhancer.rb

-      idv_please_call_visited
-      idv_start_over
-    ].freeze
+    IGNORED_METHODS = %i[


Right now this list is just any frontend events that were not previously "opted in". Hopefully big changes to this list can come in as separate PRs.

It could be argued that many events before document_capture don't benefit from having proofing_components added automatically. However:

We have much better back button support than we did when this events enhancer was first written, so proofing_components could have data

I'm planning on using this events enhancer to add more properties in the future that would be relevant for those earlier events

Don't include nil values here

Update AnalyticsEventsEnhancer so that it augments any event where the _method name_ starts with `^idv_` UNLESS it has been told not to. changelog: Internal, Analytics, Add additional data to IdV analytics events by default.

…nalyticsEvents Check each method on AnalyticsEvents that is "enhanced" by AnalyticsEventsEnhancer: 1. Includes the enhancements in its args list 2. Passes those to track_event

@param

Look for "@param" documentation for e.g. proofing_components. Autocorrect to a "TODO"

matthinz · 2024-03-18T18:51:34Z

Closing for now -- going to try and get this in as a few smaller PRs.

matthinz requested a review from a team March 11, 2024 21:53

zachmargolis reviewed Mar 11, 2024

View reviewed changes

app/services/idv/analytics_events_enhancer.rb Outdated Show resolved Hide resolved

app/services/idv/analytics_events_enhancer.rb Outdated Show resolved Hide resolved

aduth reviewed Mar 12, 2024

View reviewed changes

app/services/analytics_events.rb Show resolved Hide resolved

zachmargolis reviewed Mar 12, 2024

View reviewed changes

app/services/idv/analytics_events_enhancer.rb Outdated Show resolved Hide resolved

matthinz marked this pull request as draft March 12, 2024 21:45

zachmargolis reviewed Mar 13, 2024

View reviewed changes

matthinz commented Mar 14, 2024

View reviewed changes

matthinz mentioned this pull request Mar 15, 2024

Update ImageUploadsController spec to use have_logged_event #10258

Merged

matthinz added 2 commits March 18, 2024 10:50

compact common_analytics_attributes

055bcdf

Don't include nil values here

Refactor proofing_components into its own describe block

fd4486d

matthinz force-pushed the matthinz/12657-update-analytics-events-enchanter branch 3 times, most recently from 6e0d041 to ea9f2c1 Compare March 18, 2024 18:03

matthinz added 2 commits March 18, 2024 11:03

Add **extra to some analytics events that are missing it

6d25a57

Apply AnalyticsEventsEnhancer to more events

5154552

Update AnalyticsEventsEnhancer so that it augments any event where the _method name_ starts with `^idv_` UNLESS it has been told not to. changelog: Internal, Analytics, Add additional data to IdV analytics events by default.

matthinz force-pushed the matthinz/12657-update-analytics-events-enchanter branch from ea9f2c1 to 22d6978 Compare March 18, 2024 18:11

matthinz added 3 commits March 18, 2024 11:40

Lint that arguments added by AnalyticsEventsEnhancer are present in A…

c533aae

…nalyticsEvents Check each method on AnalyticsEvents that is "enhanced" by AnalyticsEventsEnhancer: 1. Includes the enhancements in its args list 2. Passes those to track_event

Check that "enhanced" event args have @param docs

c853bdf

Look for "@param" documentation for e.g. proofing_components. Autocorrect to a "TODO"

Turn on the EnhancedIdvEventsLinter

88e00c8

matthinz force-pushed the matthinz/12657-update-analytics-events-enchanter branch from 22d6978 to 88e00c8 Compare March 18, 2024 18:40

matthinz closed this Mar 18, 2024

This was referenced Mar 18, 2024

Make Idv::AnalyticsEventsEnhancer opt-out #10263

Merged

Rubocop linter for "enhanced" analytics events #10264

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

LG-12657: Make Idv::AnalyticsEventsEnhancer opt-out #10230

LG-12657: Make Idv::AnalyticsEventsEnhancer opt-out #10230

matthinz commented Mar 11, 2024 •

edited

Loading

zachmargolis Mar 13, 2024

matthinz Mar 14, 2024

matthinz Mar 14, 2024

matthinz Mar 14, 2024

matthinz Mar 14, 2024

matthinz Mar 14, 2024

matthinz commented Mar 18, 2024

LG-12657: Make Idv::AnalyticsEventsEnhancer opt-out #10230

LG-12657: Make Idv::AnalyticsEventsEnhancer opt-out #10230

Conversation

matthinz commented Mar 11, 2024 • edited Loading

🎫 Ticket

🛠 Summary of changes

📜 Testing Plan

zachmargolis Mar 13, 2024

Choose a reason for hiding this comment

matthinz Mar 14, 2024

Choose a reason for hiding this comment

matthinz Mar 14, 2024

Choose a reason for hiding this comment

matthinz Mar 14, 2024

Choose a reason for hiding this comment

matthinz Mar 14, 2024

Choose a reason for hiding this comment

matthinz Mar 14, 2024

Choose a reason for hiding this comment

matthinz commented Mar 18, 2024

matthinz commented Mar 11, 2024 •

edited

Loading