-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
nanosecond timestamps & JS' floating point "integers" #2935
Comments
This'll also be a concern for trace & span ids - any id above 2^53 - 1 will see more collisions than those below that limit |
@nevir did you find a workaround for this? |
Unfortunately no - I'm reporting timestamps with as much precision as I can (which is roughly microsecond precision). It's been good enough for our needs |
Hey @nevir I'm a new to DD-Open-Source. Just looking to help out and contribute where I can. Not sure if I'm fully understanding your issue, but do you mean to say that you're considering something along these lines? https://www.npmjs.com/package/nano-date |
Something along those lines, yeah - but I think it needs to be at the protocol level to make JS clients easier to manage. E.g. in addition to accepting nanosecond integers over JSON, the agent should support some alternate data structure Could be a string like nano-date's |
Hmm, I see. I'll do some more digging to see if I can find anything useful to the cause. I'd have to do some research on how, but I might be able to write a Node add-on w/ C++ to handle it. This one seems to utilize something right along the lines of what is needed: https://www.npmjs.com/package/emiflake Is there anything else I might be able to contribute to or assist with -- in terms of the library you're building? I'm pretty solid w/ JavaScript, NodeJS, ES5/6/7. Happy to take any grunt work off your back -- including documentation or admin tasks. |
I'm working on a Node.js client library for reporting traces to the agent, and JavaScript's all-numbers-are-doubles makes it a bit challenging. The largest positive integer we can represent in JS is 2^53 - 1, which is well below any recent epoch-ns timestamp.
Also, for ref - Node does support nanosecond timers via (
process.hrtime
), though they represent it as two integers (seconds, nanoseconds), to work around that limitation.I believe I can work around it by representing the timestamps in a similar way, and then doing custom msgpack encoding to convert them into a true integer over the wire. Or just drop some amount of precision to simplify the interface from the caller's perspective
However, it's pretty awkward. I also worry that it will catch folk off guard, particularly if they forward traces through a JS process (e.g. the JSON format, or something like OpenTracing).
What I'm wondering is whether it would make sense for the trace agent to support an alternative format for timestamps in its JSON encoding? (strings? millisecond timestamps w/ fractional components? etc)
The text was updated successfully, but these errors were encountered: