feat(telemetry): send command opts and args (#7251)

* build: print `callCli` output when using DEBUG_TESTS

I don't know why we weren't doing this before. This is so helpful.

* feat(telemetry): send command opts and args

It's impossible for us currently to identify the potential impact of option deprecation.
This won't address that retroactively for past CLI versions, but at least it will give us
data going forward.

Attaching the `source` is important, otherwise this would include applied default values
and we wouldn't be able to distinguish this from actual user-provided values.

Unfortunately there doesn't appear to be a commander API to get named positional
arguments, so `args` is just an array. Seems ok.

* test: undo rejection check

This is different on Windows... bleh.

Author: serhalp

src/commands/base-command.ts

@@ -7,7 +7,7 @@ import { DefaultLogger, Project } from '@netlify/build-info'
 import { NodeFS, NoopLogger } from '@netlify/build-info/node'
 import { resolveConfig } from '@netlify/config'
 import { isCI } from 'ci-info'
-import { Command, Help, Option } from 'commander'
+import { Command, Help, Option, type OptionValues } from 'commander'
 import debug from 'debug'
 import { findUp } from 'find-up'
 import inquirer from 'inquirer'
@@ -75,6 +75,23 @@ const COMMANDS_WITHOUT_WORKSPACE_OPTIONS = new Set(['api', 'recipes', 'completio
  */
 const COMMANDS_WITH_FEATURE_FLAGS = new Set(['build', 'dev', 'deploy'])
 
+/**
+ * Names of options whose values should be scrubbed
+ */
+const SCRUBBED_OPTIONS = new Set(['auth'])
+
+const getScrubbedOptions = (command: BaseCommand): Record<string, { source: OptionValues['source']; value: unknown }> =>
+  Object.entries(command.optsWithGlobals()).reduce(
+    (acc: Record<string, { source: OptionValues['source']; value: unknown }>, [key, value]) => ({
+      ...acc,
+      [key]: {
+        source: command.getOptionValueSourceWithGlobals(key),
+        value: SCRUBBED_OPTIONS.has(key) ? '********' : value,
+      },
+    }),
+    {},
+  )
+
 /** Formats a help list correctly with the correct indent */
 const formatHelpList = (textArray: string[]) => textArray.join('\n').replace(/^/gm, ' '.repeat(HELP_INDENT_WIDTH))
 
@@ -626,6 +643,8 @@ export default class BaseCommand extends Command {
       monorepo: Boolean(this.project.workspace),
       packageManager: this.project.packageManager?.name,
       buildSystem: this.project.buildSystems.map(({ id }) => id),
+      opts: getScrubbedOptions(actionCommand),
+      args: actionCommand.args,
     })
 
     // set the project and the netlify api object on the command,

src/utils/telemetry/request.ts

@@ -1,5 +1,5 @@
 // This file is being called by `src/utils/telemetry/telemetry.js` as a child process
-// to run a s a detached process
+// to run as a detached process
 import process from 'process'
 
 import fetch from 'node-fetch'

src/utils/telemetry/telemetry.ts

@@ -12,7 +12,7 @@ import isValidEventName from './validation.js'
 
 const dirPath = dirname(fileURLToPath(import.meta.url))
 
-function send(type: 'track' | 'identify', payload: object) {
+function send(type: 'track' | 'identify', payload: Record<string, unknown>) {
   const requestFile = join(dirPath, 'request.js')
   const options = JSON.stringify({
     data: payload,

tests/integration/telemetry.test.ts

@@ -74,6 +74,8 @@ await withMockApi(routes, () => {
       didEnableCompileCache: expect.any(Boolean),
       monorepo: false,
       nodejsVersion,
+      opts: expect.any(Object),
+      args: expect.any(Array),
       // TODO: this should be NPM however some CI tests are using pnpm which changes the value
       packageManager: expect.stringMatching(/npm|pnpm/),
     })
@@ -96,6 +98,8 @@ await withMockApi(routes, () => {
       didEnableCompileCache: expect.any(Boolean),
       monorepo: false,
       nodejsVersion,
+      opts: expect.any(Object),
+      args: expect.any(Array),
       // TODO: this should be NPM however some CI tests are using pnpm which changes the value
       packageManager: expect.stringMatching(/npm|pnpm/),
     })
@@ -124,4 +128,47 @@ await withMockApi(routes, () => {
       )
     })
   })
+
+  test<MockApiTestContext>('should attach user options as `opts` and positional args as `args`', async ({
+    apiUrl,
+    requests,
+  }) => {
+    await expect(
+      callCli(['blobs:get', '--filter', 'web', '-O', './output_dir', 'my-store', 'my-key'], getCLIOptions(apiUrl)),
+    ).rejects.toThrowError(/You don't appear to be in a folder that is linked to a site/)
+    const request = requests.find(({ path }) => path === '/api/v1/track')
+    expect(request).toBeDefined()
+
+    expect(request!.body).toHaveProperty('event', 'cli:command')
+    expect(request!.body).toHaveProperty(
+      'properties',
+      expect.objectContaining({
+        command: 'blobs:get',
+        opts: {
+          filter: { source: 'cli', value: 'web' },
+          output: { source: 'cli', value: './output_dir' },
+        },
+        args: ['my-store', 'my-key'],
+      }),
+    )
+  })
+
+  test<MockApiTestContext>('should scrub values of sensitive `opts`', async ({ apiUrl, requests }) => {
+    await callCli(['api', 'listSites', '--auth', 'my-sensitive-token'], getCLIOptions(apiUrl))
+    const request = requests.find(({ path }) => path === '/api/v1/track')
+    expect(request).toBeDefined()
+
+    expect(request!.body).toHaveProperty('event', 'cli:command')
+    expect(request!.body).toHaveProperty(
+      'properties',
+      expect.objectContaining({
+        command: 'api',
+        opts: {
+          auth: { source: 'cli', value: '********' },
+          list: { source: 'default', value: false },
+        },
+        args: ['listSites'],
+      }),
+    )
+  })
 })

tests/integration/utils/call-cli.ts

@@ -17,11 +17,16 @@ export const callCli = async function (
   parseJson = false,
 ): // eslint-disable-next-line @typescript-eslint/no-explicit-any
 Promise<any> {
-  const { stdout } = await execa.node(cliPath, args, {
+  const { stdout, stderr } = await execa.node(cliPath, args, {
     timeout: CLI_TIMEOUT,
     nodeOptions: [],
     ...execOptions,
   })
+  if (process.env.DEBUG_TESTS) {
+    process.stdout.write(stdout)
+    process.stderr.write(stderr)
+  }
+
   if (parseJson) {
     return JSON.parse(stdout)
   }