This is an automated email from the ASF dual-hosted git repository.
lizhimins pushed a commit to branch rocketmq-studio
in repository https://gitbox.apache.org/repos/asf/rocketmq-dashboard.git
The following commit(s) were added to refs/heads/rocketmq-studio by this push:
new baf70ae6a fix(metrics): retain newest queries when history responses
finish out of order (#6377)
baf70ae6a is described below
commit baf70ae6a759570110dce9a1973dc617433492dc
Author: Chstercc <[email protected]>
AuthorDate: Thu Oct 15 19:23:00 2026 +0800
fix(metrics): retain newest queries when history responses finish out of
order (#6377)
* fix(metrics): retain newest queries when history responses finish out of
order
* test(metrics): align the history scope case with queriedAt ordering
mergeMetricsQueryHistory now orders by queriedAt descending, so the
"unscoped vs an instance literally named global" case can no longer assert
that the incoming entry comes first. Assert what the case is actually about:
both entries survive with distinct ids, and both iteration orders converge
on
the same newest-first result.
---------
Co-authored-by: lizhimins <[email protected]>
---
.../components/__tests__/MetricsExplorer.test.tsx | 30 ++++++
web/src/utils/metricsExplorerDiagnostics.test.ts | 113 +++++++++++++++++----
web/src/utils/metricsExplorerDiagnostics.ts | 21 ++--
3 files changed, 138 insertions(+), 26 deletions(-)
diff --git a/web/src/components/__tests__/MetricsExplorer.test.tsx
b/web/src/components/__tests__/MetricsExplorer.test.tsx
index be438a7dc..21cf2c0ca 100644
--- a/web/src/components/__tests__/MetricsExplorer.test.tsx
+++ b/web/src/components/__tests__/MetricsExplorer.test.tsx
@@ -1292,6 +1292,36 @@ describe('MetricsExplorer', () => {
expect(reopened.querySelector('.ant-list-item')).toHaveTextContent('自定义查询');
});
+ it('keepsNewerCustomHistoryAheadOfASlowEarlierProfileQueryTest', async () =>
{
+ const user = userEvent.setup();
+ const pendingProfile = createDeferred<MetricData>();
+ vi.mocked(queryMetrics).mockReturnValueOnce(pendingProfile.promise);
+ renderWithProviders(<MetricsExplorer />);
+ await waitFor(() => expect(queryMetrics).toHaveBeenCalledTimes(1));
+
+ vi.mocked(Date.now).mockReturnValue(1_800_000_001_000);
+ await user.type(screen.getByLabelText('自定义查询'),
'sum(rocketmq_topic_number)');
+ await user.click(screen.getByRole('button', { name: '查询' }));
+ await screen.findByRole('img', { name: '自定义查询 time series' });
+
+ await act(async () => pendingProfile.resolve(metricData));
+ await screen.findByRole('img', { name: 'Message In TPS time series' });
+ await user.click(screen.getByRole('button', { name: '查询历史' }));
+ const dialog = await screen.findByRole('dialog', { name: '指标查询历史' });
+ const rows = dialog.querySelectorAll('.ant-list-item');
+
+ expect(rows).toHaveLength(2);
+ expect(rows[0]).toHaveTextContent('自定义查询');
+ expect(rows[1]).toHaveTextContent('Message In TPS');
+ const persisted: MetricsQueryHistoryEntry[] = JSON.parse(
+ localStorage.getItem(METRICS_QUERY_HISTORY_STORAGE_KEY)!,
+ );
+ expect(persisted.map((entry) => entry.queriedAt)).toEqual([
+ 1_800_000_001_000, 1_800_000_000_000,
+ ]);
+ expect(persisted[0].promql).toBe('sum(rocketmq_topic_number)');
+ });
+
it('restores profile, range, and source from query history', async () => {
const user = userEvent.setup();
localStorage.setItem(METRICS_QUERY_HISTORY_STORAGE_KEY,
JSON.stringify([createHistoryEntry()]));
diff --git a/web/src/utils/metricsExplorerDiagnostics.test.ts
b/web/src/utils/metricsExplorerDiagnostics.test.ts
index 90936ec67..ca12f237c 100644
--- a/web/src/utils/metricsExplorerDiagnostics.test.ts
+++ b/web/src/utils/metricsExplorerDiagnostics.test.ts
@@ -152,30 +152,44 @@ describe('metrics explorer diagnostics', () => {
const data: MetricData = {
resultType: 'matrix',
warnings: [],
- series: [{
- labels: { instance: 'broker-a' },
- values: [{ timestamp: 2, value: '12' }],
- histograms: [
- { timestamp: 3, histogram: { count: '4', sum: '20', buckets: [] } },
- { timestamp: 1, histogram: { count: '2', sum: '10', buckets: [] } },
- ],
- }],
+ series: [
+ {
+ labels: { instance: 'broker-a' },
+ values: [{ timestamp: 2, value: '12' }],
+ histograms: [
+ { timestamp: 3, histogram: { count: '4', sum: '20', buckets: [] }
},
+ { timestamp: 1, histogram: { count: '2', sum: '10', buckets: [] }
},
+ ],
+ },
+ ],
};
- expect(toMetricSeriesSamples(data.series[0]).samples.map(({ timestamp,
kind }) => ({ timestamp, kind })))
- .toEqual([
- { timestamp: 1, kind: 'histogram' },
- { timestamp: 2, kind: 'scalar' },
- { timestamp: 3, kind: 'histogram' },
- ]);
+ expect(
+ toMetricSeriesSamples(data.series[0]).samples.map(({ timestamp, kind })
=> ({
+ timestamp,
+ kind,
+ })),
+ ).toEqual([
+ { timestamp: 1, kind: 'histogram' },
+ { timestamp: 2, kind: 'scalar' },
+ { timestamp: 3, kind: 'histogram' },
+ ]);
expect(summarizeMetricData(data)).toMatchObject({
- seriesCount: 1, visibleSeriesCount: 1, sampleCount: 3,
- scalarSampleCount: 1, histogramSampleCount: 2,
- earliestTimestamp: 1, latestTimestamp: 3,
+ seriesCount: 1,
+ visibleSeriesCount: 1,
+ sampleCount: 3,
+ scalarSampleCount: 1,
+ histogramSampleCount: 2,
+ earliestTimestamp: 1,
+ latestTimestamp: 3,
});
expect(buildMetricSeriesDetailRows(data, metric)[0]).toMatchObject({
- sampleType: 'mixed', sampleCount: 3, latestTimestamp: 3,
- latestValue: 20, histogramCount: 4, histogramSum: 20,
+ sampleType: 'mixed',
+ sampleCount: 3,
+ latestTimestamp: 3,
+ latestValue: 20,
+ histogramCount: 4,
+ histogramSum: 20,
});
const rows = buildMetricCsvRows(data, metric, { profileName: 'test',
sourceName: 'test' });
expect(rows.map((row) => row.sampleType)).toEqual(['histogram', 'scalar',
'histogram']);
@@ -349,7 +363,14 @@ describe('metrics explorer diagnostics', () => {
const merged = mergeMetricsQueryHistory([existing], incoming);
- expect(merged).toEqual([incoming, existing]);
+ // The point of this case is separation: an unscoped query and one
scoped to an instance
+ // literally named "global" must survive as two entries. Ordering
follows queriedAt now,
+ // so both iteration orders converge on the same result.
+ expect(merged).toHaveLength(2);
+ expect(merged.map((entry) => entry.id)).toEqual(
+ expect.arrayContaining([unscoped.id, scoped.id]),
+ );
+ expect(merged).toEqual([scoped, unscoped]);
localStorage.clear();
expect(saveMetricsQueryHistory(merged)).toBe(true);
expect(loadMetricsQueryHistory()).toEqual([scoped, unscoped]);
@@ -390,6 +411,58 @@ describe('metrics explorer diagnostics', () => {
expect(mergeMetricsQueryHistory([implicit], explicit)).toEqual([explicit,
implicit]);
});
+ it('ordersLateCompletionsByQueryTimeWithoutMutatingInputTest', () => {
+ const newest = createHistoryEntry({ rangeId: '3h', queriedAt: 30 });
+ const oldest = createHistoryEntry({ rangeId: '1h', queriedAt: 10 });
+ const middle = createHistoryEntry({ rangeId: '2h', queriedAt: 20 });
+ const entries = [newest, oldest];
+
+ expect(mergeMetricsQueryHistory(entries, middle)).toEqual([newest, middle,
oldest]);
+ expect(entries).toEqual([newest, oldest]);
+ });
+
+ it('capsHistoryAfterOrderingSoAnOlderCompletionCannotEvictANewerQueryTest',
() => {
+ const entries = Array.from({ length: METRICS_QUERY_HISTORY_LIMIT }, (_,
index) =>
+ createHistoryEntry({ rangeId: `${index + 1}h`, queriedAt: 100 - index }),
+ );
+ const late = createHistoryEntry({ rangeId: 'old', queriedAt: 1 });
+
+ expect(mergeMetricsQueryHistory(entries, late)).toEqual(entries);
+ });
+
+ it('retainsTheNewestOccurrenceWhenAnOlderDuplicateCompletesTest', () => {
+ const newest = createHistoryEntry({ queriedAt: 30 });
+ const late = createHistoryEntry({ queriedAt: 10 });
+
+ expect(mergeMetricsQueryHistory([newest], late)).toEqual([newest]);
+ expect(mergeMetricsQueryHistory([late], newest)).toEqual([newest]);
+ });
+
+ it('keepsEqualTimeOrderingStableAcrossPersistenceReloadTest', () => {
+ localStorage.clear();
+ const first = createHistoryEntry({ rangeId: '1h', queriedAt: 10 });
+ const second = createHistoryEntry({ rangeId: '2h', queriedAt: 10 });
+ const incoming = createHistoryEntry({ rangeId: '3h', queriedAt: 10 });
+ const merged = mergeMetricsQueryHistory([first, second], incoming);
+
+ expect(merged).toEqual([incoming, first, second]);
+ expect(saveMetricsQueryHistory(merged)).toBe(true);
+ expect(loadMetricsQueryHistory()).toEqual(merged);
+ clearMetricsQueryHistory();
+ });
+
+ it('deduplicatesBeforeApplyingACustomHistoryLimitTest', () => {
+ const newest = createHistoryEntry({ rangeId: '1h', queriedAt: 30 });
+ const duplicate = createHistoryEntry({ rangeId: '1h', queriedAt: 20 });
+ const distinct = createHistoryEntry({ rangeId: '2h', queriedAt: 10 });
+ const late = createHistoryEntry({ rangeId: '3h', queriedAt: 1 });
+
+ expect(mergeMetricsQueryHistory([newest, duplicate, distinct], late,
2)).toEqual([
+ newest,
+ distinct,
+ ]);
+ });
+
// The unavailable-storage fallbacks are covered by browserStorage.test.ts,
which is where
// the try/catch now lives, so this only asserts the malformed-payload path.
it('ignores malformed persisted history entries', () => {
diff --git a/web/src/utils/metricsExplorerDiagnostics.ts
b/web/src/utils/metricsExplorerDiagnostics.ts
index 7dafacd1e..c3bfcd741 100644
--- a/web/src/utils/metricsExplorerDiagnostics.ts
+++ b/web/src/utils/metricsExplorerDiagnostics.ts
@@ -340,7 +340,9 @@ export const buildMetricSeriesDetailRows = (
seriesLabel: metricSeriesLabel(series, metric.name),
labels: stableLabelsText(series.labels),
sampleType: fromHistogram
- ? samples.some((sample) => sample.kind === 'scalar') ? 'mixed' :
'histogram'
+ ? samples.some((sample) => sample.kind === 'scalar')
+ ? 'mixed'
+ : 'histogram'
: 'scalar',
sampleCount: samples.length,
latestTimestamp: latest?.timestamp,
@@ -496,9 +498,16 @@ export const mergeMetricsQueryHistory = (
entry: MetricsQueryHistoryEntry,
limit = METRICS_QUERY_HISTORY_LIMIT,
) => {
- const duplicateKey = historyIdentity(entry);
- return [entry, ...entries.filter((item) => historyIdentity(item) !==
duplicateKey)].slice(
- 0,
- limit,
- );
+ // Responses can finish out of order. Keep the newest query for each
identity before
+ // applying the cap; a late older response must not displace more recent
history.
+ const identities = new Set<string>();
+ return [entry, ...entries]
+ .sort((left, right) => right.queriedAt - left.queriedAt)
+ .filter((item) => {
+ const identity = historyIdentity(item);
+ if (identities.has(identity)) return false;
+ identities.add(identity);
+ return true;
+ })
+ .slice(0, limit);
};
