diff --git a/cli/server.go b/cli/server.go index c47cf8271de9e..e30bf5fd71fde 100644 --- a/cli/server.go +++ b/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 diff --git a/cli/server_test.go b/cli/server_test.go index 075a3e8c38dcf..6437fc68c28e1 100644 --- a/cli/server_test.go +++ b/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(), - ) + 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 + } - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) - defer cancel() + t.Run("DBMetricsDisabled", func(t *testing.T) { + t.Parallel() - clitest.Start(t, inv) - _ = waitAccessURL(t, cfg) + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) + defer cancel() - 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() + 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(), + ) - 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 + 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()) } - if strings.HasPrefix(scanner.Text(), "coderd_api_workspace_latest_build_total") { - hasWorkspaces = true - continue + 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()) } - t.Logf("scanned %s", scanner.Text()) - } - require.NoError(t, scanner.Err()) - require.True(t, hasActiveUsers) - require.True(t, hasWorkspaces) + require.NoError(t, scanner.Err()) + require.True(t, hasDBMetrics) + }) }) t.Run("GitHubOAuth", func(t *testing.T) { t.Parallel() diff --git a/cli/testdata/coder_server_--help.golden b/cli/testdata/coder_server_--help.golden index f8f45b138d7fd..340f167855d88 100644 --- a/cli/testdata/coder_server_--help.golden +++ b/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. diff --git a/cli/testdata/server-config.yaml.golden b/cli/testdata/server-config.yaml.golden index 4755939392dd1..79a678e7abd2f 100644 --- a/cli/testdata/server-config.yaml.golden +++ b/cli/testdata/server-config.yaml.golden @@ -152,6 +152,9 @@ introspection: # Collect agent stats (may increase charges for metrics storage). # (default: , 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: , type: bool) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index bff3b11e8089b..c43da403aa372 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -7880,6 +7880,9 @@ const docTemplate = `{ "collect_agent_stats": { "type": "boolean" }, + "collect_db_metrics": { + "type": "boolean" + }, "enable": { "type": "boolean" } diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index a990b936ae2cf..c83e77a8ea8e1 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -7061,6 +7061,9 @@ "collect_agent_stats": { "type": "boolean" }, + "collect_db_metrics": { + "type": "boolean" + }, "enable": { "type": "boolean" } diff --git a/coderd/coderd.go b/coderd/coderd.go index 82a7d36e80551..1bb12c6708a14 100644 --- a/coderd/coderd.go +++ b/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, diff --git a/codersdk/deployment.go b/codersdk/deployment.go index 6972e94e3a338..6d293bde5f317 100644 --- a/codersdk/deployment.go +++ b/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", diff --git a/docs/api/general.md b/docs/api/general.md index 57fe3716efc22..22be046b47e64 100644 --- a/docs/api/general.md +++ b/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": { diff --git a/docs/api/schemas.md b/docs/api/schemas.md index 28d63f3628daa..88580cdb6b957 100644 --- a/docs/api/schemas.md +++ b/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 diff --git a/docs/cli/server.md b/docs/cli/server.md index 9dcd9e8d9cdeb..68c79cebf8f26 100644 --- a/docs/cli/server.md +++ b/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 | bool | +| Environment | $CODER_PROMETHEUS_COLLECT_DB_METRICS | +| YAML | introspection.prometheus.collect_db_metrics | +| Default | false | + +Collect database metrics (may increase charges for metrics storage). + ### --prometheus-enable | | | diff --git a/enterprise/cli/testdata/coder_server_--help.golden b/enterprise/cli/testdata/coder_server_--help.golden index f8f45b138d7fd..340f167855d88 100644 --- a/enterprise/cli/testdata/coder_server_--help.golden +++ b/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. diff --git a/scaletest/terraform/coder.tf b/scaletest/terraform/coder.tf index 29e9ffef9fce7..fe7e84374042c 100644 --- a/scaletest/terraform/coder.tf +++ b/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: diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 68304bd2a3eb5..84171129fe2ba 100644 --- a/site/src/api/typesGenerated.ts +++ b/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