From 74167b4d44119862c1246caa3367e4f488efd15e Mon Sep 17 00:00:00 2001 From: Daniel Lee Date: Wed, 7 Dec 2022 21:19:35 +0100 Subject: [PATCH] UsageInsights: Record events for Explore queries (#59931) * usageinsights: record events for Explore queries * usageinsights: make the source field optional It is not logical to have it for an event like the dashboard-view * usageinsights: add comment to Explore test Explain why we are reversing a previous decision --- .../configure-security/export-logs.md | 1 + .../grafana-runtime/src/analytics/types.ts | 3 + .../query/state/queryAnalytics.test.ts | 65 ++++++++++++++++-- .../features/query/state/queryAnalytics.ts | 68 +++++++++++-------- 4 files changed, 106 insertions(+), 31 deletions(-) diff --git a/docs/sources/setup-grafana/configure-security/export-logs.md b/docs/sources/setup-grafana/configure-security/export-logs.md index 89baf385275..6d44b982dbb 100644 --- a/docs/sources/setup-grafana/configure-security/export-logs.md +++ b/docs/sources/setup-grafana/configure-security/export-logs.md @@ -44,6 +44,7 @@ Logs of usage insights contain the following fields, where the fields followed b | `panelName` | string | Name of the panel of the query. | | `error` | string | Error returned by the query. | | `duration` | number | Duration of the query. | +| `source` | string | Source of the query. For example, `dashboard` or `explore`. | | `orgId`\* | number | ID of the user’s organization. | | `orgName`\* | string | Name of the user’s organization. | | `timestamp`\* | string | The date and time that the request was made, in Coordinated Universal Time (UTC) in [RFC3339](https://tools.ietf.org/html/rfc3339#section-5.6) format. | diff --git a/packages/grafana-runtime/src/analytics/types.ts b/packages/grafana-runtime/src/analytics/types.ts index 2a7ef01504e..5f6812ead7e 100644 --- a/packages/grafana-runtime/src/analytics/types.ts +++ b/packages/grafana-runtime/src/analytics/types.ts @@ -1,3 +1,5 @@ +import { CoreApp } from '@grafana/data'; + import { EchoEvent, EchoEventType } from '../services/EchoSrv'; /** @@ -20,6 +22,7 @@ export interface DashboardInfo { * @public */ export interface DataRequestInfo extends Partial { + source?: CoreApp | string; datasourceName: string; datasourceId: number; datasourceUid: string; diff --git a/public/app/features/query/state/queryAnalytics.test.ts b/public/app/features/query/state/queryAnalytics.test.ts index 6c847338bc0..d23a537ac6d 100644 --- a/public/app/features/query/state/queryAnalytics.test.ts +++ b/public/app/features/query/state/queryAnalytics.test.ts @@ -1,4 +1,13 @@ -import { CoreApp, DataFrame, DataQueryRequest, DataSourceApi, dateTime, LoadingState, PanelData } from '@grafana/data'; +import { + CoreApp, + DataFrame, + DataQueryError, + DataQueryRequest, + DataSourceApi, + dateTime, + LoadingState, + PanelData, +} from '@grafana/data'; import { MetaAnalyticsEventName, reportMetaAnalytics } from '@grafana/runtime'; import { DashboardModel } from '../../dashboard/state'; @@ -12,6 +21,7 @@ beforeEach(() => { const datasource = { name: 'test', id: 1, + uid: 'test', } as DataSourceApi; const dashboardModel = new DashboardModel( @@ -94,6 +104,28 @@ function getTestData(requestApp: string, series: DataFrame[] = []): PanelData { }; } +function getTestDataForExplore(requestApp: string, series: DataFrame[] = []): PanelData { + const now = dateTime(); + const error: DataQueryError = { message: 'test error' }; + + return { + request: { + app: requestApp, + dashboardId: 0, + 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); @@ -185,10 +217,35 @@ describe('emitDataRequestEvent - from a dashboard panel', () => { }); }); +// 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', () => { - const data = getTestData(CoreApp.Explore); - it('Should not report meta analytics', () => { + it('Should report meta analytics', () => { + const data = getTestDataForExplore(CoreApp.Explore); emitDataRequestEvent(datasource)(data); - expect(reportMetaAnalytics).not.toBeCalled(); + + expect(reportMetaAnalytics).toBeCalledTimes(1); + expect(reportMetaAnalytics).toBeCalledWith( + expect.objectContaining({ + eventName: MetaAnalyticsEventName.DataRequest, + source: 'explore', + datasourceName: 'test', + datasourceId: 1, + datasourceUid: 'test', + dataSize: 0, + duration: 1, + totalQueries: 0, + }) + ); + }); + + describe('emitDataRequestEvent - from Explore', () => { + it('Should not report errors', () => { + const data = getTestDataForExplore(CoreApp.Explore); + emitDataRequestEvent(datasource)(data); + + expect(reportMetaAnalytics).toBeCalledTimes(1); + expect(reportMetaAnalytics).toBeCalledWith(expect.not.objectContaining({ error: 'test error' })); + }); }); }); diff --git a/public/app/features/query/state/queryAnalytics.ts b/public/app/features/query/state/queryAnalytics.ts index ae70999ff61..8d977555b78 100644 --- a/public/app/features/query/state/queryAnalytics.ts +++ b/public/app/features/query/state/queryAnalytics.ts @@ -8,7 +8,7 @@ export function emitDataRequestEvent(datasource: DataSourceApi) { let done = false; return (data: PanelData) => { - if (!data.request || done || data.request.app === CoreApp.Explore) { + if (!data.request || done) { return; } @@ -21,6 +21,41 @@ export function emitDataRequestEvent(datasource: DataSourceApi) { return; } + const eventData: DataRequestEventPayload = { + eventName: MetaAnalyticsEventName.DataRequest, + source: data.request.app, + datasourceName: datasource.name, + datasourceId: datasource.id, + datasourceUid: datasource.uid, + datasourceType: datasource.type, + dataSize: 0, + duration: data.request.endTime! - data.request.startTime, + }; + + if (data.request.app === CoreApp.Explore) { + enrichWithExploreInfo(eventData, data); + } else { + enrichWithDashboardInfo(eventData, data); + } + + if (data.series && data.series.length > 0) { + // estimate size + eventData.dataSize = data.series.length; + } + + reportMetaAnalytics(eventData); + + // this done check is to make sure we do not double emit events in case + // there are multiple responses with done state + done = true; + }; + + function enrichWithExploreInfo(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; @@ -31,21 +66,11 @@ export function emitDataRequestEvent(datasource: DataSourceApi) { const totalQueries = Object.keys(queryCacheStatus).length; const cachedQueries = Object.values(queryCacheStatus).filter((val) => val === true).length; - const eventData: DataRequestEventPayload = { - eventName: MetaAnalyticsEventName.DataRequest, - datasourceName: datasource.name, - datasourceId: datasource.id, - datasourceUid: datasource.uid, - datasourceType: datasource.type, - panelId: data.request.panelId, - dashboardId: data.request.dashboardId, - dataSize: 0, - duration: data.request.endTime! - data.request.startTime, - totalQueries, - cachedQueries, - }; + eventData.panelId = data.request!.panelId; + eventData.dashboardId = data.request!.dashboardId; + eventData.totalQueries = totalQueries; + eventData.cachedQueries = cachedQueries; - // enrich with dashboard info const dashboard = getDashboardSrv().getCurrent(); if (dashboard) { eventData.dashboardId = dashboard.id; @@ -58,19 +83,8 @@ export function emitDataRequestEvent(datasource: DataSourceApi) { } } - if (data.series && data.series.length > 0) { - // estimate size - eventData.dataSize = data.series.length; - } - if (data.error) { eventData.error = data.error.message; } - - reportMetaAnalytics(eventData); - - // this done check is to make sure we do not double emit events in case - // there are multiple responses with done state - done = true; - }; + } }