Add adp analytics to Dynamo #10576

pinzart · 2020-04-17T15:16:02Z

Purpose

Add ADP analytics to Dynamo.

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.

Reviewers

(FILL ME IN) Reviewer 1 (If possible, assign the Reviewer for the PR)

(FILL ME IN, optional) Any additional notes to reviewers or testers.

FYIs

(FILL ME IN, Optional) Names of anyone else you wish to be notified of

mjkkirschner · 2020-04-17T16:33:59Z

src/DynamoCore/Logging/DynamoAnalyticsClient.cs

                service.Register(new GATrackerFactory(ANALYTICS_PROPERTY));

+            if (service.GetTrackerFactory(ADPTrackerFactory.Name) == null)
+                service.Register(new ADPTrackerFactory());


can this fail for any reason?

not really. At the moment...there is no more risk than the GATracker registration call.
We can wrap bot calls in a try catch...

mjkkirschner · 2020-04-17T16:34:31Z

src/DynamoCore/Logging/DynamoAnalyticsClient.cs

            var appversion = dynamoModel.AppVersion;

-            product = new ProductInfo() { Name = "Dynamo", VersionString = appversion, AppVersion = dynamoModel.Version };
+            product = new ProductInfo() { Id = "DYN", Name = "Dynamo", VersionString = appversion, AppVersion = dynamoModel.Version, BuildId = "CB159AA3-2F20-4791-AE8C-8939E90C9600", ReleaseId = "2.5.0", MasterId = "{4C7B248C-56D3-488F-8E23-8CAFD2651968}" };


is this going to effect the gathered google analytics data?

The added data should not interfere with Google analytics.

It is still designed to simplify internal testing (just test dummy identifiers).
We need to decide on proper UPI (universal product identifier) configs ...then we can properly supply them here

mjkkirschner · 2020-04-17T16:35:23Z

src/DynamoCore/Logging/DynamoAnalyticsClient.cs

            var appversion = dynamoModel.AppVersion;

-            product = new ProductInfo() { Name = "Dynamo", VersionString = appversion, AppVersion = dynamoModel.Version };
+            product = new ProductInfo() { Id = "DYN", Name = "Dynamo", VersionString = appversion, AppVersion = dynamoModel.Version, BuildId = "CB159AA3-2F20-4791-AE8C-8939E90C9600", ReleaseId = "2.5.0", MasterId = "{4C7B248C-56D3-488F-8E23-8CAFD2651968}" };


whats the process around these ids and when we increment them?
Latest released version is 2.5.3

Again...this is test dummy data.
From my discussion with other teams....it seems that the proper releaseId format is the targeted release year (ex. 2021) So we would need to change the UPI configs
THe rest of the identifiers (from my understanding) do not play any real roles (they should just reflect the configs in UPI..and they do not need to change from release to release)

Basically it sounds like the ReleaseId is the only config that needs to change from release to release.

@mjkkirschner We update them everytime we RTM, unless for a specific reason that team decides that we do not want to distinguish the newly released Dynamo with the previous release. They need to be first populated from UPI and then reflected in Code here to be passed in. These will need to be documented when this gets in.

mjkkirschner · 2020-04-17T16:36:12Z

Should we also update our readme and associated terms of use etc before this gets merged into master?
@kronz
@Amoursol
@gregmarr

QilongTang · 2020-04-17T17:00:20Z

@mjkkirschner The about box already covers ADP. We are talking with legal about this, this is not meant for merging.

pinzart · 2020-04-17T18:14:38Z

Sorry @mjkkirschner ..I should have made it clear in the description.
We should test this branch internally, with the ADPC#SDK dll. Check that we have the proper analytics showing up in Looker...then we can merge to master.

We've just talked to legal. It seems it will take a while to get proper approvals (including updating the terms of use ...if needed)

src/Dynamo.All.sln

Amoursol · 2020-04-17T19:14:18Z

100% yes, we do need to update TOS and our legal messaging with this @mjkkirschner, thanks for the ping.

@QilongTang @pinzart Am I correct in reading that you two are talking with legal (James Connor?) on this?

QilongTang · 2020-04-17T19:18:13Z

@Amoursol Yes, we already in connect with legal. For TOU, do you mean aboutbox only or if there is something else we need to watch for? I am not aware of anywhere else we mention Analytics component. In terms of agreement, we will not be using the DynamoGA one for ADP agreement.

Amoursol · 2020-04-17T19:28:04Z

@QilongTang - Brilliant, thank you. Yes, meant the About-Box in Dynamo's help menu 😊

src/DynamoCoreWpf/Properties/Resources.en-US.resx

pinzart · 2020-04-29T23:09:35Z

Changed the analytics consent related documents/resource.
I intend to enable the ADP supplied consent window through a debug mode later on (if it makes sense)

This reverts commit 1de286e.

This reverts commit 3d0bb2d.

mjkkirschner · 2020-05-05T18:05:14Z

@pinzart @QilongTang - the build is failing with:

 Configuration\DebugModes.cs(50,150): error CS1002: ; expected [c:\WorkspaceDynamo\src\DynamoCore\DynamoCore.csproj]
[2020-05-05T17:19:32.922Z] 
[2020-05-05T17:19:32.922Z]     226 Warning(s)
[2020-05-05T17:19:32.922Z]     1 Error(s)
[2020-05-05T17:19:32.922Z]

update adp dlls

pinzart · 2020-05-05T20:08:39Z

@mjkkirschner My fault...forgot a comma.
Should be fixed

src/DynamoCoreWpf/Properties/Resources.Designer.cs

inlined declare of out var seems to cause build fail (probably older compiler version)

pinzart · 2020-05-06T14:06:25Z

@mjkkirschner @QilongTang are we good to go here?
I will add a separate PR for the UI changes (opt in/out of Google and ADP analytics)

mjkkirschner · 2020-05-06T21:59:43Z

@pinzart @QilongTang @Dewb - have we done any testing in hosts that also use ADP? Could there be conflicts? What can we do in those cases?

QilongTang · 2020-05-06T22:08:08Z

@mjkkirschner We confirmed that the wrapper will be in charge of finding the correct version of ADP Client. We do not need to worry about versioning for now

* add namespace conflict tests for short name replacer and node to code (#10611) * Register custom node before package load reset (#10591) This fixes a problem where existing custom nodes in the home workspace became unresolved after a package that contained binaries was loaded. The cause of the problem was that the compiled function was not available at the time of the execution after a VM reset. Now the data is registered on package load, by queueing it in pendingCustomNodeSyncData. This results in a CompileCustomNodeAsyncTask being scheduled before the update of the home workspace graph takes place. * Increase coverage of the Core folder (#10609) * Add a test for the CrashPromptArgs class * Update DynamoCoreTests.csproj * Added NotificationObject tests * Updated the test, NotificationObject coverage at 100% * Changed some access modifiers * Added coverage for Updates/BinaryVersion * Added comments and fixed names * Revert "Added comments and fixed names" This reverts commit 42cd024. * Revert "Added coverage for Updates/BinaryVersion" This reverts commit 679deca. * Increased coverage in CrashPromptArgs * Added some Core coverage * DYN-2560 - Increase the code coverage: DynamoCore/Models Folder First Part (#10612) * DYN-2560 - Increase the code coverage: DynamoCore/Models Folder I started adding just one test method TestOnRequestDispatcherBeginInvoke() for testing the DynamoModelEvents class. * DYN-2560 - Adding test cases for DynamoModelEvents Adding test cases for DynamoModelEvents * DYN-2560 - Adding test cases for DynamoModelEvents I added all the test cases for the all events in the DynamoModelEvent class, i just need to fix the last 6 of them. * DYN-2560 - Adding test cases for DynamoModelEventsArgs I added several test cases for the classes inside the DynamoModelEventsArgs.cs file. ZoomEventArgs TaskDialogEventArgs EvaluationCompletedEventArgs DynamoModelUpdateArgs FunctionNamePromptEventArgs PresetsNamePromptEventArgs ViewOperationEventArgs PointEventArgs WorkspaceEventArgs ModelEventArgs * DYN-2560 - Code Review Comments Based in the comment done by Aaron in the GitHub pull request, I added more description comments for the method TestTaskDialogEventArgs() and also I added comments for a local function * DYN-2560 - Code Review Comments 2 There was a spelling error in two methods for the word "Internally", then I fixed this error in the two places. * Python Engine Enum (#10618) * Cherrypick * Comments * Add unit test * Comments * Handle runtime table gaps on code block deletion (#10605) When the runtime table are built there is an implicit assumption that the code block ids are consecutive. However, that is not always the case, as the deletion of a procedure causes the deletion of its child code blocks, which may generate gaps in the id numbering. In order to make the code resiliant to these gaps, the runtime tables are sized based on the largest code block id, rather than in the amount of code blocks. * Validate ASM installations before loading (#10621) The check for the specific assemblies tbb.dll and tbbmalloc.dll is generalized to a full file list validation of detected ASM locations. This way, Dynamo is guarded against any incomplete/unusual ASM binary folders that other applications might include. The lists of files for each version were taken from LibG. They cannot be reused from LibG without involving major changes in the preloader, so the lists should be kept in sync as new major release of ASM occur. * SQ bug fix (#10622) * (1) Null reference bug fix from SQ dashboard * Add support for debug modes (#10603) * Add support for debug modes * Increase coverage of the Updates folder (#10628) * Add a test for the CrashPromptArgs class * Update DynamoCoreTests.csproj * Added NotificationObject tests * Updated the test, NotificationObject coverage at 100% * Changed some access modifiers * Added coverage for Updates/BinaryVersion * Added comments and fixed names * Added asserts for happy path, changed namespace and deleted a console function * Removed using * Revert "Removed using" This reverts commit 022823d. * Removed a change that should not be there * Added coverage for Updates folder * Tests for CPython3 Engine as well as for IronPython. * [WIP][FEEDBACK] Reduce test time by substantially reducing number of serialization tests. (#10624) * reduce number of serialization tests by factor 3~ * reduce wpf json serialization tests * Handle missing instance calling method statically (#10630) A code block node calling an instance method in its static form would make Dynamo crash if the instance was not provided. An example of this would be calling 'Curve.Patch();'. The cause of the issue was that the default argument was ultimately tried to be interpreted as a pointer. By avoiding that wrong conversion the engine is now able to surface the real problem as a human-readable warning. * Python3 Selection Under Debug Modes (#10629) * Cherrypick Python3 changes * Cleanup * Use Debug Modes * CleanUp * Rename Function * Clean Up * Do not use anouymous function as handler * Revert newer language change * Add adp analytics to Dynamo (#10576) * add ADPTracker register * Fix non-array item search for dot operation (#10633) When an array is passed to the dot operation, it needs to get an item to determine the actual class being processed. This was done in ArrayUtils.GetFirstNonArrayStackValue, but the function only checked the first item of the array and its descendants. This caused the dot operation to failed when passed an array that contained an empty array as its first item. In order to fix the problem, the function was changed to check for non-array items in the entire array. The function should stop as soon as the first non-array item is found. * Addressing some comments * some more comments * Update AssemblyInfo.cs * Update AssemblyInfo.cs * Marking the python node as modified when its engine property is modified Also updated some tests. * Remove unwanted check. * changes to test * Update AssemblySharedInfo.cs * Using python engine's AcquireLock to avoid deadlock. The deadlock was happening only when multiple PythonEngine's are initialized on a thread from 2 different test fixtures in the same run. The main difference is calling this function PythonEngine.BeginAllowThreads(). Also we do not want to initialze the python engine if it is already initialized. Co-authored-by: aparajit-pratap <aparajit.pratap@autodesk.com> Co-authored-by: Martin Misol Monzo <mmisol@users.noreply.github.com> Co-authored-by: Bruno Yorda <34453173+byorda-glb@users.noreply.github.com> Co-authored-by: Roberto T <61755417+RobertGlobant20@users.noreply.github.com> Co-authored-by: Aaron (Qilong) <173288704@qq.com> Co-authored-by: Ashish Aggarwal <aggarwal.ash@husky.neu.edu> Co-authored-by: Tibi <tiberiu.pinzariu@autodesk.com> Co-authored-by: Michael Kirschner <mjk.kirschner@gmail.com>

pinzart added 8 commits March 27, 2020 15:28

add ADPTracker register

103822d

update

fcb4208

Merge branch 'master' into adp_tracker

5299f61

update

1f49c24

update for latest analytics version

5f0a7cc

Update AssemblySharedInfo.cs

6e28c86

Delete AdpSDKCSharpWrapper.dll

1d9b7f8

Update DynamoAnalyticsClient.cs

abff723

mjkkirschner reviewed Apr 17, 2020

View reviewed changes

QilongTang added the DNM Do not merge. For PRs. label Apr 17, 2020

pinzart requested a review from QilongTang April 17, 2020 18:14

QilongTang reviewed Apr 17, 2020

View reviewed changes

src/Dynamo.All.sln Outdated Show resolved Hide resolved

pinzart added 3 commits April 20, 2020 17:04

update

12090ed

update releaseid and buildid

404e8e9

upate existing consent dialog

1de286e

pinzart commented Apr 29, 2020

View reviewed changes

src/DynamoCoreWpf/Properties/Resources.en-US.resx Outdated Show resolved Hide resolved

pinzart removed the DNM Do not merge. For PRs. label Apr 29, 2020

pinzart added 4 commits April 29, 2020 19:24

fix copy consent

3d0bb2d

Merge branch 'master' into adp_tracker

304f4d3

Revert "upate existing consent dialog"

0becdcb

This reverts commit 1de286e.

Revert "fix copy consent"

c9449d8

This reverts commit 3d0bb2d.

add ADP under debug mode

1b2ef60

pinzart added 2 commits May 5, 2020 15:24

fix dbg mode

80e706a

add more info on debugmode description

f41a922

update adp dlls

pinzart commented May 5, 2020

View reviewed changes

src/DynamoCoreWpf/Properties/Resources.Designer.cs Outdated Show resolved Hide resolved

pinzart added 3 commits May 5, 2020 17:01

fix build

8b12f18

inlined declare of out var seems to cause build fail (probably older compiler version)

undo modification

6592fa9

use Analytics v2.0.0 release

e64e4d2

pinzart added 3 commits May 6, 2020 12:38

Merge branch 'master' into adp_tracker

c4bad7a

Merge branch 'master' into adp_tracker

bf297b5

update after master merge

0731fe4

pinzart merged commit f398219 into master May 6, 2020

pinzart deleted the adp_tracker branch May 6, 2020 20:45

Add adp analytics to Dynamo #10576

Add adp analytics to Dynamo #10576

Uh oh!

Conversation

pinzart commented Apr 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose

Declarations

Reviewers

FYIs

Uh oh!

mjkkirschner Apr 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pinzart Apr 17, 2020

Choose a reason for hiding this comment

Uh oh!

mjkkirschner Apr 17, 2020

Choose a reason for hiding this comment

Uh oh!

pinzart Apr 17, 2020

Choose a reason for hiding this comment

Uh oh!

mjkkirschner Apr 17, 2020

Choose a reason for hiding this comment

Uh oh!

pinzart Apr 17, 2020

Choose a reason for hiding this comment

Uh oh!

QilongTang Apr 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mjkkirschner commented Apr 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

QilongTang commented Apr 17, 2020

Uh oh!

pinzart commented Apr 17, 2020

Uh oh!

Uh oh!

Amoursol commented Apr 17, 2020

Uh oh!

QilongTang commented Apr 17, 2020

Uh oh!

Amoursol commented Apr 17, 2020

Uh oh!

Uh oh!

pinzart commented Apr 29, 2020

Uh oh!

mjkkirschner commented May 5, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pinzart commented May 5, 2020

Uh oh!

Uh oh!

pinzart commented May 6, 2020

Uh oh!

mjkkirschner commented May 6, 2020

Uh oh!

QilongTang commented May 6, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

pinzart commented Apr 17, 2020 •

edited

Loading

mjkkirschner Apr 17, 2020 •

edited

Loading

QilongTang Apr 17, 2020 •

edited

Loading

mjkkirschner commented Apr 17, 2020 •

edited

Loading

mjkkirschner commented May 5, 2020 •

edited

Loading