fix(new-webui): Modify all timestamps to UTC timezone.#1000
Conversation
WalkthroughThe changes update time handling in the log viewer web UI to consistently use UTC for all date and time calculations and display. This is achieved by updating Dayjs usage, refactoring time range preset logic to use functions returning UTC-based ranges, and adjusting related component initializations and rendering. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TimeRangeInput
participant Utils (TIME_RANGE_OPTION_DAYJS_MAP)
participant SearchState
participant TableRenderer
User->>TimeRangeInput: Selects time range
TimeRangeInput->>Utils (TIME_RANGE_OPTION_DAYJS_MAP): Calls selected time range function
Utils (TIME_RANGE_OPTION_DAYJS_MAP)-->>TimeRangeInput: Returns [startUTC, endUTC]
TimeRangeInput->>SearchState: Updates state with [startUTC, endUTC]
SearchState->>TableRenderer: Provides UTC time range for search/filter
TableRenderer->>Utils (Dayjs.utc): Formats timestamps as UTC for display
Possibly related PRs
Suggested reviewers
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
components/log-viewer-webui/client/src/pages/IngestPage/Details/TimeRange.tsx(1 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/TimeRangeInput/index.tsx(1 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/TimeRangeInput/utils.tsx(2 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/typings.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}`: - Prefer `false == ` rather than `!`.
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}: - Preferfalse == <expression>rather than!<expression>.
components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/TimeRangeInput/index.tsxcomponents/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/typings.tsxcomponents/log-viewer-webui/client/src/pages/IngestPage/Details/TimeRange.tsxcomponents/log-viewer-webui/client/src/pages/SearchPage/SearchControls/TimeRangeInput/utils.tsx
🧬 Code Graph Analysis (1)
components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/typings.tsx (1)
components/log-viewer-webui/client/src/typings/datetime.ts (1)
DATETIME_FORMAT_TEMPLATE(3-3)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: lint-check (macos-latest)
- GitHub Check: lint-check (ubuntu-latest)
🔇 Additional comments (2)
components/log-viewer-webui/client/src/pages/IngestPage/Details/TimeRange.tsx (1)
24-24: UTC formatting looks goodThe switch to
beginDate.utc()/endDate.utc()consistently aligns the ingest-page range display with the UTC standard adopted throughout the PR.components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/typings.tsx (1)
35-35: Correct use ofdayjs.utcin rendererRendering timestamps via
dayjs.utc(timestamp)guarantees they’re interpreted in UTC, eliminating client-locale drift—nice catch.
| import DayjsUtc from "dayjs/plugin/utc"; | ||
| dayjs.extend(DayjsUtc); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Centralise Dayjs UTC plugin extension to avoid repetitive side-effects
Importing and extending dayjs with the UTC plugin in every utility/component can lead to accidental duplication and harder-to-trace side-effects if future plugins are added/removed. Consider moving
import DayjsUtc from "dayjs/plugin/utc";
dayjs.extend(DayjsUtc);into a single bootstrap/module (e.g., client/src/dayjsSetup.ts) that is imported once at app start-up.
🤖 Prompt for AI Agents
In
components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/TimeRangeInput/utils.tsx
around lines 3 to 4, the Dayjs UTC plugin is imported and extended directly,
causing potential duplication and side-effects. To fix this, create a new module
(e.g., client/src/dayjsSetup.ts) where you import and extend Dayjs with the UTC
plugin once, then remove the import and extension from this file and instead
import the new setup module once at the app startup to centralize the plugin
extension.
| if (!isValidDateRange(dates)) { | ||
| return; | ||
| } | ||
|
|
||
| console.debug('Selected date range:', { | ||
| start: dates?.[0]?.format('YYYY-MM-DD HH:mm:ss Z'), | ||
| end: dates?.[1]?.format('YYYY-MM-DD HH:mm:ss Z') | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Remove dev-only logging and follow boolean-negation guideline
console.debugis handy while developing but will leak into production bundles and browser consoles. Either remove it or guard it behind an environment flag.- The code-style guideline for this repo prefers
false == <expr>over!<expr>in TS/JS files.
- if (!isValidDateRange(dates)) {
+ if (false == isValidDateRange(dates)) {
return;
}
- console.debug('Selected date range:', {
- start: dates?.[0]?.format('YYYY-MM-DD HH:mm:ss Z'),
- end: dates?.[1]?.format('YYYY-MM-DD HH:mm:ss Z')
- });
+ /* DEBUG remove or guard before ship
+ console.debug("Selected date range:", {
+ start: dates[0].format("YYYY-MM-DD HH:mm:ss Z"),
+ end: dates[1].format("YYYY-MM-DD HH:mm:ss Z"),
+ });
+ */📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!isValidDateRange(dates)) { | |
| return; | |
| } | |
| console.debug('Selected date range:', { | |
| start: dates?.[0]?.format('YYYY-MM-DD HH:mm:ss Z'), | |
| end: dates?.[1]?.format('YYYY-MM-DD HH:mm:ss Z') | |
| }); | |
| if (false == isValidDateRange(dates)) { | |
| return; | |
| } | |
| /* DEBUG remove or guard before ship | |
| console.debug("Selected date range:", { | |
| start: dates[0].format("YYYY-MM-DD HH:mm:ss Z"), | |
| end: dates[1].format("YYYY-MM-DD HH:mm:ss Z"), | |
| }); | |
| */ |
🤖 Prompt for AI Agents
In
components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/TimeRangeInput/index.tsx
around lines 45 to 52, remove the console.debug statement or wrap it with a
check for a development environment flag to prevent logging in production. Also,
replace the boolean negation !isValidDateRange(dates) with the preferred style
false == isValidDateRange(dates) to comply with the repo's code-style
guidelines.
| let stat; | ||
| if (beginDate.isValid() && endDate.isValid()) { | ||
| stat = `${beginDate.format(DATE_FORMAT)} - ${endDate.format(DATE_FORMAT)}`; | ||
| stat = `${beginDate.utc().format(DATE_FORMAT)} - ${endDate.utc().format(DATE_FORMAT)}`; |
There was a problem hiding this comment.
just a question -
would it be better to create/store a UTC object in the query store in the first place?
There was a problem hiding this comment.
Im not sure i fully get your question, but I believe the timestamp is in utc, its just that dayJs dosent know that. So this change basically lets dayJs know the timestamp is in utc
There was a problem hiding this comment.
Like shall we change these lines
setBeginDate(dayjs.utc(details.begin_timestamp));
setEndDate(dayjs.utc(details.end_timestamp));
or
setBeginDate(dayjs(details.begin_timestamp).utc());
setEndDate(dayjs(details.end_timestamp).utc());
then we can avoid the UTC conversions when the dates are consumed in the rendering functions
There was a problem hiding this comment.
okay that can work too
| start: dates[0].format("YYYY-MM-DD HH:mm:ss Z"), | ||
| end: dates[1].format("YYYY-MM-DD HH:mm:ss Z"), |
There was a problem hiding this comment.
shall we use constants for the format strings?
There was a problem hiding this comment.
I will just remove this. its not neccesary
| defaultSortOrder: "ascend", | ||
| key: "timestamp", | ||
| render: (timestamp: number) => dayjs(timestamp).format(DATETIME_FORMAT_TEMPLATE), | ||
| render: (timestamp: number) => dayjs.utc(timestamp).format(DATETIME_FORMAT_TEMPLATE), |
There was a problem hiding this comment.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/TimeRangeInput/utils.tsx (1)
1-3: Duplicate UTC-plugin bootstrap – centralise to avoid side-effectsImporting and extending the Dayjs UTC plugin in every utility duplicates side-effects and makes future plugin changes brittle. Please move the extension into a single bootstrap module that is imported once at app start-up, then remove it from this file.
components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/TimeRangeInput/index.tsx (1)
45-49: Conform to repo boolean-negation styleRepo guideline prefers
false == <expr>over!<expr>. Update the guard accordingly:- if (!isValidDateRange(dates)) { + if (false == isValidDateRange(dates)) { return; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/TimeRangeInput/index.tsx(1 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/TimeRangeInput/utils.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}`: - Prefer `false == ` rather than `!`.
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}: - Preferfalse == <expression>rather than!<expression>.
components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/TimeRangeInput/index.tsxcomponents/log-viewer-webui/client/src/pages/SearchPage/SearchControls/TimeRangeInput/utils.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/TimeRangeInput/index.tsx (1)
45-47: 🧹 Nitpick (assertive)Follow negation style guideline (
false == <expr>instead of!<expr>)- if (!isValidDateRange(dates)) { + if (false == isValidDateRange(dates)) { return; }components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/TimeRangeInput/utils.tsx (1)
2-6: 🧹 Nitpick (assertive)Centralise Dayjs-UTC plugin setup to a single bootstrap module
Extending Dayjs with
utcin multiple files risks accidental double-extension and harder plugin management. Moveimport utc from "dayjs/plugin/utc"; dayjs.extend(utc);into a shared startup module (e.g.,
client/src/dayjsSetup.ts) and import that once at app initialisation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
components/log-viewer-webui/client/src/pages/IngestPage/Details/index.tsx(1 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/TimeRangeInput/index.tsx(3 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/TimeRangeInput/utils.tsx(2 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/typings.tsx(1 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchState/index.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}`: - Prefer `false == ` rather than `!`.
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}: - Preferfalse == <expression>rather than!<expression>.
components/log-viewer-webui/client/src/pages/IngestPage/Details/index.tsxcomponents/log-viewer-webui/client/src/pages/SearchPage/SearchControls/TimeRangeInput/index.tsxcomponents/log-viewer-webui/client/src/pages/SearchPage/SearchState/index.tsxcomponents/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/typings.tsxcomponents/log-viewer-webui/client/src/pages/SearchPage/SearchControls/TimeRangeInput/utils.tsx
🧬 Code Graph Analysis (3)
components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/TimeRangeInput/index.tsx (1)
components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/TimeRangeInput/utils.tsx (1)
TIME_RANGE_OPTION_DAYJS_MAP(116-116)
components/log-viewer-webui/client/src/pages/SearchPage/SearchState/index.tsx (2)
components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/TimeRangeInput/utils.tsx (2)
TIME_RANGE_OPTION_DAYJS_MAP(116-116)DEFAULT_TIME_RANGE(113-113)components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTimeline/utils.ts (1)
computeTimelineConfig(95-95)
components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/typings.tsx (1)
components/log-viewer-webui/client/src/typings/datetime.ts (1)
DATETIME_FORMAT_TEMPLATE(3-3)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: lint-check (macos-latest)
- GitHub Check: lint-check (ubuntu-latest)
🔇 Additional comments (2)
components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/typings.tsx (1)
35-36:Details
❓ Verification inconclusive
Possible millisecond/second mismatch when parsing timestamps
dayjs.utc(timestamp)expects the input to be in milliseconds. If the backend is sending Unix-epoch seconds (common for DBs/APIs), the rendered time will be off by a factor of 1000.- render: (timestamp: number) => dayjs.utc(timestamp).format(DATETIME_FORMAT_TEMPLATE), + // ⚠ Verify units – switch to `dayjs.unix()` (seconds) or multiply by 1000 if needed. + render: (timestamp: number) => + dayjs.unix(timestamp).utc().format(DATETIME_FORMAT_TEMPLATE),Please confirm the unit used by
timestampbefore merging.
Confirm timestamp units before rendering
It’s unclear whether the
timestamp: numberpassed todayjs.utc(timestamp)is in milliseconds (the default) or in seconds. If the backend API returns Unix-epoch seconds, the displayed time will be off by 1000×. Please verify the unit and adjust as needed:· File:
components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/typings.tsx
Lines: 35–36- render: (timestamp: number) => dayjs.utc(timestamp).format(DATETIME_FORMAT_TEMPLATE), + // ⚠ Verify that `timestamp` is in ms or sec. + // If it’s seconds, switch to `dayjs.unix()`: + render: (timestamp: number) => + dayjs.unix(timestamp).utc().format(DATETIME_FORMAT_TEMPLATE),components/log-viewer-webui/client/src/pages/IngestPage/Details/index.tsx (1)
60-62: Verifydetails.*_timestampunit before converting withdayjs.utcLike the search-results table, the code assumes the server returns milliseconds. If the API provides seconds, the displayed begin/end dates will be wrong.
- setBeginDate(dayjs.utc(details.begin_timestamp)); - setEndDate(dayjs.utc(details.end_timestamp)); + // If the API returns seconds, change to `dayjs.unix(...)` + setBeginDate(dayjs.unix(details.begin_timestamp).utc()); + setEndDate(dayjs.unix(details.end_timestamp).utc());Double-check the API contract for these fields.
| timeRange: TIME_RANGE_OPTION_DAYJS_MAP[DEFAULT_TIME_RANGE](), | ||
| timeRangeOption: DEFAULT_TIME_RANGE, | ||
| timelineConfig: computeTimelineConfig(TIME_RANGE_OPTION_DAYJS_MAP[DEFAULT_TIME_RANGE]), | ||
| timelineConfig: computeTimelineConfig(TIME_RANGE_OPTION_DAYJS_MAP[DEFAULT_TIME_RANGE]()), | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid double evaluation of “now” when initialising default state
TIME_RANGE_OPTION_DAYJS_MAP[DEFAULT_TIME_RANGE]() is invoked twice, producing two slightly different ranges. This can cause the initial timelineConfig and the stored timeRange to drift by a few milliseconds.
- timeRange: TIME_RANGE_OPTION_DAYJS_MAP[DEFAULT_TIME_RANGE](),
- timeRangeOption: DEFAULT_TIME_RANGE,
- timelineConfig: computeTimelineConfig(TIME_RANGE_OPTION_DAYJS_MAP[DEFAULT_TIME_RANGE]()),
+ timeRangeOption: DEFAULT_TIME_RANGE,
+ timeRange: (() => {
+ const range = TIME_RANGE_OPTION_DAYJS_MAP[DEFAULT_TIME_RANGE]();
+ return range;
+ })(),
+ timelineConfig: (() => {
+ const sameRange = TIME_RANGE_OPTION_DAYJS_MAP[DEFAULT_TIME_RANGE]();
+ return computeTimelineConfig(sameRange);
+ })(),Capture the range once and reuse it for consistency.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In components/log-viewer-webui/client/src/pages/SearchPage/SearchState/index.tsx
around lines 26 to 29, the function
TIME_RANGE_OPTION_DAYJS_MAP[DEFAULT_TIME_RANGE]() is called twice, causing two
slightly different time ranges due to separate evaluations of "now". To fix
this, call the function once, store its result in a variable, and then use that
variable for both timeRange and timelineConfig initialization to ensure
consistency.
Description
Previously some timestamps were in user timezone. Now they are all in utc.
It appears that range picker will adopt timezone of the default option. So as long as the default is in utc,
all the new changes will also be in utc.
Checklist
breaking change.
Validation performed
Printed timestamps they appear in utc
Summary by CodeRabbit