-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
fix(node-core): Apply correct SDK metadata #17014
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Node SDK Metadata Overwrite Error
The Node SDK incorrectly reports 'sentry.javascript.node-core' metadata instead of 'sentry.javascript.node'. This occurs because the applySdkMetadata call for 'node' in the Node SDK's _init function is overwritten by a subsequent applySdkMetadata call for 'node-core' within initNodeCore, as both operate on the same options object.
packages/node/src/sdk/index.ts#L52-L59
sentry-javascript/packages/node/src/sdk/index.ts
Lines 52 to 59 in 88a975a
| ): NodeClient | undefined { | |
| applySdkMetadata(options, 'node'); | |
| const client = initNodeCore({ | |
| ...options, | |
| // Only use Node SDK defaults if none provided | |
| defaultIntegrations: options.defaultIntegrations ?? getDefaultIntegrationsImpl(options), | |
| }); |
packages/node-core/src/sdk/index.ts#L123-L124
sentry-javascript/packages/node-core/src/sdk/index.ts
Lines 123 to 124 in 88a975a
| applySdkMetadata(options, 'node-core'); |
Was this report helpful? Give feedback by reacting with 👍 or 👎
size-limit report 📦
|
While adding the SDK to the release-registry I noticed that we never apply the correct metadata to node-core.
64d056a failing tests show the node-core SDK getting incorrect
sentry.javascript.nodemetadata (fromNodeClient)88a975a applies correct metadata to both SDKs
I was debating changing the underlying
NodeClientto not automatically setsentry.javascript.nodebut wasn't sure of the implications so decided not to.