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

Add error to specs for undocumented analytics event params (LG-5969) #9946

Merged
merged 53 commits into from
Jan 29, 2024

Conversation

zachmargolis
Copy link
Contributor

@zachmargolis zachmargolis commented Jan 19, 2024

🎫 Ticket

LG-5969

🛠 Summary of changes

Adds code to the test suite that will error when undocumented params are sent to the track_event method.

Spec changes

There are two kinds of exceptions right now:

  1. We seem to intentionally log extra undocumented params to make sure they get passed through
  2. We have analytics methods that don't take params but do add extra stuff to the payload as new undocumented keys like so:
    def idv_doc_auth_randomizer_defaulted
    track_event(
    'IdV: doc_auth random vendor error',
    error: 'document_capture_session_uuid_key missing',
    )
    end

So to get around that, we have some new spec metadata that can be used to annotate:

context 'some spec', allowed_extra_analytics: [:my_param_name] do
  it 'logs the stuff' do
    analytics.track_some_event(my_param_name: true)
  end
end

changelog: Internal, Documentation, Add error for undocumented analytics event params
@zachmargolis zachmargolis requested a review from a team January 19, 2024 19:55
@zachmargolis zachmargolis marked this pull request as ready for review January 29, 2024 19:21
@zachmargolis
Copy link
Contributor Author

Ok update! The new plan here is that I added the infra needed to error on unknown params (and even document string keys), but plumbing those through was a lot of work and very tedious so I added an escape valve:

allowed_extra_analytics: [:*]

and am annotating failing specs with those so we can address documentation in a more piecemeal fashion

@zachmargolis zachmargolis merged commit fa70d13 into main Jan 29, 2024
2 checks passed
@zachmargolis zachmargolis deleted the margolis-lg-5969-undocumented-analytics-params branch January 29, 2024 21:25
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.

4 participants