From 692bb9ed1a21b97235507aa3e5b26511418e103f Mon Sep 17 00:00:00 2001 From: Carl Bergquist Date: Fri, 28 Apr 2023 15:19:06 +0200 Subject: [PATCH] Instrumentation: Add support for instrumenting database queries (#66022) Signed-off-by: bergquist Co-authored-by: Christopher Moyer <35463610+chri2547@users.noreply.github.com> --- conf/defaults.ini | 3 +++ conf/sample.ini | 3 +++ docs/sources/setup-grafana/configure-grafana/_index.md | 4 ++++ .../configure-grafana/feature-toggles/index.md | 1 - packages/grafana-data/src/types/featureToggles.gen.ts | 1 - pkg/middleware/loggermw/logger.go | 2 +- pkg/services/featuremgmt/registry.go | 6 ------ pkg/services/featuremgmt/toggles_gen.csv | 1 - pkg/services/featuremgmt/toggles_gen.go | 4 ---- pkg/services/searchV2/index.go | 4 ---- pkg/services/sqlstore/sqlstore.go | 2 +- pkg/setting/setting.go | 9 +++++++++ 12 files changed, 21 insertions(+), 19 deletions(-) diff --git a/conf/defaults.ini b/conf/defaults.ini index 086c84d7348..85a43de5f58 100644 --- a/conf/defaults.ini +++ b/conf/defaults.ini @@ -159,6 +159,9 @@ query_retries = 0 # For "sqlite" only. How many times to retry transaction in case of database is locked failures. Default is 5. transaction_retries = 5 +# Set to true to add metrics and tracing for database queries. +instrument_queries = false + #################################### Cache server ############################# [remote_cache] # Either "redis", "memcached" or "database" default is "database" diff --git a/conf/sample.ini b/conf/sample.ini index d1d788a982b..fed965002a4 100644 --- a/conf/sample.ini +++ b/conf/sample.ini @@ -161,6 +161,9 @@ # For "sqlite" only. How many times to retry transaction in case of database is locked failures. Default is 5. ;transaction_retries = 5 +# Set to true to add metrics and tracing for database queries. +;instrument_queries = false + ################################### Data sources ######################### [datasources] # Upper limit of data sources that Grafana will return. This limit is a temporary configuration and it will be deprecated when pagination will be introduced on the list data sources API. diff --git a/docs/sources/setup-grafana/configure-grafana/_index.md b/docs/sources/setup-grafana/configure-grafana/_index.md index 6072beb2ca1..13b190a52b9 100644 --- a/docs/sources/setup-grafana/configure-grafana/_index.md +++ b/docs/sources/setup-grafana/configure-grafana/_index.md @@ -404,6 +404,10 @@ This setting applies to `sqlite` only and controls the number of times the syste This setting applies to `sqlite` only and controls the number of times the system retries a transaction when the database is locked. The default value is `5`. +### instrument_queries + +Set to `true` to add metrics and tracing for database queries. The default value is `false`. +
## [remote_cache] diff --git a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md index bc03c5b2b41..bf553aa61a2 100644 --- a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md +++ b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md @@ -22,7 +22,6 @@ Some stable features are enabled by default. You can disable a stable feature by | Feature toggle name | Description | Enabled by default | | ----------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------ | | `disableEnvelopeEncryption` | Disable envelope encryption (emergency only) | | -| `database_metrics` | Add Prometheus metrics for database tables | | | `featureHighlights` | Highlight Grafana Enterprise features | | | `cloudWatchDynamicLabels` | Use dynamic labels instead of alias patterns in CloudWatch datasource | Yes | | `dataConnectionsConsole` | Enables a new top-level page called Connections. This page is an experiment that provides a better experience when you install and configure data sources and other plugins. | Yes | diff --git a/packages/grafana-data/src/types/featureToggles.gen.ts b/packages/grafana-data/src/types/featureToggles.gen.ts index 120b95f96ef..9f04ea6972c 100644 --- a/packages/grafana-data/src/types/featureToggles.gen.ts +++ b/packages/grafana-data/src/types/featureToggles.gen.ts @@ -20,7 +20,6 @@ export interface FeatureToggles { trimDefaults?: boolean; disableEnvelopeEncryption?: boolean; - database_metrics?: boolean; ['live-service-web-worker']?: boolean; queryOverLive?: boolean; panelTitleSearch?: boolean; diff --git a/pkg/middleware/loggermw/logger.go b/pkg/middleware/loggermw/logger.go index 4fea6fc0198..493d7a0e968 100644 --- a/pkg/middleware/loggermw/logger.go +++ b/pkg/middleware/loggermw/logger.go @@ -119,7 +119,7 @@ func (l *loggerImpl) prepareLogParams(c *contextmodel.ReqContext, duration time. lvl = lvl.HighestOf(errutil.LevelWarn) } - if l.flags.IsEnabled(featuremgmt.FlagDatabaseMetrics) { + if l.cfg.DatabaseInstrumentQueries { logParams = append(logParams, "db_call_count", log.TotalDBCallCount(c.Req.Context())) } diff --git a/pkg/services/featuremgmt/registry.go b/pkg/services/featuremgmt/registry.go index 552ec7e3034..6ad62a643ed 100644 --- a/pkg/services/featuremgmt/registry.go +++ b/pkg/services/featuremgmt/registry.go @@ -21,12 +21,6 @@ var ( State: FeatureStateStable, Owner: grafanaAsCodeSquad, }, - { - Name: "database_metrics", - Description: "Add Prometheus metrics for database tables", - State: FeatureStateStable, - Owner: hostedGrafanaTeam, - }, { Name: "live-service-web-worker", Description: "This will use a webworker thread to processes events rather than the main thread", diff --git a/pkg/services/featuremgmt/toggles_gen.csv b/pkg/services/featuremgmt/toggles_gen.csv index 07bee5a12b9..e3d7ba39bbd 100644 --- a/pkg/services/featuremgmt/toggles_gen.csv +++ b/pkg/services/featuremgmt/toggles_gen.csv @@ -1,7 +1,6 @@ Name,State,Owner,requiresDevMode,RequiresLicense,RequiresRestart,FrontendOnly trimDefaults,beta,@grafana/grafana-as-code,false,false,false,false disableEnvelopeEncryption,stable,@grafana/grafana-as-code,false,false,false,false -database_metrics,stable,@grafana/hosted-grafana-team,false,false,false,false live-service-web-worker,alpha,@grafana/grafana-app-platform-squad,false,false,false,true queryOverLive,alpha,@grafana/grafana-app-platform-squad,false,false,false,true panelTitleSearch,beta,@grafana/grafana-app-platform-squad,false,false,false,false diff --git a/pkg/services/featuremgmt/toggles_gen.go b/pkg/services/featuremgmt/toggles_gen.go index d7e369c0dc3..da33d9d06e3 100644 --- a/pkg/services/featuremgmt/toggles_gen.go +++ b/pkg/services/featuremgmt/toggles_gen.go @@ -15,10 +15,6 @@ const ( // Disable envelope encryption (emergency only) FlagDisableEnvelopeEncryption = "disableEnvelopeEncryption" - // FlagDatabaseMetrics - // Add Prometheus metrics for database tables - FlagDatabaseMetrics = "database_metrics" - // FlagLiveServiceWebWorker // This will use a webworker thread to processes events rather than the main thread FlagLiveServiceWebWorker = "live-service-web-worker" diff --git a/pkg/services/searchV2/index.go b/pkg/services/searchV2/index.go index 0c15395cd55..92b247611c3 100644 --- a/pkg/services/searchV2/index.go +++ b/pkg/services/searchV2/index.go @@ -591,10 +591,6 @@ func (i *searchIndex) withCtxData(ctx context.Context, params ...interface{}) [] params = append(params, "traceID", traceID) } - if i.features.IsEnabled(featuremgmt.FlagDatabaseMetrics) { - params = append(params, "db_call_count", log.TotalDBCallCount(ctx)) - } - return params } diff --git a/pkg/services/sqlstore/sqlstore.go b/pkg/services/sqlstore/sqlstore.go index ee3d5cbce97..c835ef02e6e 100644 --- a/pkg/services/sqlstore/sqlstore.go +++ b/pkg/services/sqlstore/sqlstore.go @@ -362,7 +362,7 @@ func (ss *SQLStore) initEngine(engine *xorm.Engine) error { return err } - if ss.Cfg.IsFeatureToggleEnabled(featuremgmt.FlagDatabaseMetrics) { + if ss.Cfg.DatabaseInstrumentQueries { ss.dbCfg.Type = WrapDatabaseDriverWithHooks(ss.dbCfg.Type, ss.tracer) } diff --git a/pkg/setting/setting.go b/pkg/setting/setting.go index e8bc06baf85..b7020ed8d87 100644 --- a/pkg/setting/setting.go +++ b/pkg/setting/setting.go @@ -519,6 +519,12 @@ type Cfg struct { GRPCServerTLSConfig *tls.Config CustomResponseHeaders map[string]string + + // DatabaseInstrumentQueries is used to decide if database queries + // should be instrumented with metrics, logs and traces. + // This needs to be on the global object since its used in the + // sqlstore package and HTTP middlewares. + DatabaseInstrumentQueries bool } // AddChangePasswordLink returns if login form is disabled or not since @@ -1189,6 +1195,9 @@ func (cfg *Cfg) Load(args CommandLineArgs) error { cfg.LogConfigSources() + databaseSection := iniFile.Section("database") + cfg.DatabaseInstrumentQueries = databaseSection.Key("instrument_queries").MustBool(false) + return nil }