Explore: Align time filters properly to day boundaries in query history (#88498)

public/app/core/history/RichHistoryLocalStorage.ts

@@ -173,7 +173,10 @@ function updateRichHistory(
  */
 function cleanUp(richHistory: RichHistoryLocalStorageDTO[]): RichHistoryLocalStorageDTO[] {
   const retentionPeriod: number = store.getObject(RICH_HISTORY_SETTING_KEYS.retentionPeriod, 7);
-  const retentionPeriodLastTs = createRetentionPeriodBoundary(retentionPeriod, false);
+  // We don't care about timezones that much here when creating the time stamp for deletion. First, not sure if we
+  // should be deleting entries based on timezone change that may serve only for querying and also the timezone
+  // difference would not change that much what is or isn't deleted compared to the default 2 weeks retention.
+  const retentionPeriodLastTs = createRetentionPeriodBoundary(retentionPeriod, { isLastTs: false });
 
   /* Keep only queries, that are within the selected retention period or that are starred.
    * If no queries, initialize with empty array

public/app/core/history/RichHistoryRemoteStorage.test.ts

@@ -131,7 +131,7 @@ describe('RichHistoryRemoteStorage', () => {
 
     expect(fetchMock).toHaveBeenCalledWith({
       method: 'GET',
-      url: `/api/query-history?datasourceUid=ds1&datasourceUid=ds2&searchString=${search}&sort=time-desc&to=now-${from}d&from=now-${to}d&limit=${expectedLimit}&page=${expectedPage}`,
+      url: `/api/query-history?datasourceUid=ds1&datasourceUid=ds2&searchString=${search}&sort=time-desc&to=${to}&from=${from}&limit=${expectedLimit}&page=${expectedPage}`,
       requestId: 'query-history-get-all',
     });
     expect(richHistory).toMatchObject([richHistoryQuery]);

public/app/core/history/RichHistoryRemoteStorage.ts

@@ -132,11 +132,8 @@ function buildQueryParams(filters: RichHistorySearchFilters): string {
     params = params + `&sort=${filters.sortOrder === SortOrder.Ascending ? 'time-asc' : 'time-desc'}`;
   }
   if (!filters.starred) {
-    const relativeFrom = filters.from === 0 ? 'now' : `now-${filters.from}d`;
+    params = params + `&to=${filters.to}`;
-    const relativeTo = filters.to === 0 ? 'now' : `now-${filters.to}d`;
+    params = params + `&from=${filters.from}`;
-    // TODO: Unify: remote storage from/to params are swapped comparing to frontend and local storage filters
-    params = params + `&to=${relativeFrom}`;
-    params = params + `&from=${relativeTo}`;
   }
   params = params + `&limit=100`;
   params = params + `&page=${filters.page || 1}`;

public/app/core/history/RichHistoryStorage.ts

@@ -1,4 +1,4 @@
-import { RichHistorySearchFilters, RichHistorySettings } from 'app/core/utils/richHistory';
+import { RichHistorySearchBackendFilters, RichHistorySettings } from 'app/core/utils/richHistoryTypes';
 
 import { RichHistoryQuery } from '../../types';
 
@@ -35,7 +35,7 @@ export type RichHistoryResults = { richHistory: RichHistoryQuery[]; total?: numb
  * @alpha
  */
 export default interface RichHistoryStorage {
-  getRichHistory(filters: RichHistorySearchFilters): Promise<RichHistoryResults>;
+  getRichHistory(filters: RichHistorySearchBackendFilters): Promise<RichHistoryResults>;
 
   /**
    * Creates new RichHistoryQuery, returns object with unique id and created date

public/app/core/history/richHistoryLocalStorageUtils.ts

@@ -1,5 +1,7 @@
 import { omit } from 'lodash';
 
+import { DateTime, dateTime, dateTimeForTimeZone } from '@grafana/data';
+
 import { RichHistoryQuery } from '../../types';
 import { SortOrder } from '../utils/richHistoryTypes';
 
@@ -25,22 +27,27 @@ export function filterAndSortQueries(
   return sortQueries(filteredQueriesToBeSorted, sortOrder);
 }
 
-export const createRetentionPeriodBoundary = (days: number, isLastTs: boolean) => {
+export const createRetentionPeriodBoundary = (
-  const today = new Date();
+  days: number,
-  const date = new Date(today.setDate(today.getDate() - days));
+  options: { isLastTs: boolean; tz?: string; now?: DateTime }
+): number => {
+  let now = options.now;
+  if (!now) {
+    now = options.tz ? dateTimeForTimeZone(options.tz) : dateTime();
+  }
+  now.add(-days, 'd');
+
   /*
    * As a retention period boundaries, we consider:
    * - The last timestamp equals to the 24:00 of the last day of retention
    * - The first timestamp that equals to the 00:00 of the first day of retention
    */
-  const boundary = isLastTs ? date.setHours(24, 0, 0, 0) : date.setHours(0, 0, 0, 0);
+  const boundary = options.isLastTs ? now.endOf('d') : now.startOf('d');
-  return boundary;
+  return boundary.valueOf();
 };
 
 function filterQueriesByTime(queries: RichHistoryQuery[], timeFilter: [number, number]) {
-  const filter1 = createRetentionPeriodBoundary(timeFilter[0], true); // probably the vars should have a different name
+  return queries.filter((q) => q.createdAt > timeFilter[0] && q.createdAt < timeFilter[1]);
-  const filter2 = createRetentionPeriodBoundary(timeFilter[1], false);
-  return queries.filter((q) => q.createdAt < filter1 && q.createdAt > filter2);
 }
 
 function filterQueriesByDataSource(queries: RichHistoryQuery[], listOfDatasourceFilters: string[]) {

public/app/core/utils/richHistory.ts

@@ -15,9 +15,16 @@ import {
   RichHistoryStorageWarning,
   RichHistoryStorageWarningDetails,
 } from '../history/RichHistoryStorage';
+import { createRetentionPeriodBoundary } from '../history/richHistoryLocalStorageUtils';
 import { getLocalRichHistoryStorage, getRichHistoryStorage } from '../history/richHistoryStorageProvider';
+import { contextSrv } from '../services/context_srv';
 
-import { RichHistorySearchFilters, RichHistorySettings, SortOrder } from './richHistoryTypes';
+import {
+  RichHistorySearchBackendFilters,
+  RichHistorySearchFilters,
+  RichHistorySettings,
+  SortOrder,
+} from './richHistoryTypes';
 
 export { RichHistorySearchFilters, RichHistorySettings, SortOrder };
 
@@ -100,7 +107,22 @@ export async function addToRichHistory(
 }
 
 export async function getRichHistory(filters: RichHistorySearchFilters): Promise<RichHistoryResults> {
-  return await getRichHistoryStorage().getRichHistory(filters);
+  // Transforming from frontend filters where from and to are days from now to absolute timestamps.
+  const filtersCopy: RichHistorySearchBackendFilters = {
+    ...filters,
+    from:
+      filters.to === undefined
+        ? filters.to
+        : createRetentionPeriodBoundary(filters.to, {
+            isLastTs: true,
+            tz: contextSrv.user?.timezone,
+          }),
+    to:
+      filters.from === undefined
+        ? filters.from
+        : createRetentionPeriodBoundary(filters.from, { isLastTs: true, tz: contextSrv.user?.timezone }),
+  };
+  return await getRichHistoryStorage().getRichHistory(filtersCopy);
 }
 
 export async function updateRichHistorySettings(settings: RichHistorySettings): Promise<void> {

public/app/core/utils/richHistoryTypes.ts

@@ -12,6 +12,7 @@ export enum SortOrder {
 }
 
 export interface RichHistorySettings {
+  // Number of days
   retentionPeriod: number;
   starredTabAsFirstTab: boolean;
   activeDatasourcesOnly: boolean;
@@ -23,8 +24,17 @@ export type RichHistorySearchFilters = {
   sortOrder: SortOrder;
   /** Names of data sources (not uids) - used by local and remote storage **/
   datasourceFilters: string[];
+  // from and to represent number of days from now to filter by as the front end filtering is designed that way.
+  // so the resulting timerange from this will be [now - from, now - to].
   from?: number;
   to?: number;
   starred: boolean;
   page?: number;
 };
+
+export type RichHistorySearchBackendFilters = Omit<RichHistorySearchFilters, 'from' | 'to'> & {
+  // This seems pointless but it serves as a documentation because we convert the filters from meaning days from now to
+  // mean absolute timestamps for the history backends.
+  from?: number;
+  to?: number;
+};