fix(webapp): prevent OTLP nanosecond timestamp overflow (#3292) - #4376
fix(webapp): prevent OTLP nanosecond timestamp overflow (#3292)#4376marceli1404 wants to merge 1 commit into
Conversation
…v#3292) Move BigInt() before multiplication to avoid IEEE 754 precision loss when converting epoch milliseconds to nanoseconds. Fixes 4 overflow sites: - getNowInNanoseconds(): BigInt(Date.now()) * 1_000_000n - calculateDurationFromStart(): BigInt(getTime()) * 1_000_000n - recordEvent() startTime: BigInt(getTime()) * 1_000_000n - runEngineHandlers OOM retry: BigInt(getTime()) * 1_000_000n Also adds epochMsToNanoseconds() helper to durations.ts for safe epoch-level conversions, and fixes seed script consistency.
|
Hi @marceli1404, 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. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
WalkthroughNanosecond timestamp and duration calculations now convert millisecond values to ✨ Finishing Touches🧪 Generate unit tests (beta)
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}
| /** Convert epoch milliseconds to nanoseconds using BigInt to avoid overflow. */ | ||
| export function epochMsToNanoseconds(ms: number): bigint { | ||
| return BigInt(ms) * 1_000_000n; | ||
| } |
There was a problem hiding this comment.
🟡 Public package changed without the required release-notes entry
A new function was added to the public core package (packages/core/src/v3/utils/durations.ts:36) without adding a changeset, which the repository rules require for any change to a published package.
Impact: The change to the published package will ship without an entry in the user-facing release notes, and CI/release tooling that enforces changesets may block the merge.
Repository rule requiring a changeset for public package changes
AGENTS.md states: "When modifying any public package (packages/* or integrations/*), add a changeset" via pnpm run changeset:add. CONTRIBUTING.md restates this: contributions to anything in /packages require a changeset before merge. This PR modifies packages/core/src/v3/utils/durations.ts (adding epochMsToNanoseconds) but the diff adds no file under .changeset/ — the only changeset present (.changeset/chat-agent-preserve-partial-on-stream-failure.md) is unrelated and pre-existing.
Prompt for agents
The PR adds a new exported function epochMsToNanoseconds to the public package packages/core/src/v3/utils/durations.ts, but no changeset accompanies the change. Per AGENTS.md and CONTRIBUTING.md, any modification to a public package under packages/* requires a changeset. Add a patch changeset (run pnpm run changeset:add, selecting @trigger.dev/core) with a user-facing description of the fix so the change appears in release notes.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Problem
Issue #3292: OTLP timestamps use nanoseconds. When converting epoch milliseconds to nanoseconds (\ms * 1_000_000), the result exceeds \Number.MAX_SAFE_INTEGER\ (~9 quadrillion), causing IEEE 754 floating-point precision loss. After \BigInt()\ wraps the already-corrupt value, the timestamp is silently wrong.
Fixes
All four HIGH-severity sites follow the same pattern: move \BigInt()\ before the multiplication so it happens in BigInt space.
Also adds \�pochMsToNanoseconds()\ helper to \packages/core/src/v3/utils/durations.ts\ for safe epoch-level conversions, and fixes \seed-ai-spans.mts\ for consistency.
Closes #3292