Address metrics handler review comments

app/metrics/metrics.go

@@ -24,7 +24,6 @@ import (
 type MetricsHandler struct {
 	ohm          outbound.Manager
 	statsManager feature_stats.Manager
-	observatory  extension.Observatory
 	ctx          context.Context
 	tag          string
 	listen       string
@@ -65,6 +64,13 @@ func (p *MetricsHandler) Start() error {
 		go p.serve(TCPlistener, handler)
 	}
 
+	if p.tag == "" {
+		if p.tcpListener == nil {
+			return errors.New("metrics must have a tag or listen address")
+		}
+		return nil
+	}
+
 	listener := &OutboundListener{
 		buffer: make(chan xnet.Conn, 4),
 		done:   done.New(),
@@ -100,7 +106,7 @@ func (p *MetricsHandler) Close() error {
 		errs = append(errs, p.listener.Close())
 		p.listener = nil
 	}
-	if p.ohm != nil {
+	if p.ohm != nil && p.tag != "" {
 		if err := p.ohm.RemoveHandler(context.Background(), p.tag); err != nil {
 			errors.LogInfo(context.Background(), "failed to remove metrics handler")
 		}
@@ -196,17 +202,13 @@ func (p *MetricsHandler) stats() map[string]map[string]map[string]int64 {
 }
 
 func (p *MetricsHandler) observatoryStatus() interface{} {
-	if p.observatory == nil {
-		common.Must(core.RequireFeatures(p.ctx, func(observatory extension.Observatory) error {
-			p.observatory = observatory
-			return nil
-		}))
-		if p.observatory == nil {
-			return nil
-		}
+	feature := core.MustFromContext(p.ctx).GetFeature(extension.ObservatoryType())
+	if feature == nil {
+		return nil
 	}
+	observatoryFeature := feature.(extension.Observatory)
 	resp := map[string]*observatory.OutboundStatus{}
-	if o, err := p.observatory.GetObservation(context.Background()); err != nil {
+	if o, err := observatoryFeature.GetObservation(context.Background()); err != nil {
 		return err
 	} else {
 		for _, x := range o.(*observatory.ObservationResult).GetStatus() {

app/metrics/metrics_test.go

@@ -1,7 +1,9 @@
 package metrics
 
 import (
+	"context"
 	"encoding/json"
+	stdnet "net"
 	"net/http"
 	"net/http/httptest"
 	"testing"
@@ -13,6 +15,7 @@ import (
 	appstats "github.com/xtls/xray-core/app/stats"
 	"github.com/xtls/xray-core/common/serial"
 	"github.com/xtls/xray-core/core"
+	feature_outbound "github.com/xtls/xray-core/features/outbound"
 )
 
 func TestMetricsCanRestartInSameProcess(t *testing.T) {
@@ -40,10 +43,40 @@ func TestMetricsCanRunMultipleInstancesInSameProcess(t *testing.T) {
 	readMetricsVars(t, server2)
 }
 
+func TestMetricsListenOnlyWithoutTagDoesNotRegisterOutbound(t *testing.T) {
+	listen := pickMetricsListenAddress(t)
+	server := startMetricsTestServerWithMetricsConfig(t, &Config{
+		Listen: listen,
+	})
+	t.Cleanup(func() {
+		_ = server.Close()
+	})
+
+	response, err := http.Get("http://" + listen + "/debug/vars")
+	if err != nil {
+		t.Fatalf("failed to read listen-only metrics: %v", err)
+	}
+	defer response.Body.Close()
+	if response.StatusCode != http.StatusOK {
+		t.Fatalf("unexpected listen-only metrics status: %d", response.StatusCode)
+	}
+
+	outboundManager := server.GetFeature(feature_outbound.ManagerType()).(feature_outbound.Manager)
+	if handlers := outboundManager.ListHandlers(context.Background()); len(handlers) != 0 {
+		t.Fatalf("listen-only metrics registered outbound handlers: got %d, want 0", len(handlers))
+	}
+}
+
 func startMetricsTestServer(t *testing.T) *core.Instance {
+	return startMetricsTestServerWithMetricsConfig(t, &Config{
+		Tag: "metrics_out",
+	})
+}
+
+func startMetricsTestServerWithMetricsConfig(t *testing.T, metricsConfig *Config) *core.Instance {
 	t.Helper()
 
-	server, err := core.New(metricsTestConfig())
+	server, err := core.New(metricsTestConfig(metricsConfig))
 	if err != nil {
 		t.Fatalf("failed to create metrics server: %v", err)
 	}
@@ -54,20 +87,29 @@ func startMetricsTestServer(t *testing.T) *core.Instance {
 	return server
 }
 
-func metricsTestConfig() *core.Config {
+func metricsTestConfig(metricsConfig *Config) *core.Config {
 	return &core.Config{
 		App: []*serial.TypedMessage{
 			serial.ToTypedMessage(&dispatcher.Config{}),
 			serial.ToTypedMessage(&proxyman.InboundConfig{}),
 			serial.ToTypedMessage(&proxyman.OutboundConfig{}),
 			serial.ToTypedMessage(&appstats.Config{}),
-			serial.ToTypedMessage(&Config{
-				Tag: "metrics_out",
-			}),
+			serial.ToTypedMessage(metricsConfig),
 		},
 	}
 }
 
+func pickMetricsListenAddress(t *testing.T) string {
+	t.Helper()
+
+	listener, err := stdnet.Listen("tcp", "127.0.0.1:0")
+	if err != nil {
+		t.Fatalf("failed to pick metrics listen address: %v", err)
+	}
+	defer listener.Close()
+	return listener.Addr().String()
+}
+
 func readMetricsVars(t *testing.T, server *core.Instance) {
 	t.Helper()