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

graphqlIntegration: updating Sentry config removes responseHook from otel config #16004

Closed
3 tasks done
ldiqual opened this issue Apr 7, 2025 · 2 comments · Fixed by #16016
Closed
3 tasks done

graphqlIntegration: updating Sentry config removes responseHook from otel config #16004

ldiqual opened this issue Apr 7, 2025 · 2 comments · Fixed by #16016
Assignees
Labels
Bug Package: node Issues related to the Sentry Node SDK

Comments

@ldiqual
Copy link

ldiqual commented Apr 7, 2025

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

9.11.0

Framework Version

No response

Link to Sentry event

No response

Reproduction Example/SDK Setup

Important: I'm using late initialization using the ESM preload, which explains why setConfig is called twice.

Sentry.init({
  // ...
  integrations: [
    Sentry.graphqlIntegration({
      mergeItems: true, // Passed to otel, not defined in Sentry types
      ignoreResolveSpans: false,
      useOperationNameForRootSpan: true,
    }),
  ],
})

Steps to Reproduce

I added the following log line in otel's graphql instrumentation @opentelemetry/instrumentation-graphql/build/src/instrumentation.js:

setConfig(config = {}) {
  super.setConfig(Object.assign(Object.assign({}, DEFAULT_CONFIG), config));
  console.log('>>>setConfig done', this.getConfig())
}

First init (during esm preload) logs:

>>>setConfig done {
  enabled: true,
  mergeItems: false,
  depth: -1,
  allowValues: false,
  ignoreResolveSpans: true,
  ignoreTrivialResolveSpans: true,
  useOperationNameForRootSpan: true,
  responseHook: [Function: responseHook]
}

Second init (during late Sentry initialization):

>>>setConfig done {
  enabled: true,
  mergeItems: true,
  depth: -1,
  allowValues: false,
  ignoreResolveSpans: false,
  ignoreTrivialResolveSpans: true,
  useOperationNameForRootSpan: true
  // Note the missing hook
}

Expected Result

I would expect sentry to pass the requestHook again to setConfig so that useOperationNameForRootSpan can be applied.

Actual Result

responseHook is not passed

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Apr 7, 2025
@github-actions github-actions bot added the Package: node Issues related to the Sentry Node SDK label Apr 7, 2025
@lforst
Copy link
Member

lforst commented Apr 8, 2025

@mydea do we have a way around this?

Copy link
Member

mydea commented Apr 8, 2025

yeah, we need to rewrite this integration to do the option parsing separately 🤔 let me take a look…

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

Successfully merging a pull request may close this issue.

4 participants