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

Broken custom dataloader methods #13869

Open
3 tasks done
michalsanger opened this issue Oct 4, 2024 · 3 comments
Open
3 tasks done

Broken custom dataloader methods #13869

michalsanger opened this issue Oct 4, 2024 · 3 comments
Assignees
Labels
Integration: dataloader Package: node Issues related to the Sentry Node SDK

Comments

@michalsanger
Copy link

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

8.33.1

Framework Version

fastify 5.0.0

Link to Sentry event

No response

Reproduction Example/SDK Setup

No response

Steps to Reproduce

The automated dataloader integration added in #13664 breaks my code.

I have a dataloader like this:

export class ProductDataloader extends DataLoader {
    constructor() {
        super(async (globalIds: ReadonlyArray<string>) => {
            return batchFn(globalIds);
        });
    }

    public async loadByInternalId(id: number) {
        const globalId = toGlobalId('Product', id);
        return this.load(globalId);
    }
}

Expected Result

That code worked OK until "@sentry/node": "8.30.0"

Actual Result

After upgrade to "@sentry/node": "8.31.0" or later, calling loadByInternalId method fails:

context.dataloaders.product.loadByInternalId is not a function
@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Oct 4, 2024
@github-actions github-actions bot added the Package: node Issues related to the Sentry Node SDK label Oct 4, 2024
@AbhiPrasad
Copy link
Member

Hey @michalsanger sorry for the trouble here! For now you can disable the dataloader integration like so:

Sentry.init({
  // ...
  integrations: function (integrations) {
    // filter out Dataloader integration due to bug
    return integrations.filter(function (integration) {
      return integration.name !== "Dataloader";
    });
  },
});

docs: https://docs.sentry.io/platforms/javascript/guides/node/configuration/integrations/#removing-a-default-integration

@onurtemizkan could you help investigate why this is happening?

@michalsanger
Copy link
Author

@AbhiPrasad thanks for the workaround, works OK.

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Oct 4, 2024
AbhiPrasad pushed a commit that referenced this issue Oct 4, 2024
…ons (#13873)

Ref: #13869

Removing `dataloader` instrumentation from default integrations until we
patch this upstream.
@getsantry getsantry bot removed the status in GitHub Issues with 👀 3 Oct 7, 2024
@onurtemizkan
Copy link
Collaborator

Opened a PR upstream to fix this: open-telemetry/opentelemetry-js-contrib#2498

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Integration: dataloader Package: node Issues related to the Sentry Node SDK
Projects
Status: No status
Development

No branches or pull requests

4 participants