Repository navigation
fix(webapp): prevent IEEE 754 precision loss on OTLP nanosecond conversions - #5035
PatelRavip1 wants to merge 1 commit into
Conversation
…rsions Fixes triggerdotdev#3292 Root Cause: Converting epoch milliseconds to nanoseconds via `BigInt(ms * 1_000_000)` performs multiplication in IEEE 754 float representation before casting to BigInt. Because epoch ms * 1e6 is ~1.7e18 and exceeds Number.MAX_SAFE_INTEGER (~9e15), the lowest bits lose precision, creating +-256ns non-deterministic drift in span ordering and duration metrics. Resolution: Cast epoch ms to BigInt before multiplying by BigInt(1_000_000) across all webapp event repository and run engine call sites.
|
|
Hi @PatelRavip1, thanks for your interest in contributing! This project requires that pull request authors are vouched, and you are not in the list of vouched users. This PR will be closed automatically. See https://github.com/triggerdotdev/trigger.dev/blob/main/CONTRIBUTING.md for more details. |
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configuration
📒 Files selected for processing: 4
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
|
|
||
| export function getNowInNanoseconds(): bigint { | ||
| return BigInt(new Date().getTime() * 1_000_000); | ||
| return BigInt(new Date().getTime()) * BigInt(1_000_000); |
There was a problem hiding this comment.
🟡 Span precision fix absent from release notes
When this fix changes recorded span times, getNowInNanoseconds gains no server change note. Customers miss the fix in release notes.
Learn more
A server change note is the user-facing release entry for a server-only fix. This PR changes the nanosecond timestamp used for recorded spans and their durations, but adds no entry under .server-changes/. The repository requires a note when a server-only bug fix affects behavior users could encounter; these files feed release notes for customers. See the server change rules.
Example: A customer sees span ordering drift before the upgrade and accurate ordering afterward, but the release notes omit the correction.
Recommended fix: Add a .server-changes/ Markdown file with area: webapp, type: fix, and a single user-facing sentence about more accurate span timing.
Was this helpful? React with 👍 or 👎 to provide feedback.
| it("preserves exact duration calculation", () => { | ||
| const start = 1729200000000n * 1_000_000n; | ||
| const end = new Date(1729200000500); // 500ms later | ||
| const duration = calculateDurationFromStart(end, start); | ||
| expect(duration).toBe(500_000_000); |
There was a problem hiding this comment.
🔍 Precision tests are absent from the default suite
The Vitest include list excludes this test directory. Its duration test also reverses calculateDurationFromStart arguments, so including it alone will not validate the fix.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Fixes #3292
Root Cause
Converting epoch milliseconds to nanoseconds via
BigInt(ms * 1_000_000)performs multiplication in IEEE 754 float representation before casting toBigInt. Becauseepoch ms * 1e6is ~1.7e18 and exceedsNumber.MAX_SAFE_INTEGER(~9e15), the lowest bits lose precision, creating +-256ns non-deterministic drift in span ordering and duration metrics.Changes
apps/webapp/app/v3/eventRepository/common.server.ts: Castnew Date().getTime()and$endtime.getTime()toBigIntprior to multiplying byBigInt(1_000_000).apps/webapp/app/v3/eventRepository/index.server.ts: Cast timestamp toBigIntprior to multiplication.apps/webapp/app/v3/runEngineHandlers.server.ts: Cast retry event timestamp toBigIntprior to multiplication.apps/webapp/app/v3/eventRepository/common.server.test.ts: Added unit tests verifying exact integer arithmetic and absence of float truncation.