Skip to content

Commit

Permalink
instead of patching the tracer.js file to throw on `@opentelemetry/…
Browse files Browse the repository at this point in the history
…api` imports, delete the `@opentelemetry/api` dependency itself
  • Loading branch information
dario-piotrowicz committed Jan 15, 2025
1 parent e6078b5 commit 13a5f89
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 19 deletions.
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

1 comment on commit 13a5f89

@Nopsled
Copy link

Choose a reason for hiding this comment

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

@dario-piotrowicz, Is this dev branch for this issue something possible to use at the moment. Would need to find a solution for our project which is standing still at the moment. Have an package depending on opentelemetry 1.9.0 and our "solution" at the moment has been to delete the dependencies from the lockfile. Does this commit handle it better you think?

Please sign in to comment.