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

Is telemetry synchronous? #405

Closed
janpio opened this issue Feb 13, 2019 · 5 comments
Closed

Is telemetry synchronous? #405

janpio opened this issue Feb 13, 2019 · 5 comments

Comments

@janpio
Copy link
Member

janpio commented Feb 13, 2019

I noticed some unexpected slowness in the CLI. Is it possible that telemetry calls are sync?

See the following output:

E:\Projects\throwaway\x4wTest (master -> origin) ([email protected])
λ cordova telemetry on
Thanks for opting into telemetry to help us improve cordova.

E:\Projects\throwaway\x4wTest (master -> origin) ([email protected])
λ cordova -v | gnomon
   2.1843s   8.1.2 ([email protected])
   0.0016s

     Total   2.1887s

E:\Projects\throwaway\x4wTest (master -> origin) ([email protected])
λ cordova telemetry off
You have been opted out of telemetry. To change this, run: cordova telemetry on.

E:\Projects\throwaway\x4wTest (master -> origin) ([email protected])
λ cordova -v | gnomon
   1.2091s   8.1.2 ([email protected])
   0.0017s

     Total   1.2138s

(gnomon is the tool that measures the time it takes for each line of the output to appear, install via npm install -g gnomon.)


λ cordova -v
8.1.2 ([email protected])
@raphinesse
Copy link
Contributor

The tracking should run asynchronously in a new process: yeoman/insight#34

@raphinesse
Copy link
Contributor

Well, the question in the title is answered. Should we close this or rename it to something more actionable?

@raphinesse raphinesse added info-needed We need more information from the reporter question and removed info-needed We need more information from the reporter labels Apr 11, 2019
@janpio
Copy link
Member Author

janpio commented Apr 11, 2019

Why is the CLI slower when telemtry is enabled then?
Or does it still wait for that process to finish before returning to the console?

@raphinesse
Copy link
Contributor

raphinesse commented Apr 11, 2019

I also experience the difference. 1 s without telemetry vs 2 s with. I don't see any offenders in a 0x flame graph which does suggests that a child process is responsible for the delay. However, the child process should not be waited upon, the way it is handled:
https://github.com/yeoman/insight/blob/cb32c1a043c54d50ef1aff04c7a70f761bcb8035/lib/index.js#L85-L88

So it's maybe just the overhead of spawning a new Node.js process and doing the IPC 🤷‍♂️

I think the only thing we could do is to create a minimal example that showcases this and report it at the inquirer repo. But then again, that's what yeoman/insight#34 seems to be.

@dpogue
Copy link
Member

dpogue commented Jun 18, 2024

We're removed telemetry support, so closing this issue

@dpogue dpogue closed this as completed Jun 18, 2024
@dpogue dpogue added this to the 13.0.0 milestone Jun 18, 2024
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