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

Trace spans have inconsistent timing #897

Closed
4 tasks done
ngraef opened this issue Mar 26, 2021 · 4 comments · Fixed by #899
Closed
4 tasks done

Trace spans have inconsistent timing #897

ngraef opened this issue Mar 26, 2021 · 4 comments · Fixed by #899

Comments

@ngraef
Copy link
Contributor

ngraef commented Mar 26, 2021

Prerequisites

Please answer the following questions for yourself before submitting an issue.

  • I am running the latest version
  • I checked the documentation and found no answer
  • I checked to make sure that this issue has not already been filed
  • I'm reporting the issue to the correct repository

Current Behavior

Trace spans from moleculer don't line up with spans from third-party modules or moleculer spans from remote services (different processes/brokers). For example, the red span in the image below should fully contain the green and purple spans under it.

span offset

Expected Behavior

Span times should be as consistent as possible across all modules.

Screen Shot 2021-03-26 at 4 50 01 PM

Failure Information

This is caused by clock drift in moleculer's tracing/now.js implementation, which is used to set the startTime of spans. The following code and charts test this behavior and show how much a span could drift compared to Date.now().

Reproduce code snippet

const now = require("./src/tracing/now");

// Output CSV for analysis
console.log(`"Time elapsed (ms)","Drift (ms)"`);

const startTime = Date.now();
console.log(`0,${now() - startTime}`);

setInterval(() => {
    const time = Date.now();
    console.log(`${time - startTime},${now() - time}`);
}, 5e3);

Here are charts from two sample runs:

20 minutes, 25 ms drift
span time drift

60 minutes, 42 ms max drift
span time drift 2

Context

  • Moleculer version: master
  • NodeJS version: 12.21.0
  • Operating System: macOS 10.15.7
@intech
Copy link
Member

intech commented Mar 26, 2021

@icebob maybe we should go to perf_hook?

@intech
Copy link
Member

intech commented Mar 26, 2021

@ngraef fast PR, thanks!

@intech intech added Status: Review Needed and removed Status: Need reproduce Issue waits for reproducing labels Mar 26, 2021
@ngraef
Copy link
Contributor Author

ngraef commented Mar 26, 2021

@intech I made the fix while the second test was running 😁

@intech
Copy link
Member

intech commented Mar 26, 2021

I also think that it is necessary to fix a similar problem in other places of the project, such as rate limiter and metrics. 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants