You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by "Fine0830 (via GitHub)" <gi...@apache.org> on 2026/03/30 03:48:10 UTC

[PR] fix: set the `step` to `SECOND` in the duration for Log/Trace/Alarm/Tag [skywalking-booster-ui]

Fine0830 opened a new pull request, #537:
URL: https://github.com/apache/skywalking-booster-ui/pull/537

   Video
   
   
   https://github.com/user-attachments/assets/cf24112e-5f17-411c-83f2-51ccd2f1dcaf
   
   
   Signed-off-by: Qiuxia Fan <qi...@apache.org>
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] fix: set the `step` to `SECOND` in the duration for Log/Trace/Alarm/Tag [skywalking-booster-ui]

Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng merged PR #537:
URL: https://github.com/apache/skywalking-booster-ui/pull/537


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] fix: set the `step` to `SECOND` in the duration for Log/Trace/Alarm/Tag [skywalking-booster-ui]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #537:
URL: https://github.com/apache/skywalking-booster-ui/pull/537#discussion_r3007374556


##########
src/constants/data.ts:
##########
@@ -16,6 +16,7 @@
  */
 
 export enum TimeType {
+  SENCOND_TIME = "SECOND",

Review Comment:
   `TimeType` enum member name `SENCOND_TIME` appears to be a typo ("sencond"). To match existing naming (`MINUTE_TIME`, `HOUR_TIME`, ...), rename this to `SECOND_TIME` and update all references to avoid baking the misspelling into the public constants API.
   ```suggestion
     SECOND_TIME = "SECOND",
   ```



##########
src/hooks/useDuration.ts:
##########
@@ -43,7 +45,7 @@ export function useDuration() {
   }
   function setDurationRow(data: Duration) {
     const appStore = useAppStoreWithOut();
-    durationRow = { ...data, coldStage: appStore.coldStageMode };
+    durationRow = { ...data, coldStage: appStore.coldStageMode, step: TimeType.SENCOND_TIME };
   }

Review Comment:
   `useDuration` now hard-codes `step` to SECOND in `getDuration()`, `getDurationTime()`, and even overwrites the caller-provided `data.step` in `setDurationRow()`. This changes the hook contract (callers pass a step from `timeFormat()` and tests expect it to be preserved) and can break any flow that relies on MINUTE/HOUR/DAY behavior. Consider only defaulting to SECOND when the incoming step is missing/empty, or make the desired step an explicit parameter/option so existing behavior can remain intact where needed.



##########
src/views/dashboard/related/trace/components/TraceList/SearchBar.vue:
##########
@@ -132,7 +133,7 @@ limitations under the License. -->
   const { duration: filtersDuration } = props.data.filters || {};
   const duration = ref<DurationTime>(
     filtersDuration
-      ? { start: filtersDuration.startTime || "", end: filtersDuration.endTime || "", step: filtersDuration.step || "" }
+      ? { start: filtersDuration.startTime || "", end: filtersDuration.endTime || "", step: TimeType.SENCOND_TIME }
       : appStore.durationTime,

Review Comment:
   When `filtersDuration` is not provided, this still initializes `duration` from `appStore.durationTime`, whose `step` comes from the global duration (typically MINUTE/HOUR/DAY). If the goal is to ensure Trace queries always use SECOND step, this path will continue sending a non-SECOND `queryDuration.step`. Consider deriving the default duration via `useDuration().getDurationTime()` (or otherwise formatting start/end with SECOND) so both branches use a consistent step.
   ```suggestion
         : { ...appStore.durationTime, step: TimeType.SENCOND_TIME },
   ```



##########
src/views/dashboard/related/trace/components/TraceList/Filter.vue:
##########
@@ -152,7 +153,7 @@ limitations under the License. -->
   }
 
   async function init() {
-    duration.value = filters.value.duration || appStore.durationTime;
+    duration.value = filters.value.duration || getDurationTime();

Review Comment:
   `init()` assigns `duration.value = filters.value.duration || getDurationTime();`. If `filters.value.duration` came from URL/widget state and lacks `step` (or uses `startTime/endTime` shape), this bypasses the normalization done above and can reintroduce an empty/non-SECOND step in `queryDuration`. Consider normalizing `filters.value.duration` here (mapping `startTime/endTime` to `start/end` and defaulting `step` to SECOND) to keep behavior consistent.
   ```suggestion
       const baseDuration = getDurationTime();
       if (filters.value.duration) {
         const rawDuration: any = filters.value.duration as any;
         const {
           start,
           end,
           startTime,
           endTime,
           ...rest
         } = rawDuration;
         const normalizedDuration: Duration = {
           // start from defaults (including default step, typically SECOND)
           ...baseDuration,
           // spread any additional/overriding fields from filters
           ...rest,
           // normalize start/end, giving priority to explicit start/end, then startTime/endTime
           start: start ?? startTime ?? baseDuration.start,
           end: end ?? endTime ?? baseDuration.end,
         };
         duration.value = normalizedDuration;
       } else {
         duration.value = baseDuration;
       }
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org