Skip to content

Commit b784568

Browse files
authored
v3: no bundle builds for the dev CLI (#923)
* WIP no bundling * Convert dev CLI to use an unbundled build process to support otel instrumentation * Fix pnpm lock file * A couple of fixes to get typechecking to work
1 parent 52c9d48 commit b784568

File tree

18 files changed

+490
-95
lines changed

18 files changed

+490
-95
lines changed

apps/kubernetes-provider/package.json

+2-3
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,7 @@
1010
"build:bundle": "esbuild src/index.ts --bundle --outfile=dist/index.cjs --platform=node",
1111
"build:image": "docker build -f Containerfile . -t kubernetes-provider",
1212
"dev": "tsx --no-warnings=ExperimentalWarning --require dotenv/config --watch src/index.ts",
13-
"start": "tsx src/index.ts",
14-
"typecheck": "tsc --noEmit"
13+
"start": "tsx src/index.ts"
1514
},
1615
"keywords": [],
1716
"author": "",
@@ -28,4 +27,4 @@
2827
"tsx": "^4.7.0",
2928
"typescript": "^5.3.3"
3029
}
31-
}
30+
}

packages/cli-v3/package.json

+7-2
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,12 @@
3838
"@trigger.dev/core": "workspace:*",
3939
"@trigger.dev/tsconfig": "workspace:*",
4040
"@types/gradient-string": "^1.1.2",
41+
"@types/jsonlines": "^0.1.5",
4142
"@types/mock-fs": "^4.13.1",
4243
"@types/node": "18",
4344
"@types/object-hash": "^3.0.6",
4445
"@types/react": "^18.2.48",
46+
"@types/semver": "^7.3.13",
4547
"@types/ws": "^8.5.3",
4648
"cpy-cli": "^5.0.0",
4749
"npm-run-all": "^4.1.5",
@@ -59,7 +61,7 @@
5961
"build:prod-containerfile": "src/Containerfile.prod"
6062
},
6163
"scripts": {
62-
"typecheck": "tsc",
64+
"typecheck": "tsc -p tsconfig.check.json",
6365
"build": "npm run clean && run-p build:**",
6466
"build:main": "tsup",
6567
"build:facade": "tsup --config tsup.facade.config.ts",
@@ -95,6 +97,7 @@
9597
"@opentelemetry/semantic-conventions": "^1.21.0",
9698
"@traceloop/instrumentation-openai": "^0.3.9",
9799
"@trigger.dev/core": "workspace:*",
100+
"@trigger.dev/core-apps": "workspace:*",
98101
"@types/degit": "^2.8.3",
99102
"chalk": "^5.2.0",
100103
"chokidar": "^3.5.3",
@@ -104,11 +107,12 @@
104107
"dotenv": "^16.4.4",
105108
"esbuild": "^0.19.11",
106109
"evt": "^2.4.13",
107-
"execa": "^7.0.0",
110+
"execa": "^8.0.0",
108111
"find-up": "^7.0.0",
109112
"gradient-string": "^2.0.2",
110113
"import-meta-resolve": "^4.0.0",
111114
"ink": "^4.4.1",
115+
"jsonlines": "^0.1.1",
112116
"liquidjs": "^10.9.2",
113117
"mock-fs": "^5.2.0",
114118
"nanoid": "^4.0.2",
@@ -121,6 +125,7 @@
121125
"proxy-agent": "^6.3.0",
122126
"react": "^18.2.0",
123127
"react-error-boundary": "^4.0.12",
128+
"semver": "^7.5.0",
124129
"simple-git": "^3.19.0",
125130
"socket.io-client": "^4.7.4",
126131
"source-map-support": "^0.5.21",

packages/cli-v3/src/commands/dev.tsx

+65-13
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import {
1010
import chalk from "chalk";
1111
import { watch } from "chokidar";
1212
import { Command } from "commander";
13-
import { BuildContext, context } from "esbuild";
13+
import { BuildContext, Metafile, context } from "esbuild";
1414
import { resolve as importResolve } from "import-meta-resolve";
1515
import { Box, Text, render, useApp, useInput } from "ink";
1616
import { createHash } from "node:crypto";
@@ -23,14 +23,14 @@ import React, { Suspense, useEffect } from "react";
2323
import { ClientOptions, WebSocket as wsWebSocket } from "ws";
2424
import { z } from "zod";
2525
import * as packageJson from "../../package.json";
26+
import { CliApiClient } from "../apiClient";
27+
import { CommonCommandOptions } from "../cli/common.js";
2628
import { BackgroundWorker, BackgroundWorkerCoordinator } from "../dev/backgroundWorker.js";
29+
import { getConfigPath, readConfig } from "../utilities/configFiles";
2730
import { printStandloneInitialBanner } from "../utilities/initialBanner.js";
2831
import { logger } from "../utilities/logger.js";
2932
import { isLoggedIn } from "../utilities/session.js";
30-
import { CommonCommandOptions } from "../cli/common.js";
31-
import { getConfigPath, readConfig } from "../utilities/configFiles";
3233
import { createTaskFileImports, gatherTaskFiles } from "../utilities/taskFiles";
33-
import { CliApiClient } from "../apiClient";
3434

3535
let apiClient: CliApiClient | undefined;
3636

@@ -298,10 +298,14 @@ function useDev({
298298
new URL(importResolve("./worker-facade.js", import.meta.url)).href.replace("file://", ""),
299299
"utf-8"
300300
);
301-
const entryPointContents = workerFacade.replace(
302-
"__TASKS__",
303-
createTaskFileImports(taskFiles)
304-
);
301+
302+
const registerTracingPath = new URL(
303+
importResolve("./register-tracing.js", import.meta.url)
304+
).href.replace("file://", "");
305+
306+
const entryPointContents = workerFacade
307+
.replace("__TASKS__", createTaskFileImports(taskFiles))
308+
.replace("__REGISTER_TRACING__", `import { tracingSDK } from "${registerTracingPath}";`);
305309

306310
let firstBuild = true;
307311

@@ -318,17 +322,15 @@ function useDev({
318322
write: false,
319323
minify: false,
320324
sourcemap: "external", // does not set the //# sourceMappingURL= comment in the file, we handle it ourselves
325+
packages: "external", // https://esbuild.github.io/api/#packages
321326
logLevel: "warning",
322327
platform: "node",
323-
format: "esm",
328+
format: "cjs", // This is needed to support opentelemetry instrumentation that uses module patching
324329
target: ["node18", "es2020"],
325330
outdir: "out",
326331
define: {
327332
TRIGGER_API_URL: `"${config.triggerUrl}"`,
328333
},
329-
banner: {
330-
js: `import { createRequire } from 'module';const require = createRequire(import.meta.url);`,
331-
},
332334
plugins: [
333335
{
334336
name: "trigger.dev v3",
@@ -379,7 +381,7 @@ function useDev({
379381
}
380382

381383
// Create a file at join(dir, ".trigger", path) with the fileContents
382-
const fullPath = join(config.projectDir, ".trigger", `${contentHash}.mjs`);
384+
const fullPath = join(config.projectDir, ".trigger", `${contentHash}.js`);
383385
const sourceMapPath = `${fullPath}.map`;
384386

385387
const outputFileWithSourceMap = `${
@@ -389,6 +391,10 @@ function useDev({
389391
await fs.promises.mkdir(dirname(fullPath), { recursive: true });
390392
await fs.promises.writeFile(fullPath, outputFileWithSourceMap);
391393

394+
logger.debug(`Wrote background worker to ${fullPath}`);
395+
396+
const dependencies = gatherRequiredDependencies(metaOutput);
397+
392398
if (sourceMapFile) {
393399
const sourceMapPath = `${fullPath}.map`;
394400
await fs.promises.writeFile(sourceMapPath, sourceMapFile.text);
@@ -399,6 +405,7 @@ function useDev({
399405

400406
const backgroundWorker = new BackgroundWorker(fullPath, {
401407
projectDir: config.projectDir,
408+
dependencies,
402409
env: {
403410
TRIGGER_API_URL: apiUrl,
404411
TRIGGER_API_KEY: apiKey,
@@ -608,3 +615,48 @@ function WebsocketFactory(apiKey: string) {
608615
}
609616
};
610617
}
618+
619+
// Returns the dependencies that are required by the output that are found in output and the CLI package dependencies
620+
// Returns the dependency names and the version to use (taken from the CLI deps package.json)
621+
function gatherRequiredDependencies(outputMeta: Metafile["outputs"][string]) {
622+
const dependencies: Record<string, string> = {};
623+
624+
for (const file of outputMeta.imports) {
625+
if (file.kind !== "require-call" || !file.external) {
626+
continue;
627+
}
628+
629+
const packageName = detectPackageNameFromImportPath(file.path);
630+
631+
if (dependencies[packageName]) {
632+
continue;
633+
}
634+
635+
const internalDependencyVersion = (packageJson.dependencies as Record<string, string>)[
636+
packageName
637+
];
638+
639+
if (internalDependencyVersion) {
640+
dependencies[packageName] = internalDependencyVersion;
641+
}
642+
}
643+
644+
return dependencies;
645+
}
646+
647+
// Expects path to be in the format:
648+
// - source-map-support/register.js
649+
// - @opentelemetry/api
650+
// - zod
651+
//
652+
// With the result being:
653+
// - source-map-support
654+
// - @opentelemetry/api
655+
// - zod
656+
function detectPackageNameFromImportPath(path: string): string {
657+
if (path.startsWith("@")) {
658+
return path.split("/").slice(0, 2).join("/");
659+
} else {
660+
return path.split("/")[0] as string;
661+
}
662+
}

packages/cli-v3/src/dev/backgroundWorker.ts

+10-2
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,11 @@ import chalk from "chalk";
2020
import dotenv from "dotenv";
2121
import { Evt } from "evt";
2222
import { ChildProcess, fork } from "node:child_process";
23-
import { resolve } from "node:path";
23+
import { dirname, resolve } from "node:path";
2424
import terminalLink from "terminal-link";
25-
import { logger } from "../utilities/logger.js";
2625
import { safeDeleteFileSync } from "../utilities/fileSystem.js";
26+
import { installPackages } from "../utilities/installPackages.js";
27+
import { logger } from "../utilities/logger.js";
2728

2829
export type CurrentWorkers = BackgroundWorkerCoordinator["currentWorkers"];
2930
export class BackgroundWorkerCoordinator {
@@ -202,6 +203,7 @@ class CleanupProcessError extends Error {
202203

203204
export type BackgroundWorkerParams = {
204205
env: Record<string, string>;
206+
dependencies?: Record<string, string>;
205207
projectDir: string;
206208
debuggerOn: boolean;
207209
debugOtel?: boolean;
@@ -253,6 +255,11 @@ export class BackgroundWorker {
253255
throw new Error("Worker already initialized");
254256
}
255257

258+
// Install the dependencies in dirname(this.path) using npm and child_process
259+
if (this.params.dependencies) {
260+
await installPackages(this.params.dependencies, { cwd: dirname(this.path) });
261+
}
262+
256263
let resolved = false;
257264

258265
this.tasks = await new Promise<Array<TaskMetadataWithFilePath>>((resolve, reject) => {
@@ -469,6 +476,7 @@ class TaskRunProcess {
469476
async initialize() {
470477
this._child = fork(this.path, {
471478
stdio: [/*stdin*/ "ignore", /*stdout*/ "pipe", /*stderr*/ "pipe", "ipc"],
479+
cwd: dirname(this.path),
472480
env: {
473481
...this.env,
474482
OTEL_RESOURCE_ATTRIBUTES: JSON.stringify({
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
import { Resource } from "@opentelemetry/resources";
2+
import { OpenAIInstrumentation } from "@traceloop/instrumentation-openai";
3+
import {
4+
SemanticInternalAttributes,
5+
TracingDiagnosticLogLevel,
6+
TracingSDK,
7+
} from "@trigger.dev/core/v3";
8+
9+
export const tracingSDK = new TracingSDK({
10+
url: process.env.OTEL_EXPORTER_OTLP_ENDPOINT ?? "http://0.0.0.0:4318",
11+
resource: new Resource({
12+
[SemanticInternalAttributes.CLI_VERSION]: "3.0.0",
13+
}),
14+
instrumentations: [new OpenAIInstrumentation()],
15+
diagLogLevel: (process.env.OTEL_LOG_LEVEL as TracingDiagnosticLogLevel) ?? "none",
16+
});

packages/cli-v3/src/worker-facade.ts renamed to packages/cli-v3/src/dev/worker-facade.ts

+9-20
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,9 @@
1-
import "source-map-support/register";
2-
import { TracingSDK } from "@trigger.dev/core/v3/otel";
3-
// import { OpenAIInstrumentation } from "@traceloop/instrumentation-openai";
4-
5-
// IMPORTANT: this needs to be the first import to work properly
6-
// WARNING: [WARNING] Constructing "ImportInTheMiddle" will crash at run-time because it's an import namespace object, not a constructor [call-import-namespace]
7-
// TODO: https://github.com/open-telemetry/opentelemetry-js/issues/3954
8-
const tracingSDK = new TracingSDK({
9-
url: process.env.OTEL_EXPORTER_OTLP_ENDPOINT ?? "http://0.0.0.0:4318",
10-
resource: new Resource({
11-
[SemanticInternalAttributes.CLI_VERSION]: packageJson.version,
12-
}),
13-
instrumentations: [
14-
// new OpenAIInstrumentation(),
15-
],
16-
diagLogLevel: (process.env.OTEL_LOG_LEVEL as TracingDiagnosticLogLevel) ?? "none",
17-
});
1+
import "source-map-support/register.js";
2+
import { type TracingSDK } from "@trigger.dev/core/v3";
3+
4+
__REGISTER_TRACING__;
5+
declare const __REGISTER_TRACING__: unknown;
6+
declare const tracingSDK: TracingSDK;
187

198
const otelTracer = tracingSDK.getTracer("trigger-dev-worker", packageJson.version);
209
const otelLogger = tracingSDK.getLogger("trigger-dev-worker", packageJson.version);
@@ -42,13 +31,13 @@ import {
4231
taskContextManager,
4332
workerToChildMessages,
4433
type BackgroundWorkerProperties,
34+
type TracingDiagnosticLogLevel,
4535
} from "@trigger.dev/core/v3";
46-
import * as packageJson from "../package.json";
36+
import * as packageJson from "../../package.json";
4737

4838
import { Resource } from "@opentelemetry/resources";
4939
import { flattenAttributes } from "@trigger.dev/core/v3";
50-
import { TaskMetadataWithFunctions } from "./types.js";
51-
import { TracingDiagnosticLogLevel } from "@trigger.dev/core/v3/otel/tracingSDK";
40+
import { TaskMetadataWithFunctions } from "../types.js";
5241

5342
const tracer = new TriggerTracer({ tracer: otelTracer, logger: otelLogger });
5443
const consoleInterceptor = new ConsoleInterceptor(otelLogger);

packages/cli-v3/src/prod-facade.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
// import "source-map-support/register";
2-
import { TracingSDK } from "@trigger.dev/core/v3/otel";
2+
import { TracingSDK } from "@trigger.dev/core/v3";
33
// import { OpenAIInstrumentation } from "@traceloop/instrumentation-openai";
44

55
// IMPORTANT: this needs to be the first import to work properly
@@ -48,7 +48,7 @@ import * as packageJson from "../package.json";
4848
import { Resource } from "@opentelemetry/resources";
4949
import { flattenAttributes } from "@trigger.dev/core/v3";
5050
import { TaskMetadataWithFunctions } from "./types";
51-
import { TracingDiagnosticLogLevel } from "@trigger.dev/core/v3/otel/tracingSDK";
51+
import { TracingDiagnosticLogLevel } from "@trigger.dev/core/v3";
5252

5353
const tracer = new TriggerTracer({ tracer: otelTracer, logger: otelLogger });
5454
const consoleInterceptor = new ConsoleInterceptor(otelLogger);

0 commit comments

Comments
 (0)