From 829022d488e922980d4b18ad6c3c411e9b749460 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Jamr=C3=B3z?= Date: Wed, 14 Jan 2026 12:53:06 +0100 Subject: [PATCH] add save retries and track stats --- .../history/RichHistoryLocalStorage.test.ts | 91 +++++++++++++++ .../core/history/RichHistoryLocalStorage.ts | 109 +++++++++++++++--- public/app/core/utils/richHistoryTypes.ts | 1 + .../explore/spec/queryHistory.test.tsx | 2 +- 4 files changed, 189 insertions(+), 14 deletions(-) diff --git a/public/app/core/history/RichHistoryLocalStorage.test.ts b/public/app/core/history/RichHistoryLocalStorage.test.ts index 8cf739514e2..a64aee332f0 100644 --- a/public/app/core/history/RichHistoryLocalStorage.test.ts +++ b/public/app/core/history/RichHistoryLocalStorage.test.ts @@ -1,4 +1,5 @@ import { DataQuery } from '@grafana/data'; +import { createMonitoringLogger, MonitoringLogger } from '@grafana/runtime'; import store from 'app/core/store'; import { RichHistoryQuery } from 'app/types/explore'; @@ -26,8 +27,15 @@ jest.mock('@grafana/runtime', () => ({ ...jest.requireActual('@grafana/runtime'), getBackendSrv: () => backendSrv, getDataSourceSrv: () => dsMock, + createMonitoringLogger: jest.fn().mockReturnValue({ logWarning: jest.fn() }), })); +// logger is created at import so we cannot initialize inside the test +const loggerIndex = (createMonitoringLogger as jest.Mock).mock.calls.findIndex( + (args) => args[0] === 'features.query-history.local-storage' +); +const loggerMock: MonitoringLogger = (createMonitoringLogger as jest.Mock).mock.results[loggerIndex]?.value; + interface MockQuery extends DataQuery { query: string; } @@ -223,6 +231,89 @@ describe('RichHistoryLocalStorage', () => { }); }); + describe('quota errors and retries', () => { + it('should rotate and retry saving when QuotaExceededError occurs once', async () => { + const initial = [ + { ts: Date.now(), starred: true, comment: 'starred1', queries: [], datasourceName: 'name-of-dev-test' }, + { ts: Date.now(), starred: false, comment: 'notStarred1', queries: [], datasourceName: 'name-of-dev-test' }, + { ts: Date.now(), starred: true, comment: 'starred2', queries: [], datasourceName: 'name-of-dev-test' }, + ]; + store.setObject(key, initial); + + // Spy on setObject to throw once with QuotaExceededError, then call through + const originalSetObject = store.setObject.bind(store); + jest + .spyOn(store, 'setObject') + // first attempt throws and errors + .mockImplementationOnce(() => { + const err = new Error('quota hit'); + err.name = 'QuotaExceededError'; + throw err; + }) + // second attempt calls through + .mockImplementation((k: string, value: unknown) => { + return originalSetObject(k, value); + }); + + const result = await storage.addToRichHistory({ + starred: false, + datasourceUid: 'dev-test', + datasourceName: 'name-of-dev-test', + comment: 'new', + queries: [{ refId: 'A' }], + }); + expect(result.richHistoryQuery).toBeDefined(); + + // After one failure, rotation removes one unstarred entry + const saved = store.getObject(key)!; + expect(saved).toHaveLength(3); + expect(saved).toMatchObject([ + expect.objectContaining({ comment: 'new' }), + expect.objectContaining({ comment: 'starred1' }), + expect.objectContaining({ comment: 'starred2' }), + ]); + + // Ensure logger was called for the failure, with expected flags + expect(loggerMock.logWarning).toHaveBeenCalled(); + const [message, payload] = (loggerMock.logWarning as jest.Mock).mock.calls[0]; + expect(message).toContain('Failed to save rich history to local storage'); + expect(payload.saveRetriesLeft).toBe('3'); + expect(payload.quotaExceededError).toBe('true'); + }); + + it('should throw StorageFull when QuotaExceededError persists for all retries and track attempts', async () => { + store.setObject(key, [ + { ts: Date.now(), starred: false, comment: 'notStarred1', queries: [], datasourceName: 'name-of-dev-test' }, + ]); + + const setSpy = jest.spyOn(store, 'setObject').mockImplementation(() => { + const err = new Error('quota still hit'); + err.name = 'QuotaExceededError'; + throw err; + }); + + await expect( + storage.addToRichHistory({ + starred: false, + datasourceUid: 'dev-test', + datasourceName: 'name-of-dev-test', + comment: 'new', + queries: [{ refId: 'B' }], + }) + ).rejects.toMatchObject({ name: 'StorageFull' }); + + // 3 tracking attempts should be logged (for each failed try) + expect(loggerMock.logWarning).toHaveBeenCalledTimes(3); + const calls = (loggerMock.logWarning as jest.Mock).mock.calls; + expect(calls[0][0]).toContain('Failed to save rich history to local storage'); + expect(calls[0][1].saveRetriesLeft).toBe('3'); + expect(calls[1][1].saveRetriesLeft).toBe('2'); + expect(calls[2][1].saveRetriesLeft).toBe('1'); + + setSpy.mockRestore(); + }); + }); + describe('migration', () => { afterEach(() => { storage.deleteAll(); diff --git a/public/app/core/history/RichHistoryLocalStorage.ts b/public/app/core/history/RichHistoryLocalStorage.ts index 6a88f5011c3..c1471fcdc63 100644 --- a/public/app/core/history/RichHistoryLocalStorage.ts +++ b/public/app/core/history/RichHistoryLocalStorage.ts @@ -1,7 +1,8 @@ import { find, isEqual, omit } from 'lodash'; import { DataQuery, SelectableValue } from '@grafana/data'; -import { RichHistorySearchFilters, RichHistorySettings } from 'app/core/utils/richHistoryTypes'; +import { createMonitoringLogger } from '@grafana/runtime'; +import { RichHistorySearchFilters, RichHistorySettings, SortOrder } from 'app/core/utils/richHistoryTypes'; import { RichHistoryQuery } from 'app/types/explore'; import store from '../store'; @@ -26,10 +27,18 @@ export type RichHistoryLocalStorageDTO = { queries: DataQuery[]; }; +const logger = createMonitoringLogger('features.query-history.local-storage'); + /** * Local storage implementation for Rich History. It keeps all entries in browser's local storage. */ export default class RichHistoryLocalStorage implements RichHistoryStorage { + public static getLocalStorageUsageInBytes(): number { + const richHistory: RichHistoryLocalStorageDTO[] = store.get(RICH_HISTORY_KEY) || ''; + // each character is 2 bytes + return richHistory.length * 2; + } + /** * Return history entries based on provided filters, perform migration and clean up entries not matching retention policy. */ @@ -77,21 +86,42 @@ export default class RichHistoryLocalStorage implements RichHistoryStorage { throw error; } - const { queriesToKeep, limitExceeded } = checkLimits(currentRichHistoryDTOs); + const { queriesToKeep, limitExceeded } = cleanUpUnstarredQuery(currentRichHistoryDTOs, MAX_HISTORY_ITEMS); - const updatedHistory: RichHistoryLocalStorageDTO[] = [newRichHistoryQueryDTO, ...queriesToKeep]; + let updatedHistory: RichHistoryLocalStorageDTO[] = [newRichHistoryQueryDTO, ...queriesToKeep]; - try { - store.setObject(RICH_HISTORY_KEY, updatedHistory); - } catch (error) { - if (error instanceof Error && error.name === 'QuotaExceededError') { - throwError(RichHistoryServiceError.StorageFull, `Saving rich history failed: ${error.message}`); - } else { - throw error; + let saveRetriesLeft = 3; + let saved = false; + + while (!saved && saveRetriesLeft >= 0) { + try { + store.setObject(RICH_HISTORY_KEY, updatedHistory); + saved = true; + } catch (error) { + if (saveRetriesLeft >= 1) { + await this.trackLocalStorageUsage('Failed to save rich history to local storage', { + saveRetriesLeft: saveRetriesLeft.toString(), + quotaExceededError: error instanceof Error && error.name === 'QuotaExceededError' ? 'true' : 'false', + errorMessage: error instanceof Error ? error?.message : 'unknown', + }); + + saveRetriesLeft--; + const { queriesToKeep } = cleanUpUnstarredQuery(updatedHistory, updatedHistory.length - 1); + updatedHistory = queriesToKeep; + continue; + } + + if (error instanceof Error && error.name === 'QuotaExceededError') { + throwError(RichHistoryServiceError.StorageFull, `Saving rich history failed: ${error.message}`); + } else { + throw error; + } } } if (limitExceeded) { + await this.trackLocalStorageUsage('Rich history query limit exceeded.'); + return { warning: { type: RichHistoryStorageWarning.LimitExceeded, @@ -148,6 +178,33 @@ export default class RichHistoryLocalStorage implements RichHistoryStorage { }) ); } + + private async trackLocalStorageUsage(message: string, additionalInfo?: Record) { + const allQueriesCount = + ( + await this.getRichHistory({ + search: '', + sortOrder: SortOrder.Ascending, + datasourceFilters: [], + starred: false, + }) + ).total || -1; + + const allQueriesSizeInBytes = RichHistoryLocalStorage.getLocalStorageUsageInBytes(); + + const totalLocalStorageSize = calculateTotalLocalStorageSize(); + + const localStats = { + totalLocalStorageSize: totalLocalStorageSize?.toString(), + allQueriesSizeInBytes: allQueriesSizeInBytes?.toString(), + allQueriesCount: allQueriesCount?.toString(), + }; + + logger.logWarning(message, { + ...localStats, + ...additionalInfo, + }); + } } function updateRichHistory( @@ -185,17 +242,20 @@ function cleanUp(richHistory: RichHistoryLocalStorageDTO[]): RichHistoryLocalSto } /** - * Ensures the entry can be added. Throws an error if current limit has been hit. + * Ensures the entry can be added. * Returns queries that should be saved back giving space for one extra query. */ -export function checkLimits(queriesToKeep: RichHistoryLocalStorageDTO[]): { +export function cleanUpUnstarredQuery( + queriesToKeep: RichHistoryLocalStorageDTO[], + max: number +): { queriesToKeep: RichHistoryLocalStorageDTO[]; limitExceeded: boolean; } { // remove oldest non-starred items to give space for the recent query let limitExceeded = false; let current = queriesToKeep.length - 1; - while (current >= 0 && queriesToKeep.length >= MAX_HISTORY_ITEMS) { + while (current >= 0 && queriesToKeep.length > max) { if (!queriesToKeep[current].starred) { queriesToKeep.splice(current, 1); limitExceeded = true; @@ -247,3 +307,26 @@ function throwError(name: string, message: string) { error.name = name; throw error; } + +function calculateTotalLocalStorageSize() { + try { + let total = 0; + + // eslint-disable-next-line + const ls = window.localStorage; + + for (let i = 0; i < ls.length; i++) { + const key = ls.key(i); + if (key) { + const value = ls.getItem(key); + if (value) { + total += key.length + value.length; + } + } + } + // each character is 2 bytes + return total * 2; + } catch (e) { + return -1; + } +} diff --git a/public/app/core/utils/richHistoryTypes.ts b/public/app/core/utils/richHistoryTypes.ts index 0a944eef422..d822d7b93a2 100644 --- a/public/app/core/utils/richHistoryTypes.ts +++ b/public/app/core/utils/richHistoryTypes.ts @@ -28,6 +28,7 @@ export type RichHistorySearchFilters = { // so the resulting timerange from this will be [now - from, now - to]. from?: number; to?: number; + // true if only starred entries should be returned, false if ALL entries should be returned, starred: boolean; page?: number; }; diff --git a/public/app/features/explore/spec/queryHistory.test.tsx b/public/app/features/explore/spec/queryHistory.test.tsx index ca67720d66f..f09d8b95648 100644 --- a/public/app/features/explore/spec/queryHistory.test.tsx +++ b/public/app/features/explore/spec/queryHistory.test.tsx @@ -200,7 +200,7 @@ describe('Explore: Query History', () => { await waitForExplore(); await openQueryHistory(); - jest.spyOn(localStorage, 'checkLimits').mockImplementationOnce((queries) => { + jest.spyOn(localStorage, 'cleanUpUnstarredQuery').mockImplementationOnce((queries) => { return { queriesToKeep: queries, limitExceeded: true }; });