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

feat(crwa): OpenTelemetry #7455

Closed
wants to merge 9 commits into from
9 changes: 7 additions & 2 deletions packages/create-redwood-app/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,20 @@
"@babel/core": "7.20.12",
"@babel/node": "7.20.7",
"@babel/runtime-corejs3": "7.20.7",
"@redwoodjs/internal": "3.2.0",
"@redwoodjs/telemetry": "3.2.0",
"@opentelemetry/api": "1.4.0",
"@opentelemetry/exporter-trace-otlp-http": "0.35.0",
"@opentelemetry/resources": "1.9.0",
"@opentelemetry/sdk-trace-base": "1.9.0",
"@opentelemetry/semantic-conventions": "1.9.0",
"chalk": "4.1.2",
"check-node-version": "4.2.1",
"core-js": "3.27.2",
"enquirer": "2.3.6",
"envinfo": "7.8.1",
"execa": "5.1.1",
"fs-extra": "11.1.0",
"listr2": "5.0.7",
"systeminformation": "5.17.3",
"terminal-link": "2.1.1",
"yargs": "17.6.2"
},
Expand Down
105 changes: 43 additions & 62 deletions packages/create-redwood-app/src/create-redwood-app.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,8 @@
// Usage:
// `$ yarn create redwood-app ./path/to/new-project`

import { spawn } from 'child_process'
import path from 'path'

import chalk from 'chalk'
import checkNodeVersion from 'check-node-version'
import { prompt } from 'enquirer'
import execa from 'execa'
import fs from 'fs-extra'
import { Listr, figures } from 'listr2'
import terminalLink from 'terminal-link'
import { hideBin } from 'yargs/helpers'
import yargs from 'yargs/yargs'

import { name, version } from '../package'

/**
* To keep a consistent color/style palette between cli packages, such as
* @redwood/create-redwood-app and @redwood/cli, please keep them compatible
Expand All @@ -33,7 +20,31 @@ import { name, version } from '../package'
* - packages/create-redwood-app/src/create-redwood-app.js (this file)
*
*/
import chalk from 'chalk'
import checkNodeVersion from 'check-node-version'
import { prompt } from 'enquirer'
import execa from 'execa'
import fs from 'fs-extra'
import { Listr, figures } from 'listr2'
import terminalLink from 'terminal-link'
import { hideBin } from 'yargs/helpers'
import yargs from 'yargs/yargs'

import { name, version } from '../package'

import { startTelemetry, shutdownTelemetry } from './telemetry'
;(async () => {
//

// Telemetry
if (
!process.argv.includes('--no-telemetry') && // Must include '--no-telemetry' exactly because we want to do this check before any yargs. TODO: Communicate this on cmd help
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is a little too long to be on the same line as the code:

Suggested change
!process.argv.includes('--no-telemetry') && // Must include '--no-telemetry' exactly because we want to do this check before any yargs. TODO: Communicate this on cmd help
// Must include '--no-telemetry' exactly because we want to do this check before any yargs.
// TODO: Communicate this on cmd help
!process.argv.includes('--no-telemetry') &&

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had the same problem in the CLI's index.js file. I need the value of the cwd option before I invoke yargs. You should be able to use the same solution I did:

let { cwd } = Parser(hideBin(process.argv))

So in your case...

import { hideBin, Parser } from 'yargs/helpers'

// ...

const { telemetry } = Parser(hideBin(process.argv))

Then check if telemetry is false later, no need to deal with the "no-" prefix, etc.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent I had a suspicion that it might have been a similar issue you'd dealt. I'll try out your solution for sure.

!process.env.REDWOOD_DISABLE_TELEMETRY // We should use the same condition as in full projects here too
) {
// Setup and start root span
await startTelemetry()
}

// Styles for terminal
const style = {
error: chalk.bold.red,
Expand Down Expand Up @@ -68,7 +79,6 @@ import { name, version } from '../package'
'yarn-install': yarnInstall,
typescript,
overwrite,
telemetry: telemetry,
yarn1,
'git-init': gitInit,
} = yargs(hideBin(process.argv))
Expand All @@ -92,12 +102,6 @@ import { name, version } from '../package'
type: 'boolean',
describe: "Create even if target directory isn't empty",
})
.option('telemetry', {
default: true,
type: 'boolean',
describe:
'Enables sending telemetry events for this create command and all Redwood CLI commands https://telemetry.redwoodjs.com',
})
Comment on lines -95 to -100
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using the suggestion in another comment, you can leave this here. Solves the documentation problem!

.option('yarn1', {
default: false,
type: 'boolean',
Expand Down Expand Up @@ -130,6 +134,9 @@ import { name, version } from '../package'
'my-redwood-app'
)}`
)
await shutdownTelemetry({
exception: new Error('no target directory specified'),
})
process.exit(1)
}

Expand All @@ -152,7 +159,9 @@ import { name, version } from '../package'
`\n'${newAppDir}' already exists and is not empty\n`
)
)
process.exit(1)
shutdownTelemetry({
exception: new Error('target directory not empty'),
}).finally(() => process.exit(1))
}
} else {
fs.ensureDirSync(path.dirname(newAppDir))
Expand Down Expand Up @@ -220,42 +229,6 @@ import { name, version } from '../package'
]
}

const sendTelemetry = ({ error } = {}) => {
// send 'create' telemetry event, or disable for new app
if (telemetry) {
const command = process.argv
// make command show 'create redwood-app [path] --flags'
command.splice(2, 0, 'create', 'redwood-app')
command[4] = '[path]'

let args = [
'--root',
newAppDir,
'--argv',
JSON.stringify(command),
'--duration',
Date.now() - startTime,
'--rwVersion',
version,
]
if (error) {
args = [...args, '--error', `"${error}"`]
}

spawn(process.execPath, [path.join(__dirname, 'telemetry.js'), ...args], {
detached: process.env.REDWOOD_VERBOSE_TELEMETRY ? false : true,
stdio: process.env.REDWOOD_VERBOSE_TELEMETRY ? 'inherit' : 'ignore',
}).unref()
} else {
fs.appendFileSync(
path.join(newAppDir, '.env'),
'REDWOOD_DISABLE_TELEMETRY=1\n'
)
}
}

const startTime = Date.now()

// Engine check Listr. Separate Listr to avoid https://github.com/cenk1cenk2/listr2/issues/296
// Boolean flag
let hasPassedEngineCheck = null
Expand Down Expand Up @@ -327,10 +300,18 @@ import { name, version } from '../package'
message: 'How would you like to proceed?',
choices: ['Override error and continue install', 'Quit install'],
initial: 0,
onCancel: () => process.exit(1),
onCancel: async () => {
await shutdownTelemetry({
exception: new Error('cancelled engine override'),
})
process.exit(1)
},
})
// Quit the install if user selects this option, otherwise it will proceed
if (response['override-engine-error'] === 'Quit install') {
await shutdownTelemetry({
exception: new Error('quit install at engine override'),
})
process.exit(1)
}
}
Expand Down Expand Up @@ -430,8 +411,6 @@ import { name, version } from '../package'
)
.run()
.then(() => {
sendTelemetry()

// zOMG the semicolon below is a real Prettier thing. What??
// https://prettier.io/docs/en/rationale.html#semicolons
;[
Expand Down Expand Up @@ -482,11 +461,13 @@ import { name, version } from '../package'
`${style.redwood(` > ${style.green(`yarn rw dev`)}`)}`,
'',
].map((item) => console.log(item))
shutdownTelemetry().then(() => {
process.exit(0)
})
})
.catch((e) => {
console.log()
console.log(e)
sendTelemetry({ error: e.message })

if (fs.existsSync(newAppDir)) {
console.log(
Expand All @@ -497,6 +478,6 @@ import { name, version } from '../package'
)
)
}
process.exit(1)
shutdownTelemetry({ exception: e }).finally(() => process.exit(1))
})
})()
119 changes: 114 additions & 5 deletions packages/create-redwood-app/src/telemetry.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,115 @@
const { sendTelemetry } = require('@redwoodjs/telemetry/dist/sendTelemetry')
import { diag, DiagConsoleLogger, DiagLogLevel } from '@opentelemetry/api'
import opentelemetry, { SpanStatusCode } from '@opentelemetry/api'
import { OTLPTraceExporter } from '@opentelemetry/exporter-trace-otlp-http'
import { Resource } from '@opentelemetry/resources'
import { BatchSpanProcessor } from '@opentelemetry/sdk-trace-base'
import { NodeTracerProvider } from '@opentelemetry/sdk-trace-node'
import { SemanticResourceAttributes } from '@opentelemetry/semantic-conventions'
import envinfo from 'envinfo'
import system from 'systeminformation'

// if this script is run directly by node then telemetry will be sent in immediately
;(async function () {
await sendTelemetry()
})()
/**
* @type NodeTracerProvider
*/
let traceProvider

/**
* @type BatchSpanProcessor
*/
let traceProcessor

/**
* @type OTLPTraceExporter
*/
let traceExporter

/**
* @type Span
*/
let rootSpan

export async function startTelemetry() {
// Logger
// TODO: This line should be removed or at least the log level raised after experimentation
diag.setLogger(new DiagConsoleLogger(), DiagLogLevel.INFO)

// Resources
const info = JSON.parse(
await envinfo.run(
{
System: ['OS', 'Shell'],
Binaries: ['Node', 'Yarn', 'npm'],
npmPackages: '@redwoodjs/*',
IDEs: ['VSCode'],
},
{ json: true }
)
)

// get shell name instead of path
const shell = info.System?.Shell // Windows doesn't always provide shell info, I guess
if (shell?.path?.match('/')) {
info.System.Shell.name = info.System.Shell.path.split('/').pop()
} else if (shell?.path.match('\\')) {
info.System.Shell.name = info.System.Shell.path.split('\\').pop()
}
const cpu = await system.cpu()
const mem = await system.mem()

const resource = Resource.default().merge(
new Resource({
[SemanticResourceAttributes.SERVICE_NAME]: 'create-redwood-app',
// TODO: Get a better way of recording what version of CRWA is running because this is just null...
[SemanticResourceAttributes.SERVICE_VERSION]:
info.npmPackages != null
? info.npmPackages['@redwoodjs/core']?.installed
: undefined,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason you can't just get it from this package's package.json? Like in create-redwood-app.js

import { name, version } from '../package'

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd been using that technique in my earlier experimenting and it worked fine. I only replaced it with a to-do because vscode was yelling at me. I'll happily add it back.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah gotcha—yelling at you with red squiggles?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah but it's fine now. I must have been importing it with the extension or something before.

[SemanticResourceAttributes.OS_TYPE]: info.System?.OS?.split(' ')[0],
[SemanticResourceAttributes.OS_VERSION]: info.System?.OS?.split(' ')[1],
'shell.name': info.System?.Shell?.name,
'node.version': info.Binaries?.Node?.version,
'yarn.version': info.Binaries?.Yarn?.version,
'npm.version': info.Binaries?.npm?.version,
'vscode.version': info.IDEs?.VSCode?.version,
'cpu.count': cpu.physicalCores,
'memory.gb': Math.round(mem.total / 1073741824),
})
)

// Tracing
traceProvider = new NodeTracerProvider({
resource: resource,
})
traceExporter = new OTLPTraceExporter({
// TODO: Point this to somewhere permanent
})
traceProcessor = new BatchSpanProcessor(traceExporter)
traceProvider.addSpanProcessor(traceProcessor)
traceProvider.register()

// Start root span
const tracer = opentelemetry.trace.getTracer('crwa-tracer')
rootSpan = tracer.startSpan('root', undefined, opentelemetry.context.active())
}

export async function shutdownTelemetry({ exception } = {}) {
if (rootSpan?.isRecording()) {
if (exception !== undefined) {
// TODO: Think about how best to redact this exception information
rootSpan.recordException(exception)
rootSpan.setStatus({
code: SpanStatusCode.ERROR,
message: exception.message,
})
}
rootSpan.end()
}
try {
await traceProvider?.shutdown()
await traceProcessor?.shutdown()
await traceExporter?.shutdown()
} catch (error) {
console.error('Telemetry error')
console.error(error)
}
}
Loading