-
Notifications
You must be signed in to change notification settings - Fork 0
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
[Sentry] Express is not instrumented. This is likely because you required/imported express before calling Sentry.init()
#17
Comments
Strange... I did some code refactoring, and didn't touch this part I shared at all.. somehow I'm not having this error anymore. I will close this issue for a bit, if I encounter it again, I'll reopen it |
Isse came back again, I have no idea why. I didn't change anything. tried to update to latest Sentry, didn't help. Tried to nuke node_modules. lock file and everything, still didn't work. This is some weird mystery... |
Click the checkbox below to generate a PR!
@hiroshinishio, You have 2 requests left in this cycle which refreshes on 2024-11-04 02:56:25+00:00. |
Click the checkbox below to generate a PR!
@hiroshinishio, You have 3 requests left in this cycle which refreshes on 2024-11-05 22:45:17+00:00. |
Hi there, Things changed in v8 a bit, most integrations are now automatically added and you no longer have to take care of those (with the exception of We recommend going through the migration guide here. You can use In your particular example, could you try removing |
hey, thanks for getting back ! I did follow the migration guide, and used As you suggested, i removed |
If it helps, you may check my comment on another similar issue that was opened 2 days ago: |
If that help,s we can have a short screensharing call so u can get the context you need |
Have you updated to Could you please provide
I see a |
yes, updated just yesterday. And yes, same result.
Regarding bundler: I'm using /esbuild, as it comes as built-in bundler in NX. Here is part of my NX project (this microservice) config: {
"name": "auth-service",
"$schema": "../../node_modules/nx/schemas/project-schema.json",
"sourceRoot": "apps/auth-service/src",
"projectType": "application",
"targets": {
"build": {
"executor": "/esbuild:esbuild",
"outputs": ["{projectRoot}/build"],
"defaultConfiguration": "production",
"options": {
"platform": "node",
"outputPath": "{projectRoot}/build",
"format": ["cjs"],
"bundle": true,
"main": "apps/auth-service/src/main.ts",
"tsConfig": "apps/auth-service/tsconfig.app.json",
"assets": ["apps/auth-service/src/assets"],
"generatePackageJson": true,
"esbuildOptions": {
"sourcemap": true,
"outExtension": {
".js": ".js"
}
}
},
"configurations": {
"development": {},
"production": {
"generateLockfile": true,
"esbuildOptions": {
"sourcemap": false,
"outExtension": {
".js": ".js"
}
}
}
}
}, |
I figured why I stopped having this issue at that time, I removed sentry.setupExpressErrorHandler(app), that's why I didn't get that message. But the problem was still there, although I didn't see it in the console. So this issue is persistent |
So you should def. not remove the error handler :D Do you know if your app is being run as a CommonJS or ESM app? Basically, if you look into your build/dist folder, are the files in there using |
it's CJS, for sure :) |
could you please post snippets of |
Hi, I'm also seeing:
in my application logs despite following the sentry onboarding guide for express apps. Errors are still sent to sentry and everything seems to work fine though. Here's the code that I'm running: |
Are you not using performance? This is a bug in the current version, that it will show this warning if performance is disabled - it's safe to ignore it! In 8.3.0 (which should be out soon) this warning will be fixed/removed. |
Hey , thank you for providing a sample project. I've had a quick look over it, you do have performance (tracing) enabled so that's not the issue. I see that you're using esbuild with the output format ESM. For ESM, you need to I tried that out for you, but your build output doesn't seem to be pure ESM either—I'm seeing this snippet in the output which probably throws off internals of sentry/opentelemetry. |
Thanks for taking a look! I've implemented what you've suggested and I'm no longer seeing the warning message. |
could you please update to the latest SDK? Should be |
just to make sure, |
Side note, If you enable |
I'm also getting this error with Sentry v8.10.0 and it's preventing me from migrating to v8:
This seems to be related to esbuild and how it bundles code. Our code is bundled using esbuild (CJS), and in the bundled code there doesn't seem to be any guarantee that Sentry is initialized before Koa is imported, no matter how early I initialize Sentry. Is there any workaround? I tried late initialization (https://docs.sentry.io/platforms/javascript/guides/koa/install/late-initializtion/) but that didn't help. |
Hello , enable tracing is set to true yet I'm still getting:
This is my set up
if i remove adding
Node version - 20.14.0 |
in your case, if you read the warning message carefully it will tell you the problem, I believe - you have to use & : If you are already following https://docs.sentry.io/platforms/javascript/guides/express/install/commonjs/, then the problem is probably that you are bundling your code. If this is the case, you need to define the packages you want to instrument (e.g. or koa fastify) as externals in your build process. For now, there is sadly no way around this :( We have not documented this properly yet, but it's on our radar: getsentry/sentry-docs#10416 basically, the problem is that the underlying instrumentation works by patching FWIW basic instrumentation should still work, so you should still be seeing http.server spans and basic other things. But you'll miss nice route parametrization and middleware spans, which is obviously not ideal. |
adding the
And since I can't upgrade Mongoose from v6xx to 7xx due to breaking changes, I switched |
Damn, that sucks. We have a tracking issue for this here: getsentry#11499, we hope to find some time to resolve this soon. For now, you should be able to remove this integration, avoiding the build error: // instrument.js
Sentry.init({
integrations: integrations => {
return integrations.filter(integration => integration.name !== 'Mongoose')
}
}); |
Yep, thanks. I'd try this during my spare time and give feedback soon. But for now, the version works. I'm also keeping an eye out for both issues. |
Thank you for your response . I got this working by configuring import { initSentry } from "./sentry";
initSentry(); When I look at the bundled code in the generated import "./sentry"; |
Having the same issue. Not using TS, plain mjs. Node 20, /node:8.15.0,/profiling-node:8.15.0. |
can you share the contents of your sentry.mjs file or even better share a reproduction example we can use to debug locally? Thanks! |
I don't have a production example, I am new to sentry and was following the docs: https://docs.sentry.io/platforms/javascript/guides/express/ My
However, I was using hyper-express instead of plain express. Maybe that was the issue. Express seems to be abandoned, stuck at v4 since 2016, yet it is maintained. |
Well that is an important detail. Assuming you mean: https://www.npmjs.com/package/hyper-express We don't support |
I am facing the same thing. I notice there is an uncaught error happening in an opentelemetry package after the express warning is issued, related to fetch(''). Not sure if that is worth me giving more detail on or not.
With my makeSentry() method here:
|
Hello . Thanks for adding more context to this. We'll take a look. |
What error exactly are you getting? |
I tried generating logs to Sentry via JEST testing. I'm wondering if that's part of the issue? Some googling on the error code below has a lot of JEST related hits. Have you seen this before? Just as a last ditch, I deleted by node_modules folders and tried it again, with the same result. Also, the uncaught error I mentioned above stopped happening for me, so I'm unable to help there, sorry! I'm pretty sure I didn't halucinate it, but after switching branches etc, I guess something "fixed" itself there.
|
Can you elaborate what you are trying to do? I don't fully understand how jest is involved or what you are trying to achieve. |
Is this only happening in Jest tests? This is not really something we support (well) right now, because the Jest environment is not a proper/full Node environment, so stuff may not be working there properly 🤔 |
I don't know if it's happening correctly in prod / stage. I couldn't generate the correct events in test, so I didn't go further. , I just used my tests as an easy way to generate events for Sentry (normally I would turn off events from local / test). First time I've used Sentry with Node. I'll try running it for real and see what happens. |
/ : I guess I should apologize since I can see events in my sentry account when I just run it normally. We're moving really fast here, but I should have tried this before logging a ticket. My bad. If Sentry doesn't work within a Jest context, perhaps Sentry can test for this and log a warning to the console for a better developer experience? Not sure if that's easy to detect or not. Anyways, glad we can get up and running with the product! |
🙏 all good, that's great to hear! I think we could potentially add a check for |
/ : I'm not entirely sure what's happened, but I managed to get some events, but I'm also still getting the warning and most of the events are not happening. It's intermittent. I think it's because we are using express v5.0.0-beta.3 on most of our services. I downgraded to 4, and I don't see the Sentry warning. But I'm not convinced this is 100% it. Also, I get this error consistently.
|
We'll continue to look into this next week as this week is Hackweek at Sentry. |
That is very likely the cause as the OpenTelemetry express instrumentation is version-locked to version 4: https://github.com/open-telemetry/opentelemetry-js-contrib/blob/de7a6cb77e643ed0de82e514510089fba5ae0405/plugins/node/opentelemetry-instrumentation-express/src/instrumentation.ts#L62 |
We should add an option to opt-out of these warnings, IMHO. I opened an issue for this: getsentry#13471 In this case it is true/correct that express will not be properly performance instrumented. But there is nothing you can do about this as a user, and error monitoring should still work, so you can safely ignore this warning - you'll get basic (un-parametrized) http.server spans through the http instrumentation, but no express-specific tracing data, sadly. I think it is unlikely that OpenTelemetry support will be added before v5 of express is out of beta (which, who nows if/when this will happen :O ) |
if Sentry will simply not work with Express v5, you really need to put that on your documentation. This page would be a good spot. I reviewed it and I didn't see any block indicating express version that Sentry is compatible with. https://docs.sentry.io/platforms/javascript/guides/express/ Tooling with express is a critical requirement for what we need, so I have a hard choice to either downgrade express or find a different provider for error capture. |
So error monitoring should still work, I believe. As long as middlewares have not changed, ours should still be compatible. Only tracing (performance monitoring) will not work in detail, as mentioned above - meaning you will still get traces, only without parametrisation, and no middleware spans. But there should still be enough details to see what's generally going on! Generally we do not support beta-level releases, unless otherwise stated. When express v5 goes stable, we will make sure to support it as soon as possible. From what I see from release notes etc. middlewares should not (?) have changed, so you can ignore the warning that instrumentation did not work for now. Alternatively, if you really want to get rid of the warning, you can also manually add the middleware like this: import * as Sentry from '/node';
app.use(Sentry.expressErrorHandler()); |
Issue with Feathers FrameworkIn case anyone finds this issue and uses feathers with express: The warning is generated because the function Solution/WorkaroundSkip the check. function setupExpressErrorHandler(
app,
options,
) {
app.use(expressErrorHandler(options));
ensureIsWrapped(app.use, 'express');
} Therefore, you can simply replace |
I removed this error middleware: and instead used this one: The [Sentry] Express is not instrumented warning went away. |
Hi, I wouldn't recommend removing the getsentry#13471 will provide you an option to silence these warnings once it is merged and released (soon). |
FYI you can now configure see getsentry#13693 |
Sorry, we have an error. Please try again. Have feedback or need help? |
@hiroshinishio Pull request completed! Check it out here #21 🚀 |
@hiroshinishio Pull request completed! Check it out here #24 🚀 Note: I automatically create a pull request for an unassigned and open issue in order from oldest to newest once a day at 00:00 UTC, as long as you have remaining automation usage. Should you have any questions or wish to change settings or limits, please feel free to contact [email protected] or invite us to Slack Connect. |
Is there an existing issue for this?
How do you use Sentry?
Sentry Saas (sentry.io)
Which SDK are you using?
@sentry/node
SDK Version
8.0.0
Framework Version
express@^4.18.1, @nx/express": "18.2.2
Link to Sentry event
No response
SDK Setup
Steps to Reproduce
I did right as you suggested it in your docs. Imprted and initialised Sentry first thing first. But still, it's not working
Expected Result
Do not have [Sentry] Express is not instrumented. This is likely because you required/imported express before calling
Sentry.init()
message, andActual Result
Having error message: [Sentry] Express is not instrumented. This is likely because you required/imported express before calling
Sentry.init()
message, andThe text was updated successfully, but these errors were encountered: