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

instead of patching the tracer.js file to throw on @opentelemetry/api imports, delete the @opentelemetry/api dependency itself #259

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all 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
9 changes: 9 additions & 0 deletions .changeset/forty-jobs-press.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
"@opennextjs/cloudflare": patch
---

instead of patching the `tracer.js` file to throw on `@opentelemetry/api` imports, delete the `@opentelemetry/api` dependency itself

the problem that this addresses is that the `@opentelemetry/api` package is not only imported by the `tracer.js` file
we patch, so just deleting the library itself makes sure that all files requiring it get the same throwing behavior
(besides decreasing the overall worker size)
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { openNextReplacementPlugin } from "@opennextjs/aws/plugins/replacement.j
import { openNextResolvePlugin } from "@opennextjs/aws/plugins/resolve.js";
import type { FunctionOptions, SplittedFunctionOptions } from "@opennextjs/aws/types/open-next.js";

import { deleteOpenTelemetryDep } from "../patches/index.js";
import { normalizePath } from "../utils/index.js";

export async function createServerBundle(options: buildHelper.BuildOptions) {
Expand Down Expand Up @@ -86,12 +87,14 @@ export async function createServerBundle(options: buildHelper.BuildOptions) {
}

// Generate default function
await generateBundle("default", options, {
const outputPath = await generateBundle("default", options, {
...defaultFn,
// @ts-expect-error - Those string are RouteTemplate
routes: Array.from(remainingRoutes),
patterns: ["*"],
});

deleteOpenTelemetryDep(outputPath);
}

async function generateBundle(
Expand Down Expand Up @@ -250,6 +253,8 @@ CMD ["node", "index.mjs"]
`
);
}

return outputPath;
}

function shouldGenerateDockerfile(options: FunctionOptions) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { rmSync } from "node:fs";
import { join } from "node:path";

/**
* Given a directory path, it deletes a `node_modules/@opentelemetry` subdirectory if present.
*
* Explanation:
* The standard `@opentelemetry/api` library doesn't work in workerd since there are paths that it can't resolve (without a
* compilation step), fortunately Next.js has a try-catch statement that replaces, when failing, `require('@opentelemetry/api')`
* calls with a precompiled version of the library ('next/dist/compiled/@opentelemetry/api') which does properly in our runtime
* (source code: https://github.com/vercel/next.js/blob/9e8266a7/packages/next/src/server/lib/trace/tracer.ts#L27-L31)
*
* So this function is used to delete the `@opentelemetry` dependency entirely so to guarantee that
* `require('@opentelemetry/api')` fail ensuring that the precompiled version is used
*/
export async function deleteOpenTelemetryDep(path: string): Promise<void> {
const nodeModulesDirPath = join(path, "node_modules");

rmSync(join(nodeModulesDirPath, "@opentelemetry"), {
recursive: true,
force: true,
});
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
export * from "./copy-package-cli-files.js";
export * from "./delete-open-telemetry-dep.js";
export * from "./patch-cache.js";
export * from "./patch-require.js";
export * from "./update-webpack-chunks-file/index.js";
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import * as ts from "ts-morph";

import { Config } from "../../../config.js";
import { tsParseFile } from "../../utils/index.js";
import { deleteOpenTelemetryDep } from "../investigated/delete-open-telemetry-dep.js";

export function patchWranglerDeps(config: Config) {
console.log("# patchWranglerDeps");
Expand All @@ -28,24 +29,7 @@ export function patchWranglerDeps(config: Config) {

patchRequireReactDomServerEdge(config);

// Patch .next/standalone/node_modules/next/dist/server/lib/trace/tracer.js
//
// Remove the need for an alias in wrangler.toml:
//
// [alias]
// # @opentelemetry/api is `require`d when running wrangler dev, so we need to stub it out
// # IMPORTANT: we shim @opentelemetry/api to the throwing shim so that it will throw right away, this is so that we throw inside the
// # try block here: https://github.com/vercel/next.js/blob/9e8266a7/packages/next/src/server/lib/trace/tracer.ts#L27-L31
// # causing the code to require the 'next/dist/compiled/@opentelemetry/api' module instead (which properly works)
// #"@opentelemetry/api" = "./.next/standalone/node_modules/cf/templates/shims/throw.ts"
const tracerFile = join(distPath, "server", "lib", "trace", "tracer.js");

const patchedTracer = readFileSync(tracerFile, "utf-8").replaceAll(
/\w+\s*=\s*require\([^/]*opentelemetry.*\)/g,
`throw new Error("@opentelemetry/api")`
);

writeFileSync(tracerFile, patchedTracer);
deleteOpenTelemetryDep(config.paths.output.standaloneRoot);
}

/**
Expand Down
Loading