fix: prevent nanosecond overflow in OTLP timestamps - #4463
Conversation
Multiply after BigInt conversion to avoid IEEE 754 precision loss. Affects 4 locations where epoch-ms * 1e6 exceeds Number.MAX_SAFE_INTEGER. Fixes triggerdotdev#3292
|
|
Hi @deepshekhardas, 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. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| ## What | ||
|
|
||
| This PR updates route components to use the animated Resizable panel pattern: | ||
| - Converts conditionally-rendered panels to always-mounted animated panels with `collapsible`, `collapsed`, and `collapseAnimation` props | ||
| - Adds `useFrozenValue` hook usage to keep last selected item visible during panel collapse animation | ||
| - Minor UI polish on logs, runs, schedules, waitpoints pages | ||
|
|
||
| ## Changes | ||
|
|
||
| - **Resizable.tsx** — skipped (main already has these exports) | ||
| - **Route files** (logs, runs, schedules, batches, etc.) — convert panel pattern, add `useFrozenValue` | ||
| - **LogsTable, TreeView, GitMetadata** — minor improvements | ||
|
|
||
| ## Manual resolution | ||
|
|
||
| Merge had 3 conflicts: | ||
| - `Resizable.tsx`: kept main's version (already has animated panel exports with Firefox workarounds) | ||
| - `logs/route.tsx` line 419: removed pointless `?? undefined` (PR's cleaner version) | ||
| - `runs.$runParam/route.tsx`: kept `panel-run-parent-v3` autosaveId, removed `?? undefined` | ||
|
|
||
| ## Original PRs | ||
| - Original: #3267 | ||
| - This replaces: #3319 | ||
|
|
||
| Co-authored-by: James Ritchie <james@trigger.dev> |
There was a problem hiding this comment.
🟡 Unrelated draft notes files accidentally committed to the repository root
Two unrelated scratch text files describing entirely different pull requests are added to the repository root (pr-body-3321.txt at pr-body-3321.txt:1, plus pr-body-3322.txt), so this change ships content unrelated to the timestamp fix.
Impact: Junk files land in the repo and the change no longer addresses only a single issue, contrary to the contribution rules.
Why this violates the repository rules
CONTRIBUTING.md states: "We only accept PRs that address a single issue. Please do not submit PRs containing multiple unrelated fixes or features." The two files (pr-body-3321.txt, pr-body-3322.txt) contain draft PR bodies for unrelated work (animated resizable panels, OpenClaw agent integration) and are clearly leftover local artifacts. They should be deleted from the branch.
Prompt for agents
The branch contains two stray files at the repository root, pr-body-3321.txt and pr-body-3322.txt, which are draft PR descriptions for unrelated work (animated resizable panels and an OpenClaw agent integration). They are unrelated to the nanosecond overflow fix and should be removed from the branch before merge.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
|
||
| 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.
🟡 Server-only change is missing the required release-notes entry
The change modifies only webapp server code (apps/webapp/app/v3/eventRepository/common.server.ts:24) without adding the required release-notes entry file, so the fix will not appear in the release notes.
Impact: The bug fix is invisible to users reading release notes.
Rule requiring a .server-changes entry
CONTRIBUTING.md ("Adding server changes") and CLAUDE.md require: when a PR only changes server components (apps/webapp/, apps/supervisor/, ...) with no package changes, add a markdown file under .server-changes/ with area and type frontmatter. This PR touches only apps/webapp files and adds no .server-changes/ entry (see git diff --stat).
Prompt for agents
Add a .server-changes/ markdown file (e.g. .server-changes/fix-otlp-nanosecond-overflow.md) with frontmatter `area: webapp` and `type: fix`, and a one-line body describing the nanosecond overflow fix, as required by CONTRIBUTING.md for server-only changes.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
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)
WalkthroughMillisecond-to-nanosecond conversions now cast 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 Warning |
1 participant
Fixes #3292 - nanosecond overflow when converting timestamps to OTLP format.