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

Mongoose tracing plugin instrumentation does not trace useful timings #4078

Closed
1 task done
andreialecu opened this issue Oct 19, 2021 · 5 comments
Closed
1 task done

Comments

@andreialecu
Copy link
Contributor

Package + Version

  • @sentry/tracing: 6.13.3

Version:

6.13.3

Description

Using the mongoose tracing plugin does not produce useful data because it seems to patch functions like .find() which are usually chained:

Calls usually look like:

      const results = await model.find({
        ...query
      })
      .sort({ scheduledDate: 1 })
      .lean();

However, the plugin attaches to the find method which returns immediately. Proof of this here:

Screenshot 2021-10-19 at 15 41 43

Instead of attaching to find, it should attach to exec.

Reference for properly instrumenting mongoose: https://github.com/aspecto-io/opentelemetry-ext-js/blob/master/packages/instrumentation-mongoose/src/mongoose.ts

Originally implemented in:
#3252
#3176

@github-actions
Copy link
Contributor

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

@ptrin
Copy link

ptrin commented Feb 21, 2024

It's misleading that this was closed as completed. There is still no useful tracing integration for Mongoose, despite Sentry advertising that Mongoose is auto-discovered with the Node.js performance integrations.

@iker-barriocanal Can you please add the label Status: Backlog so there can be a faint hope of this bug in the integration being fixed?

@iker-barriocanal
Copy link
Contributor

Maybe somebody from @getsentry/team-web-sdk-frontend can take a look?

@Lms24
Copy link
Member

Lms24 commented Feb 22, 2024

Thanks for the ping @iker-barriocanal!

@ptrin, there is indeed some hope ;) We're currently working on our new SDK major version v8 (#9508) where one of the core changes will be that our Node instrumentations (including DB integrations like Mongoose) will be based on OpenTelemetry. So chances are that the issue you're experiencing will be fixed with v8. We'll release a first alpha version next week and we'd love to get some feedback on it if you have time.

@AbhiPrasad
Copy link
Member

I think we have a bug with auto discovery - that dynamic require needs to look at mongoose instead:

const integration = dynamicRequire(module, './mongo') as {
Mongo: IntegrationClass<LazyLoadedIntegration>;
};
return new integration.Mongo({ mongoose: true });

For now you can just add Sentry.Integrations.Mongo yourself to init to get this working.

Sentry.init({
  integrations: [
    new Sentry.Integrations.Mongo({
      useMongoose: true // Default: false
    }),
  ],
});

and yes as @Lms24 mentioned above with v8 this will be fixed (and the instrumentation will be higher quality!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants