-
Notifications
You must be signed in to change notification settings - Fork 340
Add basic Analytics #43
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
Seems we were way out of date. This allows accessing env.appName.
|
@devoncarew I added some basic analytics (to see how many people are using and which versions - I don't thing VSCode auto-updates or prompts uses to). After the discussion about TODOs I also added an event for TODOs being enabled/disabled (this is easily extendable to other toggles). I also set an isDevelopment flag when in the dev host (not sure if I did this the best way...) so I can filter these out in Analytics. The numbers are obviously 0 for now, but if you're interested in access to the Analytics account let me know. |
|
This fixes #20. |
| let data = { | ||
| v: "1", // API Version. | ||
| tid: "UA-2201586-19", | ||
| cid: env.machineId, |
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.
I don't know if this value will work. When I run this locally I see 'someValue.machineId'. analytics wants this value to be something like a UUID v4 - https://developers.google.com/analytics/devguides/collection/analyticsjs/field-reference#clientId.
There's some sample code for creating one here: https://github.com/dart-lang/usage/blob/master/lib/src/uuid.dart.
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.
It could be that env.machineId is normally in a good form for analytics to use, but theres something funky going on on my machine -
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.
I actually say the same thing in the debugger and wondered if it was the dev host. I made a note to double-check what's really being sent from the release version but haven't had chance yet. I'll be sure to do it tonight!
If it doesn't work, I'll create a random ID and stash it in the extension context to use instead (and raise a bug with MS!)
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.
Ok, confirmed that machineId is a big long GUID in the real host; it's only that placeholder in the extension host. That might make a better check than my "-dev" version check so I'll see if I can get MS to confirm if that's by design and stable.
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.
Ah, cool; it's nice to not have to write code to generate one.
|
Awesome, analytics in great to have. It can be sensitive for some, so probably best to add an opt-out as a preference (VSCode has an opt-out for this as well). Adding a preference would also allow us to filter out dev hosts as well. |
|
I've added a setting to opt-out of analytics. There is a dev flag passed to GA (based on version number until MS implement this). In the analytics account I have a view for "All" (which includes dev, to aid testing) and "Live" (filters dev) so you don't need to disable it if you're just worried about polluting stats (I think missing live usage would be worse), though obviously the option is there if you wish! Looking into the weird machine ID thing now... |
No description provided.