feat: add metricsServer for health checks, metrics and pprof (#5578)

Add a separate HTTP listener for observability, configured through metricsServer in both frps and frpc. It serves /healthz and, when pprofEnable is set, the pprof handlers except /debug/pprof/cmdline. It never serves the dashboard, static assets or the admin API, and it does not touch the global assets filesystem. frps also serves /metrics on this listener with both runtime and business metrics, enabled by metricsServer.port alone. The in-memory collector still starts only with the dashboard, and the Prometheus collector starts either with metricsServer or with enablePrometheus plus webServer, so a configuration that collected nothing before still collects nothing. The /healthz, /metrics and pprof endpoints on webServer, and enablePrometheus, keep working but are deprecated and log a hint at startup. frpc does not serve /metrics yet.
2026-10-19 20:08:39 +08:00
parent 00e4fc4dd3
commit f13fdd58d4
20 changed files with 858 additions and 27 deletions
@@ -0,0 +1,108 @@
+// Copyright 2026 The frp Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package server
+
+import (
+	"context"
+	"fmt"
+	"io"
+	"net"
+	"net/http"
+	"path/filepath"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+
+	v1 "github.com/fatedier/frp/pkg/config/v1"
+)
+
+func TestServiceMetricsServerWithoutWebServer(t *testing.T) {
+	cfg := &v1.ServerConfig{
+		BindAddr: "127.0.0.1", ProxyBindAddr: "127.0.0.1",
+		MetricsServer: v1.MetricsServerConfig{Port: freeLifecycleTCPPort(t)},
+	}
+	require.NoError(t, cfg.Complete())
+	cfg.BindPort = 0
+	svr, err := NewService(cfg)
+	require.NoError(t, err)
+	require.Nil(t, svr.webServer)
+	address := svr.metricsServer.Address()
+	done := make(chan struct{})
+	go func() { svr.Run(context.Background()); close(done) }()
+	defer func() {
+		require.NoError(t, svr.Close())
+		waitForSignal(t, done, "metrics service shutdown")
+		assertLifecyclePorts(t, []string{address}, true)
+	}()
+
+	client := &http.Client{Timeout: 3 * time.Second}
+	resp, err := client.Get("http://" + address + "/healthz")
+	require.NoError(t, err)
+	defer resp.Body.Close()
+	require.Equal(t, http.StatusOK, resp.StatusCode)
+	body, err := io.ReadAll(resp.Body)
+	require.NoError(t, err)
+	require.Empty(t, body)
+}
+
+func TestNewServiceReleasesResourcesOnMetricsServerFailure(t *testing.T) {
+	for _, mode := range []string{"port conflict", "invalid TLS certificate"} {
+		t.Run(mode, func(t *testing.T) {
+			cfg := &v1.ServerConfig{BindAddr: "127.0.0.1", ProxyBindAddr: "127.0.0.1"}
+			require.NoError(t, cfg.Complete())
+			// Reserve distinct ports together, including the second HTTP listener.
+			ports := []*int{&cfg.BindPort, &cfg.VhostHTTPPort, &cfg.WebServer.Port, &cfg.MetricsServer.Port}
+			reservations := make([]net.Listener, 0, len(ports))
+			addresses := make([]string, 0, len(ports))
+			for _, port := range ports {
+				ln, err := net.Listen("tcp", "127.0.0.1:0")
+				require.NoError(t, err)
+				t.Cleanup(func() { _ = ln.Close() })
+				*port = ln.Addr().(*net.TCPAddr).Port
+				reservations = append(reservations, ln)
+				addresses = append(addresses, ln.Addr().String())
+			}
+			for _, ln := range reservations[:len(reservations)-1] {
+				require.NoError(t, ln.Close())
+			}
+			if mode == "invalid TLS certificate" {
+				require.NoError(t, reservations[len(reservations)-1].Close())
+				cfg.MetricsServer.TLS = &v1.TLSConfig{
+					CertFile: filepath.Join(t.TempDir(), "missing.crt"), KeyFile: "missing.key",
+				}
+			} else {
+				// This port belongs to the test, not to the failed service.
+				addresses = addresses[:len(addresses)-1]
+			}
+			workersBefore := countPortManagerWorkers(t)
+			for range 5 {
+				svr, err := NewService(cfg)
+				if svr != nil {
+					_ = svr.Close()
+				}
+				require.Error(t, err)
+				require.Nil(t, svr)
+				if mode == "port conflict" {
+					require.ErrorContains(t, err, fmt.Sprint(cfg.MetricsServer.Port))
+				} else {
+					require.ErrorContains(t, err, "missing.crt")
+				}
+				assertLifecyclePorts(t, addresses, true)
+			}
+			require.Equal(t, workersBefore, countPortManagerWorkers(t), "failed construction leaked port-manager workers")
+		})
+	}
+}
@@ -31,6 +31,7 @@ import (
 	"github.com/fatedier/golib/crypto"
 	"github.com/fatedier/golib/net/mux"
 	fmux "github.com/fatedier/yamux"
+	"github.com/prometheus/client_golang/prometheus/promhttp"
 	quic "github.com/quic-go/quic-go"
 	"github.com/samber/lo"

@@ -120,7 +121,8 @@ type Service struct {
 	rc *controller.ResourceController

 	// web server for dashboard UI and apis
-	webServer *httppkg.Server
+	webServer     *httppkg.Server
+	metricsServer *httppkg.Server
 	// HTTP vhost resources are started and owned by NewService.
 	httpVhostServer   *http.Server
 	httpVhostListener net.Listener
@@ -347,8 +349,8 @@ func NewService(cfg *v1.ServerConfig) (*Service, error) {
 	}
 	svr.rc.NatHoleController = nc

-	// Create the web server after all fallible steps so its listener is not
-	// leaked when an earlier error causes NewService to return.
+	// Create HTTP servers after all other fallible steps, rolling back listeners
+	// if either server cannot be created.
 	if cfg.WebServer.Port > 0 {
 		ws, err := httppkg.NewServer(cfg.WebServer)
 		if err != nil {
@@ -357,11 +359,24 @@ func NewService(cfg *v1.ServerConfig) (*Service, error) {
 			return nil, err
 		}
 		svr.webServer = ws
-
-		modelmetrics.EnableMem()
-		if cfg.EnablePrometheus {
-			modelmetrics.EnablePrometheus()
+	}
+	if cfg.MetricsServer.Port > 0 {
+		ms, err := httppkg.NewMetricsServer(cfg.MetricsServer, promhttp.Handler())
+		if err != nil {
+			_ = svr.Close()
+			return nil, err
 		}
+		svr.metricsServer = ms
+	}
+
+	// Collectors are enabled at most once. The in-memory collector is only read
+	// by the dashboard API, and Prometheus metrics need an endpoint to serve them.
+	enableMem, enablePrometheus := metricsCollectors(cfg)
+	if enableMem {
+		modelmetrics.EnableMem()
+	}
+	if enablePrometheus {
+		modelmetrics.EnablePrometheus()
 	}

 	// Start port-manager cleanup workers only after all fallible steps have succeeded.
@@ -375,6 +390,16 @@ func NewService(cfg *v1.ServerConfig) (*Service, error) {
 	return svr, nil
 }

+// metricsCollectors reports which collectors should be started for cfg. The
+// in-memory collector only backs the dashboard API (webServer), while the
+// Prometheus collector needs a listener that serves /metrics: either the new
+// metricsServer, or the legacy webServer endpoint gated by enablePrometheus.
+func metricsCollectors(cfg *v1.ServerConfig) (mem bool, prometheus bool) {
+	mem = cfg.WebServer.Port > 0
+	prometheus = cfg.MetricsServer.Port > 0 || (cfg.EnablePrometheus && mem)
+	return mem, prometheus
+}
+
 func (svr *Service) Run(ctx context.Context) {
 	ctx, cancel := context.WithCancel(ctx)
 	svr.lifecycleMu.Lock()
@@ -396,6 +421,15 @@ func (svr *Service) Run(ctx context.Context) {

 	// run dashboard web server.
 	if svr.webServer != nil {
+		if svr.cfg.WebServer.PprofEnable {
+			log.Warnf("webServer pprof endpoints are deprecated; use metricsServer.pprofEnable instead")
+		}
+		if svr.cfg.EnablePrometheus {
+			log.Warnf("webServer /metrics is deprecated; configure metricsServer.port instead")
+		}
+		if svr.cfg.MetricsServer.Port == 0 {
+			log.Infof("webServer /healthz is deprecated; health probes can migrate to metricsServer.port")
+		}
 		go func() {
 			log.Infof("dashboard listen on %s", svr.webServer.Address())
 			if err := svr.webServer.Run(); err != nil {
@@ -404,6 +438,15 @@ func (svr *Service) Run(ctx context.Context) {
 		}()
 	}

+	if svr.metricsServer != nil {
+		go func() {
+			log.Infof("metrics server listen on %s", svr.metricsServer.Address())
+			if err := svr.metricsServer.Run(); err != nil && !errors.Is(err, http.ErrServerClosed) {
+				log.Warnf("metrics server exit with error: %v", err)
+			}
+		}()
+	}
+
 	go svr.HandleListener(svr.sshTunnelListener, true)

 	if svr.kcpListener != nil {
@@ -462,6 +505,9 @@ func (svr *Service) closeResources() {
 	if svr.webServer != nil {
 		svr.webServer.Close()
 	}
+	if svr.metricsServer != nil {
+		svr.metricsServer.Close()
+	}
 	if svr.httpVhostServer != nil {
 		svr.httpVhostServer.Close()
 		// Close even if Serve has not registered its listener yet.
@@ -276,6 +276,50 @@ func TestServiceRunAfterEarlyShutdown(t *testing.T) {
 	}
 }

+func TestMetricsCollectors(t *testing.T) {
+	for _, tc := range []struct {
+		name                    string
+		webPort, metricsPort    int
+		enablePrometheus        bool
+		wantMem, wantPrometheus bool
+	}{
+		{name: "nothing enabled"},
+		{name: "web server only", webPort: 7500, wantMem: true},
+		{
+			name: "web server with enablePrometheus", webPort: 7500, enablePrometheus: true,
+			wantMem: true, wantPrometheus: true,
+		},
+		{
+			// enablePrometheus alone has no endpoint to serve metrics from, so the
+			// Prometheus collector must stay off, as it did before metricsServer.
+			name: "enablePrometheus without listeners", enablePrometheus: true,
+		},
+		{
+			// The in-memory collector only backs the dashboard API.
+			name: "metrics server only", metricsPort: 7501, wantPrometheus: true,
+		},
+		{
+			name: "both servers", webPort: 7500, metricsPort: 7501,
+			wantMem: true, wantPrometheus: true,
+		},
+		{
+			name: "both servers with enablePrometheus", webPort: 7500, metricsPort: 7501, enablePrometheus: true,
+			wantMem: true, wantPrometheus: true,
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			cfg := &v1.ServerConfig{
+				WebServer:        v1.WebServerConfig{Port: tc.webPort},
+				MetricsServer:    v1.MetricsServerConfig{Port: tc.metricsPort},
+				EnablePrometheus: tc.enablePrometheus,
+			}
+			mem, prometheus := metricsCollectors(cfg)
+			require.Equal(t, tc.wantMem, mem, "EnableMem")
+			require.Equal(t, tc.wantPrometheus, prometheus, "EnablePrometheus")
+		})
+	}
+}
+
 func TestServiceCloseMarksClosedBeforeCleanup(t *testing.T) {
 	svr, listener, addresses := newLifecycleTestService(t, false)
 	listener.closeEntered, listener.closeResume = make(chan struct{}), make(chan struct{})