This is an automated email from the ASF dual-hosted git repository.
wu-sheng pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/skywalking-horizon-ui.git
The following commit(s) were added to refs/heads/main by this push:
new 4a39381 fix(layer): resolve top-N compare sort direction from the
MQE, not the data (#54)
4a39381 is described below
commit 4a39381892e9ff18cf360ae19a09a5625b84b607
Author: 吴晟 Wu Sheng <[email protected]>
AuthorDate: Mon Jun 15 22:57:08 2026 +0800
fix(layer): resolve top-N compare sort direction from the MQE, not the data
(#54)
In multi-entity compare, the merged "All" tab inferred asc/des from a
single probe entity's values. That flipped the merge whenever the probe
entity's values were flat/equal (e.g. instances all at 100% success rate)
or when no entity had >=2 rows (it defaulted to descending). So "Top 10
instances by success rate" — top_n(service_instance_sla,10,asc), which is
deliberately worst-first for triage — rendered best-first in the "All"
list, while the per-entity tabs (native order) stayed correct.
Resolve the direction BFF-side at template-resolve time instead: a new
DashboardWidget.topNOrder field is parsed from the widget's first top_n()
expression in widgetsForScope (top/record widgets only; pass-through and
reference-stable otherwise). The UI's multiTopGroups reads topNOrder
(default des) and drops the data-probe — so the UI no longer parses MQE
or guesses direction from one entity's data.
---
apps/bff/src/logic/layers/loader.test.ts | 13 ++++++
apps/bff/src/logic/layers/loader.ts | 32 ++++++++++++--
apps/bff/src/logic/layers/topNOrder.test.ts | 51 ++++++++++++++++++++++
.../render/layer-dashboard/LayerDashboardsView.vue | 15 ++++---
packages/api-client/src/dashboard.ts | 6 +++
5 files changed, 107 insertions(+), 10 deletions(-)
diff --git a/apps/bff/src/logic/layers/loader.test.ts
b/apps/bff/src/logic/layers/loader.test.ts
index bf5aa8f..455c101 100644
--- a/apps/bff/src/logic/layers/loader.test.ts
+++ b/apps/bff/src/logic/layers/loader.test.ts
@@ -94,6 +94,19 @@ describe('widgetsForScope — scope-resolution + fallback
chain', () => {
expect(widgetsForScope(t, 'service')).toEqual([]);
expect(widgetsForScope(t, 'instance')).toBe(instance);
});
+
+ it('resolves topNOrder on top / record widgets from their top_n()
expression', () => {
+ const top: DashboardWidget = {
+ id: 'sla',
+ title: 'Top instances by success rate',
+ type: 'top',
+ expressions: ['top_n(service_instance_sla,10,asc)/100'],
+ };
+ const line = widget('cpm-line'); // non-top widget rides through untouched
+ const out = widgetsForScope(tpl({ dashboards: { service: [top, line] } }),
'service');
+ expect(out.find((w) => w.id === 'sla')?.topNOrder).toBe('asc');
+ expect(out.find((w) => w.id === 'cpm-line')).toBe(line); // unchanged
reference
+ });
});
describe('topologyConfigFor / endpointDependencyConfigFor / tracesConfigFor —
defaults', () => {
diff --git a/apps/bff/src/logic/layers/loader.ts
b/apps/bff/src/logic/layers/loader.ts
index 78b7f7f..8f903db 100644
--- a/apps/bff/src/logic/layers/loader.ts
+++ b/apps/bff/src/logic/layers/loader.ts
@@ -560,14 +560,40 @@ function load(): Map<string, LayerTemplate> {
return out;
}
-/** Resolve the widget set for a given scope, falling back to service. */
+/** Sort direction (`asc`/`des`) of the FIRST `top_n(…)` expression in the
+ * list, or `undefined` when none declares one. Resolved here at template
+ * time so the UI never parses MQE or guesses direction from the data —
+ * today top_n's order is a plain literal arg: `top_n(<metric>, <N>,
asc|des)`. */
+export function topNOrderOf(
+ expressions: readonly string[] | undefined,
+): 'asc' | 'des' | undefined {
+ for (const e of expressions ?? []) {
+ const m = /\btop_n\s*\([^)]*,\s*(asc|des)\b/i.exec(e);
+ if (m) return m[1].toLowerCase() as 'asc' | 'des';
+ }
+ return undefined;
+}
+
+/** Resolve the widget set for a given scope, falling back to service.
+ * Enriches `top` / `record` widgets with the resolved `topNOrder` (from
+ * their first `top_n(…)`) so the multi-entity compare grid merges the
+ * per-entity "All" list in the MQE's own direction — without the UI parsing
+ * MQE or inferring asc/des from one entity's (possibly flat) values. */
export function widgetsForScope(
template: LayerTemplate,
scope: DashboardScope,
): DashboardWidget[] {
const d = template.dashboards;
- if (!d) return template.widgets ?? [];
- return d[scope] ?? d.service ?? template.widgets ?? [];
+ const raw = !d ? (template.widgets ?? []) : (d[scope] ?? d.service ??
template.widgets ?? []);
+ // Pass the resolved array straight through unless a top / record widget
+ // needs its sort direction resolved — keeps the common case allocation-free
+ // (and reference-stable) and only copies the widgets that gain `topNOrder`.
+ if (!raw.some((w) => w.type === 'top' || w.type === 'record')) return raw;
+ return raw.map((w) => {
+ if (w.type !== 'top' && w.type !== 'record') return w;
+ const order = topNOrderOf(w.expressions);
+ return order ? { ...w, topNOrder: order } : w;
+ });
}
/** Resolve the topology config — operator override if present, else
diff --git a/apps/bff/src/logic/layers/topNOrder.test.ts
b/apps/bff/src/logic/layers/topNOrder.test.ts
new file mode 100644
index 0000000..3280526
--- /dev/null
+++ b/apps/bff/src/logic/layers/topNOrder.test.ts
@@ -0,0 +1,51 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { describe, expect, it } from 'vitest';
+import { topNOrderOf } from './loader';
+
+describe('topNOrderOf — resolve top_n sort direction from the MQE', () => {
+ it('reads the explicit asc / des argument', () => {
+
expect(topNOrderOf(['top_n(service_instance_sla,10,asc)/100'])).toBe('asc');
+ expect(topNOrderOf(['top_n(endpoint_cpm,20,des)'])).toBe('des');
+ });
+
+ it('takes the FIRST top_n expression when several are present (tab-switch
widgets)', () => {
+ // The compare grid merges the first expression (topGroups[0]), so its
order wins.
+ expect(
+ topNOrderOf(['top_n(endpoint_cpm,20,des)',
'top_n(endpoint_sla,20,asc)/100']),
+ ).toBe('des');
+ });
+
+ it('tolerates whitespace and case', () => {
+ expect(topNOrderOf(['top_n( service_instance_sla , 10 , ASC
)/100'])).toBe('asc');
+ });
+
+ it('is undefined when no order argument is declared', () => {
+ expect(topNOrderOf(['top_n(service_instance_cpm,10)'])).toBeUndefined();
+ });
+
+ it('is undefined for non-top_n expressions (record sources, plain metrics)',
() => {
+ expect(topNOrderOf(['top_n_service_database_statement'])).toBeUndefined();
+ expect(topNOrderOf(['service_instance_cpm'])).toBeUndefined();
+ });
+
+ it('is undefined for empty / missing input', () => {
+ expect(topNOrderOf([])).toBeUndefined();
+ expect(topNOrderOf(undefined)).toBeUndefined();
+ });
+});
diff --git a/apps/ui/src/render/layer-dashboard/LayerDashboardsView.vue
b/apps/ui/src/render/layer-dashboard/LayerDashboardsView.vue
index bfa97fc..5995e13 100644
--- a/apps/ui/src/render/layer-dashboard/LayerDashboardsView.vue
+++ b/apps/ui/src/render/layer-dashboard/LayerDashboardsView.vue
@@ -662,15 +662,16 @@ interface TopGroup {
expression: string;
items: TopItem[];
}
-// "All" follows the response's own asc/desc (MQE-fixed) then prefixes the
-// entity; per-entity groups keep each entity's native order.
+// "All" merges every entity's rows and re-sorts them in the widget's OWN
+// MQE direction; per-entity groups keep each entity's native order. The
+// direction is resolved BFF-side at template time (`topNOrder`), NOT inferred
+// from the data — inferring from one probe entity flips the sort when that
+// entity's values are flat/equal (e.g. instances all at 100% success rate) or
+// when no entity has ≥2 rows. Default `des` for record widgets / expressions
+// without an explicit order.
function multiTopGroups(wid: string): TopGroup[] {
const ents = compareEntities.value;
- // Detect the MQE's fixed asc/desc from the FIRST entity that actually has
- // >=2 items — probing only ents[0] inverts the merged sort when the primary
- // happens to be sparse while a pinned entity carries the full ranking.
- const probe = ents.map((e) => topItemsFor(wid, e)).find((it) => it.length >=
2) ?? [];
- const desc = probe.length < 2 || (probe[0].value ?? 0) >=
(probe[probe.length - 1].value ?? 0);
+ const desc = widgets.value.find((w) => w.id === wid)?.topNOrder !== 'asc';
const all: TopItem[] = ents.flatMap((e) =>
topItemsFor(wid, e).map((it) => ({ name: `${entityLabel(e)} · ${it.name}`,
value: it.value })),
);
diff --git a/packages/api-client/src/dashboard.ts
b/packages/api-client/src/dashboard.ts
index 24864e6..0b6419c 100644
--- a/packages/api-client/src/dashboard.ts
+++ b/packages/api-client/src/dashboard.ts
@@ -205,6 +205,12 @@ export interface DashboardWidget {
* multi-entity compare; the remainder fold into one `(others)` row.
* Defaults to 8. */
labelTopN?: number;
+ /** Sort direction of the widget's first `top_n(…)` expression, resolved
+ * BFF-side at template-resolve time (not inferred from data). Lets the
+ * multi-entity compare grid merge the per-entity "All" list in the MQE's
+ * own order — `asc` (worst/lowest first, e.g. success-rate) vs `des`.
+ * Absent for non-`top_n` widgets; the UI then falls back to `des`. */
+ topNOrder?: 'asc' | 'des';
/** Legacy 24-col grid coordinates — kept for back-compat during the
* span-based flow-layout migration. New widgets should leave these
* unset and use `span` / `rowSpan` instead. */
