sentry-javascript icon indicating copy to clipboard operation
sentry-javascript copied to clipboard

feat(node): Add `@sentry/node/preload` hook

Open mydea opened this issue 1 year ago • 8 comments

This PR adds a new way to initialize @sentry/node, which allows to use the SDK with performance instrumentation even if you cannot (for whatever reason) call Sentry.init() at the very start of your app.

CJS usage

In CommonJS mode, you can run the SDK like this:

node --require @sentry/node/preload ./app.js
// app.js
const express = require('express');
const Sentry = require('@sentry/node');

const dsn = await getSentryDsn();
Sentry.init({ dsn });

// express is instrumented even though we initialized Sentry late

ESM usage

in ESM mode, you can run the SDK like this:

node --import @sentry/node/preload ./app.mjs
// app.mjs
import express from 'express';
import * as Sentry from '@sentry/node';

const dsn = await getSentryDsn();
Sentry.init({ dsn });

// express is instrumented even though we initialized Sentry late

Configuration options

This script will by default preload all opentelemetry instrumentation. You can choose to instrument only specific packages like this:

SENTRY_PRELOAD_INTEGRATIONS="Http,Express,Graphql" --import @sentry/node/preload ./app.mjs

You can also enable debug logging for the script via SENTRY_DEBUG=true.

Manually preloading

It is also possible to manually call preloadOpenTelemetry() to achieve the same thing. For example, in a CJS app you could do the following thing if you want to initialize late but don't want to use --require:

// preload.js
const Sentry = require('@sentry/node');
Sentry.preloadOpenTelemetry();

// app.js
// call this first, before any other requires!
require('./preload.js');
// Then, other stuff
const express = require('express');
const Sentry = require('@sentry/node');

const dsn = await getSentryDsn();
Sentry.init({ dsn });

mydea avatar May 24 '24 13:05 mydea

What happens if a user manually initializes OpenTelemetry? Can they still delay calling Sentry.init?

AbhiPrasad avatar May 24 '24 13:05 AbhiPrasad

How does this differ from using --import @sentry/node/import? I can see the code differenced but why are these differences required?

I assumed --import @sentry/node/import would load import-in-the-middle and then import order in the app doesn't matter, it still instruments libraries.

timfish avatar May 24 '24 13:05 timfish

size-limit report 📦

Path Size
@sentry/browser 21.74 KB (0%)
@sentry/browser (incl. Tracing) 32.77 KB (0%)
@sentry/browser (incl. Tracing, Replay) 68.24 KB (0%)
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 61.66 KB (0%)
@sentry/browser (incl. Tracing, Replay with Canvas) 72.29 KB (0%)
@sentry/browser (incl. Tracing, Replay, Feedback) 84.32 KB (0%)
@sentry/browser (incl. Feedback) 37.75 KB (0%)
@sentry/browser (incl. sendFeedback) 26.31 KB (0%)
@sentry/browser (incl. FeedbackAsync) 30.73 KB (0%)
@sentry/react 24.43 KB (0%)
@sentry/react (incl. Tracing) 35.77 KB (0%)
@sentry/vue 25.68 KB (0%)
@sentry/vue (incl. Tracing) 34.58 KB (0%)
@sentry/svelte 21.88 KB (0%)
CDN Bundle 24.28 KB (0%)
CDN Bundle (incl. Tracing) 34.22 KB (0%)
CDN Bundle (incl. Tracing, Replay) 68.03 KB (0%)
CDN Bundle (incl. Tracing, Replay, Feedback) 73.02 KB (0%)
CDN Bundle - uncompressed 71.46 KB (0%)
CDN Bundle (incl. Tracing) - uncompressed 101.55 KB (0%)
CDN Bundle (incl. Tracing, Replay) - uncompressed 211.46 KB (0%)
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 223.81 KB (0%)
@sentry/nextjs (client) 35.12 KB (0%)
@sentry/sveltekit (client) 33.36 KB (0%)
@sentry/node 114.6 KB (+0.25% 🔺)
@sentry/aws-serverless 103.29 KB (+0.09% 🔺)

github-actions[bot] avatar May 24 '24 13:05 github-actions[bot]

How does this differ from using --import @sentry/node/import? I can see the code differenced but why are these differences required?

I assumed --import @sentry/node/import would load import-in-the-middle and then import order in the app doesn't matter, it still instruments libraries.

In our tests, import/execution order sadly still mattered. If something was imported before we added the OTEL instrumentation, it did not work :( Actually, the test is not "good" because in this specific test stuff does work. I updated this now, it fails when something is used before Sentry is run. E.g.:

import express from 'express';

const app = express();
// setup app...

// somewhere later
setTimeout(function() {
  Sentry.init(...);
}, 1000);

In this case, express is already uninstrumented before we run init :/

What happens if a user manually initializes OpenTelemetry? Can they still delay calling Sentry.init?

This should all still work, basically this does nothing except setup the instrumentations, it does not actually setup otel itself (e.g. no span processor etc. is created yet).

mydea avatar May 24 '24 13:05 mydea

How does this differ from using --import @sentry/node/import? I can see the code differenced but why are these differences required?

I assumed --import @sentry/node/import would load import-in-the-middle and then import order in the app doesn't matter, it still instruments libraries.

If I understood correctly, some people didn't have a DSN ready by the time we need to import/init everything for instrumentation to work. This would be a workaround.

andreiborza avatar May 24 '24 13:05 andreiborza

How does this differ from using --import @sentry/node/import? I can see the code differenced but why are these differences required? I assumed --import @sentry/node/import would load import-in-the-middle and then import order in the app doesn't matter, it still instruments libraries.

If I understood correctly, some people didn't have a DSN ready by the time we need to import/init everything for instrumentation to work. This would be a workaround.

You can use @sentry/node/import without DSN, which does setup up import-in-the-middle correctly. but this still requires the otel instrumentation to be loaded before a module is used :/

mydea avatar May 24 '24 13:05 mydea

// somewhere later
setTimeout(function() {
  Sentry.init(...);
}, 1000);

In this case, express is already uninstrumented before we run init :/

Ah that does make sense for that case.

Is there any reason not to encourage using an instrument.js instead?

node --require ./instrument.js app.js
// instrument.js
const Sentry = require('@sentry/node');
const dsn = await getSentryDsn();
Sentry.init({ dsn });

// app.js
const express = require('express');

timfish avatar May 24 '24 14:05 timfish

// somewhere later
setTimeout(function() {
  Sentry.init(...);
}, 1000);

In this case, express is already uninstrumented before we run init :/

Ah that does make sense for that case.

Is there any reason not to encourage using an instrument.js instead?

node --require ./instrument.js app.js
// instrument.js
const Sentry = require('@sentry/node');
const dsn = await getSentryDsn();
Sentry.init({ dsn });

// app.js
const express = require('express');

Yeah, we generally do recommend that, but this is not possible if users cannot init at this point - e.g. if they load their DSN from somewhere else (something that we don't recommend, but there are people out there that have setups like this...)

So this is really just an alternative way to get stuff running for these people, and will def. not be the recommended way to init sentry, but just an escape hatch!

mydea avatar May 24 '24 14:05 mydea

My only minor concern is that we're racking up quite a few different ways to initialise the SDK.

yeah I also feel this, but I think this is an important thing to expose, so let's release. Maybe we can iterate and combine @sentry/node/preload and @sentry/node/import somehow 🤔.

We could, eventually, make import be a variant of preload with some env var set 🤔 but let's get this out for now and tweak stuff later as needed!

mydea avatar May 27 '24 07:05 mydea

Out of curiosity, what's a use case for this? I can't imagine any situation in which you don't have control over the entry point of your code. Maybe with frameworks like Next?

jeengbe avatar May 27 '24 18:05 jeengbe