From 30d9d8475810a904541a6276865951e9ccb84136 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Wed, 28 Feb 2024 17:58:03 +0100 Subject: [PATCH] fix: use flag to enable Prometheus (#12345) --- cli/clitest/clitest.go | 10 ++ docs/admin/provisioners.md | 14 +++ docs/admin/workspace-proxies.md | 11 +++ docs/cli/provisionerd_start.md | 10 ++ enterprise/cli/provisionerdaemons.go | 10 +- enterprise/cli/provisionerdaemons_test.go | 21 +++-- enterprise/cli/proxyserver_test.go | 92 +++++++++++++++++++ .../coder_provisionerd_start_--help.golden | 3 + 8 files changed, 160 insertions(+), 11 deletions(-) diff --git a/cli/clitest/clitest.go b/cli/clitest/clitest.go index 451757debf..93131a4dad 100644 --- a/cli/clitest/clitest.go +++ b/cli/clitest/clitest.go @@ -141,6 +141,10 @@ func extractTar(t *testing.T, data []byte, directory string) { // Start runs the command in a goroutine and cleans it up when the test // completed. func Start(t *testing.T, inv *clibase.Invocation) { + StartWithAssert(t, inv, nil) +} + +func StartWithAssert(t *testing.T, inv *clibase.Invocation, assertCallback func(t *testing.T, err error)) { //nolint:revive t.Helper() closeCh := make(chan struct{}) @@ -155,6 +159,12 @@ func Start(t *testing.T, inv *clibase.Invocation) { go func() { defer close(closeCh) err := waiter.Wait() + + if assertCallback != nil { + assertCallback(t, err) + return + } + switch { case errors.Is(err, context.Canceled): return diff --git a/docs/admin/provisioners.md b/docs/admin/provisioners.md index 2e1a0e16b5..e20b86f5a0 100644 --- a/docs/admin/provisioners.md +++ b/docs/admin/provisioners.md @@ -229,3 +229,17 @@ This can be disabled with a server-wide ```shell coder server --provisioner-daemons=0 ``` + +## Prometheus metrics + +Coder provisioner daemon exports metrics via the HTTP endpoint, which can be +enabled using either the environment variable `CODER_PROMETHEUS_ENABLE` or the +flag `--prometheus-enable`. + +The Prometheus endpoint address is `http://localhost:2112/` by default. You can +use either the environment variable `CODER_PROMETHEUS_ADDRESS` or the flag +`--prometheus-address :` to select a different listen +address. + +If you have provisioners daemons deployed as pods, it is advised to monitor them +separately. diff --git a/docs/admin/workspace-proxies.md b/docs/admin/workspace-proxies.md index 972475a3a7..eb7ac7754f 100644 --- a/docs/admin/workspace-proxies.md +++ b/docs/admin/workspace-proxies.md @@ -184,3 +184,14 @@ goes offline, the session will fall back to the primary proxy. This could take up to 60 seconds. ![Workspace proxy picker](../images/admin/workspace-proxy-picker.png) + +## Step 3: Observability + +Coder workspace proxy exports metrics via the HTTP endpoint, which can be +enabled using either the environment variable `CODER_PROMETHEUS_ENABLE` or the +flag `--prometheus-enable`. + +The Prometheus endpoint address is `http://localhost:2112/` by default. You can +use either the environment variable `CODER_PROMETHEUS_ADDRESS` or the flag +`--prometheus-address :` to select a different listen +address. diff --git a/docs/cli/provisionerd_start.md b/docs/cli/provisionerd_start.md index 4decf76915..f7a13102c3 100644 --- a/docs/cli/provisionerd_start.md +++ b/docs/cli/provisionerd_start.md @@ -98,6 +98,16 @@ Deprecated and ignored. The bind address to serve prometheus metrics. +### --prometheus-enable + +| | | +| ----------- | ------------------------------------- | +| Type | bool | +| Environment | $CODER_PROMETHEUS_ENABLE | +| Default | false | + +Serve prometheus metrics on the address defined by prometheus address. + ### --psk | | | diff --git a/enterprise/cli/provisionerdaemons.go b/enterprise/cli/provisionerdaemons.go index 37b9a629ea..5943758a7d 100644 --- a/enterprise/cli/provisionerdaemons.go +++ b/enterprise/cli/provisionerdaemons.go @@ -71,6 +71,7 @@ func (r *RootCmd) provisionerDaemonStart() *clibase.Cmd { preSharedKey string verbose bool + prometheusEnable bool prometheusAddress string ) client := new(codersdk.Client) @@ -171,7 +172,7 @@ func (r *RootCmd) provisionerDaemonStart() *clibase.Cmd { }() var metrics *provisionerd.Metrics - if prometheusAddress != "" { + if prometheusEnable { logger.Info(ctx, "starting Prometheus endpoint", slog.F("address", prometheusAddress)) prometheusRegistry := prometheus.NewRegistry() @@ -315,6 +316,13 @@ func (r *RootCmd) provisionerDaemonStart() *clibase.Cmd { Value: clibase.StringArrayOf(&logFilter), Default: "", }, + { + Flag: "prometheus-enable", + Env: "CODER_PROMETHEUS_ENABLE", + Description: "Serve prometheus metrics on the address defined by prometheus address.", + Value: clibase.BoolOf(&prometheusEnable), + Default: "false", + }, { Flag: "prometheus-address", Env: "CODER_PROMETHEUS_ADDRESS", diff --git a/enterprise/cli/provisionerdaemons_test.go b/enterprise/cli/provisionerdaemons_test.go index e0999a4ef8..e53fadcb4a 100644 --- a/enterprise/cli/provisionerdaemons_test.go +++ b/enterprise/cli/provisionerdaemons_test.go @@ -170,15 +170,6 @@ func TestProvisionerDaemon_SessionToken(t *testing.T) { t.Run("PrometheusEnabled", func(t *testing.T) { t.Parallel() - // Helper function to find a free random port - 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 - } prometheusPort := randomPort(t) // Configure CLI client @@ -191,7 +182,7 @@ func TestProvisionerDaemon_SessionToken(t *testing.T) { }, }) anotherClient, _ := coderdtest.CreateAnotherUser(t, client, admin.OrganizationID, rbac.RoleTemplateAdmin()) - inv, conf := newCLI(t, "provisionerd", "start", "--name", "daemon-with-prometheus", "--prometheus-address", fmt.Sprintf("127.0.0.1:%d", prometheusPort)) + inv, conf := newCLI(t, "provisionerd", "start", "--name", "daemon-with-prometheus", "--prometheus-enable", "--prometheus-address", fmt.Sprintf("127.0.0.1:%d", prometheusPort)) clitest.SetupConfig(t, anotherClient, conf) pty := ptytest.New(t).Attach(inv) ctx, cancel := context.WithTimeout(inv.Context(), testutil.WaitLong) @@ -251,3 +242,13 @@ func TestProvisionerDaemon_SessionToken(t *testing.T) { require.True(t, hasPromHTTP, "Prometheus HTTP metrics are missing") }) } + +// randomPort is a helper function to find a free random port, for instance to spawn Prometheus endpoint. +func randomPort(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 +} diff --git a/enterprise/cli/proxyserver_test.go b/enterprise/cli/proxyserver_test.go index 72f576008f..05e1aeb0fa 100644 --- a/enterprise/cli/proxyserver_test.go +++ b/enterprise/cli/proxyserver_test.go @@ -1,16 +1,23 @@ package cli_test import ( + "bufio" + "context" + "errors" "fmt" "net/http" "net/http/httptest" + "strings" + "sync" "sync/atomic" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/coder/coder/v2/cli/clitest" "github.com/coder/coder/v2/pty/ptytest" + "github.com/coder/coder/v2/testutil" ) func Test_Headers(t *testing.T) { @@ -52,3 +59,88 @@ func Test_Headers(t *testing.T) { assert.EqualValues(t, 1, atomic.LoadInt64(&called)) } + +func TestWorkspaceProxy_Server_PrometheusEnabled(t *testing.T) { + t.Parallel() + + prometheusPort := randomPort(t) + + var wg sync.WaitGroup + wg.Add(1) + + // Start fake coderd + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/api/v2/workspaceproxies/me/register" { + // Give fake app_security_key (96 bytes) + w.WriteHeader(http.StatusCreated) + _, _ = w.Write([]byte(`{"app_security_key": "012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789123456012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789123456"}`)) + return + } + if r.URL.Path == "/api/v2/workspaceproxies/me/coordinate" { + // Slow down proxy registration, so that test runner can check if Prometheus endpoint is exposed. + wg.Wait() + + // Does not matter, we are not going to implement a real workspace proxy. + w.WriteHeader(http.StatusNotImplemented) + return + } + + w.Header().Add("Content-Type", "application/json") + _, _ = w.Write([]byte(`{}`)) // build info can be ignored + })) + defer srv.Close() + defer wg.Done() + + // Configure CLI client + inv, _ := newCLI(t, "wsproxy", "server", + "--primary-access-url", srv.URL, + "--proxy-session-token", "test-token", + "--access-url", "http://foobar:3001", + "--http-address", fmt.Sprintf("127.0.0.1:%d", randomPort(t)), + "--prometheus-enable", + "--prometheus-address", fmt.Sprintf("127.0.0.1:%d", prometheusPort), + ) + pty := ptytest.New(t).Attach(inv) + + ctx, cancel := context.WithTimeout(inv.Context(), testutil.WaitLong) + defer cancel() + + // Start "wsproxy server" command + clitest.StartWithAssert(t, inv, func(t *testing.T, err error) { + assert.Error(t, err) + assert.False(t, errors.Is(err, context.Canceled), "error was expected, but context was canceled") + }) + pty.ExpectMatchContext(ctx, "Started HTTP listener at") + + // Fetch metrics from Prometheus endpoint + var res *http.Response + require.Eventually(t, func() bool { + req, err := http.NewRequestWithContext(ctx, "GET", fmt.Sprintf("http://127.0.0.1:%d", prometheusPort), nil) + assert.NoError(t, err) + // nolint:bodyclose + res, err = http.DefaultClient.Do(req) + return err == nil + }, testutil.WaitShort, testutil.IntervalFast) + defer res.Body.Close() + + // Scan for metric patterns + scanner := bufio.NewScanner(res.Body) + hasGoStats := false + hasPromHTTP := false + for scanner.Scan() { + if strings.HasPrefix(scanner.Text(), "go_goroutines") { + hasGoStats = true + continue + } + if strings.HasPrefix(scanner.Text(), "promhttp_metric_handler_requests_total") { + hasPromHTTP = true + continue + } + t.Logf("scanned %s", scanner.Text()) + } + require.NoError(t, scanner.Err()) + + // Verify patterns + require.True(t, hasGoStats, "Go stats are missing") + require.True(t, hasPromHTTP, "Prometheus HTTP metrics are missing") +} diff --git a/enterprise/cli/testdata/coder_provisionerd_start_--help.golden b/enterprise/cli/testdata/coder_provisionerd_start_--help.golden index fcf2e5ef2f..90694af40f 100644 --- a/enterprise/cli/testdata/coder_provisionerd_start_--help.golden +++ b/enterprise/cli/testdata/coder_provisionerd_start_--help.golden @@ -35,6 +35,9 @@ OPTIONS: --prometheus-address string, $CODER_PROMETHEUS_ADDRESS (default: 127.0.0.1:2112) The bind address to serve prometheus metrics. + --prometheus-enable bool, $CODER_PROMETHEUS_ENABLE (default: false) + Serve prometheus metrics on the address defined by prometheus address. + --psk string, $CODER_PROVISIONER_DAEMON_PSK Pre-shared key to authenticate with Coder server.