From 42a3f36c1827001e5b2239dbe1397acc3f0ba3b7 Mon Sep 17 00:00:00 2001 From: Giordano Ricci Date: Wed, 15 Nov 2023 16:32:28 +0000 Subject: [PATCH] Explore: Fix queries (cached & non) count in usage insights (#78097) * Fix: Fix queries (cached & non) count in usage insights * also keep deprecated error property * Fix & refactor tests --- .../query/state/queryAnalytics.test.ts | 296 ++++++++++-------- .../features/query/state/queryAnalytics.ts | 31 +- 2 files changed, 183 insertions(+), 144 deletions(-) diff --git a/public/app/features/query/state/queryAnalytics.test.ts b/public/app/features/query/state/queryAnalytics.test.ts index b4a7de819f2..cfaefbb3cea 100644 --- a/public/app/features/query/state/queryAnalytics.test.ts +++ b/public/app/features/query/state/queryAnalytics.test.ts @@ -2,11 +2,12 @@ import { CoreApp, DataFrame, DataQueryError, - DataQueryRequest, + getDefaultTimeRange, DataSourceApi, dateTime, LoadingState, PanelData, + DataQueryRequest, } from '@grafana/data'; import { MetaAnalyticsEventName, reportMetaAnalytics } from '@grafana/runtime'; @@ -84,155 +85,196 @@ const multipleDataframesWithSameRefId = [ }, ]; -function getTestData(requestApp: string, series: DataFrame[] = []): PanelData { +function getTestData( + overrides: Partial = {}, + series?: DataFrame[], + errors?: DataQueryError[] +): PanelData { const now = dateTime(); return { request: { - app: requestApp, - panelId: 2, + app: CoreApp.Dashboard, startTime: now.unix(), endTime: now.add(1, 's').unix(), - } as DataQueryRequest, - series, - state: LoadingState.Done, - timeRange: { - from: dateTime(), - to: dateTime(), - raw: { from: '1h', to: 'now' }, + interval: '1s', + intervalMs: 1000, + range: getDefaultTimeRange(), + requestId: '1', + scopedVars: {}, + targets: [], + timezone: 'utc', + ...overrides, }, + series: series || [], + state: LoadingState.Done, + timeRange: getDefaultTimeRange(), + errors, }; } -function getTestDataForExplore(requestApp: string, series: DataFrame[] = []): PanelData { - const now = dateTime(); - const error: DataQueryError = { message: 'test error' }; - - return { - request: { - app: requestApp, - startTime: now.unix(), - endTime: now.add(1, 's').unix(), - } as DataQueryRequest, - series, - state: LoadingState.Done, - timeRange: { - from: dateTime(), - to: dateTime(), - raw: { from: '1h', to: 'now' }, - }, - error: error, - }; -} - -describe('emitDataRequestEvent - from a dashboard panel', () => { - it('Should report meta analytics', () => { - const data = getTestData(CoreApp.Dashboard); - emitDataRequestEvent(datasource)(data); - - expect(reportMetaAnalytics).toBeCalledTimes(1); - expect(reportMetaAnalytics).toBeCalledWith( - expect.objectContaining({ - eventName: MetaAnalyticsEventName.DataRequest, - datasourceName: datasource.name, - datasourceUid: datasource.uid, - datasourceType: datasource.type, - source: 'dashboard', +describe('emitDataRequestEvent', () => { + describe('From a dashboard panel', () => { + it('Should report meta analytics', () => { + const data = getTestData({ panelId: 2, - dashboardUid: 'test', // from dashboard srv - dataSize: 0, - duration: 1, - totalQueries: 0, - cachedQueries: 0, - }) - ); - }); + }); + emitDataRequestEvent(datasource)(data); - it('Should report meta analytics with counts for cached and total queries', () => { - const data = getTestData(CoreApp.Dashboard, partiallyCachedSeries); - emitDataRequestEvent(datasource)(data); + expect(reportMetaAnalytics).toBeCalledTimes(1); + expect(reportMetaAnalytics).toBeCalledWith( + expect.objectContaining({ + eventName: MetaAnalyticsEventName.DataRequest, + datasourceName: datasource.name, + datasourceUid: datasource.uid, + datasourceType: datasource.type, + source: CoreApp.Dashboard, + panelId: 2, + dashboardUid: 'test', // from dashboard srv + dataSize: 0, + duration: 1, + totalQueries: 0, + cachedQueries: 0, + }) + ); + }); - expect(reportMetaAnalytics).toBeCalledTimes(1); - expect(reportMetaAnalytics).toBeCalledWith( - expect.objectContaining({ - eventName: MetaAnalyticsEventName.DataRequest, - datasourceName: datasource.name, - datasourceUid: datasource.uid, - datasourceType: datasource.type, - source: 'dashboard', - panelId: 2, - dashboardUid: 'test', - dataSize: 2, - duration: 1, - totalQueries: 2, - cachedQueries: 1, - }) - ); - }); + it('Should report meta analytics with counts for cached and total queries', () => { + const data = getTestData( + { + panelId: 2, + }, + partiallyCachedSeries + ); + emitDataRequestEvent(datasource)(data); - it('Should report meta analytics with counts for cached and total queries when same refId spread across multiple DataFrames', () => { - const data = getTestData(CoreApp.Dashboard, multipleDataframesWithSameRefId); - emitDataRequestEvent(datasource)(data); + expect(reportMetaAnalytics).toBeCalledTimes(1); + expect(reportMetaAnalytics).toBeCalledWith( + expect.objectContaining({ + eventName: MetaAnalyticsEventName.DataRequest, + datasourceName: datasource.name, + datasourceUid: datasource.uid, + datasourceType: datasource.type, + source: CoreApp.Dashboard, + panelId: 2, + dashboardUid: 'test', + dataSize: 2, + duration: 1, + totalQueries: 2, + cachedQueries: 1, + }) + ); + }); - expect(reportMetaAnalytics).toBeCalledTimes(1); - expect(reportMetaAnalytics).toBeCalledWith( - expect.objectContaining({ - eventName: MetaAnalyticsEventName.DataRequest, - datasourceName: datasource.name, - datasourceUid: datasource.uid, - datasourceType: datasource.type, - source: 'dashboard', - panelId: 2, - dashboardUid: 'test', // from dashboard srv - dataSize: 2, - duration: 1, - totalQueries: 1, - cachedQueries: 1, - }) - ); - }); + it('Should report meta analytics with counts for cached and total queries when same refId spread across multiple DataFrames', () => { + const data = getTestData( + { + panelId: 2, + }, + multipleDataframesWithSameRefId + ); + emitDataRequestEvent(datasource)(data); - it('Should not report meta analytics twice if the request receives multiple responses', () => { - const data = getTestData(CoreApp.Dashboard); - const fn = emitDataRequestEvent(datasource); - fn(data); - fn(data); - expect(reportMetaAnalytics).toBeCalledTimes(1); - }); + expect(reportMetaAnalytics).toBeCalledTimes(1); + expect(reportMetaAnalytics).toBeCalledWith( + expect.objectContaining({ + eventName: MetaAnalyticsEventName.DataRequest, + datasourceName: datasource.name, + datasourceUid: datasource.uid, + datasourceType: datasource.type, + source: CoreApp.Dashboard, + panelId: 2, + dashboardUid: 'test', // from dashboard srv + dataSize: 2, + duration: 1, + totalQueries: 1, + cachedQueries: 1, + }) + ); + }); - it('Should not report meta analytics in edit mode', () => { - mockGetUrlSearchParams.mockImplementationOnce(() => { - return { editPanel: 2 }; + it('Should not report meta analytics twice if the request receives multiple responses', () => { + const data = getTestData(); + const fn = emitDataRequestEvent(datasource); + fn(data); + fn(data); + expect(reportMetaAnalytics).toBeCalledTimes(1); + }); + + it('Should not report meta analytics in edit mode', () => { + mockGetUrlSearchParams.mockImplementationOnce(() => { + return { editPanel: 2 }; + }); + const data = getTestData(); + emitDataRequestEvent(datasource)(data); + expect(reportMetaAnalytics).not.toBeCalled(); }); - const data = getTestData(CoreApp.Dashboard); - emitDataRequestEvent(datasource)(data); - expect(reportMetaAnalytics).not.toBeCalled(); }); -}); -// Previously we filtered out Explore events due to too many errors being generated while a user is building a query -// This tests that we send an event for Explore queries but do not record errors -describe('emitDataRequestEvent - from Explore', () => { - it('Should report meta analytics', () => { - const data = getTestDataForExplore(CoreApp.Explore); - emitDataRequestEvent(datasource)(data); - - expect(reportMetaAnalytics).toBeCalledTimes(1); - expect(reportMetaAnalytics).toBeCalledWith( - expect.objectContaining({ - eventName: MetaAnalyticsEventName.DataRequest, - source: 'explore', - datasourceName: 'test', - datasourceUid: 'test', - dataSize: 0, - duration: 1, - totalQueries: 0, - }) + // Previously we filtered out Explore and Correlations events due to too many errors being generated while a user is building a query + // This tests that we send an event for both queries but do not record errors + describe('From Explore', () => { + const data = getTestData( + { + app: CoreApp.Explore, + }, + undefined, + [{ message: 'test error' }] ); + + it('Should report meta analytics', () => { + emitDataRequestEvent(datasource)(data); + + expect(reportMetaAnalytics).toBeCalledTimes(1); + expect(reportMetaAnalytics).toBeCalledWith( + expect.objectContaining({ + eventName: MetaAnalyticsEventName.DataRequest, + source: CoreApp.Explore, + datasourceName: 'test', + datasourceUid: 'test', + dataSize: 0, + duration: 1, + totalQueries: 0, + }) + ); + }); + + it('Should not report errors', () => { + emitDataRequestEvent(datasource)(data); + + expect(reportMetaAnalytics).toBeCalledTimes(1); + expect(reportMetaAnalytics).toBeCalledWith(expect.not.objectContaining({ error: 'test error' })); + }); }); - describe('emitDataRequestEvent - from Explore', () => { + // Previously we filtered out Explore and Correlations events due to too many errors being generated while a user is building a query + // This tests that we send an event for both queries but do not record errors + describe('From Correlations', () => { + const data = getTestData( + { + app: CoreApp.Correlations, + }, + undefined, + [{ message: 'some error' }] + ); + + it('Should report meta analytics', () => { + emitDataRequestEvent(datasource)(data); + + expect(reportMetaAnalytics).toBeCalledTimes(1); + expect(reportMetaAnalytics).toBeCalledWith( + expect.objectContaining({ + eventName: MetaAnalyticsEventName.DataRequest, + source: CoreApp.Correlations, + datasourceName: 'test', + datasourceUid: 'test', + dataSize: 0, + duration: 1, + totalQueries: 0, + }) + ); + }); + it('Should not report errors', () => { - const data = getTestDataForExplore(CoreApp.Explore); emitDataRequestEvent(datasource)(data); expect(reportMetaAnalytics).toBeCalledTimes(1); diff --git a/public/app/features/query/state/queryAnalytics.ts b/public/app/features/query/state/queryAnalytics.ts index 71c261d18bd..51b25443db7 100644 --- a/public/app/features/query/state/queryAnalytics.ts +++ b/public/app/features/query/state/queryAnalytics.ts @@ -1,4 +1,4 @@ -import { PanelData, LoadingState, DataSourceApi, CoreApp, urlUtil } from '@grafana/data'; +import { PanelData, LoadingState, DataSourceApi, urlUtil, CoreApp } from '@grafana/data'; import { reportMetaAnalytics, MetaAnalyticsEventName, DataRequestEventPayload } from '@grafana/runtime'; import { getDashboardSrv } from '../../dashboard/services/DashboardSrv'; @@ -31,10 +31,10 @@ export function emitDataRequestEvent(datasource: DataSourceApi) { duration: data.request.endTime! - data.request.startTime, }; - if (data.request.app === CoreApp.Explore || data.request.app === CoreApp.Correlations) { - enrichWithInfo(eventData, data); - } else { - enrichWithDashboardInfo(eventData, data); + enrichWithInfo(eventData, data); + + if (data.request.app !== CoreApp.Explore && data.request.app !== CoreApp.Correlations) { + enrichWithErrorData(eventData, data); } if (data.series && data.series.length > 0) { @@ -50,11 +50,6 @@ export function emitDataRequestEvent(datasource: DataSourceApi) { }; function enrichWithInfo(eventData: DataRequestEventPayload, data: PanelData) { - const totalQueries = Object.keys(data.series).length; - eventData.totalQueries = totalQueries; - } - - function enrichWithDashboardInfo(eventData: DataRequestEventPayload, data: PanelData) { const queryCacheStatus: { [key: string]: boolean } = {}; for (let i = 0; i < data.series.length; i++) { const refId = data.series[i].refId; @@ -62,12 +57,10 @@ export function emitDataRequestEvent(datasource: DataSourceApi) { queryCacheStatus[refId] = data.series[i].meta?.isCachedResponse ?? false; } } - const totalQueries = Object.keys(queryCacheStatus).length; - const cachedQueries = Object.values(queryCacheStatus).filter((val) => val === true).length; + eventData.totalQueries = Object.keys(queryCacheStatus).length; + eventData.cachedQueries = Object.values(queryCacheStatus).filter((val) => val === true).length; eventData.panelId = data.request!.panelId; - eventData.totalQueries = totalQueries; - eventData.cachedQueries = cachedQueries; const dashboard = getDashboardSrv().getCurrent(); if (dashboard) { @@ -76,9 +69,13 @@ export function emitDataRequestEvent(datasource: DataSourceApi) { eventData.dashboardUid = dashboard.uid; eventData.folderName = dashboard.meta.folderTitle; } + } +} - if (data.error) { - eventData.error = data.error.message; - } +function enrichWithErrorData(eventData: DataRequestEventPayload, data: PanelData) { + if (data.errors?.length) { + eventData.error = data.errors.join(', '); + } else if (data.error) { + eventData.error = data.error.message; } }