Telemetry (#465)

.github/workflows/ci.yml

@@ -78,9 +78,6 @@ jobs:
       - name: Install yarn dependencies
         run: git config --global url."https://".insteadOf ssh:// && yarn install
         if: steps.cache_node.outputs.cache-hit != 'true'
-      - name: Run yarn postinstall if cache hitted
-        run: yarn run postinstall
-        if: steps.cache_node.outputs.cache-hit == 'true'
       - name: Build packages
         run: yarn build
       - name: Check licenses

docs/command-line-interface/config.md

@@ -41,13 +41,16 @@ Configure running node information for propagating transactions to network
 ```
 USAGE
   $ celocli config:set [-n <value>] [--globalHelp] [--derivationPath <value>]
+    [--telemetry 1|0]
 
 FLAGS
   -n, --node=<value>            URL of the node to run commands against or an alias
       --derivationPath=<value>  Set the default derivation path used by account:new and
                                 when using --useLedger flag. Options: 'eth',
                                 'celoLegacy', or a custom derivation path
       --globalHelp              View all available global flags
+      --telemetry=<option>      Whether to enable or disable telemetry
+                                <options: 1|0>
 
 DESCRIPTION
   Configure running node information for propagating transactions to network
@@ -75,6 +78,10 @@ EXAMPLES
 
   set --derivationPath celoLegacy
 
+  set --telemetry 0 # disable telemetry
+
+  set --telemetry 1 # enable telemetry
+
 FLAG DESCRIPTIONS
   -n, --node=<value>  URL of the node to run commands against or an alias
 

package.json

@@ -24,7 +24,7 @@
     "check-licenses": "yarn licenses list --prod | grep '\\(─ GPL\\|─ (GPL-[1-9]\\.[0-9]\\+ OR GPL-[1-9]\\.[0-9]\\+)\\)' && echo 'Found GPL license(s). Use 'yarn licenses list --prod' to look up the offending package' || echo 'No GPL licenses found'",
     "report-coverage": "yarn workspaces foreach -piv --all run test-coverage",
     "test:watch": "node node_modules/jest/bin/jest.js --watch",
-    "postinstall": "husky install && yarn workspaces foreach -piv --all run postinstall",
+    "postinstall": "husky install",
     "release": "yarn clean && yarn build && yarn workspace @celo/celocli run prepack && yarn cs publish",
     "version-and-reinstall": "yarn changeset version && yarn install --no-immutable",
     "celocli": "yarn workspace @celo/celocli run --silent celocli"

packages/cli/src/base-l2.test.ts

@@ -2,12 +2,14 @@ import { Connection } from '@celo/connect'
 import { testWithAnvilL2 } from '@celo/dev-utils/lib/anvil-test'
 import * as WalletLedgerExports from '@celo/wallet-ledger'
 import { Config, ux } from '@oclif/core'
+import http from 'http'
 import { tmpdir } from 'os'
 import Web3 from 'web3'
 import { BaseCommand } from './base'
 import Set from './commands/config/set'
 import CustomHelp from './help'
 import { stripAnsiCodesFromNestedArray, testLocallyWithWeb3Node } from './test-utils/cliUtils'
+import * as config from './utils/config'
 import { readConfig } from './utils/config'
 
 process.env.NO_SYNCCHECK = 'true'
@@ -75,8 +77,14 @@ describe('flags', () => {
   })
 })
 
+// Make sure telemetry tests are deterministic, otherwise we'd have to update tests every release
+jest.mock('../package.json', () => ({
+  version: '5.2.3',
+}))
+
 testWithAnvilL2('BaseCommand', (web3: Web3) => {
   const logSpy = jest.spyOn(console, 'log').mockImplementation()
+
   beforeEach(() => {
     logSpy.mockClear()
   })
@@ -289,4 +297,183 @@ testWithAnvilL2('BaseCommand', (web3: Web3) => {
       ]
     `)
   })
+
+  describe('telemetry', () => {
+    afterEach(() => {
+      jest.clearAllMocks()
+      jest.restoreAllMocks()
+    })
+
+    it('sends telemetry data successfuly on success', async () => {
+      class TestTelemetryCommand extends BaseCommand {
+        id = 'test:telemetry-success'
+
+        async run() {
+          console.log('Successful run')
+        }
+      }
+
+      // here we test also that it works without this env var
+      delete process.env.TELEMETRY_ENABLED
+      process.env.TELEMETRY_URL = 'https://telemetry.example.org'
+
+      jest.spyOn(config, 'readConfig').mockImplementation((_: string) => {
+        return { telemetry: true } as config.CeloConfig
+      })
+
+      const fetchMock = jest.fn().mockResolvedValue({
+        ok: true,
+      })
+      const fetchSpy = jest.spyOn(global, 'fetch').mockImplementation(fetchMock)
+
+      await TestTelemetryCommand.run([])
+
+      // Assert it was called at all in the first place
+      expect(fetchSpy.mock.calls.length).toEqual(1)
+
+      expect(fetchSpy.mock.calls[0][0]).toMatchInlineSnapshot(`"https://telemetry.example.org"`)
+      expect(fetchSpy.mock.calls[0][1]?.body).toMatchInlineSnapshot(`
+        "
+        celocli_invocation{success="true", version="5.2.3", command="test:telemetry-success"} 1
+        "
+      `)
+      expect(fetchSpy.mock.calls[0][1]?.headers).toMatchInlineSnapshot(`
+        {
+          "Content-Type": "application/octet-stream",
+        }
+      `)
+      expect(fetchSpy.mock.calls[0][1]?.method).toMatchInlineSnapshot(`"POST"`)
+      expect(fetchSpy.mock.calls[0][1]?.signal).toBeInstanceOf(AbortSignal)
+      // Make sure the request was not aborted
+      expect(fetchSpy.mock.calls[0][1]?.signal?.aborted).toBe(false)
+    })
+
+    it('sends telemetry data successfuly on error', async () => {
+      class TestTelemetryCommand extends BaseCommand {
+        id = 'test:telemetry-error'
+
+        async run() {
+          throw new Error('test error')
+        }
+      }
+
+      jest.spyOn(config, 'readConfig').mockImplementation((_: string) => {
+        return { telemetry: true } as config.CeloConfig
+      })
+
+      // here we test also that it works with this env var set to 1 explicitly
+      process.env.TELEMETRY_ENABLED = '1'
+      process.env.TELEMETRY_URL = 'https://telemetry.example.org'
+
+      const fetchMock = jest.fn().mockResolvedValue({
+        ok: true,
+      })
+      const fetchSpy = jest.spyOn(global, 'fetch').mockImplementation(fetchMock)
+
+      await expect(TestTelemetryCommand.run([])).rejects.toMatchInlineSnapshot(
+        `[Error: test error]`
+      )
+
+      // Assert it was called at all in the first place
+      expect(fetchSpy.mock.calls.length).toEqual(1)
+
+      expect(fetchSpy.mock.calls[0][0]).toMatchInlineSnapshot(`"https://telemetry.example.org"`)
+      expect(fetchSpy.mock.calls[0][1]?.body).toMatchInlineSnapshot(`
+        "
+        celocli_invocation{success="false", version="5.2.3", command="test:telemetry-error"} 1
+        "
+      `)
+      expect(fetchSpy.mock.calls[0][1]?.headers).toMatchInlineSnapshot(`
+        {
+          "Content-Type": "application/octet-stream",
+        }
+      `)
+      expect(fetchSpy.mock.calls[0][1]?.method).toMatchInlineSnapshot(`"POST"`)
+      expect(fetchSpy.mock.calls[0][1]?.signal).toBeInstanceOf(AbortSignal)
+      // Make sure the request was not aborted
+      expect(fetchSpy.mock.calls[0][1]?.signal?.aborted).toBe(false)
+    })
+
+    it('does not send telemetry when disabled by config', async () => {
+      class TestTelemetryCommand extends BaseCommand {
+        id = 'test:telemetry-should-not-be-sent'
+
+        async run() {
+          console.log('Successful run')
+        }
+      }
+
+      jest.spyOn(config, 'readConfig').mockImplementation((_: string) => {
+        return { telemetry: false } as config.CeloConfig
+      })
+
+      // we leave it here to double check that it is not sent even if the env var is set
+      process.env.TELEMETRY_ENABLED = '1'
+      process.env.TELEMETRY_URL = 'https://telemetry.example.org'
+
+      const fetchMock = jest.fn().mockResolvedValue({
+        ok: true,
+      })
+      const fetchSpy = jest.spyOn(global, 'fetch').mockImplementation(fetchMock)
+
+      await TestTelemetryCommand.run([])
+
+      expect(fetchSpy).not.toHaveBeenCalled()
+    })
+
+    it('times out after TIMEOUT', async () => {
+      return new Promise<void>((resolve, _) => {
+        const EXPECTED_COMMAND_RESULT = 'Successful run'
+
+        class TestTelemetryCommand extends BaseCommand {
+          id = 'test:telemetry-timeout'
+
+          async run() {
+            return EXPECTED_COMMAND_RESULT
+          }
+        }
+
+        jest.spyOn(config, 'readConfig').mockImplementation((_: string) => {
+          return { telemetry: true } as config.CeloConfig
+        })
+
+        delete process.env.TELEMETRY_ENABLED
+        process.env.TELEMETRY_URL = 'http://localhost:3000/'
+
+        const fetchSpy = jest.spyOn(global, 'fetch')
+
+        const server = http.createServer((_, res) => {
+          setTimeout(() => {
+            res.end()
+          }, 5000) // Higher timeout than the telemetry logic uses
+        })
+
+        server.listen(3000, async () => {
+          // Make sure the command actually returns
+          await expect(TestTelemetryCommand.run([])).resolves.toBe(EXPECTED_COMMAND_RESULT)
+
+          expect(fetchSpy.mock.calls.length).toEqual(1)
+
+          expect(fetchSpy.mock.calls[0][0]).toMatchInlineSnapshot(`"http://localhost:3000/"`)
+          expect(fetchSpy.mock.calls[0][1]?.body).toMatchInlineSnapshot(`
+            "
+            celocli_invocation{success="true", version="5.2.3", command="test:telemetry-timeout"} 1
+            "
+          `)
+          expect(fetchSpy.mock.calls[0][1]?.headers).toMatchInlineSnapshot(`
+                    {
+                      "Content-Type": "application/octet-stream",
+                    }
+                `)
+          expect(fetchSpy.mock.calls[0][1]?.method).toMatchInlineSnapshot(`"POST"`)
+          expect(fetchSpy.mock.calls[0][1]?.signal).toBeInstanceOf(AbortSignal)
+          // Make sure the request was aborted
+          expect(fetchSpy.mock.calls[0][1]?.signal?.aborted).toBe(true)
+
+          server.close()
+          resolve()
+        })
+      })
+    })
+  })
 })

packages/cli/src/base.ts

@@ -15,6 +15,7 @@ import { CustomFlags } from './utils/command'
 import { getDefaultDerivationPath, getNodeUrl } from './utils/config'
 import { getFeeCurrencyContractWrapper } from './utils/fee-currency'
 import { requireNodeIsSynced } from './utils/helpers'
+import { reportUsageStatisticsIfTelemetryEnabled } from './utils/telemetry'
 
 export abstract class BaseCommand extends Command {
   static flags: FlagInput = {
@@ -247,6 +248,8 @@ export abstract class BaseCommand extends Command {
 
   async finally(arg: Error | undefined): Promise<any> {
     try {
+      await reportUsageStatisticsIfTelemetryEnabled(this.config.configDir, !arg, this.id)
+
       if (arg) {
         if (!(arg instanceof CLIError)) {
           console.error(

packages/cli/src/commands/config/get.test.ts

@@ -14,10 +14,11 @@ testWithAnvilL2('config:get cmd', (web3: Web3) => {
   it('shows the config', async () => {
     const logMock = jest.spyOn(console, 'log').mockImplementation()
     await testLocallyWithWeb3Node(Get, [], web3)
-    expect(stripAnsiCodesAndTxHashes(logMock.mock.calls[0][0].replace(/:\d\d\d\d/, ':PORT')))
+    expect(stripAnsiCodesAndTxHashes(logMock.mock.calls[0][0].replace(/:\d+/, ':PORT')))
       .toMatchInlineSnapshot(`
       "node: http://127.0.0.1:PORT
-      derivationPath: m/44'/52752'/0'"
+      derivationPath: m/44'/52752'/0'
+      telemetry: true"
     `)
   })
 })

packages/cli/src/commands/config/set.test.ts

@@ -3,17 +3,32 @@ import { testWithAnvilL1 } from '@celo/dev-utils/lib/anvil-test'
 import { ux } from '@oclif/core'
 import Web3 from 'web3'
 import { stripAnsiCodesFromNestedArray, testLocallyWithWeb3Node } from '../../test-utils/cliUtils'
+import * as cliUtils from '../../utils/cli'
 import * as config from '../../utils/config'
+import Get from './get'
 import Set from './set'
 
 process.env.NO_SYNCCHECK = 'true'
 
-afterEach(async () => {
-  jest.clearAllMocks()
-  jest.restoreAllMocks()
-})
-
 testWithAnvilL1('config:set cmd', (web3: Web3) => {
+  afterEach(async () => {
+    jest.clearAllMocks()
+    jest.restoreAllMocks()
+
+    // cleanup to defaults after each test to have deterministic results
+    await testLocallyWithWeb3Node(
+      Set,
+      [
+        '--telemetry',
+        config.defaultConfig.telemetry ? '1' : '0',
+        '--derivationPath',
+        config.defaultConfig.derivationPath,
+        // node is injected by default by testLocallyWithWeb3Node
+      ],
+      web3
+    )
+  })
+
   describe('--derivationPath', () => {
     it('sets with bip44 path', async () => {
       const writeMock = jest.spyOn(config, 'writeConfig')
@@ -79,9 +94,57 @@ testWithAnvilL1('config:set cmd', (web3: Web3) => {
         ],
       ]
     `)
+
     expect(writeMock).toHaveBeenCalledTimes(1)
     expect(writeMock.mock.calls[0][0]).toMatch('.config/@celo/celocli')
     expect(writeMock.mock.calls[0][1]).toMatchObject({ derivationPath: "m/44'/52752'/0'" })
     expect(writeMock.mock.calls[0][1]).not.toHaveProperty('gasCurrency')
   })
+
+  it('allows to disable telemetry', async () => {
+    const writeMock = jest.spyOn(config, 'writeConfig')
+    const configDir = '.config/@celo/celocli'
+    const printValueMapSpy = jest.spyOn(cliUtils, 'printValueMap')
+
+    await testLocallyWithWeb3Node(Set, ['--telemetry', '0'], web3)
+
+    expect(writeMock).toHaveBeenCalledTimes(1)
+    expect(writeMock.mock.calls[0][0]).toMatch(configDir)
+    expect(writeMock.mock.calls[0][1]).toMatchObject({
+      telemetry: false,
+    })
+
+    await testLocallyWithWeb3Node(Get, [], web3)
+    expect(printValueMapSpy).toHaveBeenCalledTimes(1)
+    expect(printValueMapSpy.mock.calls[0][0].telemetry).toEqual(false)
+
+    // Setting other config value should not change telemetry
+    // In this case --node flag is passed by default so we don't
+    // need to specify any other flags
+    await testLocallyWithWeb3Node(Set, [], web3)
+
+    expect(writeMock).toHaveBeenCalledTimes(2)
+    expect(writeMock.mock.calls[1][0]).toMatch(configDir)
+    expect(writeMock.mock.calls[1][1]).toMatchObject({
+      telemetry: false,
+    })
+
+    // Check that it's not overwritten
+    await testLocallyWithWeb3Node(Get, [], web3)
+    expect(printValueMapSpy).toHaveBeenCalledTimes(2)
+    expect(printValueMapSpy.mock.calls[1][0].telemetry).toEqual(false)
+
+    // Now let's check if we can enable it back
+    await testLocallyWithWeb3Node(Set, ['--telemetry', '1'], web3)
+
+    expect(writeMock).toHaveBeenCalledTimes(3)
+    expect(writeMock.mock.calls[2][0]).toMatch(configDir)
+    expect(writeMock.mock.calls[2][1]).toMatchObject({
+      telemetry: true,
+    })
+
+    await testLocallyWithWeb3Node(Get, [], web3)
+    expect(printValueMapSpy).toHaveBeenCalledTimes(3)
+    expect(printValueMapSpy.mock.calls[2][0].telemetry).toEqual(true)
+  })
 })