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

mysql2 requests aren't being attached to root scope #946

Closed
watchinharrison opened this issue Dec 17, 2018 · 9 comments
Closed

mysql2 requests aren't being attached to root scope #946

watchinharrison opened this issue Dec 17, 2018 · 9 comments
Assignees
Labels
api: cloudtrace Issues related to the googleapis/cloud-trace-nodejs API. external fix 🚨 This issue needs some love. status: blocked Resolving the issue is dependent on other work. triage me I really want to be triaged.

Comments

@watchinharrison
Copy link

The log is outputting INFO TraceApi#createChildSpan: [mysql2@1.6.4:lib/connection.js] Creating phantom child span [mysql-query] because there is no root span..

My setup is using knex with the client mysql2. The query is running in the knex raw method.

This is an example app: https://github.com/davidharrisonlb/mysql-tracer-test

@kjin
Copy link
Contributor

kjin commented Dec 17, 2018

Thanks for reporting this -- I'm looking into it now.

@watchinharrison
Copy link
Author

No problem. I spent the evening trying to work it out too. Think it could be knex’s use of bluebird for promises. Will update with anything further I find today.

@JustinBeckwith JustinBeckwith added the triage me I really want to be triaged. label Dec 18, 2018
@kjin
Copy link
Contributor

kjin commented Dec 18, 2018

I think I've found the issue -- it seems that the Trace Agent (or anything reliant on async_hooks, for that matter) currently doesn't work for code that awaits a custom thenable, like a Bluebird Promise (see this minimum repro). (Edit: The repro is inaccurate.)

This is a problem that is only addressable in Node core, I'll scan the issue list shortly and add it if it's not there. In the meantime, it seems like this functions as a workaround:

  // explicitly call the first `.then` rather than let it be implicitly be called via the `await` keyword
  const result = await database.raw('SELECT 1').then(_ => _);

@kjin kjin added priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. external fix type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. and removed triage me I really want to be triaged. labels Dec 18, 2018
@JustinBeckwith JustinBeckwith added the triage me I really want to be triaged. label Dec 18, 2018
@kjin
Copy link
Contributor

kjin commented Dec 18, 2018

It seems like it is indeed a variant of an existing issue: nodejs/node#22360.

On that note, however, it seems like a more immediate solution is to move forward with the bluebird plugin PR that I had open a while ago. I'll try to amend it to work for this use case as well and see if I can land it this week.

@kjin kjin removed the triage me I really want to be triaged. label Dec 19, 2018
@JustinBeckwith JustinBeckwith added triage me I really want to be triaged. 🚨 This issue needs some love. labels Dec 19, 2018
@majelbstoat
Copy link

I'm glad to see this issue - I've been struggling with connecting distributed traces from a js grpc client, but I too am promisifying those methods with bluebird, so hopefully that PR will help.

@kjin kjin self-assigned this Jan 7, 2019
@kjin kjin removed the triage me I really want to be triaged. label Jan 7, 2019
@kjin
Copy link
Contributor

kjin commented Feb 13, 2019

Sorry that this has been stalled. I believe this is a fix that must be done in Node itself and not in userspace, so unfortunately I can't amend the bluebird PR to address this issue. You can follow the Node bug here: nodejs/node#26064

@kjin kjin added status: blocked Resolving the issue is dependent on other work. and removed 🚨 This issue needs some love. labels Feb 13, 2019
@yoshi-automation yoshi-automation added the 🚨 This issue needs some love. label Feb 13, 2019
@ofrobots ofrobots removed priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. labels Feb 20, 2019
@yoshi-automation yoshi-automation added the triage me I really want to be triaged. label Feb 20, 2019
@kjin kjin closed this as completed in #981 Feb 28, 2019
@giuliano-barberi-tf
Copy link

It looks like Bluebird has been removed in the newest release of knex. Does anyone know if this issue is fixed now?

@s5no5t
Copy link

s5no5t commented Mar 19, 2020

@giuliano-barberi-tf I have tested this today, and it isn't fixed.

@samcfinan
Copy link

Experiencing a similar issue with Sequelize v5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: cloudtrace Issues related to the googleapis/cloud-trace-nodejs API. external fix 🚨 This issue needs some love. status: blocked Resolving the issue is dependent on other work. triage me I really want to be triaged.
Projects
None yet
Development

No branches or pull requests

9 participants