[WIP] Telemetry

.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'
@@ -67,8 +69,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()
   })
@@ -281,4 +289,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 = {
@@ -252,6 +253,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/set-l2.test.ts

@@ -0,0 +1,66 @@
+import { newKitFromWeb3 } from '@celo/contractkit'
+import { testWithAnvilL2 } from '@celo/dev-utils/lib/anvil-test'
+import { ux } from '@oclif/core'
+import Web3 from 'web3'
+import {
+  extractHostFromWeb3,
+  stripAnsiCodesFromNestedArray,
+  testLocallyWithWeb3Node,
+} from '../../test-utils/cliUtils'
+import * as config from '../../utils/config'
+import Set from './set'
+
+process.env.NO_SYNCCHECK = 'true'
+
+afterEach(async () => {
+  jest.clearAllMocks()
+  jest.restoreAllMocks()
+})
+
+testWithAnvilL2('config:set cmd', (web3: Web3) => {
+  it('shows a warning if gasCurrency is passed', async () => {
+    const kit = newKitFromWeb3(web3)
+    const feeCurrencyWhitelist = await kit.contracts.getFeeCurrencyDirectory()
+    const consoleMock = jest.spyOn(ux, 'warn')
+    const writeMock = jest.spyOn(config, 'writeConfig')
+
+    await testLocallyWithWeb3Node(
+      Set,
+      ['--gasCurrency', (await feeCurrencyWhitelist.getCurrencies())[0]],
+      web3
+    )
+    expect(stripAnsiCodesFromNestedArray(consoleMock.mock.calls as string[][]))
+      .toMatchInlineSnapshot(`
+      [
+        [
+          "
+      Setting a default gasCurrency has been removed from the config, you may still use the --gasCurrency on every command.
+      Did you value this feature a lot and would like to see it back? Let us know at https://github.com/celo-org/developer-tooling/discussions/92",
+        ],
+      ]
+    `)
+    expect(writeMock.mock.calls[0][1]).toMatchInlineSnapshot(`
+      {
+        "derivationPath": "m/44'/52752'/0'",
+        "node": ${extractHostFromWeb3(web3)},
+        "telemetry": true,
+      }
+    `)
+  })
+
+  it('allows to disable telemetry', async () => {
+    const writeMock = jest.spyOn(config, 'writeConfig')
+
+    await testLocallyWithWeb3Node(Set, ['--telemetry', '0'], web3)
+
+    // --node is passed by testLocalWithWeb3Node, so it will
+    // in the config as well
+    expect(writeMock.mock.calls[0][1]).toMatchInlineSnapshot(`
+      {
+        "derivationPath": "m/44'/52752'/0'",
+        "node": ${extractHostFromWeb3(web3)},
+        "telemetry": false,
+      }
+    `)
+  })
+})

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

@@ -2,7 +2,11 @@ import { newKitFromWeb3 } from '@celo/contractkit'
 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 {
+  extractHostFromWeb3,
+  stripAnsiCodesFromNestedArray,
+  testLocallyWithWeb3Node,
+} from '../../test-utils/cliUtils'
 import * as config from '../../utils/config'
 import Set from './set'
 
@@ -79,6 +83,14 @@ testWithAnvilL1('config:set cmd', (web3: Web3) => {
         ],
       ]
     `)
+    expect(writeMock.mock.calls[0][1]).toMatchInlineSnapshot(`
+      {
+        "derivationPath": "m/44'/52752'/0'",
+        "node": ${extractHostFromWeb3(web3)},
+        "telemetry": true,
+      }
+    `)
+
     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'" })