Hotfix(metrics): CNX-9196 add object type event tracking for mixpanel in autocad, rhino, and revit #3257
Conversation
ConnectorAutocadCivil/ConnectorAutocadCivil/UI/ConnectorBindingsAutocadCivil.Receive.cs
Show resolved
Hide resolved
Core/Core/Logging/Analytics.cs
Outdated
| /// <summary> | ||
| /// Event triggered on send, capturing the object type counts of the sent commit | ||
| /// </summary> | ||
| SendObjectReport, |
There was a problem hiding this comment.
Can we just make these without setup or do we need to setup on MixPanel side, presumably it's either automatic or we did the setup? Just asking? Also, do we know what part of our estate is release and what part is WIP?
There was a problem hiding this comment.
I would not introduce two new events to track this; instead, we should attach this information to the existing Send and Receive events.
Also, I did not see any instance of SendObjectReport and ReceiveObjectReport in mixpanel. Has this code been tested? What's the payload like?
ConnectorAutocadCivil/ConnectorAutocadCivil/UI/ConnectorBindingsAutocadCivil.Receive.cs
Outdated
Show resolved
Hide resolved
…mixpanel-for-AutoCAD-Rhino-and-Revit-on-send-and-receive
Since we are tracking this event from the connector binding (not DUI2), wouldn't this create 2 send events for every send then? Originally I introduced a new event to avoid this issue, but perhaps I'm misunderstanding how the send event is tracked |
There was a problem hiding this comment.
Checking your sample requests I can see that the object counts have been added as top level properties: https://eu.mixpanel.com/project/2605593/view/3144341/app/events#Q987uPpF8rvz
This will make it difficult to analyze the data in Mixpanel, I think they should instead be sent as nested object properties. This will make sure we keep our lexicon clean and data will be easier to parse.
Please also note the following limits:
- Each event must be smaller than 1MB of uncompressed JSON.
- Each event must have fewer than 255 properties.
- All nested object properties must have fewer than 255 keys and max nesting depth is 3.
|
Just refactored the payload to be an array instead of a dictionary! Sample Mixpanel report below: |
There was a problem hiding this comment.
Were building up a fair bit of duplicate code.
I think @AlanRynne and @BovineOx were discussing consolidating the send + receive bindings into a shared project (so all connectors use the same) this would likely fix things here.
Not a blocker for this pr, but just extra fuel for us needing to re-consider where this lives.
ConnectorAutocadCivil/ConnectorAutocadCivil/UI/ConnectorBindingsAutocadCivil.Receive.cs
Outdated
Show resolved
Hide resolved
| .Select(o => new { TypeName = o.Key, Count = o.Value }) | ||
| .OrderBy(pair => pair.Count) | ||
| .Reverse() | ||
| .Take(250); |
There was a problem hiding this comment.
Perhaps we can encode this value as a constant inside the Analytics class, just so we avoid having magic numbers
There was a problem hiding this comment.
SInce the limit in mixpanel is 255, and we might be adding some other default props, I would lower it to 230 or so to be safe, maybe even 200.
ConnectorAutocadCivil/ConnectorAutocadCivil/UI/ConnectorBindingsAutocadCivil.Receive.cs
Outdated
Show resolved
Hide resolved
| .Select(o => new { TypeName = o.Key, Count = o.Value }) | ||
| .OrderBy(pair => pair.Count) | ||
| .Reverse() | ||
| .Take(250); |
There was a problem hiding this comment.
SInce the limit in mixpanel is 255, and we might be adding some other default props, I would lower it to 230 or so to be safe, maybe even 200.
| var typeCountArray = typeCountDict | ||
| .ToArray() | ||
| .Select(o => new { TypeName = o.Key, Count = o.Value }) | ||
| .OrderBy(pair => pair.Count) | ||
| .Reverse() | ||
| .Take(250); | ||
|
|
||
| Analytics.TrackEvent( | ||
| Analytics.Events.ConvertToSpeckle, | ||
| new Dictionary<string, object>() { { "typeCount", typeCountArray } } | ||
| ); |
There was a problem hiding this comment.
Wondering if this could go in a helper function, but up to you guys...
clairekuang
deleted the
CNX-9196-Add-object-type-event-tracking-for-mixpanel-for-AutoCAD-Rhino-and-Revit-on-send-and-receive
branch
Description & motivation
Adds two new events for mixpanel tracking:
SendObjectReportandReceiveObjectReport.Attaches a dictionary containing each Speckle type found in the sent/received commit and their count, and tracks this event separately from send and receive events.
Added for AutoCAD, Rhino, and Revit.