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/04/27 09:50:13 UTC
[PR] feat: support trace v1 in trace single page [skywalking-booster-ui]
Fine0830 opened a new pull request, #549:
URL: https://github.com/apache/skywalking-booster-ui/pull/549
Closes https://github.com/apache/skywalking/issues/13842
screenshot
<img width="1534" height="1042" alt="1" src="https://github.com/user-attachments/assets/65282bef-7003-44bb-beee-577d46101c0e" />
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] feat: support trace v1 in trace single page [skywalking-booster-ui]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #549:
URL: https://github.com/apache/skywalking-booster-ui/pull/549#discussion_r3146387834
##########
src/store/modules/trace.ts:
##########
@@ -193,14 +194,14 @@ export const traceStore = defineStore({
this.loading = false;
return response;
}
- this.getTraceSpans({ traceId: response.data.data.traces[0].traceIds[0] });
this.traceList = response.data.data.traces.map((d: Trace) => {
d.traceIds = d.traceIds.map((id: string) => {
return { value: id, label: id };
});
return d;
});
this.setCurrentTrace(response.data.data.traces[0] || {});
+ await this.getTraceSpans({ traceId: this.currentTrace?.traceId || "" });
Review Comment:
In the v1 (queryBasicTraces) path, traces don’t include a `traceId` field (only `traceIds`), so calling `getTraceSpans` with `this.currentTrace?.traceId || ""` will query spans with an empty/undefined ID and fail to load spans. Use the first trace ID from `traceIds` (after mapping, `traceIds[0].value`) when requesting spans.
```suggestion
const currentTrace = this.traceList[0] || {};
this.setCurrentTrace(currentTrace);
await this.getTraceSpans({ traceId: currentTrace.traceIds?.[0]?.value || "" });
```
##########
src/store/modules/trace.ts:
##########
@@ -177,10 +177,11 @@ export const traceStore = defineStore({
return response;
},
async getTraces() {
+ this.loading = true;
+ await this.getHasQueryTracesV2Support();
if (this.hasQueryTracesV2Support) {
return this.fetchV2Traces();
Review Comment:
`getTraces()` now calls `getHasQueryTracesV2Support()` on every invocation, which adds an extra network round-trip per trace query even though this capability is effectively static for a backend. Consider caching/memoizing this (e.g., only query when the value is still “unknown”, or store it in app init) to avoid unnecessary requests.
##########
src/store/modules/trace.ts:
##########
@@ -301,6 +302,37 @@ export const traceStore = defineStore({
this.setCurrentTrace(trace || {});
return response;
},
+
+ async getTraceByTraceId(traceId: string) {
+ this.loading = true;
+ this.setTraceCondition({
+ traceId: traceId,
+ });
+ await this.getHasQueryTracesV2Support();
+ if (this.hasQueryTracesV2Support) {
+ await this.fetchV2Traces();
+ this.loading = false;
+ return;
+ }
+ this.setCurrentTrace({
+ duration: 0,
+ endpointNames: [],
+ isError: false,
+ key: traceId,
+ label: traceId,
+ operationNames: [],
+ segmentId: "",
+ serviceCode: "",
+ spans: [],
+ start: "0",
+ traceIds: [{ value: traceId, label: traceId }],
+ traceId: traceId,
+ id: traceId,
+ parentId: "",
+ });
+ await this.getTraceSpans({ traceId });
Review Comment:
`getTraceByTraceId` seeds `currentTrace` with `start: "0"`, `duration: 0`, and empty `endpointNames`. Since `SpanList.vue` uses these fields for the header and for min/max timestamps, the trace detail view can render with a blank title and a 0–0 time range even after spans load. Consider deriving `start`, `duration`, `endpointNames` (and optionally `serviceCode`/`isError`) from the fetched spans once `getTraceSpans` completes, so the charts render with a correct time window.
```suggestion
await this.getTraceSpans({ traceId });
const spans = Array.isArray(this.traceSpans) ? this.traceSpans : [];
if (spans.length) {
const rootSpan =
spans.find((span: Span) => span.parentSpanId === -1 && span.refs.length === 0) || spans[0];
const start = spans.reduce(
(min: number, span: Span) => Math.min(min, span.startTime),
spans[0].startTime
);
const end = spans.reduce(
(max: number, span: Span) => Math.max(max, span.endTime),
spans[0].endTime
);
const endpointNames = Array.from(
new Set(
spans
.map((span: Span) => span.endpointName)
.filter((endpointName: string) => Boolean(endpointName))
)
);
const serviceCode =
rootSpan.serviceCode || spans.find((span: Span) => Boolean(span.serviceCode))?.serviceCode || "";
const endpointName = rootSpan.endpointName || endpointNames[0] || "";
this.setCurrentTrace({
duration: end - start,
endpointNames,
isError: spans.some((span: Span) => Boolean(span.isError)),
key: traceId,
label: endpointName ? `${serviceCode}: ${endpointName}` : traceId,
operationNames: [],
segmentId: "",
serviceCode,
spans,
start,
traceIds: [{ value: traceId, label: traceId }],
traceId: traceId,
id: traceId,
parentId: "",
});
}
```
##########
src/views/dashboard/related/trace/components/TraceList/SpanList.vue:
##########
@@ -67,6 +67,12 @@ limitations under the License. -->
:traceId="traceStore.currentTrace?.traceIds?.[0]?.value"
:showBtnDetail="false"
:headerType="WidgetType.Trace"
+ :minTimestamp="Number(traceStore.currentTrace?.start)"
+ :maxTimestamp="
+ traceStore.currentTrace
+ ? Number(traceStore.currentTrace.start) + Number(traceStore.currentTrace.duration)
Review Comment:
`minTimestamp`/`maxTimestamp` are derived from `currentTrace.start` and `currentTrace.duration`. If those fields are unset (or seeded as 0 for v1 before enrichment), this can yield `NaN`/0–0 ranges and cause the D3 graphs to render incorrectly. Consider computing the time bounds from `traceStore.traceSpans` when available, or only passing these props when both values are valid numbers.
```suggestion
:minTimestamp="
[Number(traceStore.currentTrace?.start), Number(traceStore.currentTrace?.duration)].every(
Number.isFinite,
) &&
(Number(traceStore.currentTrace?.start) !== 0 || Number(traceStore.currentTrace?.duration) !== 0)
? Number(traceStore.currentTrace?.start)
: undefined
"
:maxTimestamp="
[Number(traceStore.currentTrace?.start), Number(traceStore.currentTrace?.duration)].every(
Number.isFinite,
) &&
(Number(traceStore.currentTrace?.start) !== 0 || Number(traceStore.currentTrace?.duration) !== 0)
? Number(traceStore.currentTrace?.start) + Number(traceStore.currentTrace?.duration)
```
##########
src/store/modules/trace.ts:
##########
@@ -301,6 +302,37 @@ export const traceStore = defineStore({
this.setCurrentTrace(trace || {});
return response;
},
+
+ async getTraceByTraceId(traceId: string) {
+ this.loading = true;
+ this.setTraceCondition({
+ traceId: traceId,
+ });
+ await this.getHasQueryTracesV2Support();
+ if (this.hasQueryTracesV2Support) {
+ await this.fetchV2Traces();
+ this.loading = false;
+ return;
+ }
+ this.setCurrentTrace({
+ duration: 0,
+ endpointNames: [],
+ isError: false,
+ key: traceId,
+ label: traceId,
+ operationNames: [],
+ segmentId: "",
+ serviceCode: "",
+ spans: [],
+ start: "0",
+ traceIds: [{ value: traceId, label: traceId }],
+ traceId: traceId,
+ id: traceId,
+ parentId: "",
+ });
+ await this.getTraceSpans({ traceId });
+ this.loading = false;
Review Comment:
`getTraceByTraceId` always sets `loading = false` after awaiting `getTraceSpans`, but if `getTraceSpans` throws/rejects (or returns errors) the store can be left with a placeholder `currentTrace` and stale/empty `traceSpans` without any “not found” state. Wrapping the body in `try/finally` and clearing `currentTrace`/`traceSpans` on error would avoid the blank page state and allow the UI to show a proper “No trace found” message.
```suggestion
try {
this.setTraceCondition({
traceId: traceId,
});
await this.getHasQueryTracesV2Support();
if (this.hasQueryTracesV2Support) {
await this.fetchV2Traces();
return;
}
this.setCurrentTrace({
duration: 0,
endpointNames: [],
isError: false,
key: traceId,
label: traceId,
operationNames: [],
segmentId: "",
serviceCode: "",
spans: [],
start: "0",
traceIds: [{ value: traceId, label: traceId }],
traceId: traceId,
id: traceId,
parentId: "",
});
const response = await this.getTraceSpans({ traceId });
if (response?.errors) {
this.currentTrace = null;
this.traceSpans = [];
}
} catch (error) {
this.currentTrace = null;
this.traceSpans = [];
throw error;
} finally {
this.loading = false;
}
```
##########
src/views/dashboard/Trace.vue:
##########
@@ -14,10 +14,16 @@ See the License for the specific language governing permissions and
limitations under the License. -->
<template>
<div style="padding: 20px">
- <TraceContent v-if="traceStore.currentTrace" :trace="traceStore.currentTrace" />
- <div style="text-align: center; padding: 20px" v-if="!traceStore.loading && !traceStore.currentTrace"
- >No trace found</div
- >
+ <div v-if="traceStore.traceSpans.length">
+ <TraceContent
+ v-if="traceStore.hasQueryTracesV2Support && traceStore.currentTrace"
+ :trace="traceStore.currentTrace"
+ />
+ <SpanList v-if="!traceStore.hasQueryTracesV2Support" />
+ </div>
+ <div style="text-align: center; padding: 20px" v-if="!traceStore.loading && !traceStore.currentTrace">
+ No trace found
+ </div>
Review Comment:
The rendering logic can result in a blank page: the wrapper is gated by `traceStore.traceSpans.length`, but the “No trace found” message is gated by `!traceStore.currentTrace`. Since the store typically sets `currentTrace` to `{}` (truthy) even on empty/error cases, the “No trace found” block may never render while `traceSpans` is empty. Consider basing the empty-state on `!loading && traceSpans.length === 0` (and/or making `currentTrace` truly `null` when not found), and render the detail component without requiring `traceSpans.length` first so loading/empty states are visible.
--
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] feat: support trace v1 in trace single page [skywalking-booster-ui]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng merged PR #549:
URL: https://github.com/apache/skywalking-booster-ui/pull/549
--
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