Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Better approach for asynchronous spans #27

Open
SimonSimCity opened this issue Aug 15, 2019 · 7 comments
Open

Better approach for asynchronous spans #27

SimonSimCity opened this issue Aug 15, 2019 · 7 comments

Comments

@SimonSimCity
Copy link
Member

The spans - specially the span async - are often very misleading. This was already an issue in Kadira.

Imagine a meteor method is executed which connects to the database. While it is waiting for the database another call comes in - let's say, an HTTP request. NodeJS will start processing the second request, which replaces the agent.currentTransaction. If the second process now starts waiting for the database and the first comes back and you do some work asynchronously, those might get logged to the wrong request.

A better approach would be to write the current transaction as a reference to the fibers instance, but this is something I have to dive into. One idea would also be to look into async_hooks: https://nodejs.org/api/async_hooks.html

@kschingiz
Copy link
Collaborator

@SimonSimCity elastic apm is already tracing async executions. Depending on the async context agent.currentTransaction will be different.
Take a look at this script:
https://github.com/elastic/apm-agent-nodejs/blob/master/lib/instrumentation/async-hooks.js
They are defining currentTransaction which returns transactions depending on the execution context.
So for meteor method and it's child async execution: current transaction is one
For http request and it's child async execution: current transaction is different.
Maybe Kadira had issues on async tracking, but I don't think that elastic agent has also too.

@kschingiz
Copy link
Collaborator

@SimonSimCity if you have sample project where tracing is broken, that would be helpful.

@SimonSimCity
Copy link
Member Author

I can tell you so far that I've disabled async spans for now since it was too much chaos. Furthermore I found some db-spans within the transaction of a REST request which show a stack-trace which shows that this db-span actually belongs to a transaction of a meteor-method.

If you have the time, I'd very much appreciate it if you would take a look into it. We're currently in our high-season which forces me to focus on short-term fixes to keep everything running for this season, but I'll definitively come back to this once it's a bit more relaxed over here.

@kschingiz
Copy link
Collaborator

Never encountered this issue. Let's keep this issue open, I will investigate it. Unfortunately I am also experiencing lack of time.

@SimonSimCity
Copy link
Member Author

I've just noticed this PR which could be related ... elastic/apm-agent-nodejs#1306

@linegel
Copy link
Contributor

linegel commented Aug 6, 2020

Does this PR elastic/apm-agent-nodejs#1306 actually solved problem for you @SimonSimCity?

@SimonSimCity
Copy link
Member Author

A good question ... I've disabled the async-spans as they just filled my monitoring without adding additional value. If someone coudl confirm that this works on all of the processes on a busy machine, that would be nice.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants