diff --git a/cobra.go b/cobra.go index a87c733..857ce9b 100644 --- a/cobra.go +++ b/cobra.go @@ -114,13 +114,13 @@ func NewCobraOpts(cmd *cobra.Command, opts *TemporalOpts) *TemporalOpts { viper.SetDefault("health_listen_address", "0.0.0.0:3000") cmd.Flags().StringVar( &opts.HealthListenAddress, "health-listen-address", - viper.GetString("health_listen_address"), "Address of health server", + viper.GetString("health_listen_address"), "Address of health server. If the port is 0, a random available port is assigned.", ) viper.SetDefault("metrics_listen_address", "0.0.0.0:9090") cmd.Flags().StringVar( &opts.MetricsListenAddress, "metrics-listen-address", - viper.GetString("metrics_listen_address"), "Address of Prometheus metrics server", + viper.GetString("metrics_listen_address"), "Address of Prometheus metrics server. If the port is 0, a random available port is assigned.", ) cmd.Flags().StringVar( diff --git a/health.go b/health.go index bf901c8..cf51454 100644 --- a/health.go +++ b/health.go @@ -229,16 +229,16 @@ func NewHealthCheck(ctx context.Context, taskQueues []string, address string, c Handler: mux, } - l := log.With(). - Str("address", address). - Int("taskQueueCount", len(taskQueues)). - Logger() - listener, err := (&net.ListenConfig{}).Listen(ctx, "tcp", address) if err != nil { return fmt.Errorf("error listening on %s: %w", address, err) } + l := log.With(). + Str("address", listener.Addr().String()). + Int("taskQueueCount", len(taskQueues)). + Logger() + go func() { <-ctx.Done() diff --git a/health_test.go b/health_test.go index f295faa..cf3ec0a 100644 --- a/health_test.go +++ b/health_test.go @@ -683,6 +683,32 @@ func TestNewHealthCheck(t *testing.T) { assert.JSONEq(t, `{"healthy":true}`, body, "the fake client reports a healthy Temporal") }) + t.Run("port 0 serves on an assigned port and logs it", func(t *testing.T) { + logs := captureLogs(t) + + ctx, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + + require.NoError(t, NewHealthCheck(ctx, []string{testTaskQueue}, "127.0.0.1:0", newFakeTemporalClient())) + + // NewHealthCheck does not return the address, so the log is the only + // way to find the port the OS assigned. + entry := logs.waitForMessage(t, "Starting healthcheck service") + address := requireAssignedAddress(t, entry, "127.0.0.1") + assert.InDelta(t, 1, entry["taskQueueCount"], 0) + + httpClient := newTestHTTPClient(t) + + status, body, err := getLiveness(t, httpClient, address) + require.NoError(t, err, "the healthcheck serves on the logged address") + assert.Equal(t, http.StatusOK, status) + assert.JSONEq(t, `{"healthy":true}`, body) + + cancel() + + requireNotServing(t, httpClient, address) + }) + t.Run("shuts down when the context is cancelled", func(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() diff --git a/helpers_test.go b/helpers_test.go index 73f37e7..e90faea 100644 --- a/helpers_test.go +++ b/helpers_test.go @@ -23,14 +23,20 @@ import ( "crypto/tls" "crypto/x509" "crypto/x509/pkix" + "encoding/json" "encoding/pem" "math/big" + "net" "os" "path/filepath" "reflect" + "strconv" + "sync" "testing" "time" + "github.com/rs/zerolog" + "github.com/rs/zerolog/log" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.temporal.io/sdk/client" @@ -154,3 +160,104 @@ func writeFile(t *testing.T, name, content string) string { return path } + +// logWaitTimeout bounds how long a test waits for a log entry written from +// another goroutine. It only exists so a broken implementation fails rather than +// hangs. +const logWaitTimeout = 5 * time.Second + +// logCapture records the JSON entries written by a zerolog logger. It is mutex +// guarded because servers log from their own goroutines. +type logCapture struct { + mu sync.Mutex + entries []map[string]any + // written is signalled after every entry, so waiters never poll. + written chan struct{} +} + +func (c *logCapture) Write(p []byte) (int, error) { + entry := map[string]any{} + if err := json.Unmarshal(p, &entry); err != nil { + return 0, err + } + + c.mu.Lock() + c.entries = append(c.entries, entry) + c.mu.Unlock() + + select { + case c.written <- struct{}{}: + default: + } + + return len(p), nil +} + +func (c *logCapture) find(message string) (map[string]any, bool) { + c.mu.Lock() + defer c.mu.Unlock() + + for _, entry := range c.entries { + if entry[zerolog.MessageFieldName] == message { + return entry, true + } + } + + return nil, false +} + +// waitForMessage returns the first entry with the given message, waiting a +// bounded time for it to be written. +func (c *logCapture) waitForMessage(t *testing.T, message string) map[string]any { + t.Helper() + + timeout := time.NewTimer(logWaitTimeout) + defer timeout.Stop() + + for { + if entry, ok := c.find(message); ok { + return entry + } + + select { + case <-c.written: + case <-timeout.C: + t.Fatalf("no %q log entry after %s", message, logWaitTimeout) + } + } +} + +// captureLogs swaps the global zerolog logger for one that records its entries +// and restores the original when the test finishes. Loggers derived from the +// global one before the swap is undone keep writing to the capture, so entries +// logged from a goroutine are still recorded. Callers must not be parallel. +func captureLogs(t *testing.T) *logCapture { + t.Helper() + + original := log.Logger + t.Cleanup(func() { log.Logger = original }) + + c := &logCapture{written: make(chan struct{}, 1)} + log.Logger = zerolog.New(c) + + return c +} + +// requireAssignedAddress asserts that a logged address is on the expected host +// with a real, OS-assigned port rather than 0, and returns it. +func requireAssignedAddress(t *testing.T, entry map[string]any, expectedHost string) string { + t.Helper() + + address, ok := entry["address"].(string) + require.True(t, ok, "the log entry has an address: %v", entry) + + host, port, err := net.SplitHostPort(address) + require.NoError(t, err) + assert.Equal(t, expectedHost, host) + + p, err := strconv.Atoi(port) + require.NoError(t, err) + require.NotZero(t, p, "the logged port is the assigned one, not 0") + + return address +} diff --git a/prometheus.go b/prometheus.go index 1cfc81a..7fb0a02 100644 --- a/prometheus.go +++ b/prometheus.go @@ -17,8 +17,12 @@ package temporal import ( + "context" + "errors" "fmt" "io" + "net" + "net/http" "time" prom "github.com/prometheus/client_golang/prometheus" @@ -37,18 +41,39 @@ import ( // handler, and is responsible for calling [PrometheusHandler.Close]. type PrometheusHandler struct { client.MetricsHandler - closer io.Closer + closer io.Closer + server *http.Server + listener net.Listener } -// Close flushes and releases the underlying Tally scope, and reports whatever -// the scope reports. A handler with no scope to release, such as the zero -// value, closes without error. +// Close stops the owned HTTP server and listener, then flushes and releases the +// underlying Tally scope. It returns any errors encountered during cleanup. +// The zero value closes without error. An endpoint registered on Go's default +// HTTP mux remains registered. func (h *PrometheusHandler) Close() error { - if h.closer == nil { - return nil + var errs []error + + if h.server != nil { + if err := h.server.Close(); err != nil { + errs = append(errs, err) + } + } + + // Also close the listener directly in case Close was called before + // the Serve goroutine registered it with the HTTP server. + if h.listener != nil { + if err := h.listener.Close(); err != nil && !errors.Is(err, net.ErrClosed) { + errs = append(errs, err) + } + } + + if h.closer != nil { + if err := h.closer.Close(); err != nil { + errs = append(errs, err) + } } - return h.closer.Close() + return errors.Join(errs...) } // NewPrometheusHandler creates a Prometheus-backed metrics handler for a @@ -56,9 +81,11 @@ func (h *PrometheusHandler) Close() error { // [PrometheusHandler.Close] when it is no longer needed, usually with defer. // // Metrics are served on listenAddress under /metrics, with prefix prepended to -// every metric name. An empty listenAddress attaches the metrics endpoint to -// Go's default HTTP mux instead of starting a server of its own. A nil registry -// uses the Prometheus default registry. Timers are reported as histograms. +// every metric name. A port of 0 (for example "0.0.0.0:0") lets the OS pick a +// free port, which is logged. An empty listenAddress attaches the metrics +// endpoint to Go's default HTTP mux instead of starting a server of its own. A +// nil registry uses the Prometheus default registry. Timers are reported as +// histograms. // // The optional onError argument decides how the reporter reports its own // failures: @@ -69,20 +96,14 @@ func (h *PrometheusHandler) Close() error { // behaviour applies // - more than one: an error is returned and no handler is created // -// Reporter failures are not returned from here. A listen address that cannot be -// bound, for example, is reported asynchronously through onError once the -// reporter is running. +// Listen failures are returned synchronously. Metric registration and unexpected +// HTTP serving failures are reported through onError. func NewPrometheusHandler( listenAddress, prefix string, registry *prom.Registry, onError ...func(error), ) (*PrometheusHandler, error) { - c := prometheus.Configuration{ - ListenAddress: listenAddress, - TimerType: "histogram", - } - errorHandler := func(err error) { log.Fatal().Err(err).Msg("Error in Prometheus reporter") } @@ -93,14 +114,35 @@ func NewPrometheusHandler( errorHandler = onError[0] } - reporter, err := c.NewReporter( - prometheus.ConfigurationOptions{ - Registry: registry, - OnError: errorHandler, - }, - ) - if err != nil { - return nil, fmt.Errorf("error creating reporter: %w", err) + reporterOpts := prometheus.Options{ + DefaultTimerType: prometheus.HistogramTimerType, + OnRegisterError: errorHandler, + } + if registry != nil { + reporterOpts.Registerer = registry + reporterOpts.Gatherer = registry + } + + reporter := prometheus.NewReporter(reporterOpts) + + var listener net.Listener + var server *http.Server + + if listenAddress == "" { + http.Handle("/metrics", reporter.HTTPHandler()) + } else { + var err error + listener, err = (&net.ListenConfig{}).Listen(context.Background(), "tcp", listenAddress) + if err != nil { + return nil, fmt.Errorf("error listening for Prometheus: %w", err) + } + + mux := http.NewServeMux() + mux.Handle("/metrics", reporter.HTTPHandler()) + server = &http.Server{ + Handler: mux, + ReadHeaderTimeout: 5 * time.Second, + } } scopeOpts := tally.ScopeOptions{ @@ -112,9 +154,25 @@ func NewPrometheusHandler( scope, closer := tally.NewRootScope(scopeOpts, time.Second) scope = sdktally.NewPrometheusNamingScope(scope) - log.Info().Str("address", listenAddress).Msg("Starting Prometheus service") - return &PrometheusHandler{ + handler := &PrometheusHandler{ MetricsHandler: sdktally.NewMetricsHandler(scope), closer: closer, - }, nil + server: server, + listener: listener, + } + + if server != nil { + log.Info().Str("address", listener.Addr().String()).Msg("Starting Prometheus service") + + go func() { + if err := server.Serve(listener); err != nil && !errors.Is(err, http.ErrServerClosed) { + if errorHandler == nil { + panic(err) + } + errorHandler(err) + } + }() + } + + return handler, nil } diff --git a/prometheus_test.go b/prometheus_test.go index b5468c4..1470988 100644 --- a/prometheus_test.go +++ b/prometheus_test.go @@ -21,11 +21,13 @@ import ( "context" "fmt" "io" + "net" "net/http" "net/http/httptest" "os" "os/exec" "sync" + "syscall" "testing" "time" @@ -43,11 +45,9 @@ const freeListenAddress = "127.0.0.1:0" // selects which failure that child triggers. const promFatalEnv = "TEMPORAL_HELPERS_TEST_PROMETHEUS_FATAL" -// The failures the fatal child process can trigger. -const ( - fatalModeListen = "listen" - fatalModeRegister = "register" -) +// fatalModeRegister selects the registration failure in the fatal child +// process. +const fatalModeRegister = "register" // Names used by the error handler tests. The metric is registered up front by // the test so that tally's own registration of the same name fails. @@ -60,9 +60,99 @@ const ( // more than one error handler. const errorHandlerMessage = "only a single error handler may be supplied" -// metricsPath is the path tally registers its handler on. +// metricsPath is the path the metrics handler is served on. const metricsPath = "/metrics" +// prometheusStartMessage is logged by NewPrometheusHandler when it starts a +// server of its own. +const prometheusStartMessage = "Starting Prometheus service" + +// newTestPrometheusHandler builds a handler, failing the test if that does not +// work, and closes it when the test finishes. Tests that assert on Close call it +// themselves; the cleanup is only a backstop. +func newTestPrometheusHandler( + t *testing.T, + listenAddress, prefix string, + registry *prom.Registry, + onError ...func(error), +) *PrometheusHandler { + t.Helper() + + handler, err := NewPrometheusHandler(listenAddress, prefix, registry, onError...) + require.NoError(t, err) + require.NotNil(t, handler) + + t.Cleanup(func() { _ = handler.Close() }) + + return handler +} + +// startExplicitPrometheusHandler starts a handler on an explicit loopback port, +// moving to a different port if the one it picked was claimed in the meantime. +// An explicit port has to be known before the handler binds it, so this follows +// the same pick-and-retry approach as startHealthCheck. Any error other than +// "address already in use" fails the test immediately. +func startExplicitPrometheusHandler( + t *testing.T, + prefix string, + registry *prom.Registry, + onError func(error), +) (handler *PrometheusHandler, address string) { + t.Helper() + + for range maxAddressAttempts { + address = freeLoopbackAddress(t) + + handler, err := NewPrometheusHandler(address, prefix, registry, onError) + if err == nil { + t.Cleanup(func() { _ = handler.Close() }) + + return handler, address + } + + require.ErrorIs(t, err, syscall.EADDRINUSE, "unexpected error starting the Prometheus handler") + } + + t.Fatalf("no free loopback port found after %d attempts", maxAddressAttempts) + + return nil, "" +} + +// getMetrics fetches /metrics from address and returns the body. The listener +// is bound before NewPrometheusHandler returns, so the request is accepted even +// if the server has not been scheduled yet and no retry is needed. +func getMetrics(t *testing.T, address string) string { + t.Helper() + + ctx, cancel := context.WithTimeout(context.Background(), requestTimeout) + defer cancel() + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, "http://"+address+metricsPath, http.NoBody) + require.NoError(t, err) + + resp, err := newTestHTTPClient(t).Do(req) + require.NoError(t, err, "%s is serving metrics", address) + + defer func() { _ = resp.Body.Close() }() + + raw, err := io.ReadAll(resp.Body) + require.NoError(t, err) + require.Equal(t, http.StatusOK, resp.StatusCode) + + return string(raw) +} + +// startedAddress returns the address a handler logged as listening on. +func startedAddress(t *testing.T, logs *logCapture) string { + t.Helper() + + // The entry is written before NewPrometheusHandler returns. + entry, ok := logs.find(prometheusStartMessage) + require.True(t, ok, "the start message is logged") + + return requireAssignedAddress(t, entry, "127.0.0.1") +} + // collidingCounter builds a counter registered under the name tally derives for // prefix and name - "__total" - but with a different help string, // so registering both in one registry is a guaranteed error. @@ -164,10 +254,7 @@ func TestNewPrometheusHandler(t *testing.T) { registry := prom.NewRegistry() - handler, err := NewPrometheusHandler(freeListenAddress, "myprefix", registry) - - require.NoError(t, err) - require.NotNil(t, handler) + handler := newTestPrometheusHandler(t, freeListenAddress, "myprefix", registry) // Creating a metric registers it with the registry immediately. handler.Counter("things_done").Inc(1) @@ -180,9 +267,7 @@ func TestNewPrometheusHandler(t *testing.T) { registry := prom.NewRegistry() - handler, err := NewPrometheusHandler(freeListenAddress, "", registry) - - require.NoError(t, err) + handler := newTestPrometheusHandler(t, freeListenAddress, "", registry) handler.Counter("unprefixed_things").Inc(1) @@ -194,9 +279,7 @@ func TestNewPrometheusHandler(t *testing.T) { registry := prom.NewRegistry() - handler, err := NewPrometheusHandler(freeListenAddress, "my.prefix", registry) - - require.NoError(t, err) + handler := newTestPrometheusHandler(t, freeListenAddress, "my.prefix", registry) handler.Counter("things.done").Inc(1) handler.Gauge("things.pending").Update(1) @@ -212,9 +295,7 @@ func TestNewPrometheusHandler(t *testing.T) { registry := prom.NewRegistry() - handler, err := NewPrometheusHandler(freeListenAddress, "tagged", registry) - - require.NoError(t, err) + handler := newTestPrometheusHandler(t, freeListenAddress, "tagged", registry) handler.WithTags(map[string]string{"task.queue": "my-queue"}).Counter("tagged_things").Inc(1) @@ -238,6 +319,102 @@ func TestNewPrometheusHandler(t *testing.T) { // Label names and values are both sanitised by the configured options. assert.Equal(t, []string{"task_queue=my_queue"}, labels) }) + + t.Run("rejects a listen address without a port", func(t *testing.T) { + t.Parallel() + + handler, err := NewPrometheusHandler("not-a-valid-address", "noport", prom.NewRegistry()) + + require.Error(t, err) + assert.ErrorContains(t, err, "error listening for Prometheus") + assert.Nil(t, handler) + }) + + t.Run("returns a bind failure synchronously", func(t *testing.T) { + t.Parallel() + + // The test holds the port for its whole duration, so the bind is + // guaranteed to fail. + occupied, err := (&net.ListenConfig{}).Listen(t.Context(), "tcp", freeListenAddress) + require.NoError(t, err) + t.Cleanup(func() { _ = occupied.Close() }) + + recorder := &errorRecorder{} + + handler, err := NewPrometheusHandler(occupied.Addr().String(), "occupied", prom.NewRegistry(), recorder.onError) + + require.Error(t, err) + assert.ErrorContains(t, err, "error listening for Prometheus") + assert.ErrorIs(t, err, syscall.EADDRINUSE) + assert.Nil(t, handler) + assert.Empty(t, recorder.errors(), "a bind failure is returned, not reported") + }) + + t.Run("serves on an explicit port until closed", func(t *testing.T) { + t.Parallel() + + recorder := &errorRecorder{} + + handler, address := startExplicitPrometheusHandler(t, "explicitport", prom.NewRegistry(), recorder.onError) + + handler.Counter("things_done").Inc(1) + + assert.Contains(t, getMetrics(t, address), "explicitport_things_done_total") + + require.NoError(t, handler.Close()) + + requireNotServing(t, newTestHTTPClient(t), address) + assert.Empty(t, recorder.errors(), "an expected shutdown is not reported as an error") + }) +} + +// TestPrometheusHandlerZeroValueClose covers Close on a handler that owns +// nothing. +func TestPrometheusHandlerZeroValueClose(t *testing.T) { + t.Parallel() + + var handler PrometheusHandler + + assert.NoError(t, handler.Close()) +} + +// TestNewPrometheusHandlerAssignedPort covers port 0, where the log is the only +// way to learn the address. It is not parallel because it captures the global +// logger. +func TestNewPrometheusHandlerAssignedPort(t *testing.T) { + t.Run("serves on the logged address until closed", func(t *testing.T) { + logs := captureLogs(t) + recorder := &errorRecorder{} + + handler := newTestPrometheusHandler(t, freeListenAddress, "assignedport", prom.NewRegistry(), recorder.onError) + address := startedAddress(t, logs) + + handler.Counter("things_done").Inc(1) + + assert.Contains(t, getMetrics(t, address), "assignedport_things_done_total", "metrics are served on the logged address") + + require.NoError(t, handler.Close()) + + // The Serve goroutine cannot be observed directly. Waiting until the + // address stops answering means it has seen the closed listener, and + // a regression that reported the expected shutdown would do so then. + requireNotServing(t, newTestHTTPClient(t), address) + assert.Empty(t, recorder.errors(), "an expected shutdown is not reported as an error") + }) + + t.Run("closing straight after construction stops the server", func(t *testing.T) { + logs := captureLogs(t) + recorder := &errorRecorder{} + + handler := newTestPrometheusHandler(t, freeListenAddress, "closedearly", prom.NewRegistry(), recorder.onError) + address := startedAddress(t, logs) + + // Close may run before the Serve goroutine has been scheduled. + require.NoError(t, handler.Close()) + + requireNotServing(t, newTestHTTPClient(t), address) + assert.Empty(t, recorder.errors(), "an expected shutdown is not reported as an error") + }) } // TestNewPrometheusHandlerNilRegistry covers the branch where no registry is @@ -247,10 +424,9 @@ func TestNewPrometheusHandler(t *testing.T) { // test or leak into anything else that uses the real default registry. func TestNewPrometheusHandlerNilRegistry(t *testing.T) { registry := isolateDefaultRegistry(t) + logs := captureLogs(t) - handler, err := NewPrometheusHandler(freeListenAddress, "defaultregistry", nil) - - require.NoError(t, err) + handler := newTestPrometheusHandler(t, freeListenAddress, "defaultregistry", nil) handler.Counter("things_done").Inc(1) @@ -264,21 +440,27 @@ func TestNewPrometheusHandlerNilRegistry(t *testing.T) { assert.Contains(t, names, "defaultregistry_things_done_total") assert.Equal(t, names, registeredMetricNames(t, registry), "the default gatherer is the registry the metric was registered with") + + body := getMetrics(t, startedAddress(t, logs)) + assert.Contains(t, body, "defaultregistry_things_done_total", "the server gathers from the default registry") } // TestNewPrometheusHandlerWithoutListenAddress covers the branch where no listen // address is given and the metrics handler is instead attached to Go's default // HTTP mux. It is not parallel and installs its own mux for the duration of the -// test, because the underlying library registers "/metrics" on whatever mux is -// installed and a mux panics if the same path is registered twice. +// test, because "/metrics" is registered on whatever mux is installed and a mux +// panics if the same path is registered twice. It also captures the global +// logger, to show that no server of its own is started. func TestNewPrometheusHandlerWithoutListenAddress(t *testing.T) { mux := isolateDefaultServeMux(t) + logs := captureLogs(t) registry := prom.NewRegistry() - handler, err := NewPrometheusHandler("", "defaultmux", registry) + handler := newTestPrometheusHandler(t, "", "defaultmux", registry) - require.NoError(t, err) + _, ok := logs.find(prometheusStartMessage) + assert.False(t, ok, "no server is started, so no listening address is logged") handler.Counter("things_done").Inc(1) @@ -299,6 +481,10 @@ func TestWithPrometheusMetrics(t *testing.T) { require.NotNil(t, o.MetricsHandler) + handler, ok := o.MetricsHandler.(*PrometheusHandler) + require.True(t, ok, "the option installs a *PrometheusHandler") + t.Cleanup(func() { _ = handler.Close() }) + o.MetricsHandler.Counter("things_done").Inc(1) assert.Contains(t, registeredMetricNames(t, registry), "clientoption_things_done_total") @@ -362,10 +548,7 @@ func TestNewPrometheusHandlerErrorHandler(t *testing.T) { registry, fqName := collidingRegistry(t, prefix) recorder := &errorRecorder{} - handler, err := NewPrometheusHandler(freeListenAddress, prefix, registry, recorder.onError) - - require.NoError(t, err) - require.NotNil(t, handler) + handler := newTestPrometheusHandler(t, freeListenAddress, prefix, registry, recorder.onError) // The supplied handler swallows the error, so allocating the metric // neither panics (tally's default) nor exits (our default). @@ -385,10 +568,7 @@ func TestNewPrometheusHandlerErrorHandler(t *testing.T) { registry, fqName := collidingRegistry(t, prefix) - handler, err := NewPrometheusHandler(freeListenAddress, prefix, registry, nil) - - require.NoError(t, err) - require.NotNil(t, handler) + handler := newTestPrometheusHandler(t, freeListenAddress, prefix, registry, nil) // tally treats a nil ConfigurationOptions.OnError as "use my default", // and its default panics with the registration error. Recovering here @@ -412,12 +592,11 @@ func TestNewPrometheusHandlerErrorHandler(t *testing.T) { } // TestNewPrometheusHandlerDefaultErrorHandler documents that with no onError -// argument the default handler is still log.Fatal, for both of the failures -// tally reports: a listen address it cannot bind, which it reports -// asynchronously from the reporter's own goroutine, and a metric registration it -// cannot make, which it reports synchronously. Neither produces an error from -// NewPrometheusHandler, and log.Fatal terminates the process, so both have to be -// observed from a child process. +// argument the default handler is still log.Fatal for a metric registration +// that cannot be made. That failure produces no error from NewPrometheusHandler, +// and log.Fatal terminates the process, so it has to be observed from a child +// process. Listen failures are returned synchronously and are covered by +// TestNewPrometheusHandler. func TestNewPrometheusHandlerDefaultErrorHandler(t *testing.T) { if mode := os.Getenv(promFatalEnv); mode != "" { runDefaultErrorHandlerChild(mode) @@ -431,11 +610,6 @@ func TestNewPrometheusHandlerDefaultErrorHandler(t *testing.T) { // expected fragments of the child's stderr expected []string }{ - { - name: "listen failure", - mode: fatalModeListen, - expected: []string{"Error in Prometheus reporter", "not-a-valid-address"}, - }, { name: "registration failure", mode: fatalModeRegister, @@ -454,19 +628,10 @@ func TestNewPrometheusHandlerDefaultErrorHandler(t *testing.T) { cmd := exec.CommandContext(ctx, os.Args[0], "-test.run=^TestNewPrometheusHandlerDefaultErrorHandler$") cmd.Env = append(os.Environ(), promFatalEnv+"="+test.mode) - // The child may block on stdin until it is killed, so it never - // trips the Go deadlock detector and the test needs no sleeps. - stdin, err := cmd.StdinPipe() - require.NoError(t, err) - - defer func() { - _ = stdin.Close() - }() - var stderr bytes.Buffer cmd.Stderr = &stderr - err = cmd.Run() + err := cmd.Run() var exitErr *exec.ExitError require.ErrorAs(t, err, &exitErr, "stderr: %s", stderr.String()) @@ -485,11 +650,6 @@ func TestNewPrometheusHandlerDefaultErrorHandler(t *testing.T) { // the one under test, and it expects never to return. func runDefaultErrorHandlerChild(mode string) { switch mode { - case fatalModeListen: - mustChildHandler("not-a-valid-address", "child", prom.NewRegistry()) - - // Block until the reporter's goroutine terminates the process. - _, _ = io.Copy(io.Discard, os.Stdin) case fatalModeRegister: const prefix = "child"