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

[Sentry] Express is not instrumented. This is likely because you required/imported express before calling Sentry.init() #12056

Open
3 tasks done
amiranvarov opened this issue May 15, 2024 · 51 comments
Labels
Package: node Issues related to the Sentry Node SDK Waiting for: Community

Comments

@amiranvarov
Copy link

amiranvarov commented May 15, 2024

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

// main.ts
import * as Sentry from "@sentry/node";
import { nodeProfilingIntegration } from "@sentry/profiling-node";
Sentry.init({
	serverName: "auth-service",
	dsn: process.env.SENTRY_DSN,
	environment: process.env.NODE_ENV,
	enabled: true,
	debug: process.env.NODE_ENV !== "production",
	integrations: [Sentry.httpIntegration(), Sentry.expressIntegration(), nodeProfilingIntegration()],
	// Set tracesSampleRate to 1.0 to capture 100%
	// of transactions for performance monitoring.
	// We recommend adjusting this value in production
	tracesSampleRate: 1.0,
	beforeSend(event, hint) {
		const error = hint.originalException as Error;
		if (error?.message?.match(/database unavailable/i)) {
			event.fingerprint = ["database-unavailable"];
		}

		// Modify or drop the event here
		if (event.user) {
			// Don't send user's email address
			event.user.email = undefined;
		}

		return event;
	},

	// Called for transaction events
	beforeSendTransaction(event) {
		// Modify or drop the event here
		if (event.transaction === "/unimportant/route") {
			// Don't send the event to Sentry
			return null;
		}
		return event;
	},
	beforeBreadcrumb(breadcrumb, hint) {
		// Check if the breadcrumb has sensitive data like user email
		if (breadcrumb.data?.["user"]?.email) {
			// Remove the user email from the breadcrumb
			breadcrumb.data["user"].email = undefined;
		}
		return breadcrumb;
	},
	includeLocalVariables: true,
	attachStacktrace: true,
});

/// rest of the app

Steps to Reproduce

  1. install NX
  2. Install NX's express
  3. run nx run --target=serve

I did right as you suggested it in your docs. Imprted and initialised Sentry first thing first. But still, it's not working
image

Expected Result

Do not have [Sentry] Express is not instrumented. This is likely because you required/imported express before calling Sentry.init() message, and

Actual Result

Having error message: [Sentry] Express is not instrumented. This is likely because you required/imported express before calling Sentry.init() message, and
image

@github-actions github-actions bot added the Package: node Issues related to the Sentry Node SDK label May 15, 2024
@amiranvarov
Copy link
Author

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

@amiranvarov
Copy link
Author

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...

@andreiborza
Copy link
Member

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 nodeProfilingIntegration in your example).

We recommend going through the migration guide here. You can use npx @sentry/migr8@latest to take care of it for you, or follow the steps to manually set it up here.

In your particular example, could you try removing Sentry.httpIntegration() and Sentry.expressIntegration() and report back?

@amiranvarov
Copy link
Author

amiranvarov commented May 19, 2024

hey, thanks for getting back @andreiborza! I did follow the migration guide, and used npx @sentry/migr8@latest right from the start. But no, it didn't help.

As you suggested, i removed Sentry.httpIntegration and Sentry.expressIntegration() it didn't help. Then i removed all of the integrations, didn't help either. still getting this message.

@amiranvarov
Copy link
Author

If it helps, you may check my comment on another similar issue that was opened 2 days ago:
#12105

@amiranvarov
Copy link
Author

If that help,s we can have a short screensharing call so u can get the context you need

@andreiborza
Copy link
Member

Have you updated to 8.2.1 yet? Same result?

Could you please provide

  • node version you are running
  • the updated code snippets after your migration attempts (instrument.ts and main.ts)
  • the way you run your application (i.e. the script behind auth-service:build:development)

I see a .ts ending in the comment, what are you using to bundle your app?

@amiranvarov
Copy link
Author

yes, updated just yesterday. And yes, same result.

  • node v20.13.1
  • will provide later today
  • I'm not sure what exactly do you mean? the whole code of my microservice? if so, can't share much of it, it's private :( If you mean the way u run the script itself: just nx run auth-service:build:development. In the screen you may say that I didn't type it manually, that's because I'm using NX's VScode extension that will run this script for me with one click, but it should not matter for your debugging as it's still running nx run auth-service:build:development

Regarding bundler: I'm using @nx/esbuild, as it comes as built-in bundler in NX.

Here is part of my NX project (this microservice) config:
apps/auth-service/project.json

{
	"name": "auth-service",
	"$schema": "../../node_modules/nx/schemas/project-schema.json",
	"sourceRoot": "apps/auth-service/src",
	"projectType": "application",
	"targets": {
		"build": {
			"executor": "@nx/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"
						}
					}
				}
			}
		},

@amiranvarov
Copy link
Author

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...

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

@mydea
Copy link
Member

mydea commented May 22, 2024

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 import or require?

@amiranvarov
Copy link
Author

it's CJS, for sure :)

@andreiborza
Copy link
Member

could you please post snippets of instrument.ts and the top of your app where you import sentry and express and setup the handler?

@Nabil372
Copy link

Hi, I'm also seeing:

[Sentry] Express is not instrumented. This is likely because you required/imported express before calling `Sentry.init()`.

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:
https://github.com/Nabil372/sentry-playground

@mydea
Copy link
Member

mydea commented May 22, 2024

Hi, I'm also seeing:

[Sentry] Express is not instrumented. This is likely because you required/imported express before calling `Sentry.init()`.

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: https://github.com/Nabil372/sentry-playground

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.

@mydea
Copy link
Member

mydea commented Aug 14, 2024

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 🤔

@gdbjohnson
Copy link

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. @lforst , 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.

@gdbjohnson
Copy link

@mydea / @lforst : 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!

@mydea
Copy link
Member

mydea commented Aug 16, 2024

🙏 all good, that's great to hear!

I think we could potentially add a check for process.env.JEST_WORKER_ID when calling init in @sentry/node and log a small, helpful warning there. PRs for this are welcome! :)

@gdbjohnson
Copy link

gdbjohnson commented Aug 17, 2024

@mydea / @lforst : 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.

Exception has occurred: TypeError: Failed to parse URL from 
  at new Request (node:internal/deps/undici/undici:5855:19)
    at fetch (node:internal/deps/undici/undici:10123:25)
    at Object.fetch (node:internal/deps/undici/undici:12344:10)
    at fetch (node:internal/process/pre_execution:336:27)
    at loadFetch (/Users/gregory/Work/booking-backend/node_modules/.pnpm/[email protected]_@[email protected]/node_modules/opentelemetry-instrumentation-fetch-node/src/index.ts:81:11)
    at new FetchInstrumentation (/Users/gregory/Work/booking-backend/node_modules/.pnpm/[email protected]_@[email protected]/node_modules/opentelemetry-instrumentation-fetch-node/src/index.ts:121:5)

@Lms24
Copy link
Member

Lms24 commented Aug 19, 2024

We'll continue to look into this next week as this week is Hackweek at Sentry.

@lforst
Copy link
Member

lforst commented Aug 26, 2024

@mydea
Copy link
Member

mydea commented Aug 27, 2024

We should add an option to opt-out of these warnings, IMHO. I opened an issue for this: #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 )

@gdbjohnson
Copy link

@mydea 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.

@mydea
Copy link
Member

mydea commented Aug 28, 2024

@mydea 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 '@sentry/node';

app.use(Sentry.expressErrorHandler());

@SteffenLanger
Copy link

Issue with Feathers Framework

In case anyone finds this issue and uses feathers with express: The warning is generated because the function setupExpressErrorHandler() checks if the express function app.use() was wrapped by Sentry. Even if it was, the check always fails and produces a warning because the feathers function feathersExpress() overwrites the express app's app.use() with its own implementation - thus the warning.

Solution/Workaround

Skip the check.
Sentry's setupExpressErrorHandler() looks like this:

function setupExpressErrorHandler(
  app,
  options,
) {
  app.use(expressErrorHandler(options));
  ensureIsWrapped(app.use, 'express');
}

Therefore, you can simply replace Sentry.setupExpressErrorHandler(app, options) with app.use(expressErrorHandler(options));

@yogendrarana
Copy link

I removed this error middleware:
Sentry.setupExpressErrorHandler(app);

and instead used this one:
app.use(Sentry.expressErrorHandler());

The [Sentry] Express is not instrumented warning went away.

@Lms24
Copy link
Member

Lms24 commented Sep 18, 2024

Hi, I wouldn't recommend removing the setupExpressErrorHandler function as we could theoretically also use it for more things than just for adding the middleware. Unless of course, you know what you're doing and you're willing to take that chance :)

#13471 will provide you an option to silence these warnings once it is merged and released (soon).

@mydea
Copy link
Member

mydea commented Sep 25, 2024

FYI you can now configure disableInstrumentationWarnings: true in your init to avoid the warning!

see #13693

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Package: node Issues related to the Sentry Node SDK Waiting for: Community
Projects
Status: Waiting for: Community
Development

No branches or pull requests