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

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

Open
KeKs0r opened this issue Aug 27, 2024 · 8 comments
Open
Labels
bug Something that isn't working

Comments

@KeKs0r
Copy link

KeKs0r commented Aug 27, 2024

Which Cloudflare product(s) does this pertain to?

Workers Vitest Integration

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

[email protected], @cloudflare/[email protected], @microlabs/[email protected]

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 #5127 (comment) 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

> [email protected] 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"
@KeKs0r KeKs0r added the bug Something that isn't working label Aug 27, 2024
@github-project-automation github-project-automation bot moved this to Untriaged in workers-sdk Aug 27, 2024
@andyjessop andyjessop moved this from Untriaged to Backlog in workers-sdk Sep 9, 2024
@sam-rusty
Copy link

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

@jahands
Copy link
Contributor

jahands commented Sep 21, 2024

Running into this as well :(

@jahands
Copy link
Contributor

jahands commented Sep 30, 2024

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.

@depombo
Copy link

depombo commented Oct 9, 2024

running into this issue

@romeupalos
Copy link
Contributor

Running into this issue as well.

@tsteckenborn
Copy link

According to the issue linked above apparently fixable on the pool worker side

@sannajammeh
Copy link

sannajammeh commented Dec 20, 2024

You can shim this. This is only an example that works for me, you'll have to adapt the shim to proxy methods that you use.
Then make sure you re-export your worker handler without the otel instrumentation.

test/otel-shim.js

export const createCFOtel = () => {
	return {
		handler: (handler) => {
			return handler;
		},
		durableObject: (handler) => {
			return handler;
		},
	};
};

export const trace = {
	getActiveSpan: () => {
		return {
			setAttribute: () => {},
		};
	},
};

vitest.config.mts

import { defineWorkersConfig } from '@cloudflare/vitest-pool-workers/config';
import path from 'node:path';

export default defineWorkersConfig({
	resolve: {
		alias: {
			'@opentelemetry/api': path.join(process.cwd(), 'test', 'otel-shim.js'),
			'@frend-digital/worker-utils/otel': path.join(process.cwd(), 'test', 'otel-shim.js'),
		},
	},

	test: {
		poolOptions: {
			workers: {
				main: './test/worker-test.ts',
				wrangler: { configPath: './wrangler.toml' },
			},
		},
	},
});

@JeongJuhyeon
Copy link

Also running this issue with libraries that depend on opentelemetry such as @lmnr-ai/lmnr in my case or one of the other 1,493+ libraries that depend on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something that isn't working
Projects
Status: Backlog
Development

No branches or pull requests

8 participants