Skip to content
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

[WIP] Telemetry #465

Draft
wants to merge 22 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 18 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 9 additions & 3 deletions docs/command-line-interface/config.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,13 @@ Configure running node information for propagating transactions to network

```
USAGE
$ celocli config:set [-n <value>] [--globalHelp]
$ celocli config:set [-n <value>] [--globalHelp] [--telemetry 1|0]

FLAGS
-n, --node=<value> URL of the node to run commands against or an alias
--globalHelp View all available global flags
-n, --node=<value> URL of the node to run commands against or an alias
--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
Expand All @@ -66,6 +68,10 @@ EXAMPLES

set --node <geth-location>/geth.ipc

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

Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
181 changes: 181 additions & 0 deletions packages/cli/src/base-l2.test.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import { Connection } from '@celo/connect'
import { testWithAnvilL2 } from '@celo/dev-utils/lib/anvil-test'
import { Config, ux } from '@oclif/core'
import http from 'http'
import Web3 from 'web3'
import { BaseCommand } from './base'
import CustomHelp from './help'
import { stripAnsiCodesFromNestedArray, testLocallyWithWeb3Node } from './test-utils/cliUtils'
import * as config from './utils/config'

process.env.NO_SYNCCHECK = 'true'

Expand Down Expand Up @@ -51,7 +53,16 @@ describe('flags', () => {
})
})

jest.mock('../package.json', () => ({
version: '5.2.3',
}))

testWithAnvilL2('BaseCommand', (web3: Web3) => {
afterEach(() => {
jest.clearAllMocks()
jest.restoreAllMocks()
})

it('logs command execution error', async () => {
class TestErrorCommand extends BaseCommand {
async run() {
Expand Down Expand Up @@ -112,4 +123,174 @@ testWithAnvilL2('BaseCommand', (web3: Web3) => {
]
`)
})

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.com'

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.com"`)
expect(fetchSpy.mock.calls[0][1]?.body).toMatchInlineSnapshot(`
"
test_pag_celocli{success="true", version="5.2.3", command="test:telemetry-success", network="alfajores"} 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.com'

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.com"`)
expect(fetchSpy.mock.calls[0][1]?.body).toMatchInlineSnapshot(`
"
test_pag_celocli{success="false", version="5.2.3", command="test:telemetry-error", network="alfajores"} 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.com'

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(`
"
test_pag_celocli{success="true", version="5.2.3", command="test:telemetry-timeout", network="alfajores"} 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()
})
})
})
})
3 changes: 3 additions & 0 deletions packages/cli/src/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { CustomFlags } from './utils/command'
import { 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 = {
Expand Down Expand Up @@ -245,6 +246,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(
Expand Down
64 changes: 64 additions & 0 deletions packages/cli/src/commands/config/set-l2.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
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(`
{
"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(`
{
"node": "${extractHostFromWeb3(web3)}",
"telemetry": false,
}
`)
})
})
1 change: 1 addition & 0 deletions packages/cli/src/commands/config/set.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ testWithAnvilL1('config:set cmd', (web3: Web3) => {
expect(writeMock.mock.calls[0][1]).toMatchInlineSnapshot(`
{
"node": "${extractHostFromWeb3(web3)}",
"telemetry": true,
}
`)
})
Expand Down
11 changes: 10 additions & 1 deletion packages/cli/src/commands/config/set.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ux } from '@oclif/core'
import { Flags, ux } from '@oclif/core'
import chalk from 'chalk'
import { BaseCommand } from '../../base'
import { CeloConfig, readConfig, writeConfig } from '../../utils/config'
Expand All @@ -12,6 +12,11 @@ export default class Set extends BaseCommand {
...BaseCommand.flags.node,
hidden: false,
},
telemetry: Flags.string({
options: ['1', '0'],
description: 'Whether to enable or disable telemetry',
required: false,
}),
}

static examples = [
Expand All @@ -23,6 +28,8 @@ export default class Set extends BaseCommand {
'set --node local # alias for http://localhost:8545',
'set --node ws://localhost:2500',
'set --node <geth-location>/geth.ipc',
'set --telemetry 0 # disable telemetry',
'set --telemetry 1 # enable telemetry',
]

requireSynced = false
Expand All @@ -31,6 +38,7 @@ export default class Set extends BaseCommand {
const res = await this.parse(Set)
const curr = readConfig(this.config.configDir)
const node = res.flags.node ?? curr.node
const telemetry = res.flags.telemetry === '0' ? false : true
const gasCurrency = res.flags.gasCurrency

if (gasCurrency) {
Expand All @@ -43,6 +51,7 @@ export default class Set extends BaseCommand {

await writeConfig(this.config.configDir, {
node,
telemetry,
} as CeloConfig)
}
}
3 changes: 3 additions & 0 deletions packages/cli/src/scripts/postinstall.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import { printTelemetryInformation } from '../utils/telemetry'

printTelemetryInformation()
2 changes: 2 additions & 0 deletions packages/cli/src/test-utils/setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,5 @@ jest.mock('@ledgerhq/hw-transport-node-hid', () => {
},
}
})

process.env.TELEMETRY_ENABLED = '0'
aaronmgdr marked this conversation as resolved.
Show resolved Hide resolved
Loading
Loading