fix: only collect prometheus database metrics when explicitly enabled (#8045)

* fix: only collect prometheus database metrics when explicitly enabled

* add missing test

* de-duplicate wrapping

cli/server.go

@@ -589,7 +589,7 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
 
 			if cfg.InMemoryDatabase {
 				// This is only used for testing.
-				options.Database = dbmetrics.New(dbfake.New(), options.PrometheusRegistry)
+				options.Database = dbfake.New()
 				options.Pubsub = pubsub.NewInMemory()
 			} else {
 				sqlDB, err := connectToPostgres(ctx, logger, sqlDriver, cfg.PostgresURL.String())
@@ -600,7 +600,7 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
 					_ = sqlDB.Close()
 				}()
 
-				options.Database = dbmetrics.New(database.New(sqlDB), options.PrometheusRegistry)
+				options.Database = database.New(sqlDB)
 				options.Pubsub, err = pubsub.New(ctx, sqlDB, cfg.PostgresURL.String())
 				if err != nil {
 					return xerrors.Errorf("create pubsub: %w", err)
@@ -608,6 +608,10 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
 				defer options.Pubsub.Close()
 			}
 
+			if options.DeploymentValues.Prometheus.Enable && options.DeploymentValues.Prometheus.CollectDBMetrics {
+				options.Database = dbmetrics.New(options.Database, options.PrometheusRegistry)
+			}
+
 			var deploymentID string
 			err = options.Database.InTx(func(tx database.Store) error {
 				// This will block until the lock is acquired, and will be

cli/server_test.go

@@ -881,59 +881,115 @@ func TestServer(t *testing.T) {
 	})
 	t.Run("Prometheus", func(t *testing.T) {
 		t.Parallel()
-		random, err := net.Listen("tcp", "127.0.0.1:0")
-		require.NoError(t, err)
-		_ = random.Close()
-		tcpAddr, valid := random.Addr().(*net.TCPAddr)
-		require.True(t, valid)
-		randomPort := tcpAddr.Port
 
-		inv, cfg := clitest.New(t,
-			"server",
-			"--in-memory",
-			"--http-address", ":0",
-			"--access-url", "http://example.com",
-			"--provisioner-daemons", "1",
-			"--prometheus-enable",
-			"--prometheus-address", ":"+strconv.Itoa(randomPort),
-			"--cache-dir", t.TempDir(),
-		)
-
-		ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort)
-		defer cancel()
-
-		clitest.Start(t, inv)
-		_ = waitAccessURL(t, cfg)
-
-		var res *http.Response
-		require.Eventually(t, func() bool {
-			req, err := http.NewRequestWithContext(ctx, "GET", fmt.Sprintf("http://127.0.0.1:%d", randomPort), nil)
-			assert.NoError(t, err)
-			// nolint:bodyclose
-			res, err = http.DefaultClient.Do(req)
-			return err == nil
-		}, testutil.WaitShort, testutil.IntervalFast)
-		defer res.Body.Close()
-
-		scanner := bufio.NewScanner(res.Body)
-		hasActiveUsers := false
-		hasWorkspaces := false
-		for scanner.Scan() {
-			// This metric is manually registered to be tracked in the server. That's
-			// why we test it's tracked here.
-			if strings.HasPrefix(scanner.Text(), "coderd_api_active_users_duration_hour") {
-				hasActiveUsers = true
-				continue
-			}
-			if strings.HasPrefix(scanner.Text(), "coderd_api_workspace_latest_build_total") {
-				hasWorkspaces = true
-				continue
-			}
-			t.Logf("scanned %s", scanner.Text())
+		randomPort := func(t *testing.T) int {
+			random, err := net.Listen("tcp", "127.0.0.1:0")
+			require.NoError(t, err)
+			_ = random.Close()
+			tcpAddr, valid := random.Addr().(*net.TCPAddr)
+			require.True(t, valid)
+			return tcpAddr.Port
 		}
-		require.NoError(t, scanner.Err())
-		require.True(t, hasActiveUsers)
-		require.True(t, hasWorkspaces)
+
+		t.Run("DBMetricsDisabled", func(t *testing.T) {
+			t.Parallel()
+
+			ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort)
+			defer cancel()
+
+			randPort := randomPort(t)
+			inv, cfg := clitest.New(t,
+				"server",
+				"--in-memory",
+				"--http-address", ":0",
+				"--access-url", "http://example.com",
+				"--provisioner-daemons", "1",
+				"--prometheus-enable",
+				"--prometheus-address", ":"+strconv.Itoa(randPort),
+				// "--prometheus-collect-db-metrics", // disabled by default
+				"--cache-dir", t.TempDir(),
+			)
+
+			clitest.Start(t, inv)
+			_ = waitAccessURL(t, cfg)
+
+			var res *http.Response
+			require.Eventually(t, func() bool {
+				req, err := http.NewRequestWithContext(ctx, "GET", fmt.Sprintf("http://127.0.0.1:%d", randPort), nil)
+				assert.NoError(t, err)
+				// nolint:bodyclose
+				res, err = http.DefaultClient.Do(req)
+				return err == nil
+			}, testutil.WaitShort, testutil.IntervalFast)
+			defer res.Body.Close()
+
+			scanner := bufio.NewScanner(res.Body)
+			hasActiveUsers := false
+			hasWorkspaces := false
+			for scanner.Scan() {
+				// This metric is manually registered to be tracked in the server. That's
+				// why we test it's tracked here.
+				if strings.HasPrefix(scanner.Text(), "coderd_api_active_users_duration_hour") {
+					hasActiveUsers = true
+					continue
+				}
+				if strings.HasPrefix(scanner.Text(), "coderd_api_workspace_latest_build_total") {
+					hasWorkspaces = true
+					continue
+				}
+				if strings.HasPrefix(scanner.Text(), "coderd_db_query_latencies_seconds") {
+					t.Fatal("db metrics should not be tracked when --prometheus-collect-db-metrics is not enabled")
+				}
+				t.Logf("scanned %s", scanner.Text())
+			}
+			require.NoError(t, scanner.Err())
+			require.True(t, hasActiveUsers)
+			require.True(t, hasWorkspaces)
+		})
+
+		t.Run("DBMetricsEnabled", func(t *testing.T) {
+			t.Parallel()
+
+			ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort)
+			defer cancel()
+
+			randPort := randomPort(t)
+			inv, cfg := clitest.New(t,
+				"server",
+				"--in-memory",
+				"--http-address", ":0",
+				"--access-url", "http://example.com",
+				"--provisioner-daemons", "1",
+				"--prometheus-enable",
+				"--prometheus-address", ":"+strconv.Itoa(randPort),
+				"--prometheus-collect-db-metrics",
+				"--cache-dir", t.TempDir(),
+			)
+
+			clitest.Start(t, inv)
+			_ = waitAccessURL(t, cfg)
+
+			var res *http.Response
+			require.Eventually(t, func() bool {
+				req, err := http.NewRequestWithContext(ctx, "GET", fmt.Sprintf("http://127.0.0.1:%d", randPort), nil)
+				assert.NoError(t, err)
+				// nolint:bodyclose
+				res, err = http.DefaultClient.Do(req)
+				return err == nil
+			}, testutil.WaitShort, testutil.IntervalFast)
+			defer res.Body.Close()
+
+			scanner := bufio.NewScanner(res.Body)
+			hasDBMetrics := false
+			for scanner.Scan() {
+				if strings.HasPrefix(scanner.Text(), "coderd_db_query_latencies_seconds") {
+					hasDBMetrics = true
+				}
+				t.Logf("scanned %s", scanner.Text())
+			}
+			require.NoError(t, scanner.Err())
+			require.True(t, hasDBMetrics)
+		})
 	})
 	t.Run("GitHubOAuth", func(t *testing.T) {
 		t.Parallel()

cli/testdata/coder_server_--help.golden

@@ -93,6 +93,9 @@ Use a YAML configuration file when your server launch become unwieldy.
       --prometheus-collect-agent-stats bool, $CODER_PROMETHEUS_COLLECT_AGENT_STATS
           Collect agent stats (may increase charges for metrics storage).
 
+      --prometheus-collect-db-metrics bool, $CODER_PROMETHEUS_COLLECT_DB_METRICS (default: false)
+          Collect database metrics (may increase charges for metrics storage).
+
       --prometheus-enable bool, $CODER_PROMETHEUS_ENABLE
           Serve prometheus metrics on the address defined by prometheus address.
 

cli/testdata/server-config.yaml.golden

@@ -152,6 +152,9 @@ introspection:
     # Collect agent stats (may increase charges for metrics storage).
     # (default: <unset>, type: bool)
     collect_agent_stats: false
+    # Collect database metrics (may increase charges for metrics storage).
+    # (default: false, type: bool)
+    collect_db_metrics: false
   pprof:
     # Serve pprof metrics on the address defined by pprof address.
     # (default: <unset>, type: bool)

coderd/apidoc/docs.go

@@ -7880,6 +7880,9 @@ const docTemplate = `{
                 "collect_agent_stats": {
                     "type": "boolean"
                 },
+                "collect_db_metrics": {
+                    "type": "boolean"
+                },
                 "enable": {
                     "type": "boolean"
                 }

coderd/apidoc/swagger.json

@@ -7061,6 +7061,9 @@
         "collect_agent_stats": {
           "type": "boolean"
         },
+        "collect_db_metrics": {
+          "type": "boolean"
+        },
         "enable": {
           "type": "boolean"
         }

coderd/coderd.go

@@ -47,7 +47,6 @@ import (
 	"github.com/coder/coder/coderd/awsidentity"
 	"github.com/coder/coder/coderd/database"
 	"github.com/coder/coder/coderd/database/dbauthz"
-	"github.com/coder/coder/coderd/database/dbmetrics"
 	"github.com/coder/coder/coderd/database/pubsub"
 	"github.com/coder/coder/coderd/gitauth"
 	"github.com/coder/coder/coderd/gitsshkey"
@@ -191,10 +190,6 @@ func New(options *Options) *API {
 	if options.Authorizer == nil {
 		options.Authorizer = rbac.NewCachingAuthorizer(options.PrometheusRegistry)
 	}
-	// The below are no-ops if already wrapped.
-	if options.PrometheusRegistry != nil {
-		options.Database = dbmetrics.New(options.Database, options.PrometheusRegistry)
-	}
 	options.Database = dbauthz.New(
 		options.Database,
 		options.Authorizer,

codersdk/deployment.go

@@ -229,6 +229,7 @@ type PrometheusConfig struct {
 	Enable            clibase.Bool     `json:"enable" typescript:",notnull"`
 	Address           clibase.HostPort `json:"address" typescript:",notnull"`
 	CollectAgentStats clibase.Bool     `json:"collect_agent_stats" typescript:",notnull"`
+	CollectDBMetrics  clibase.Bool     `json:"collect_db_metrics" typescript:",notnull"`
 }
 
 type PprofConfig struct {
@@ -760,6 +761,16 @@ when required by your organization's security policy.`,
 			Group:       &deploymentGroupIntrospectionPrometheus,
 			YAML:        "collect_agent_stats",
 		},
+		{
+			Name:        "Prometheus Collect Database Metrics",
+			Description: "Collect database metrics (may increase charges for metrics storage).",
+			Flag:        "prometheus-collect-db-metrics",
+			Env:         "CODER_PROMETHEUS_COLLECT_DB_METRICS",
+			Value:       &c.Prometheus.CollectDBMetrics,
+			Group:       &deploymentGroupIntrospectionPrometheus,
+			YAML:        "collect_db_metrics",
+			Default:     "false",
+		},
 		// Pprof settings
 		{
 			Name:        "pprof Enable",

docs/api/general.md

@@ -275,6 +275,7 @@ curl -X GET http://coder-server:8080/api/v2/deployment/config \
         "port": "string"
       },
       "collect_agent_stats": true,
+      "collect_db_metrics": true,
       "enable": true
     },
     "provisioner": {

docs/api/schemas.md

@@ -1943,6 +1943,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in
         "port": "string"
       },
       "collect_agent_stats": true,
+      "collect_db_metrics": true,
       "enable": true
     },
     "provisioner": {
@@ -2270,6 +2271,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in
       "port": "string"
     },
     "collect_agent_stats": true,
+    "collect_db_metrics": true,
     "enable": true
   },
   "provisioner": {
@@ -3092,6 +3094,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in
     "port": "string"
   },
   "collect_agent_stats": true,
+  "collect_db_metrics": true,
   "enable": true
 }
 ```
@@ -3102,6 +3105,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in
 | --------------------- | ------------------------------------ | -------- | ------------ | ----------- |
 | `address`             | [clibase.HostPort](#clibasehostport) | false    |              |             |
 | `collect_agent_stats` | boolean                              | false    |              |             |
+| `collect_db_metrics`  | boolean                              | false    |              |             |
 | `enable`              | boolean                              | false    |              |             |
 
 ## codersdk.ProvisionerConfig

docs/cli/server.md

@@ -565,6 +565,17 @@ The bind address to serve prometheus metrics.
 
 Collect agent stats (may increase charges for metrics storage).
 
+### --prometheus-collect-db-metrics
+
+|             |                                                          |
+| ----------- | -------------------------------------------------------- |
+| Type        | <code>bool</code>                                        |
+| Environment | <code>$CODER_PROMETHEUS_COLLECT_DB_METRICS</code>        |
+| YAML        | <code>introspection.prometheus.collect_db_metrics</code> |
+| Default     | <code>false</code>                                       |
+
+Collect database metrics (may increase charges for metrics storage).
+
 ### --prometheus-enable
 
 |             |                                              |

enterprise/cli/testdata/coder_server_--help.golden

@@ -93,6 +93,9 @@ Use a YAML configuration file when your server launch become unwieldy.
       --prometheus-collect-agent-stats bool, $CODER_PROMETHEUS_COLLECT_AGENT_STATS
           Collect agent stats (may increase charges for metrics storage).
 
+      --prometheus-collect-db-metrics bool, $CODER_PROMETHEUS_COLLECT_DB_METRICS (default: false)
+          Collect database metrics (may increase charges for metrics storage).
+
       --prometheus-enable bool, $CODER_PROMETHEUS_ENABLE
           Serve prometheus metrics on the address defined by prometheus address.
 

scaletest/terraform/coder.tf

@@ -102,6 +102,8 @@ coder:
       value: "true"
     - name: "CODER_PROMETHEUS_COLLECT_AGENT_STATS"
       value: "true"
+    - name: "CODER_PROMETHEUS_COLLECT_DB_METRICS"
+      value: "true"
     - name: "CODER_VERBOSE"
       value: "true"
   image:

site/src/api/typesGenerated.ts

@@ -606,6 +606,7 @@ export interface PrometheusConfig {
   // eslint-disable-next-line @typescript-eslint/no-explicit-any -- External type
   readonly address: any
   readonly collect_agent_stats: boolean
+  readonly collect_db_metrics: boolean
 }
 
 // From codersdk/deployment.go