From f5dab6b5a5ce3c4d6e627b3aeef34171ab1295a1 Mon Sep 17 00:00:00 2001 From: Gilles De Mey Date: Thu, 7 Mar 2024 16:41:38 +0100 Subject: [PATCH] Alerting: Refactor analytics to use pushMeasurements (#83850) --- packages/grafana-runtime/src/utils/logging.ts | 25 ++++++ .../features/alerting/unified/Analytics.ts | 78 ++++++++++--------- .../alerting/unified/api/alertingApi.ts | 19 +++-- .../alerting/unified/api/alertmanagerApi.ts | 2 +- .../unified/api/featureDiscoveryApi.ts | 2 +- .../alerting/unified/state/actions.ts | 29 +++---- 6 files changed, 93 insertions(+), 62 deletions(-) diff --git a/packages/grafana-runtime/src/utils/logging.ts b/packages/grafana-runtime/src/utils/logging.ts index 251719a9339..d7c94da53d8 100644 --- a/packages/grafana-runtime/src/utils/logging.ts +++ b/packages/grafana-runtime/src/utils/logging.ts @@ -58,6 +58,22 @@ export function logError(err: Error, contexts?: LogContext) { } } +/** + * Log a measurement + * + * @public + */ +export type MeasurementValues = Record; +export function logMeasurement(type: string, values: MeasurementValues, context?: LogContext) { + if (config.grafanaJavascriptAgent.enabled) { + faro.api.pushMeasurement({ + type, + values, + context, + }); + } +} + /** * Creates a monitoring logger with four levels of logging methods: `logDebug`, `logInfo`, `logWarning`, and `logError`. * These methods use `faro.api.pushX` web SDK methods to report these logs or errors to the Faro collector. @@ -70,6 +86,7 @@ export function logError(err: Error, contexts?: LogContext) { * - `logInfo(message: string, contexts?: LogContext)`: Logs an informational message. * - `logWarning(message: string, contexts?: LogContext)`: Logs a warning message. * - `logError(error: Error, contexts?: LogContext)`: Logs an error message. + * - `logMeasurement(measurement: Omit, contexts?: LogContext)`: Logs a measurement. * Each method combines the `defaultContext` (if provided), the `source`, and an optional `LogContext` parameter into a full context that is included with the log message. */ export function createMonitoringLogger(source: string, defaultContext?: LogContext) { @@ -107,5 +124,13 @@ export function createMonitoringLogger(source: string, defaultContext?: LogConte * @param {LogContext} [contexts] - Optional additional context to be included. */ logError: (error: Error, contexts?: LogContext) => logError(error, createFullContext(contexts)), + + /** + * Logs an measurement with optional additional context. + * @param {MeasurementEvent} measurement - The measurement object to be recorded. + * @param {LogContext} [contexts] - Optional additional context to be included. + */ + logMeasurement: (type: string, measurement: MeasurementValues, contexts?: LogContext) => + logMeasurement(type, measurement, createFullContext(contexts)), }; } diff --git a/public/app/features/alerting/unified/Analytics.ts b/public/app/features/alerting/unified/Analytics.ts index 049243309a2..81f0c397739 100644 --- a/public/app/features/alerting/unified/Analytics.ts +++ b/public/app/features/alerting/unified/Analytics.ts @@ -26,29 +26,29 @@ export const LogMessages = { unknownMessageFromError: 'unknown messageFromError', }; -const alertingLogger = createMonitoringLogger('features.alerting', { module: 'Alerting' }); +const { logInfo, logError, logMeasurement } = createMonitoringLogger('features.alerting', { module: 'Alerting' }); -export function logInfo(message: string, context?: Record) { - alertingLogger.logInfo(message, context); -} - -export function logError(error: Error, context?: Record) { - alertingLogger.logError(error, context); -} +export { logInfo, logError, logMeasurement }; // eslint-disable-next-line @typescript-eslint/no-explicit-any export function withPerformanceLogging Promise>( + type: string, func: TFunc, - message: string, context: Record ): (...args: Parameters) => Promise>> { return async function (...args) { const startLoadingTs = performance.now(); + const response = await func(...args); - logInfo(message, { - loadTimeMs: (performance.now() - startLoadingTs).toFixed(0), - ...context, - }); + const loadTimesMs = performance.now() - startLoadingTs; + + logMeasurement( + type, + { + loadTimesMs, + }, + context + ); return response; }; @@ -56,8 +56,8 @@ export function withPerformanceLogging Promise // eslint-disable-next-line @typescript-eslint/no-explicit-any export function withPromRulesMetadataLogging Promise>( + type: string, func: TFunc, - message: string, context: Record ) { return async (...args: Parameters) => { @@ -66,13 +66,16 @@ export function withPromRulesMetadataLogging P const { namespacesCount, groupsCount, rulesCount } = getPromRulesMetadata(response); - logInfo(message, { - loadTimeMs: (performance.now() - startLoadingTs).toFixed(0), - namespacesCount, - groupsCount, - rulesCount, - ...context, - }); + logMeasurement( + type, + { + loadTimeMs: performance.now() - startLoadingTs, + namespacesCount, + groupsCount, + rulesCount, + }, + context + ); return response; }; } @@ -83,9 +86,9 @@ function getPromRulesMetadata(promRules: RuleNamespace[]) { const rulesCount = promRules.flatMap((ns) => ns.groups).flatMap((g) => g.rules).length; const metadata = { - namespacesCount: namespacesCount.toFixed(0), - groupsCount: groupsCount.toFixed(0), - rulesCount: rulesCount.toFixed(0), + namespacesCount: namespacesCount, + groupsCount: groupsCount, + rulesCount: rulesCount, }; return metadata; @@ -93,8 +96,8 @@ function getPromRulesMetadata(promRules: RuleNamespace[]) { // eslint-disable-next-line @typescript-eslint/no-explicit-any export function withRulerRulesMetadataLogging Promise>( + type: string, func: TFunc, - message: string, context: Record ) { return async (...args: Parameters) => { @@ -103,26 +106,29 @@ export function withRulerRulesMetadataLogging const { namespacesCount, groupsCount, rulesCount } = getRulerRulesMetadata(response); - logInfo(message, { - loadTimeMs: (performance.now() - startLoadingTs).toFixed(0), - namespacesCount, - groupsCount, - rulesCount, - ...context, - }); + logMeasurement( + type, + { + namespacesCount, + groupsCount, + rulesCount, + loadTimeMs: performance.now() - startLoadingTs, + }, + context + ); return response; }; } function getRulerRulesMetadata(rulerRules: RulerRulesConfigDTO) { - const namespacesCount = Object.keys(rulerRules).length; + const namespaces = Object.keys(rulerRules); const groups = Object.values(rulerRules).flatMap((groups) => groups); const rules = groups.flatMap((group) => group.rules); return { - namespacesCount: namespacesCount.toFixed(0), - groupsCount: groups.length.toFixed(0), - rulesCount: rules.length.toFixed(0), + namespacesCount: namespaces.length, + groupsCount: groups.length, + rulesCount: rules.length, }; } diff --git a/public/app/features/alerting/unified/api/alertingApi.ts b/public/app/features/alerting/unified/api/alertingApi.ts index d4f9118395e..5d90c4c58a1 100644 --- a/public/app/features/alerting/unified/api/alertingApi.ts +++ b/public/app/features/alerting/unified/api/alertingApi.ts @@ -3,7 +3,7 @@ import { lastValueFrom } from 'rxjs'; import { BackendSrvRequest, getBackendSrv } from '@grafana/runtime'; -import { logInfo } from '../Analytics'; +import { logMeasurement } from '../Analytics'; export const backendSrvBaseQuery = (): BaseQueryFn => async (requestOptions) => { try { @@ -11,12 +11,17 @@ export const backendSrvBaseQuery = (): BaseQueryFn => async ( const { data, ...meta } = await lastValueFrom(getBackendSrv().fetch(requestOptions)); - logInfo('Request finished', { - loadTimeMs: (performance.now() - requestStartTs).toFixed(0), - url: requestOptions.url, - method: requestOptions.method ?? '', - responseStatus: meta.statusText, - }); + logMeasurement( + 'backendSrvBaseQuery', + { + loadTimeMs: performance.now() - requestStartTs, + }, + { + url: requestOptions.url, + method: requestOptions.method ?? 'GET', + responseStatus: meta.statusText, + } + ); return { data, meta }; } catch (error) { diff --git a/public/app/features/alerting/unified/api/alertmanagerApi.ts b/public/app/features/alerting/unified/api/alertmanagerApi.ts index c0c71092210..ea0f1e948d9 100644 --- a/public/app/features/alerting/unified/api/alertmanagerApi.ts +++ b/public/app/features/alerting/unified/api/alertmanagerApi.ts @@ -164,8 +164,8 @@ export const alertmanagerApi = alertingApi.injectEndpoints({ // wrap our fetchConfig function with some performance logging functions const fetchAMconfigWithLogging = withPerformanceLogging( + 'unifiedalerting/fetchAmConfig', fetchAlertManagerConfig, - `[${alertmanagerSourceName}] Alertmanager config loaded`, { dataSourceName: alertmanagerSourceName, thunk: 'unifiedalerting/fetchAmConfig', diff --git a/public/app/features/alerting/unified/api/featureDiscoveryApi.ts b/public/app/features/alerting/unified/api/featureDiscoveryApi.ts index 659cc191f76..45f59b75ea7 100644 --- a/public/app/features/alerting/unified/api/featureDiscoveryApi.ts +++ b/public/app/features/alerting/unified/api/featureDiscoveryApi.ts @@ -28,8 +28,8 @@ export const featureDiscoveryApi = alertingApi.injectEndpoints({ } const discoverFeaturesWithLogging = withPerformanceLogging( + 'unifiedalerting/featureDiscoveryApi/discoverDsFeatures', discoverFeatures, - `[${rulesSourceName}] Rules source features discovered`, { dataSourceName: rulesSourceName, endpoint: 'unifiedalerting/featureDiscoveryApi/discoverDsFeatures', diff --git a/public/app/features/alerting/unified/state/actions.ts b/public/app/features/alerting/unified/state/actions.ts index 0daafcaca65..5d743ce7265 100644 --- a/public/app/features/alerting/unified/state/actions.ts +++ b/public/app/features/alerting/unified/state/actions.ts @@ -2,6 +2,7 @@ import { AsyncThunk, createAsyncThunk } from '@reduxjs/toolkit'; import { isEmpty } from 'lodash'; import { locationService } from '@grafana/runtime'; +import { logMeasurement } from '@grafana/runtime/src/utils/logging'; import { AlertmanagerAlert, AlertManagerCortexConfig, @@ -122,11 +123,10 @@ export const fetchPromRulesAction = createAsyncThunk( ): Promise => { await thunkAPI.dispatch(fetchRulesSourceBuildInfoAction({ rulesSourceName })); - const fetchRulesWithLogging = withPromRulesMetadataLogging( - fetchRules, - `[${rulesSourceName}] Prometheus rules loaded`, - { dataSourceName: rulesSourceName, thunk: 'unifiedalerting/fetchPromRules' } - ); + const fetchRulesWithLogging = withPromRulesMetadataLogging('unifiedalerting/fetchPromRules', fetchRules, { + dataSourceName: rulesSourceName, + thunk: 'unifiedalerting/fetchPromRules', + }); return await withSerializedError( fetchRulesWithLogging(rulesSourceName, filter, limitAlerts, matcher, state, identifier) @@ -164,8 +164,8 @@ export const fetchRulerRulesAction = createAsyncThunk( const rulerConfig = getDataSourceRulerConfig(getState, rulesSourceName); const fetchRulerRulesWithLogging = withRulerRulesMetadataLogging( + 'unifiedalerting/fetchRulerRules', fetchRulerRules, - `[${rulesSourceName}] Ruler rules loaded`, { dataSourceName: rulesSourceName, thunk: 'unifiedalerting/fetchRulerRules', @@ -205,14 +205,9 @@ export function fetchPromAndRulerRulesAction({ export const fetchSilencesAction = createAsyncThunk( 'unifiedalerting/fetchSilences', (alertManagerSourceName: string): Promise => { - const fetchSilencesWithLogging = withPerformanceLogging( - fetchSilences, - `[${alertManagerSourceName}] Silences loaded`, - { - dataSourceName: alertManagerSourceName, - thunk: 'unifiedalerting/fetchSilences', - } - ); + const fetchSilencesWithLogging = withPerformanceLogging('unifiedalerting/fetchSilences', fetchSilences, { + dataSourceName: alertManagerSourceName, + }); return withSerializedError(fetchSilencesWithLogging(alertManagerSourceName)); } @@ -265,8 +260,8 @@ export const fetchRulesSourceBuildInfoAction = createAsyncThunk( const { id, name } = ds; const discoverFeaturesWithLogging = withPerformanceLogging( + 'unifiedalerting/fetchPromBuildinfo', discoverFeatures, - `[${rulesSourceName}] Rules source features discovered`, { dataSourceName: rulesSourceName, thunk: 'unifiedalerting/fetchPromBuildinfo', @@ -338,8 +333,8 @@ export function fetchAllPromAndRulerRulesAction( }) ); - logInfo('All Prom and Ruler rules loaded', { - loadTimeMs: (performance.now() - allStartLoadingTs).toFixed(0), + logMeasurement('unifiedalerting/fetchAllPromAndRulerRulesAction', { + loadTimeMs: performance.now() - allStartLoadingTs, }); }; }