cloudflare / workers-sdk

⛅️ Home to Wrangler, the CLI for Cloudflare Workers®
https://developers.cloudflare.com/workers/
Apache License 2.0
2.6k stars 674 forks source link

🐛 BUG vitest-pool-workers loads opentelemetry for Node not the browser #6581

Open KeKs0r opened 1 month ago

KeKs0r commented 1 month ago

Which Cloudflare product(s) does this pertain to?

Workers Vitest Integration

What version(s) of the tool(s) are you using?

wrangler@3.63.2, @cloudflare/vitest-pool-workers@0.4.25, @microlabs/otel-cf-workers@1.0.0-rc.45

What version of Node are you using?

v22.3.0

What operating system and version are you using?

OSX Sonoma 14.5, Macbook Air M2

Describe the Bug

Observed behavior

When running tests via vitest and vitest-pool-workers, it crashes due when I am also using the opentelemetry packages, since their node version is imported, instead of the compatible browser version. This only happens when running the tests, but not when running the dev server or deploying to cloudflare.

Expected behavior

I would expect it to work the same way it does for the dev server and my deployed applicatin

Steps to reproduce

I provided a simple reproduction here: https://github.com/KeKs0r/cf-vitest-otel It just requires to install deps and run the tests.

Rererences

It feels very similar to Issue 1 described in this comment https://github.com/cloudflare/workers-sdk/issues/5127#issuecomment-1989115175 by @mrbbot But potentially the package.json from open telemetry is different in some way.

Here their exports for reference:

{
  "name": "@opentelemetry/core",
  "version": "1.25.1",
  "description": "OpenTelemetry Core provides constants and utilities shared by all OpenTelemetry SDK packages.",
  "main": "build/src/index.js",
  "module": "build/esm/index.js",
  "esnext": "build/esnext/index.js",
  "browser": {
    "./src/platform/index.ts": "./src/platform/browser/index.ts",
    "./build/esm/platform/index.js": "./build/esm/platform/browser/index.js",
    "./build/esnext/platform/index.js": "./build/esnext/platform/browser/index.js",
    "./build/src/platform/index.js": "./build/src/platform/browser/index.js"
  },
  "types": "build/src/index.d.ts",
  "repository": "open-telemetry/opentelemetry-js",
}

Please provide a link to a minimal reproduction

https://github.com/KeKs0r/cf-vitest-otel

Please provide any relevant error logs

> cf-vitest-otel@0.0.0 test
> vitest

 DEV  v1.5.0 /Users/marc/Workspace/cf-vitest-otel

[vpw:inf] Starting isolated runtimes for vitest.config.mts...
workerd/server/server.c++:2882: error: Fallback service failed to fetch module; payload = ; spec = /?specifier=%2FUsers%2Fmarc%2FWorkspace%2Fcf-vitest-otel%2Fnode_modules%2F%40opentelemetry%2Fcore%2Fbuild%2Fesm%2Fplatform%2Fnode%2Fperf_hooks&referrer=%2FUsers%2Fmarc%2FWorkspace%2Fcf-vitest-otel%2Fnode_modules%2F%40opentelemetry%2Fcore%2Fbuild%2Fesm%2Fplatform%2Fnode%2Fperformance.js&rawSpecifier=perf_hooks
 ❯ test/index.spec.ts (0)

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Failed Suites 1 ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯

 FAIL  test/index.spec.ts [ test/index.spec.ts ]
Error: No such module "Users/marc/Workspace/cf-vitest-otel/node_modules/@opentelemetry/core/build/esm/platform/node/perf_hooks".
  imported from "Users/marc/Workspace/cf-vitest-otel/node_modules/@opentelemetry/core/build/esm/platform/node/performance.js"
sam-rusty commented 2 weeks ago

i am getting the same issue. is there any workaround to bypass open telemetry integration when running test cases?

jahands commented 1 week ago

Running into this as well :(

jahands commented 23 hours ago

After messing with this for way too long, I found a solution that worked for me. I created a package that bundles otel-cf-workers and @opentelemetry/api using esbuild in bundle mode, which seems to treeshake out all the Node-specific things: https://www.npmjs.com/package/@jahands/otel-cf-workers

Not sure what the real fix would be here. I think what's happening is that Vitest is loading all files without any treeshaking and then complaining because of Node-specific stuff that isn't supported in Workers. That's why pre-bundling with esbuild resolves the issue (and also speeds up vitest because it doesn't have to load nearly as many files.)

If anyone wants to use this, I'd recommend copying this package for your own use, rather than relying on my version published to NPM.