open-telemetry / opentelemetry.io

The OpenTelemetry website and documentation
https://opentelemetry.io
Creative Commons Attribution 4.0 International
501 stars 1.07k forks source link

OpenTelemetry with NodeJs and Typescript #4812

Closed g1nsu closed 3 weeks ago

g1nsu commented 1 month ago

When setting up NodeJs with Typescript, the documentation suggests using the --require flag. However, I received an error ("Cannot use import statement outside of a module.") when taking this approach.

When I reviewed the NodeJs documentation for the --require flag from the link in the OpenTelemetry page, it suggested using the --import flag for ECMA modules. Once I made this change, everything started working. I must also note that I am using tsx to run the project.

Mentioning the --import flag may help some users when setting up OpenTelemetry for the first time.

svrnm commented 1 month ago

Thanks @g1nsu

@open-telemetry/javascript-approvers can you take a look?

trentm commented 1 month ago

Hi @g1nsu, Are you able to share your tsconfig.json? My guess is that your tsconfig is set so that the generated .js -- from your .ts (or .tsx?) source files -- is generating ESM code.

You are right that the TypeScript option in the current docs is assuming that instrumentation.ts is converted to CommonJS (require-using) code.

I agree that mentioning using --import could be helpful. That note should mention:

g1nsu commented 1 month ago

Hi @trentm, Yes, my tsconfig is generating ESM code. "target": "es2022",

It looks like I missed the assumption that the code was compiled to CommonJS since the TypeScript command still shows .ts files. ts-node --require ./instrumentation.ts app.ts

I think your proposed changes look great. I just thought I could save someone else a few minutes.

Thanks!

trentm commented 1 month ago

It looks like I missed the assumption

I'm pretty sure it isn't explicitly stated anywhere. Thanks for opening the issue.

svrnm commented 1 month ago

@g1nsu would you be interested to update the documentation accordingly? Could be a simple note that people might run into that and what to do in that situation

g1nsu commented 1 month ago

@svrnm Yes, I can certainly update the documentation. Should I just check out the Contributing documention and make a PR?

svrnm commented 1 month ago

@svrnm Yes, I can certainly update the documentation. Should I just check out the Contributing documention and make a PR?

Thanks, that would be great! Yes, please give them a look! Especially pay attention to the CLA that we have to ask you to sign (except you have contributed to any other CNCF/LF project already). I call this out because this is a blocker for people sometimes.

JamieDanielson commented 1 month ago

Thanks for reporting @g1nsu ! We definitely have some work to do in documenting ESM instrumentation, and have created an issue recently to help track some of the work. I'm noting this to help link for future reference as well.

g1nsu commented 1 month ago

After some more testing, the documentation does work as written using ts-node. Running with ts-node only works when the tsconfig has set "module": "CommonJS".

The issue I encountered has to do with tsx. The tsx package only works with the --import flag and seems to ignore the tsconfig module configuration.

If you would still like a documentation note, I can do so. But since this is a tsx specific detail, we can close this as resolved.