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

Instrument the child_process module #920

Closed
juanjoDiaz opened this issue Apr 17, 2020 · 9 comments
Closed

Instrument the child_process module #920

juanjoDiaz opened this issue Apr 17, 2020 · 9 comments

Comments

@juanjoDiaz
Copy link

Would it be possible to instrument the child_process module so time spent on launching external programs and errors on those are included in the traces?

@rochdev
Copy link
Member

rochdev commented Apr 17, 2020

Could you describe your use case? My worry is that child_process can be used for very long lived processes, which could result in traces that last days or weeks.

@juanjoDiaz
Copy link
Author

Hi @rochdev,

There are plenty of npm libraries that just wrap binaries which they call using child_process.
In my case, I have my own module that wraps VboxManage the VirtualBox CLI tool.

I think that this can be a fairly common use case.

@rochdev
Copy link
Member

rochdev commented Apr 20, 2020

Does this module always call child_process as part of an existing trace, for example in an HTTP request? One way to prevent long lived spans would be to only create a child_process span when that's the case.

@juanjoDiaz
Copy link
Author

In 99% of the cases, it does.
In 1% of the cases, it runs as part of a cron job that runs periodically to clean up stuff.

Just for my own understanding, by trying to avoid this long spans, are you trying to avoid some issue with how this library gathers the information or just trying to avoid the users to shoot their own feet?

@rochdev
Copy link
Member

rochdev commented Apr 22, 2020

Is the 1% where it's a cron job interesting for you? Or would the 99% where something is waiting be enough?

Just for my own understanding, by trying to avoid this long spans, are you trying to avoid some issue with how this library gathers the information or just trying to avoid the users to shoot their own feet?

In this case it's really to avoid having an auto-instrumentation that results in never-ending traces by default. If you have use cases without a parent that you are interested in, I could always add a configuration option on the plugin to configure that behavior. But I want to make sure that the default won't break tracing.

@juanjoDiaz
Copy link
Author

Hi,

Sorry I totally missed your last question.

Ideally one always wants 100% visibility. Although 99% visibility is better than the current 0%.

I think that child_process calls to the shell will finish in most cases. I wouldn't really make sense otherwise.
If some particular process is launched that never ends, it doesn't sound like a big drama to me (although maybe I'm missing something...)

@rochdev
Copy link
Member

rochdev commented Sep 24, 2020

That makes sense. This is not in our plans short-term however, so for your use case specifically are you (or a library you own) calling child_process directly, or at least synchronously? If that's the case then it should be straightforward to instrument it manually.

For example:

tracer.trace('child_process.exec', (span, cb) => {
  child_process.exec('echo "hello"', err => {
    cb(err)
    // your code
  })
})

Does that work for your use case at least for now?

@bengl
Copy link
Collaborator

bengl commented May 19, 2021

The manual instrumentation option was given here, and I don't think we're likely to instrument child_process for reasons @rochdev mentioned. We're happy to accept feature requests or PRs for instrumenting libraries that are explicitly not running long processes. I'll close this for now, but lease feel free to reopen or comment if you need to.

@bengl bengl closed this as completed May 19, 2021
@ikonst
Copy link
Contributor

ikonst commented May 13, 2024

It looks like current dd-trace-js does instrument child_process, which in turns results in exactly the problems described by @rochdev above:
#3608

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

4 participants