Skip to content

Conversation

@BogdanZavu
Copy link
Contributor

@BogdanZavu BogdanZavu commented Jan 13, 2023

Purpose

Avoid hang or long startup times when there are issues with ADP connection.
The number of retries in Analytics.NET IsOptedIn. is too high and it takes a failed call 30s to conclude.
In my case the return code was 11 but there could be other codes as well.
On Dynamo startup there is a high number of track event calls which can easily add up to a lot of time.
I believe this is the reason for other reports that we have with Dynamo or Player starting slowly when startup sequence takes too long.
Imho one quick possible solution for this release R2024 is to interrogate once per session for IsOptedIn value and cache it. Also limit the number of retries and overall isOpt check should not tale longer than 3s or other value.
While this will prevent the analytics behavior to be changed during the same session I think we can live with that.
This is somewhat already happening when you are set to false since trackers are only registered on startup if set to true.

Declarations

Check these if you believe they are true

  • The codebase is in a better state after this PR
  • Is documented according to the standards
  • The level of testing this PR includes is appropriate
  • User facing strings, if any, are extracted into *.resx files
  • All tests pass using the self-service CI.
  • Snapshot of UI changes, if any.
  • Changes to the API follow Semantic Versioning and are documented in the API Changes document.
  • This PR modifies some build requirements and the readme is updated

FYIs

Copy link
Member

@mjkkirschner mjkkirschner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with this if the potential for missing analytics is acceptable to @QilongTang.

I guess a future improvement could be update this cached value at some interval or in the background/from another thread.

{
get { return Logging.AnalyticsService.IsADPOptedIn; }
get
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pls add some comments here as to why we are only checking a single time.

@BogdanZavu BogdanZavu requested a review from QilongTang January 13, 2023 22:46
@BogdanZavu BogdanZavu merged commit fc30a2d into DynamoDS:master Jan 17, 2023
@BogdanZavu BogdanZavu deleted the AGD-2857 branch January 17, 2023 14:16
BogdanZavu added a commit to BogdanZavu/Dynamo that referenced this pull request Jan 17, 2023
* cache adp analytics optin

* add comment

Co-authored-by: Bogdan Zavu <bogdan.zavu@autodesk.com>
QilongTang pushed a commit that referenced this pull request Jan 18, 2023
* cache adp analytics optin

* add comment

Co-authored-by: Bogdan Zavu <bogdan.zavu@autodesk.com>

Co-authored-by: Bogdan Zavu <bogdan.zavu@autodesk.com>
twastvedt pushed a commit that referenced this pull request Oct 18, 2024
* cache adp analytics optin

* add comment

Co-authored-by: Bogdan Zavu <bogdan.zavu@autodesk.com>

Co-authored-by: Bogdan Zavu <bogdan.zavu@autodesk.com>
(cherry picked from commit 806231e)

# Conflicts:
#	src/DynamoCore/Configuration/PreferenceSettings.cs
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.

5 participants