Repository navigation
fix(webapp): prevent IEEE 754 precision loss on OTLP nanosecond conversions #5035
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| import { describe, it, expect } from "vitest"; | ||
| import { getNowInNanoseconds, calculateDurationFromStart } from "./common.server"; | ||
|
|
||
| describe("OTLP nanosecond precision", () => { | ||
| it("guarantees exact integer arithmetic without float truncation", () => { | ||
| const sampleMs = 1729200000123; | ||
| const sampleDate = new Date(sampleMs); | ||
| const expectedNano = BigInt(sampleMs) * 1_000_000n; | ||
|
|
||
| // Buggy float math produces 1729200000123000064n (+64ns error) | ||
| const buggyNano = BigInt(sampleMs * 1_000_000); | ||
| expect(buggyNano).not.toBe(expectedNano); | ||
|
|
||
| // Fixed calculation produces exact 1729200000123000000n | ||
| const fixedNano = BigInt(sampleDate.getTime()) * BigInt(1_000_000); | ||
| expect(fixedNano).toBe(expectedNano); | ||
| }); | ||
|
|
||
| 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); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,7 +26,7 @@ export function extractContextFromCarrier(carrier: Record<string, unknown>) { | |
| } | ||
|
|
||
| export function getNowInNanoseconds(): bigint { | ||
| return BigInt(new Date().getTime() * 1_000_000); | ||
| return BigInt(new Date().getTime()) * BigInt(1_000_000); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Span precision fix absent from release notes When this fix changes recorded span times, Learn moreA 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 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 Was this helpful? React with 👍 or 👎 to provide feedback. |
||
| } | ||
|
|
||
| export function getDateFromNanoseconds(nanoseconds: bigint): Date { | ||
|
|
@@ -40,7 +40,7 @@ export function calculateDurationFromStart( | |
| ) { | ||
| const $endtime = typeof endTime === "string" ? new Date(endTime) : endTime; | ||
|
|
||
| const duration = Number(BigInt($endtime.getTime() * 1_000_000) - startTime); | ||
| const duration = Number(BigInt($endtime.getTime()) * BigInt(1_000_000) - startTime); | ||
|
|
||
| if (minimumDuration && duration < minimumDuration) { | ||
| return minimumDuration; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔍 Precision tests are absent from the default suite
The Vitest include list excludes this test directory. Its duration test also reverses
calculateDurationFromStartarguments, so including it alone will not validate the fix.Was this helpful? React with 👍 or 👎 to provide feedback.