| CARVIEW |
Navigation Menu
-
Notifications
You must be signed in to change notification settings - Fork 666
DYN-9718: Add analytics to track MLTermsOfUse #16628
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
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-9718
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds analytics tracking for the ML Terms of Use preference state at application shutdown. The change moves the tracking event from the property setter in PreferenceSettings.cs to DynamoModel.ShutDown(), capturing the final user preference state just before the analytics service is shut down. This approach avoids tracking each individual toggle of the preference and instead focuses on the final state to measure opt-out rates.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/DynamoCore/Models/DynamoModel.cs | Added analytics tracking call for ML Terms of Use status before analytics service shutdown |
| src/DynamoCore/Configuration/PreferenceSettings.cs | Removed analytics tracking from the IsMLAutocompleteTOUApproved property setter |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
johnpierson
left a comment
johnpierson
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
src/DynamoCore/Models/DynamoModel.cs
Outdated
|
|
||
| // Track ML Terms of Use status before shutting down the analytics service. | ||
| // This captures the final user preference state to measure opt-out rates. | ||
| Analytics.TrackPreference("MLTermsOfUse", "", PreferenceSettings.Instance?.IsMLAutocompleteTOUApproved == true ? 1 : 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although this would work, but we have one existing pattern that we can follow - report this when initializing Dynamo, and the analytics client.
You can refer to https://github.com/DynamoDS/Dynamo/blob/master/src/DynamoCore/Logging/DynamoAnalyticsClient.cs#L179
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @QilongTang thanks for your comments! I referred to the code you suggested and moved the analytics to when Dynamo and analytics client is initialized. 🙌
kalunkuo
changed the title
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> (cherry picked from commit e86af97)
|
Successfully created backport PR for |
Purpose
IsMLAutocompleteTOUApprovedstatus during initiationPreferenceSettings.csto avoid tracking each individual selection eventDeclarations
Check these if you believe they are true
Release Notes
Add analytics to track MLTermsOfUse at shut down
Reviewers
@DynamoDS/eidos @DynamoDS/synapse
FYIs
(FILL ME IN, Optional) Names of anyone else you wish to be notified of