-
Notifications
You must be signed in to change notification settings - Fork 230
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
Knex database spans not recorded when using async/await #873
Comments
Does the database span it self show up - just without the SQL query - or is the entire database span missing from the transaction? Would you mind sharing a code snippet of how you're making the query? |
The entire database span is missing. I created a project with an example: |
Thanks for sharing the test app. I've been able to reproduce the problem now. I don't think it's directly async/await related, but you're right that this is what triggers it in this case. Other apps with async/await do work though. So I guess it's something with the combination of how knex operates. I'll investigate further... |
Update: It's somehow related to a combination of async/await and async hooks. A temporary workaround is to set I'll continue to investigate to find a more permanent solution... |
This issue might be the same as discussed in #897 |
Same problem here, knex + async/await + asyncHooks (+ something else? not sure) = no active transaction for mysql spans. I'm on node 12. Turning off |
We are seeing this problem. Screenshot 1 here is with Screenshot 2 is the same transaction path with This is just a series of SQL statements being executed in order via Note: in the first screenshot there is a ping: @Qard |
FWI, we have the same issue when we use Objection.js (an SQL ORM): queries are not properly associated with the current transaction. The trace logs show that no transaction is found, even though in our case we are processing a route response from hapi. If we manually create a span it is attached to the correct transaction, and knex requests are properly instrumented and associated with the transaction. What is the recommended course of action? |
I've been subscribed to this issue because I was curious if there was a solution (proper tracing in node is a difficult thing to do). So this is mostly just a complete guess here, but I wonder if this is because knex now uses a module called "tarn" as its pooling library: https://github.com/vincit/tarn.js/ It looks like there's a instrumentation for generic-pool, which knex previously used: Perhaps adding a similar shim for tarn would help associate the trace? |
It's because the promise hooks feature in V8, which Node.js relies on for async_hooks, does not work with Thenables, which both Knex and Objection.js are using. This is unfixable without investing some time in fixing the issue in V8. See: nodejs/node#22360 |
Having this issue as well now, which led me to this thread. Are there any workarounds? |
The issue mentioned above by @Qard seems to be fixed on this PR nodejs/node#33778 (by @Qard). Looks like the fix will be released with Node.js v14.6.0 nodejs/node#34371 |
Yep, it's fixed in 14.6.x+. It hasn't been backported to 12.x yet, but shouldn't be too hard to backport. |
Can confirm that this issue is fixed when running the apm agent on Node.js >= 14.6 |
Describe the bug
When a define a route wich uses knex to connect do the database, and use async await at the function definition, the apm agent does not send the sql span, not showing the sql info at the request timeline at kibana. The apm agent does not support async/await
To Reproduce
Steps to reproduce the behavior:
Expected behavior
To show the sql span at the request timeline.
Environment (please complete the following information)
How are you starting the agent? (please tick one of the boxes)
agent.start()
directly (e.g.require('elastic-apm-node').start(...)
)elastic-apm-node/start
from within the source code-r elastic-apm-node/start
Additional context
Add any other context about the problem here.
Agent config options
Click to expand
package.json
dependencies:Click to expand
The text was updated successfully, but these errors were encountered: