Validate event name for frontend logging

[mitchellhenke]

🛠 Summary of changes

Action item from the 2023-09-07 contingency planning scenario to require validation of events logged from the frontend

Related to #9421

[aduth]

There's a couple more through ClickObserverComponent event_name:

identity-idp/app/views/idv/in_person/ready_to_verify/show.html.erb

Line 51 in 43e1303

<%= render ClickObserverComponent.new(event_name: 'IdV: user clicked what to bring link on ready to verify page') do %>

identity-idp/app/views/idv/in_person/ready_to_verify/show.html.erb

Line 96 in 43e1303

<%= render ClickObserverComponent.new(event_name: 'IdV: user clicked sp link on ready to verify page') do %>

[zachmargolis]

Is **extra unused on purpose?

[mitchellhenke]

yeah, frontend events only accept keys that are defined (due to this) so passing extra into the logging is not doing anything. I've updated the other frontend events accordingly.

[mitchellhenke]

There's a couple more through ClickObserverComponent event_name:

identity-idp/app/views/idv/in_person/ready_to_verify/show.html.erb

Line 51 in 43e1303

<%= render ClickObserverComponent.new(event_name: 'IdV: user clicked what to bring link on ready to verify page') do %>

identity-idp/app/views/idv/in_person/ready_to_verify/show.html.erb

Line 96 in 43e1303

<%= render ClickObserverComponent.new(event_name: 'IdV: user clicked sp link on ready to verify page') do %>

added in 5e03e1b9e3fe128f95d6e938e7973fd117f59951

[aduth]

I think this looks good, I'll take a detailed look a little later.

[aduth]

To the earlier point about this **_extra, can we just exclude it? Or is the problem that the send will always try to pass a hash of values; in which case, could we make sure that only happens if it's non-empty? I guess I'm thinking that we should either never expect **extra to be passed, or we could just log the **extra for consistency with other events (except for methods like this which don't have any arguments).

[mitchellhenke]

We're limited by https://github.com/18F/identity-idp/blob/d099da2181ab73c733f8a0dea561ca4152e5aa10/lib/analytics_events_documenter.rb#L114-L116 which ends up failing lint with:

app/services/analytics_events.rb:704 idv_back_image_clicked missing **extra

[zachmargolis]

LGTM

[zachmargolis]

We could add a { ... }.compact inside AnalyticsEvents if we wanted to minimize diff noise here

[mitchellhenke]

Removing nil values is a change to downstream logging behavior and I'm not sure if the presence of nil is important or depended on in some way :/

[zachmargolis]

usually nil values are elided by CW, so it's not meaningful, but happy to err on the conservative side of this and leave these nils in there