From 572c182a818bfb3c002ee0d986a033d238d8af85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Jamr=C3=B3z?= Date: Thu, 1 Feb 2024 15:08:40 +0100 Subject: [PATCH] Unify frontend monitoring (#80075) * Unify frontend monitoring * Add missing mock * Add missing mock * Keep source:sandbox * Create separate logger for plugins/sql package * chore: rename "logAlertingError" to "logError" * Use internal Faro logging for debugging instead of redundant browser logging * Post-merge fix * Add more docs about debug levels * Unify logger names * Update packages/grafana-runtime/src/utils/logging.ts Co-authored-by: Ivan Ortega Alba * Update packages/grafana-runtime/src/utils/logging.ts Co-authored-by: Ivan Ortega Alba --------- Co-authored-by: Gilles De Mey Co-authored-by: Ivan Ortega Alba --- conf/defaults.ini | 5 ++ packages/grafana-runtime/src/index.ts | 2 +- packages/grafana-runtime/src/utils/logging.ts | 54 ++++++++++++++++++- .../useMigrateDatabaseFields.test.ts | 3 ++ .../configuration/useMigrateDatabaseFields.ts | 7 +-- packages/grafana-sql/src/utils/logging.ts | 3 ++ .../setting_grafana_javascript_agent.go | 2 + .../GrafanaJavascriptAgentBackend.ts | 3 +- .../features/alerting/unified/Analytics.ts | 19 +++---- .../receivers/form/ReceiverForm.tsx | 4 +- .../alerting/unified/useRouteGroupsMatcher.ts | 4 +- .../features/correlations/useCorrelations.ts | 8 +-- public/app/features/correlations/utils.ts | 4 +- .../dashboard/components/GenAI/hooks.ts | 6 ++- .../plugins/components/AppRootPage.tsx | 6 +-- public/app/features/plugins/sandbox/utils.ts | 30 +++-------- public/app/features/plugins/utils.ts | 3 ++ public/app/features/query/state/runRequest.ts | 6 ++- public/app/features/query/utils.ts | 3 ++ 19 files changed, 113 insertions(+), 59 deletions(-) create mode 100644 packages/grafana-sql/src/utils/logging.ts create mode 100644 public/app/features/query/utils.ts diff --git a/conf/defaults.ini b/conf/defaults.ini index d26ea453e3f..3570b50ab47 100644 --- a/conf/defaults.ini +++ b/conf/defaults.ini @@ -1047,6 +1047,11 @@ instrumentations_console_enabled = false # Should webvitals instrumentation be enabled, only affects Grafana Javascript Agent instrumentations_webvitals_enabled = false +# level of internal logging for debugging Grafana Javascript Agent. +# possible values are: 0 = OFF, 1 = ERROR, 2 = WARN, 3 = INFO, 4 = VERBOSE +# more details: https://github.com/grafana/faro-web-sdk/blob/v1.3.7/docs/sources/tutorials/quick-start-browser.md#how-to-activate-debugging +internal_logger_level = 0 + # Api Key, only applies to Grafana Javascript Agent provider api_key = diff --git a/packages/grafana-runtime/src/index.ts b/packages/grafana-runtime/src/index.ts index 9092afd51ea..fe98d4ab619 100644 --- a/packages/grafana-runtime/src/index.ts +++ b/packages/grafana-runtime/src/index.ts @@ -15,7 +15,7 @@ export { } from './utils/plugin'; export { reportMetaAnalytics, reportInteraction, reportPageview, reportExperimentView } from './analytics/utils'; export { featureEnabled } from './utils/licensing'; -export { logInfo, logDebug, logWarning, logError } from './utils/logging'; +export { logInfo, logDebug, logWarning, logError, createMonitoringLogger } from './utils/logging'; export { DataSourceWithBackend, HealthCheckError, diff --git a/packages/grafana-runtime/src/utils/logging.ts b/packages/grafana-runtime/src/utils/logging.ts index c37861c35dd..251719a9339 100644 --- a/packages/grafana-runtime/src/utils/logging.ts +++ b/packages/grafana-runtime/src/utils/logging.ts @@ -1,4 +1,4 @@ -import { faro, LogLevel, LogContext } from '@grafana/faro-web-sdk'; +import { faro, LogContext, LogLevel } from '@grafana/faro-web-sdk'; import { config } from '../config'; @@ -57,3 +57,55 @@ export function logError(err: Error, contexts?: LogContext) { }); } } + +/** + * 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. + * + * @param {string} source - Identifier for the source of the log messages. + * @param {LogContext} [defaultContext] - Context to be included in every log message. + * + * @returns {Object} Logger object with four methods: + * - `logDebug(message: string, contexts?: LogContext)`: Logs a debug message. + * - `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. + * 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) { + const createFullContext = (contexts?: LogContext) => ({ + source: source, + ...defaultContext, + ...contexts, + }); + + return { + /** + * Logs a debug message with optional additional context. + * @param {string} message - The debug message to be logged. + * @param {LogContext} [contexts] - Optional additional context to be included. + */ + logDebug: (message: string, contexts?: LogContext) => logDebug(message, createFullContext(contexts)), + + /** + * Logs an informational message with optional additional context. + * @param {string} message - The informational message to be logged. + * @param {LogContext} [contexts] - Optional additional context to be included. + */ + logInfo: (message: string, contexts?: LogContext) => logInfo(message, createFullContext(contexts)), + + /** + * Logs a warning message with optional additional context. + * @param {string} message - The warning message to be logged. + * @param {LogContext} [contexts] - Optional additional context to be included. + */ + logWarning: (message: string, contexts?: LogContext) => logWarning(message, createFullContext(contexts)), + + /** + * Logs an error with optional additional context. + * @param {Error} error - The error object to be logged. + * @param {LogContext} [contexts] - Optional additional context to be included. + */ + logError: (error: Error, contexts?: LogContext) => logError(error, createFullContext(contexts)), + }; +} diff --git a/packages/grafana-sql/src/components/configuration/useMigrateDatabaseFields.test.ts b/packages/grafana-sql/src/components/configuration/useMigrateDatabaseFields.test.ts index cda44ce5924..a193b4a273e 100644 --- a/packages/grafana-sql/src/components/configuration/useMigrateDatabaseFields.test.ts +++ b/packages/grafana-sql/src/components/configuration/useMigrateDatabaseFields.test.ts @@ -16,6 +16,9 @@ jest.mock('@grafana/runtime', () => { }, }, logDebug: jest.fn(), + createMonitoringLogger: jest.fn().mockReturnValue({ + logDebug: jest.fn(), + }), }; }); diff --git a/packages/grafana-sql/src/components/configuration/useMigrateDatabaseFields.ts b/packages/grafana-sql/src/components/configuration/useMigrateDatabaseFields.ts index 2a7ffbe8458..fec87709753 100644 --- a/packages/grafana-sql/src/components/configuration/useMigrateDatabaseFields.ts +++ b/packages/grafana-sql/src/components/configuration/useMigrateDatabaseFields.ts @@ -1,9 +1,10 @@ import { useEffect } from 'react'; import { DataSourcePluginOptionsEditorProps } from '@grafana/data'; -import { logDebug, config } from '@grafana/runtime'; +import { config } from '@grafana/runtime'; import { SQLOptions } from '../../types'; +import { sqlPluginLogger } from '../../utils/logging'; /** * 1. Moves the database field from the options object to jsonData.database and empties the database field. @@ -20,7 +21,7 @@ export function useMigrateDatabaseFields({ // Migrate the database field from the column into the jsonData object if (options.database) { - logDebug(`Migrating from options.database with value ${options.database} for ${options.name}`); + sqlPluginLogger.logDebug(`Migrating from options.database with value ${options.database} for ${options.name}`); newOptions.database = ''; newOptions.jsonData = { ...jsonData, database: options.database }; optionsUpdated = true; @@ -35,7 +36,7 @@ export function useMigrateDatabaseFields({ ) { const { maxOpenConns, maxIdleConns } = config.sqlConnectionLimits; - logDebug( + sqlPluginLogger.logDebug( `Setting default max open connections to ${maxOpenConns} and setting max idle connection to ${maxIdleConns}` ); diff --git a/packages/grafana-sql/src/utils/logging.ts b/packages/grafana-sql/src/utils/logging.ts new file mode 100644 index 00000000000..95900184359 --- /dev/null +++ b/packages/grafana-sql/src/utils/logging.ts @@ -0,0 +1,3 @@ +import { createMonitoringLogger } from '@grafana/runtime'; + +export const sqlPluginLogger = createMonitoringLogger('features.plugins.sql'); diff --git a/pkg/setting/setting_grafana_javascript_agent.go b/pkg/setting/setting_grafana_javascript_agent.go index cfc04d5136d..a623bd242ae 100644 --- a/pkg/setting/setting_grafana_javascript_agent.go +++ b/pkg/setting/setting_grafana_javascript_agent.go @@ -8,6 +8,7 @@ type GrafanaJavascriptAgent struct { ErrorInstrumentalizationEnabled bool `json:"errorInstrumentalizationEnabled"` ConsoleInstrumentalizationEnabled bool `json:"consoleInstrumentalizationEnabled"` WebVitalsInstrumentalizationEnabled bool `json:"webVitalsInstrumentalizationEnabled"` + InternalLoggerLevel int `json:"internalLoggerLevel"` ApiKey string `json:"apiKey"` } @@ -21,6 +22,7 @@ func (cfg *Cfg) readGrafanaJavascriptAgentConfig() { ErrorInstrumentalizationEnabled: raw.Key("instrumentations_errors_enabled").MustBool(true), ConsoleInstrumentalizationEnabled: raw.Key("instrumentations_console_enabled").MustBool(true), WebVitalsInstrumentalizationEnabled: raw.Key("instrumentations_webvitals_enabled").MustBool(true), + InternalLoggerLevel: raw.Key("internal_logger_level").MustInt(0), ApiKey: raw.Key("api_key").String(), } } diff --git a/public/app/core/services/echo/backends/grafana-javascript-agent/GrafanaJavascriptAgentBackend.ts b/public/app/core/services/echo/backends/grafana-javascript-agent/GrafanaJavascriptAgentBackend.ts index f454486f5be..c20b8ec34f1 100644 --- a/public/app/core/services/echo/backends/grafana-javascript-agent/GrafanaJavascriptAgentBackend.ts +++ b/public/app/core/services/echo/backends/grafana-javascript-agent/GrafanaJavascriptAgentBackend.ts @@ -1,5 +1,5 @@ import { BuildInfo } from '@grafana/data'; -import { BaseTransport } from '@grafana/faro-core'; +import { BaseTransport, defaultInternalLoggerLevel } from '@grafana/faro-core'; import { initializeFaro, BrowserConfig, @@ -74,6 +74,7 @@ export class GrafanaJavascriptAgentBackend batching: { sendTimeout: 1000, }, + internalLoggerLevel: options.internalLoggerLevel || defaultInternalLoggerLevel, }; this.faroInstance = initializeFaro(grafanaJavaScriptAgentOptions); diff --git a/public/app/features/alerting/unified/Analytics.ts b/public/app/features/alerting/unified/Analytics.ts index 2cc69d578dd..012bbd988e0 100644 --- a/public/app/features/alerting/unified/Analytics.ts +++ b/public/app/features/alerting/unified/Analytics.ts @@ -1,6 +1,5 @@ import { dateTime } from '@grafana/data'; -import { faro, LogLevel as GrafanaLogLevel } from '@grafana/faro-web-sdk'; -import { getBackendSrv, logError } from '@grafana/runtime'; +import { createMonitoringLogger, getBackendSrv } from '@grafana/runtime'; import { config, reportInteraction } from '@grafana/runtime/src'; import { contextSrv } from 'app/core/core'; @@ -22,18 +21,14 @@ export const LogMessages = { unknownMessageFromError: 'unknown messageFromError', }; -// logInfo from '@grafana/runtime' should be used, but it doesn't handle Grafana JS Agent correctly -export function logInfo(message: string, context: Record = {}) { - if (config.grafanaJavascriptAgent.enabled) { - faro.api.pushLog([message], { - level: GrafanaLogLevel.INFO, - context: { ...context, module: 'Alerting' }, - }); - } +const alertingLogger = createMonitoringLogger('features.alerting', { module: 'Alerting' }); + +export function logInfo(message: string, context?: Record) { + alertingLogger.logInfo(message, context); } -export function logAlertingError(error: Error, context: Record = {}) { - logError(error, { ...context, module: 'Alerting' }); +export function logError(error: Error, context?: Record) { + alertingLogger.logError(error, context); } // eslint-disable-next-line @typescript-eslint/no-explicit-any diff --git a/public/app/features/alerting/unified/components/receivers/form/ReceiverForm.tsx b/public/app/features/alerting/unified/components/receivers/form/ReceiverForm.tsx index 50621e5c619..a5bc8ccf287 100644 --- a/public/app/features/alerting/unified/components/receivers/form/ReceiverForm.tsx +++ b/public/app/features/alerting/unified/components/receivers/form/ReceiverForm.tsx @@ -10,7 +10,7 @@ import { useCleanup } from 'app/core/hooks/useCleanup'; import { AlertManagerCortexConfig } from 'app/plugins/datasource/alertmanager/types'; import { getMessageFromError } from '../../../../../../core/utils/errors'; -import { logAlertingError } from '../../../Analytics'; +import { logError } from '../../../Analytics'; import { isOnCallFetchError } from '../../../api/onCallApi'; import { useControlledFieldArray } from '../../../hooks/useControlledFieldArray'; import { ChannelValues, CommonSettingsComponentType, ReceiverFormValues } from '../../../types/receiver-form'; @@ -103,7 +103,7 @@ export function ReceiverForm({ const error = new Error('Failed to save the contact point'); error.cause = e; - logAlertingError(error); + logError(error); } throw e; } diff --git a/public/app/features/alerting/unified/useRouteGroupsMatcher.ts b/public/app/features/alerting/unified/useRouteGroupsMatcher.ts index e70f1d0f79e..2b57e4e0d34 100644 --- a/public/app/features/alerting/unified/useRouteGroupsMatcher.ts +++ b/public/app/features/alerting/unified/useRouteGroupsMatcher.ts @@ -1,12 +1,10 @@ import * as comlink from 'comlink'; import { useCallback, useEffect } from 'react'; -import { logError } from '@grafana/runtime'; - import { AlertmanagerGroup, RouteWithID } from '../../../plugins/datasource/alertmanager/types'; import { Labels } from '../../../types/unified-alerting-dto'; -import { logInfo } from './Analytics'; +import { logError, logInfo } from './Analytics'; import { createWorker } from './createRouteGroupsMatcherWorker'; import type { RouteGroupsMatcher } from './routeGroupsMatcher'; diff --git a/public/app/features/correlations/useCorrelations.ts b/public/app/features/correlations/useCorrelations.ts index 7ed2dfde644..7b1a9c83c8a 100644 --- a/public/app/features/correlations/useCorrelations.ts +++ b/public/app/features/correlations/useCorrelations.ts @@ -2,7 +2,7 @@ import { useAsyncFn } from 'react-use'; import { lastValueFrom } from 'rxjs'; import { DataSourceInstanceSettings } from '@grafana/data'; -import { getDataSourceSrv, FetchResponse, logWarning } from '@grafana/runtime'; +import { getDataSourceSrv, FetchResponse } from '@grafana/runtime'; import { useGrafana } from 'app/core/context/GrafanaContext'; import { @@ -15,6 +15,7 @@ import { UpdateCorrelationParams, UpdateCorrelationResponse, } from './types'; +import { correlationsLogger } from './utils'; export interface CorrelationsResponse { correlations: Correlation[]; @@ -47,7 +48,7 @@ const toEnrichedCorrelationData = ({ // This logging is to check if there are any customers who did not migrate existing correlations. // See Deprecation Notice in https://github.com/grafana/grafana/pull/72258 for more details if (correlation?.orgId === undefined || correlation?.orgId === null || correlation?.orgId === 0) { - logWarning('Invalid correlation config: Missing org id.', { module: 'Explore' }); + correlationsLogger.logWarning('Invalid correlation config: Missing org id.'); } if ( @@ -62,8 +63,7 @@ const toEnrichedCorrelationData = ({ target: targetDatasource, }; } else { - logWarning(`Invalid correlation config: Missing source or target.`, { - module: 'Explore', + correlationsLogger.logWarning(`Invalid correlation config: Missing source or target.`, { source: JSON.stringify(sourceDatasource), target: JSON.stringify(targetDatasource), }); diff --git a/public/app/features/correlations/utils.ts b/public/app/features/correlations/utils.ts index 09cba67f87e..64d4cf386b9 100644 --- a/public/app/features/correlations/utils.ts +++ b/public/app/features/correlations/utils.ts @@ -1,7 +1,7 @@ import { lastValueFrom } from 'rxjs'; import { DataFrame, DataLinkConfigOrigin } from '@grafana/data'; -import { getBackendSrv, getDataSourceSrv } from '@grafana/runtime'; +import { createMonitoringLogger, getBackendSrv, getDataSourceSrv } from '@grafana/runtime'; import { ExploreItemState } from 'app/types'; import { formatValueName } from '../explore/PrometheusListView/ItemLabels'; @@ -108,3 +108,5 @@ export const generateDefaultLabel = async (sourcePane: ExploreItemState, targetP : ''; }); }; + +export const correlationsLogger = createMonitoringLogger('features.correlations'); diff --git a/public/app/features/dashboard/components/GenAI/hooks.ts b/public/app/features/dashboard/components/GenAI/hooks.ts index 93579660bfd..f8ae8968ee2 100644 --- a/public/app/features/dashboard/components/GenAI/hooks.ts +++ b/public/app/features/dashboard/components/GenAI/hooks.ts @@ -3,7 +3,7 @@ import { useAsync } from 'react-use'; import { Subscription } from 'rxjs'; import { llms } from '@grafana/experimental'; -import { logError } from '@grafana/runtime'; +import { createMonitoringLogger } from '@grafana/runtime'; import { useAppNotification } from 'app/core/copy/appNotification'; import { isLLMPluginEnabled, DEFAULT_OAI_MODEL } from './utils'; @@ -12,6 +12,8 @@ import { isLLMPluginEnabled, DEFAULT_OAI_MODEL } from './utils'; // Ideally we will want to move the hook itself to a different scope later. type Message = llms.openai.Message; +const genAILogger = createMonitoringLogger('features.dashboards.genai'); + export enum StreamStatus { IDLE = 'idle', GENERATING = 'generating', @@ -62,7 +64,7 @@ export function useOpenAIStream( `Please try again or if the problem persists, contact your organization admin.` ); console.error(e); - logError(e, { messages: JSON.stringify(messages), model, temperature: String(temperature) }); + genAILogger.logError(e, { messages: JSON.stringify(messages), model, temperature: String(temperature) }); }, [messages, model, temperature, notifyError] ); diff --git a/public/app/features/plugins/components/AppRootPage.tsx b/public/app/features/plugins/components/AppRootPage.tsx index f66d4531f03..95a9cc1f33b 100644 --- a/public/app/features/plugins/components/AppRootPage.tsx +++ b/public/app/features/plugins/components/AppRootPage.tsx @@ -13,7 +13,7 @@ import { PluginType, PluginContextProvider, } from '@grafana/data'; -import { config, locationSearchToObject, logError } from '@grafana/runtime'; +import { config, locationSearchToObject } from '@grafana/runtime'; import { Alert } from '@grafana/ui'; import { Page } from 'app/core/components/Page/Page'; import PageLoader from 'app/core/components/PageLoader/PageLoader'; @@ -25,7 +25,7 @@ import { getMessageFromError } from 'app/core/utils/errors'; import { getPluginSettings } from '../pluginSettings'; import { importAppPlugin } from '../plugin_loader'; -import { buildPluginSectionNav } from '../utils'; +import { buildPluginSectionNav, pluginsLogger } from '../utils'; import { buildPluginPageContext, PluginPageContext } from './PluginPageContext'; @@ -193,7 +193,7 @@ async function loadAppPlugin(pluginId: string, dispatch: React.Dispatch { console.error('runRequest.catchError', err); - logError(err); + queryLogger.logError(err); return of({ ...state.panelData, state: LoadingState.Error, diff --git a/public/app/features/query/utils.ts b/public/app/features/query/utils.ts new file mode 100644 index 00000000000..32cd897d8ec --- /dev/null +++ b/public/app/features/query/utils.ts @@ -0,0 +1,3 @@ +import { createMonitoringLogger } from '@grafana/runtime'; + +export const queryLogger = createMonitoringLogger('features.query');