Add cross-product uuid to all telemetry events (#1619)

* add cross product uuid to all telemetry calls

* remove need to ts-ignore

* add some unit tests

* rework creation to new spec

* update creation of uuid and config migration

* condense if statements

Author: mattseddon

.vscode/settings.json

@@ -8,6 +8,7 @@
   "editor.tabSize": 2,
   "typescript.updateImportsOnFileMove.enabled": "prompt",
   "cSpell.words": [
+    "appdirs",
     "camelcase",
     "chokidar",
     "conda",

extension/package.json

@@ -1464,6 +1464,7 @@
   "dependencies": {
     "@hediet/std": "^0.6.0",
     "@vscode/extension-telemetry": "^0.4.10",
+    "appdirs": "^1.1.0",
     "execa": "^5.1.1",
     "fs-extra": "^10.0.0",
     "js-yaml": "^4.1.0",
@@ -1473,7 +1474,8 @@
     "lodash.isequal": "^4.5.0",
     "lodash.merge": "^4.6.2",
     "lodash.omit": "^4.5.0",
-    "tree-kill": "^1.2.2"
+    "tree-kill": "^1.2.2",
+    "uuid": "^8.3.2"
   },
   "devDependencies": {
     "@types/chai": "^4.2.22",
@@ -1493,6 +1495,7 @@
     "@types/node": "^16.11.8",
     "@types/react-vega": "^7.0.0",
     "@types/sinon-chai": "^3.2.6",
+    "@types/uuid": "^8.3.4",
     "@types/vscode": "^1.64.0",
     "@vscode/test-electron": "^2.1.3",
     "chai": "^4.2.0",

extension/src/fileSystem/index.ts

@@ -4,7 +4,8 @@ import {
   lstatSync,
   readdir,
   readFileSync,
-  removeSync
+  removeSync,
+  writeFileSync
 } from 'fs-extra'
 import { load } from 'js-yaml'
 import { Uri } from 'vscode'
@@ -91,3 +92,16 @@ export const loadYaml = <T>(path: string): T | undefined => {
     Logger.error(`failed to load yaml ${path}`)
   }
 }
+
+export const loadJson = <T>(path: string): T | undefined => {
+  try {
+    return JSON.parse(readFileSync(path).toString()) as T
+  } catch {
+    Logger.error(`failed to load JSON from ${path}`)
+  }
+}
+
+export const writeJson = <T extends Record<string, unknown>>(
+  path: string,
+  obj: T
+): void => writeFileSync(path, JSON.stringify(obj))

extension/src/telemetry/index.test.ts

@@ -6,6 +6,7 @@ import {
   EXTENSION_ID,
   IEventNamePropertyMapping
 } from './constants'
+import { getUserId } from './uuid'
 
 const mockedTelemetryReporter = jest.mocked(TelemetryReporter)
 
@@ -18,7 +19,9 @@ const mockedPackageJSON = {
   version: '0.1.0'
 }
 const mockedSendTelemetryEvent = jest.fn()
+const mockedGetUserId = jest.mocked(getUserId)
 
+jest.mock('./uuid')
 jest.mock('@vscode/extension-telemetry')
 jest.mock('vscode')
 
@@ -70,6 +73,8 @@ describe('getTelemetryReporter', () => {
 
 describe('sendTelemetryEvent', () => {
   it('should call the reporter with the correct event name and sanitized parameters', () => {
+    const mockedUserId = 'fbaff2be-6cde-4c94-ae98-b2e35e562712'
+    mockedGetUserId.mockReturnValueOnce(mockedUserId)
     const mockedEventName = 'mockedEvent' as keyof IEventNamePropertyMapping
     const mockedEventProperties = {
       a: 1,
@@ -95,7 +100,8 @@ describe('sendTelemetryEvent', () => {
         a: '1',
         b: '{"c":2,"d":{"e":"3"}}',
         h: 'some string',
-        i: 'true'
+        i: 'true',
+        user_id: mockedUserId
       },
       mockedMeasurements
     )

extension/src/telemetry/index.ts

@@ -5,6 +5,7 @@ import {
   IEventNamePropertyMapping,
   ViewOpenedEventName
 } from './constants'
+import { getUserId } from './uuid'
 import { Logger } from '../common/logger'
 import { getExtensionVersion } from '../vscode/extensions'
 
@@ -90,7 +91,7 @@ export const sendTelemetryEvent = <
     : undefined
   reporter.sendTelemetryEvent(
     eventName as string,
-    sanitizedProperties,
+    { ...sanitizedProperties, user_id: getUserId() },
     measurements
   )
 }

extension/src/telemetry/uuid.test.ts

@@ -0,0 +1,109 @@
+import { sep } from 'path'
+import { v4 } from 'uuid'
+import { getUserId, readOrCreateUserId } from './uuid'
+import { exists, loadJson, writeJson } from '../fileSystem'
+
+const mockedExists = jest.mocked(exists)
+const mockedReadJson = jest.mocked(loadJson)
+const mockedWriteJson = jest.mocked(writeJson)
+
+jest.mock('../fileSystem')
+
+beforeEach(() => {
+  jest.resetAllMocks()
+})
+
+const mockedUserId = v4()
+const mockedConfig = { 'do-not-track': true, user_id: mockedUserId }
+
+describe('readOrCreateUserId', () => {
+  it('should create two completely new configs if neither config can be found', () => {
+    mockedExists.mockReturnValueOnce(false).mockReturnValueOnce(false)
+
+    readOrCreateUserId()
+    expect(mockedReadJson).not.toBeCalled()
+    expect(mockedWriteJson).toBeCalledTimes(2)
+    expect(mockedWriteJson).toBeCalledWith(
+      expect.stringContaining(sep + 'telemetry'),
+      expect.objectContaining({ user_id: expect.stringContaining('-') })
+    )
+  })
+
+  it('should migrate the legacy DVC config if the new config cannot be found', () => {
+    mockedExists.mockReturnValueOnce(true).mockReturnValueOnce(false)
+    mockedReadJson.mockReturnValueOnce(mockedConfig)
+
+    readOrCreateUserId()
+    expect(mockedWriteJson).toBeCalledTimes(1)
+    expect(mockedWriteJson).toBeCalledWith(
+      expect.stringContaining(sep + 'telemetry'),
+      mockedConfig
+    )
+    expect(mockedWriteJson).not.toBeCalledWith(
+      expect.stringContaining('dvc'),
+      mockedConfig
+    )
+  })
+
+  it('should create a legacy config if only the new one can be found', () => {
+    mockedExists.mockReturnValueOnce(false).mockReturnValueOnce(true)
+    mockedReadJson.mockReturnValueOnce(mockedConfig)
+
+    readOrCreateUserId()
+    expect(mockedReadJson).toBeCalledTimes(1)
+    expect(mockedWriteJson).toBeCalledTimes(1)
+
+    expect(mockedWriteJson).not.toBeCalledWith(
+      expect.stringContaining(sep + 'telemetry'),
+      mockedConfig
+    )
+
+    expect(mockedWriteJson).toBeCalledWith(
+      expect.stringContaining('dvc'),
+      expect.objectContaining({ user_id: expect.stringContaining('-') })
+    )
+  })
+
+  it('should overwrite the user_id in the new config if it differs from the legacy id', () => {
+    mockedExists.mockReturnValueOnce(true).mockReturnValueOnce(true)
+    mockedReadJson
+      .mockReturnValueOnce(mockedConfig)
+      .mockReturnValueOnce({ 'some-other-info': true, user_id: 'bogus-id' })
+
+    readOrCreateUserId()
+    expect(mockedReadJson).toBeCalledTimes(2)
+    expect(mockedWriteJson).toBeCalledTimes(1)
+
+    expect(mockedWriteJson).toBeCalledWith(
+      expect.stringContaining(sep + 'telemetry'),
+      {
+        'do-not-track': true,
+        'some-other-info': true,
+        user_id: expect.not.stringMatching('bogus-id')
+      }
+    )
+  })
+})
+
+describe('getUserId', () => {
+  it('should only try to access the value from the fileSystem on the first call', () => {
+    mockedExists.mockReturnValueOnce(true)
+    mockedReadJson.mockReturnValueOnce(mockedConfig)
+
+    const user_id = getUserId()
+
+    expect(user_id).toStrictEqual(mockedUserId)
+    expect(mockedExists).toBeCalledTimes(2)
+    expect(mockedReadJson).toBeCalledTimes(1)
+
+    mockedExists.mockClear()
+    mockedReadJson.mockClear()
+
+    getUserId()
+    getUserId()
+    getUserId()
+
+    expect(mockedExists).not.toBeCalled()
+    expect(mockedReadJson).not.toBeCalled()
+  })
+})

extension/src/telemetry/uuid.ts

@@ -0,0 +1,83 @@
+import { join } from 'path'
+import isEmpty from 'lodash.isempty'
+import { v4 } from 'uuid'
+import { getProcessPlatform } from '../env'
+import { exists, loadJson, writeJson } from '../fileSystem'
+
+type UserConfig = {
+  user_id?: string
+}
+
+const loadConfig = (configPath: string): UserConfig => {
+  if (!exists(configPath)) {
+    return {}
+  }
+
+  return loadJson<UserConfig>(configPath) || {}
+}
+
+const writeMissingConfigs = (
+  user_id: string,
+  legacyConfig: UserConfig,
+  legacyConfigPath: string,
+  config: UserConfig,
+  configPath: string
+) => {
+  if (isEmpty(legacyConfig) && isEmpty(config)) {
+    writeJson(legacyConfigPath, { user_id })
+    writeJson(configPath, { user_id })
+    return
+  }
+
+  if (isEmpty(config) || config.user_id !== user_id) {
+    writeJson(configPath, { ...config, ...legacyConfig, user_id })
+    return
+  }
+
+  if (isEmpty(legacyConfig)) {
+    writeJson(legacyConfigPath, { ...config, user_id })
+  }
+}
+
+const readOrCreateConfig = (): string | undefined => {
+  const { userConfigDir } = require('appdirs') as {
+    userConfigDir: (appName: string) => string
+  }
+
+  const legacyDirectory =
+    getProcessPlatform() === 'win32'
+      ? join('iterative', 'dvc', 'user_id')
+      : join('dvc', 'user_id')
+
+  const legacyConfigPath = userConfigDir(legacyDirectory)
+  const legacyConfig = loadConfig(legacyConfigPath)
+
+  const configPath = userConfigDir(join('iterative', 'telemetry'))
+  const config = loadConfig(configPath)
+
+  const user_id = legacyConfig.user_id || config.user_id || v4()
+
+  if (legacyConfig.user_id !== user_id || config.user_id !== user_id) {
+    writeMissingConfigs(
+      user_id,
+      legacyConfig,
+      legacyConfigPath,
+      config,
+      configPath
+    )
+  }
+
+  return user_id
+}
+
+export const readOrCreateUserId = () => {
+  return readOrCreateConfig() || 'unknown'
+}
+
+let user_id: string
+export const getUserId = (): string => {
+  if (!user_id) {
+    user_id = readOrCreateUserId()
+  }
+  return user_id
+}

yarn.lock

@@ -3466,6 +3466,11 @@
   resolved "https://registry.yarnpkg.com/@types/unist/-/unist-2.0.6.tgz#250a7b16c3b91f672a24552ec64678eeb1d3a08d"
   integrity sha512-PBjIUxZHOuj0R15/xuwJYjFi+KZdNFrehocChv4g5hu6aFroHue8m0lBP0POdK2nKzbw0cgV1mws8+V/JAcEkQ==
 
+"@types/uuid@^8.3.4":
+  version "8.3.4"
+  resolved "https://registry.yarnpkg.com/@types/uuid/-/uuid-8.3.4.tgz#bd86a43617df0594787d38b735f55c805becf1bc"
+  integrity sha512-c/I8ZRb51j+pYGAu5CrFMRxqZ2ke4y2grEBO5AUjgSkSk+qT2Ea+OdWElz/OiMf5MNpn2b17kuVBwZLQJXzihw==
+
 "@types/vscode@^1.64.0":
   version "1.64.0"
   resolved "https://registry.yarnpkg.com/@types/vscode/-/vscode-1.64.0.tgz#bfd82c8d92dc7824c1be084be1ab46ce20d7fb55"
@@ -4250,6 +4255,11 @@ app-root-dir@^1.0.2:
   resolved "https://registry.yarnpkg.com/app-root-dir/-/app-root-dir-1.0.2.tgz#38187ec2dea7577fff033ffcb12172692ff6e118"
   integrity sha1-OBh+wt6nV3//Az/8sSFyaS/24Rg=
 
+appdirs@^1.1.0:
+  version "1.1.0"
+  resolved "https://registry.yarnpkg.com/appdirs/-/appdirs-1.1.0.tgz#38b9c9e1d7e76908fa9f5c69b79cb03074f8395a"
+  integrity sha1-OLnJ4dfnaQj6n1xpt5ywMHT4OVo=
+
 append-transform@^2.0.0:
   version "2.0.0"
   resolved "https://registry.yarnpkg.com/append-transform/-/append-transform-2.0.0.tgz#99d9d29c7b38391e6f428d28ce136551f0b77e12"