fix(cli): Telemetry improvements

packages/cli/src/index.js

@@ -8,6 +8,7 @@ import { config } from 'dotenv-defaults'
 import { hideBin, Parser } from 'yargs/helpers'
 import yargs from 'yargs/yargs'
 
+import { recordTelemetryAttributes } from '@redwoodjs/cli-helpers'
 import { telemetryMiddleware } from '@redwoodjs/telemetry'
 
 import * as buildCommand from './commands/build'
@@ -59,10 +60,10 @@ import { startTelemetry, shutdownTelemetry } from './telemetry/index'
 // yarn rw info
 // ```
 
-// Telemetry is enabled by default, but can be disabled in two ways
-// - by passing a `--telemetry false` option
-// - by setting a `REDWOOD_DISABLE_TELEMETRY` env var
-let { cwd, telemetry } = Parser(hideBin(process.argv), {
+let { cwd, telemetry, help, version } = Parser(hideBin(process.argv), {
+  // Telemetry is enabled by default, but can be disabled in two ways
+  // - by passing a `--telemetry false` option
+  // - by setting a `REDWOOD_DISABLE_TELEMETRY` env var
   boolean: ['telemetry'],
   default: {
     telemetry:
@@ -119,17 +120,34 @@ async function main() {
   // Execute CLI within a span, this will be the root span
   const tracer = trace.getTracer('redwoodjs')
   await tracer.startActiveSpan('cli', async (span) => {
+    // Ensure telemetry ends after a maximum of 5 minutes
+    const timeoutTimer = setTimeout(() => {
+      shutdownTelemetry()
+    }, 300000)
+
+    // Record if --help or --version were given because we will never hit a handler which will specify the command
+    if (version) {
+      recordTelemetryAttributes({ command: '--version' })
+    }
+    if (help) {
+      recordTelemetryAttributes({ command: '--help' })
+    }
+
     try {
       // Run the command via yargs
       await runYargs()
-
-      // Span housekeeping
-      span?.setStatus({ code: SpanStatusCode.OK })
     } catch (error) {
       exitWithError(error)
-    } finally {
+    }
+
+    // Span housekeeping
+    if (span?.isRecording()) {
+      span?.setStatus({ code: SpanStatusCode.OK })
       span?.end()
     }
+
+    // Clear the timeout timer since we haven't timed out
+    clearTimeout(timeoutTimer)
   })
 
   // Shutdown telemetry, ensures data is sent before the process exits

packages/cli/src/telemetry/index.js

@@ -5,6 +5,7 @@ import opentelemetry from '@opentelemetry/api'
 import {
   NodeTracerProvider,
   SimpleSpanProcessor,
+  SamplingDecision,
 } from '@opentelemetry/sdk-trace-node'
 
 import { spawnBackgroundProcess } from '../lib/background'
@@ -46,12 +47,26 @@ export async function startTelemetry() {
     diag.setLogger(new DiagConsoleLogger(), DiagLogLevel.ERROR)
 
     // Tracing
-    traceProvider = new NodeTracerProvider()
+    traceProvider = new NodeTracerProvider({
+      sampler: {
+        shouldSample: () => {
+          return {
+            decision: isShutdown
+              ? SamplingDecision.NOT_RECORD
+              : SamplingDecision.RECORD_AND_SAMPLED,
+          }
+        },
+        toString: () => {
+          return 'AlwaysSampleWhenNotShutdown'
+        },
+      },
+    })
     traceExporter = new CustomFileExporter()
     traceProcessor = new SimpleSpanProcessor(traceExporter)
     traceProvider.addSpanProcessor(traceProcessor)
     traceProvider.register()
 
+    // Ensure to shutdown telemetry when the process exits
     process.on('exit', () => {
       shutdownTelemetry()
     })

packages/cli/src/telemetry/send.js

@@ -9,6 +9,14 @@ import { getPaths } from '@redwoodjs/project-config'
 import { getResources } from './resource'
 
 async function main() {
+  // Log out the telemetry notice
+  console.log(
+    "You can disable telemetry by:\n - setting the 'REDWOOD_DISABLE_TELEMETRY' environment variable\n - passing the '--no-telemetry' flag when using the CLI"
+  )
+  console.log(
+    'Information about Redwood telemetry can be found at:\n - https://telemetry.redwoodjs.com\n'
+  )
+
   // Get all telemetry files
   const telemetryDir = path.join(getPaths().generated.base, 'telemetry')
   fs.ensureDirSync(telemetryDir)
@@ -50,6 +58,14 @@ async function main() {
       continue
     }
 
+    if (!Array.isArray(spans)) {
+      console.error(
+        `Telemetry file '${file}' does not contain an array of spans. Deleting this file to prevent further errors.`
+      )
+      fs.unlinkSync(path.join(telemetryDir, file))
+      continue
+    }
+
     /**
      * We have to fix some of the span properties because we serialized the span
      * to JSON and then deserialized it. This means that some of the properties that
@@ -88,29 +104,19 @@ async function main() {
   // Shutdown to ensure all spans are sent
   traceExporter.shutdown()
 
-  // Verbose means we keep the last 8 telemetry files as a log otherwise we delete all of them
-  if (process.env.REDWOOD_VERBOSE_TELEMETRY) {
-    console.log(
-      "Keeping only the last 8 telemetry files for inspection because 'REDWOOD_VERBOSE_TELEMETRY' is set."
-    )
-    const sortedTelemetryFiles = telemetryFiles.sort((a, b) => {
-      return (
-        parseInt(b.split('.')[0].replace('_', '')) -
-        parseInt(a.split('.')[0].replace('_', ''))
-      )
-    })
-    for (let i = 8; i < sortedTelemetryFiles.length; i++) {
-      console.log(`Removing telemetry file '${sortedTelemetryFiles[i]}'`)
-      fs.unlinkSync(path.join(telemetryDir, sortedTelemetryFiles[i]))
-    }
-  } else {
-    console.log(
-      "Removing telemetry files, set 'REDWOOD_VERBOSE_TELEMETRY' to keep the last 8 telemetry files for inspection."
+  // We keep the last 8 telemetry files for visibility/transparency
+  console.log(
+    'Keeping the lastest 8 telemetry files for visibility/transparency.'
+  )
+  const sortedTelemetryFiles = telemetryFiles.sort((a, b) => {
+    return (
+      parseInt(b.split('.')[0].replace('_', '')) -
+      parseInt(a.split('.')[0].replace('_', ''))
     )
-    telemetryFiles.forEach((file) => {
-      console.log(`Removing telemetry file '${file}'`)
-      fs.unlinkSync(path.join(telemetryDir, file))
-    })
+  })
+  for (let i = 8; i < sortedTelemetryFiles.length; i++) {
+    console.log(`Removing telemetry file '${sortedTelemetryFiles[i]}'`)
+    fs.unlinkSync(path.join(telemetryDir, sortedTelemetryFiles[i]))
   }
 }