feat(nestjs): add orchestrion integration#21796
Draft
isaacs wants to merge 5 commits into
Draft
Conversation
Port the entire vendored OTel core `instrumentation-nestjs-core` functionality. Still remaining are the 4 Sentry decorator instrumentations and then wiring up the final `experimentalUseDiagnosticsChannelIntegration()` piece with `replacedOtelIntegrationNames: ['Nest']` in the Node SDK, and an E2E test to verify that it matches the OTel functionality.
Port the @Injectable and @catch decorators, so that the entire NestJS OTel instrumentation is ported to Sentry Orchestrion implementation. Not yet wired up into the `experimentalUseDiagnosticsChannelInjection`, so still dormant at this stage. Pieces to come in following commits: - schedule (@Cron/@Interval/@timeout): error capture + isolation scope, no spans created - event (@onevent), bullmq (@processor): all the same astQuery inner-arrow-function pattern. - Final wire-in: add `experimentalUseDiagnosticsChannelInjection` + `replacedOtelIntegrationNames: ['Nest']` + opt-in e2e diffing against the OTel baseline.
Connect the `@Cron`/`@interval`/`@Timeout` (schedule), `@OnEvent` (event), and `@Processor` (bullmq) instrumentations in the orchestrion implementation. At this point, it's still not wired up by default into the SDK, but all of the functionality is there. Next step is the final wire-up and opt-in to swap out the OTel NestJS for this Orchestrion implementation.
Add `'Nest'` to the set of integrations that are implemented using Orchestrion, and which override a prior OTel based integration. The integration swap is moved into the `_init` method in the Node SDK, because the NestJS SDK (and other framework SDKs) will pass in its own defaultIntegrations array, which would bypass the old swap location. Now the swap is uniform for every framework SDK based on Node init, and respects `defaultIntegrations: false`. A new unit test is added that proves the opt-out leaves the defaults untouched, and opt-in replaces the named OTel integration with channel integrations. Full E2E testing is deferred until `@apm-js-collab/code-transformer` updates are published, because this depends on changes upstream, and E2E tests will pull in the published version.
156fa0f to
25cb29e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add
'Nest'to the set of integrations that are implemented using Orchestrion, and which override a prior OTel based integration.This is a port of the entire vendored OTel NestJS functionality to orchestrion-injected diagnostics channels.
The integration swap is moved into the
_initmethod in the Node SDK, because the NestJS SDK (and other framework SDKs) will pass in its own defaultIntegrations array, which would bypass the old swap location. Now the swap is uniform for every framework SDK based on Node init, and respectsdefaultIntegrations: false.Note: Depends on changes made upstream in
@apm-js-collab/code-transformer:When those are landed and published, an E2E test can be run that will actually pass. Code currently in place works with a local check-out, but obviously this is going to fail in CI for the time being.