feat(node): Experimentally add orchestrion support#20900
Conversation
size-limit report 📦
|
|
Note: dependency warning stuff should be addressed when this is merged/released: apm-js-collab/code-transformer-bundler-plugins#2 |
b1b6ed6 to
9e8b070
Compare
9e8b070 to
26ccdf4
Compare
26ccdf4 to
c8be420
Compare
a38481b to
c095626
Compare
5d957bf to
12ac21c
Compare
|
I also added an e2e test app using the vite plugin, however this is failing, will need to wait on v0.2.0 of the plugins as this updates to the latest version of the code transformer, which we need here. |
7d6f1f7 to
3de78da
Compare
isaacs
left a comment
There was a problem hiding this comment.
This is looking very good!
My only concern/suggestion is that it should really not be node-specific. Deno and Bun are both supported by orchestrion now, so it'd be good to move this logic into a shared location so they can all benefit from it as we add instrumentations.
I'm not sure if it's best to (a) land this first, and then refactor this and the other @sentry/core/server stuff into @sentry-internal/server-utils, or (b) if we should bite that bullet first and then move this implementation on top of it. If we go with (a), then the move would be to land this now, and do server-utils as a second step. If (b), then it'd be good to get that done, and port this on top of it.
jup, eventually this should go into a new server-utils package or similar I guess - we can move this up any time! |
Finish the the orchestrion.js-based auto-instrumentation so the Node SDK
presents a minimal API that does not expose the internal "orchestrion"
term.
A single opt-in method now does the entire Orchestrion setup.
```ts
Sentry.experimentalUseDiagnosticsChannelInjection();
// orchestrion integrations loaded and ready to go
Sentry.init({
dsn: '__DSN__'
tracesSampleRate: 1,
})
```
Assuming that this is run in a user's local `--import` module (or,
failing that, before init is called and instrumented modules are
loaded), then the appropriate hooks will be synchronously added, and
Orchestrion diagnostics_channel-based integrations will be used instead
of the legacy OTel integrations.
The previous three-step setup (`_experimentalUseOrchestrion` flag,
`--import @sentry/node/orchestrion`, plus a separate
`_experimentalSetupOrchestrion()` method call) is removed.
The channel-injection hooks are registered synchronously (mirroring
`esmLoader.ts`: `Module.register(...)` + `ModulePatch` on Node <24.13,
`Module.registerHooks` on newer / Deno 2.8+), so they are in place
before the app's own `import`s resolve. (A dynamic `import()` would have
raced module loading.)
The `@sentry/node/import` loader hook script injects the channels
**unconditionally**, as the presence of the channels is harmless when
they are not being used. They are only *subscribed* to if the opt-in
flag is set.
The version detection is moved to a single source of truth in
`server-utils/orchestrion/runtime/register.ts`, so that it can be
initialized synchronously as part of `Sentry.init()`, or in the
`import-hook.mjs` loader.
All references to `orchestrion` are removed from the public API surface.
The term remains in `server-utils`, of course, as this is an internal
package designed to house shared implementation details across
server-side JS platforms.
The `@sentry/node/orchestrion/vite` subpath export is removed, as that
really isn't used by anything, and was just a pass-through for
`@sentry/server-utils/orchestrion/vite` anyway. The Bun and Edge
computing SDKs will use this directly to instrument using Orchestrion.
Also, only install diagnostics channel injection when tracing enabled.
Every Orchestrion integration we're currently adding (ie, Mysql) or
likely to in the very near future, replaces an OTel integration that is
gated on span creation. So, there's no need to do this work if spans are
not enabled.
An e2e test is added to ensure that the orchestrion functionality is
*not* added in bundlers that do not explicitly opt into it.
fc56193 to
7d1f62e
Compare
Finish the the orchestrion.js-based auto-instrumentation so the Node SDK
presents a minimal API that does not expose the internal "orchestrion"
term.
A single opt-in method now does the entire Orchestrion setup.
```ts
Sentry.experimentalUseDiagnosticsChannelInjection();
// orchestrion integrations loaded and ready to go
Sentry.init({
dsn: '__DSN__'
tracesSampleRate: 1,
})
```
Assuming that this is run in a user's local `--import` module (or,
failing that, before init is called and instrumented modules are
loaded), then the appropriate hooks will be synchronously added, and
Orchestrion diagnostics_channel-based integrations will be used instead
of the legacy OTel integrations.
The previous three-step setup (`_experimentalUseOrchestrion` flag,
`--import @sentry/node/orchestrion`, plus a separate
`_experimentalSetupOrchestrion()` method call) is removed.
The channel-injection hooks are registered synchronously (mirroring
`esmLoader.ts`: `Module.register(...)` + `ModulePatch` on Node <24.13,
`Module.registerHooks` on newer / Deno 2.8+), so they are in place
before the app's own `import`s resolve. (A dynamic `import()` would have
raced module loading.)
The `@sentry/node/import` loader hook script injects the channels
**unconditionally**, as the presence of the channels is harmless when
they are not being used. They are only *subscribed* to if the opt-in
flag is set.
The version detection is moved to a single source of truth in
`server-utils/orchestrion/runtime/register.ts`, so that it can be
initialized synchronously as part of `Sentry.init()`, or in the
`import-hook.mjs` loader.
All references to `orchestrion` are removed from the public API surface.
The term remains in `server-utils`, of course, as this is an internal
package designed to house shared implementation details across
server-side JS platforms.
The `@sentry/node/orchestrion/vite` subpath export is removed, as that
really isn't used by anything, and was just a pass-through for
`@sentry/server-utils/orchestrion/vite` anyway. The Bun and Edge
computing SDKs will use this directly to instrument using Orchestrion.
Also, only install diagnostics channel injection when tracing enabled.
Every Orchestrion integration we're currently adding (ie, Mysql) or
likely to in the very near future, replaces an OTel integration that is
gated on span creation. So, there's no need to do this work if spans are
not enabled.
An e2e test is added to ensure that the orchestrion functionality is
*not* added in bundlers that do not explicitly opt into it.
7d1f62e to
7323f1a
Compare
|
@timfish Updated this with the fixes that we discussed. PTAL when you get a chance! :) |
| dependencies: | ||
| json-schema-to-ts "^3.1.1" | ||
|
|
||
| "@apm-js-collab/code-transformer-bundler-plugins@^0.3.0": |
There was a problem hiding this comment.
should we bump to 0.4.0 right away, while we're at it?
|
Can't submit a review because it is my PR lol xD but this is good to go from my POV, left some small nits but nothing important, maybe the only thing would be to bump the orchestrion dependencies to latest while we're at it. |
| // esbuild and friends rewrite `import.meta.url` to `{}` for CJS output, | ||
| // which would make `createRequire(undefined)` throw. | ||
| // Only use `import.meta.url` in true ESM, where there's no `require` | ||
| const nodeRequire = typeof require === 'function' ? require : createRequire(import.meta.url); |
There was a problem hiding this comment.
Unrelated specifically to this PR but I think we should move towards a bundler plugin with magic comments like we have here for dev/prod. This way we can write "pure" ESM/CJS and have the bundler keep the correct one.
Something like:
/*! rollup-include-cjs-only */
const nodeRequire = require;
/*! rollup-include-csj-only-end */
/*! rollup-include-esm-only */
const nodeRequire = createRequire(import.meta.url);
/*! rollup-include-esm-only-end */This is a WIP POC trying out usage of orchestrion-js for node SDK
instrumentation.
1. Built a general plan document outlining how this can/should work
2. Implemented the generic utilities and building blocks needed
3. Implemented a example integration for mysql package using the new
pieces
Honestly it seems pretty straightforward... Usage for this POC is:
```bash
node --import @sentry/node/orchestrion app.mjs
```
And then
```js
// app.mjs
import * as Sentry from '@sentry/node';
const client = Sentry.init({
// regular setup...
_experimentalUseOrchestrion: true,
});
// Split this way for better tree shaking
Sentry._experimentalSetupOrchestrion(client);
```
This will disable the otel instrumentation that is already converted to
orchestrion (in this PR, only Mysql) and add the respective
orchestrion-based integrations instead. The exact API here is WIP and
really just geared towards experimentation, so could change, and it's
easy to see how this would be easier in v11 with this being the default.
Some general benefits of this approach:
1. preload becomes unnecessary as this approach generally behaves like
preload - the `--import` script only registers the mappings for
orchestrion, all actual code registering stuff etc. happens in
`Sentry.init()`. This makes a bunch of things easier...
2. Not tested here, but this should generally work exactly the same if
you add the respective vite (and others in the future) plugin,
allowing you to skip the `--import`. This also works when deploying
to e.g. cloudflare etc. as long as one of the bundler plugins is
used.
3. The whole approach is much easier to reconcile with dual-system
approaches where newer versions have native DC/TC support - just need
to register different channel names mostly to get stuff working.
Also, fix the `import ... from 'node:module'` in the import-hook.mjs, which was broken.
Finish the the orchestrion.js-based auto-instrumentation so the Node SDK
presents a minimal API that does not expose the internal "orchestrion"
term.
A single opt-in method now does the entire Orchestrion setup.
```ts
Sentry.experimentalUseDiagnosticsChannelInjection();
// orchestrion integrations loaded and ready to go
Sentry.init({
dsn: '__DSN__'
tracesSampleRate: 1,
})
```
Assuming that this is run in a user's local `--import` module (or,
failing that, before init is called and instrumented modules are
loaded), then the appropriate hooks will be synchronously added, and
Orchestrion diagnostics_channel-based integrations will be used instead
of the legacy OTel integrations.
The previous three-step setup (`_experimentalUseOrchestrion` flag,
`--import @sentry/node/orchestrion`, plus a separate
`_experimentalSetupOrchestrion()` method call) is removed.
The channel-injection hooks are registered synchronously (mirroring
`esmLoader.ts`: `Module.register(...)` + `ModulePatch` on Node <24.13,
`Module.registerHooks` on newer / Deno 2.8+), so they are in place
before the app's own `import`s resolve. (A dynamic `import()` would have
raced module loading.)
The `@sentry/node/import` loader hook script injects the channels
**unconditionally**, as the presence of the channels is harmless when
they are not being used. They are only *subscribed* to if the opt-in
flag is set.
The version detection is moved to a single source of truth in
`server-utils/orchestrion/runtime/register.ts`, so that it can be
initialized synchronously as part of `Sentry.init()`, or in the
`import-hook.mjs` loader.
All references to `orchestrion` are removed from the public API surface.
The term remains in `server-utils`, of course, as this is an internal
package designed to house shared implementation details across
server-side JS platforms.
The `@sentry/node/orchestrion/vite` subpath export is removed, as that
really isn't used by anything, and was just a pass-through for
`@sentry/server-utils/orchestrion/vite` anyway. The Bun and Edge
computing SDKs will use this directly to instrument using Orchestrion.
Also, only install diagnostics channel injection when tracing enabled.
Every Orchestrion integration we're currently adding (ie, Mysql) or
likely to in the very near future, replaces an OTel integration that is
gated on span creation. So, there's no need to do this work if spans are
not enabled.
An e2e test is added to ensure that the orchestrion functionality is
*not* added in bundlers that do not explicitly opt into it.
7323f1a to
92b508d
Compare
I think we only do that for server-utils and other "implementation detail" type things. Is there a piece that I missed? I assume that most users will just use the Node/Bun/Deno SDK surfaces, which only refer to this as |
Finish the the orchestrion.js-based auto-instrumentation so the Node SDK
presents a minimal API that does not expose the internal "orchestrion"
term.
A single opt-in method now does the entire Orchestrion setup.
```ts
Sentry.experimentalUseDiagnosticsChannelInjection();
// orchestrion integrations loaded and ready to go
Sentry.init({
dsn: '__DSN__'
tracesSampleRate: 1,
})
```
Assuming that this is run in a user's local `--import` module (or,
failing that, before init is called and instrumented modules are
loaded), then the appropriate hooks will be synchronously added, and
Orchestrion diagnostics_channel-based integrations will be used instead
of the legacy OTel integrations.
The previous three-step setup (`_experimentalUseOrchestrion` flag,
`--import @sentry/node/orchestrion`, plus a separate
`_experimentalSetupOrchestrion()` method call) is removed.
The channel-injection hooks are registered synchronously (mirroring
`esmLoader.ts`: `Module.register(...)` + `ModulePatch` on Node <24.13,
`Module.registerHooks` on newer / Deno 2.8+), so they are in place
before the app's own `import`s resolve. (A dynamic `import()` would have
raced module loading.)
The `@sentry/node/import` loader hook script injects the channels
**unconditionally**, as the presence of the channels is harmless when
they are not being used. They are only *subscribed* to if the opt-in
flag is set.
The version detection is moved to a single source of truth in
`server-utils/orchestrion/runtime/register.ts`, so that it can be
initialized synchronously as part of `Sentry.init()`, or in the
`import-hook.mjs` loader.
All references to `orchestrion` are removed from the public API surface.
The term remains in `server-utils`, of course, as this is an internal
package designed to house shared implementation details across
server-side JS platforms.
The `@sentry/node/orchestrion/vite` subpath export is removed, as that
really isn't used by anything, and was just a pass-through for
`@sentry/server-utils/orchestrion/vite` anyway. The Bun and Edge
computing SDKs will use this directly to instrument using Orchestrion.
Also, only install diagnostics channel injection when tracing enabled.
Every Orchestrion integration we're currently adding (ie, Mysql) or
likely to in the very near future, replaces an OTel integration that is
gated on span creation. So, there's no need to do this work if spans are
not enabled.
An e2e test is added to ensure that the orchestrion functionality is
*not* added in bundlers that do not explicitly opt into it.
92b508d to
a1fba56
Compare
Finish the the orchestrion.js-based auto-instrumentation so the Node SDK
presents a minimal API that does not expose the internal "orchestrion"
term.
A single opt-in method now does the entire Orchestrion setup.
```ts
Sentry.experimentalUseDiagnosticsChannelInjection();
// orchestrion integrations loaded and ready to go
Sentry.init({
dsn: '__DSN__'
tracesSampleRate: 1,
})
```
Assuming that this is run in a user's local `--import` module (or,
failing that, before init is called and instrumented modules are
loaded), then the appropriate hooks will be synchronously added, and
Orchestrion diagnostics_channel-based integrations will be used instead
of the legacy OTel integrations.
The previous three-step setup (`_experimentalUseOrchestrion` flag,
`--import @sentry/node/orchestrion`, plus a separate
`_experimentalSetupOrchestrion()` method call) is removed.
The channel-injection hooks are registered synchronously (mirroring
`esmLoader.ts`: `Module.register(...)` + `ModulePatch` on Node <24.13,
`Module.registerHooks` on newer / Deno 2.8+), so they are in place
before the app's own `import`s resolve. (A dynamic `import()` would have
raced module loading.)
The `@sentry/node/import` loader hook script injects the channels
**unconditionally**, as the presence of the channels is harmless when
they are not being used. They are only *subscribed* to if the opt-in
flag is set.
The version detection is moved to a single source of truth in
`server-utils/orchestrion/runtime/register.ts`, so that it can be
initialized synchronously as part of `Sentry.init()`, or in the
`import-hook.mjs` loader.
All references to `orchestrion` are removed from the public API surface.
The term remains in `server-utils`, of course, as this is an internal
package designed to house shared implementation details across
server-side JS platforms.
The `@sentry/node/orchestrion/vite` subpath export is removed, as that
really isn't used by anything, and was just a pass-through for
`@sentry/server-utils/orchestrion/vite` anyway. The Bun and Edge
computing SDKs will use this directly to instrument using Orchestrion.
Also, only install diagnostics channel injection when tracing enabled.
Every Orchestrion integration we're currently adding (ie, Mysql) or
likely to in the very near future, replaces an OTel integration that is
gated on span creation. So, there's no need to do this work if spans are
not enabled.
An e2e test is added to ensure that the orchestrion functionality is
*not* added in bundlers that do not explicitly opt into it.
a1fba56 to
ce95b7a
Compare
This is a WIP POC trying out usage of orchestrion-js for node SDK
instrumentation.
1. Built a general plan document outlining how this can/should work
2. Implemented the generic utilities and building blocks needed
3. Implemented a example integration for mysql package using the new
pieces
Honestly it seems pretty straightforward... Usage for this POC is:
```bash
node --import @sentry/node/orchestrion app.mjs
```
And then
```js
// app.mjs
import * as Sentry from '@sentry/node';
const client = Sentry.init({
// regular setup...
_experimentalUseOrchestrion: true,
});
// Split this way for better tree shaking
Sentry._experimentalSetupOrchestrion(client);
```
This will disable the otel instrumentation that is already converted to
orchestrion (in this PR, only Mysql) and add the respective
orchestrion-based integrations instead. The exact API here is WIP and
really just geared towards experimentation, so could change, and it's
easy to see how this would be easier in v11 with this being the default.
Some general benefits of this approach:
1. preload becomes unnecessary as this approach generally behaves like
preload - the `--import` script only registers the mappings for
orchestrion, all actual code registering stuff etc. happens in
`Sentry.init()`. This makes a bunch of things easier...
2. Not tested here, but this should generally work exactly the same if
you add the respective vite (and others in the future) plugin,
allowing you to skip the `--import`. This also works when deploying
to e.g. cloudflare etc. as long as one of the bundler plugins is
used.
3. The whole approach is much easier to reconcile with dual-system
approaches where newer versions have native DC/TC support - just need
to register different channel names mostly to get stuff working.
EDIT(@isaacs): updated description to reflect where this PR landed, for more straightforward review.
Use orchestrion for instrumentation, providing the shared implementation in
packages/server-utils, and the Node integration. (Deno and Bun in subsequent PRs.)This implementation avoids exposing the "orchestrion" terminology to any user-facing API surface. Instead, the Node SDK presents a minimal API surface, with an opt-in that triggers the orchestrion instrumentations to replace their OTel counterparts. (Currently, this is just Mysql, but more will be added in the future.)
A single opt-in method now does the entire Orchestrion setup.
Assuming that this is run in a user's local
--importmodule (or, failing that, before the instrumented modules are loaded), then the appropriate hooks will be synchronously added, and Orchestrion diagnostics_channel-based integrations will be used instead of the legacy OTel integrations.The channel-injection hooks are registered synchronously (mirroring
esmLoader.ts:Module.register(...)+ModulePatchon Node <24.13,Module.registerHookson newer / Deno 2.8+), so they are in place before the app's ownimports resolve.The
@sentry/node/importloader hook script injects the channels unconditionally, as the presence of the channels is harmless when they are not being used. They are only subscribed to if the opt-in flag is set. (If tracing is disabled, then they're not enabled at all, since the integrations would not be in use. When and if we have an Orchestrion-based integration that is not spans-only, we can make this check more elaborate, but it seems like overkill for now.)The version detection lives in a single source of truth in
server-utils/orchestrion/runtime/register.ts, so that it can be initialized synchronously as part ofSentry.init(), or in theimport-hook.mjsloader, and enables the appropriate module hooks for Deno 2.8.0+ and supported Node versions.