Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ interface TimeRangeProps {
const TimeRange = ({beginDate, endDate}: TimeRangeProps) => {
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)}`;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a question -

would it be better to create/store a UTC object in the query store in the first place?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@junhaoliao junhaoliao Jun 11, 2025 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like shall we change these lines

setBeginDate(dayjs(details.begin_timestamp));
setEndDate(dayjs(details.end_timestamp));
as:

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay that can work too

} else {
stat = "No timestamp data";
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,12 @@ const TimeRangeInput = () => {
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"),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shall we use constants for the format strings?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will just remove this. its not neccesary

});

updateTimeRange(dates);
};

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import dayjs from "dayjs";

import DayjsUtc from "dayjs/plugin/utc";
dayjs.extend(DayjsUtc);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 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.


/**
* Time range options.
Expand All @@ -22,31 +23,33 @@ const DEFAULT_TIME_RANGE = TIME_RANGE_OPTION.ALL_TIME;

/* eslint-disable no-magic-numbers */
const TIME_RANGE_OPTION_DAYJS_MAP: Record<TIME_RANGE_OPTION, [dayjs.Dayjs, dayjs.Dayjs]> = {
[TIME_RANGE_OPTION.LAST_15_MINUTES]: [dayjs().subtract(15, "minute"),
dayjs()],
[TIME_RANGE_OPTION.LAST_HOUR]: [dayjs().subtract(1, "hour"),
dayjs()],
[TIME_RANGE_OPTION.TODAY]: [dayjs().startOf("day"),
dayjs().endOf("day")],
[TIME_RANGE_OPTION.YESTERDAY]: [dayjs().subtract(1, "d"),
dayjs().subtract(2, "d")],
[TIME_RANGE_OPTION.LAST_7_DAYS]: [dayjs().subtract(7, "d"),
dayjs()],
[TIME_RANGE_OPTION.LAST_30_DAYS]: [dayjs().subtract(30, "d"),
dayjs()],
[TIME_RANGE_OPTION.LAST_12_MONTHS]: [dayjs().subtract(12, "month"),
dayjs()],
[TIME_RANGE_OPTION.MONTH_TO_DATE]: [dayjs().startOf("month"),
dayjs()],
[TIME_RANGE_OPTION.YEAR_TO_DATE]: [dayjs().startOf("year"),
dayjs()],
[TIME_RANGE_OPTION.ALL_TIME]: [dayjs(0),
dayjs().add(1, "year")],
[TIME_RANGE_OPTION.LAST_15_MINUTES]: [dayjs.utc().subtract(15, "minute"),
dayjs.utc()],
[TIME_RANGE_OPTION.LAST_HOUR]: [dayjs.utc().subtract(1, "hour"),
dayjs.utc()],
[TIME_RANGE_OPTION.TODAY]: [dayjs.utc().startOf("day"),
dayjs.utc().endOf("day")],
[TIME_RANGE_OPTION.YESTERDAY]: [dayjs.utc().subtract(1, "d")
.startOf("day"),
dayjs.utc().subtract(1, "d")
.endOf("day")],
[TIME_RANGE_OPTION.LAST_7_DAYS]: [dayjs.utc().subtract(7, "d"),
dayjs.utc()],
[TIME_RANGE_OPTION.LAST_30_DAYS]: [dayjs.utc().subtract(30, "d"),
dayjs.utc()],
[TIME_RANGE_OPTION.LAST_12_MONTHS]: [dayjs.utc().subtract(12, "month"),
dayjs.utc()],
[TIME_RANGE_OPTION.MONTH_TO_DATE]: [dayjs.utc().startOf("month"),
dayjs.utc()],
[TIME_RANGE_OPTION.YEAR_TO_DATE]: [dayjs.utc().startOf("year"),
dayjs.utc()],
[TIME_RANGE_OPTION.ALL_TIME]: [dayjs.utc(0),
dayjs.utc().add(1, "year")],

Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
// Custom option is just a placeholder for typing purposes, its DayJs values should not
// be used.
[TIME_RANGE_OPTION.CUSTOM]: [dayjs(),
dayjs()],
[TIME_RANGE_OPTION.CUSTOM]: [dayjs.utc(),
dayjs.utc()],
};


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const searchResultsTableColumns: NonNullable<TableProps<SearchResult>["columns"]
dataIndex: "timestamp",
defaultSortOrder: "ascend",
key: "timestamp",
render: (timestamp: number) => dayjs(timestamp).format(DATETIME_FORMAT_TEMPLATE),
render: (timestamp: number) => dayjs.utc(timestamp).format(DATETIME_FORMAT_TEMPLATE),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorter: (a, b) => a.timestamp - b.timestamp,

// Specifying a third sort direction removes ability for user to cancel sorting.
Expand Down