From 40ec335fee660700738276253c93420db7546991 Mon Sep 17 00:00:00 2001 From: rootkiller6788 Date: Fri, 21 Aug 2026 03:08:44 +0800 Subject: [PATCH] fix(ratelimiter): log metrics server bind failures at error level setupMetrics discards the http.ListenAndServe error in an empty branch, so a Prometheus listener that fails to bind leaves the service running with no metrics and no signal in the pod logs. Log the failure at error level, matching setupPprof and setupOlricStats in the same file. Add a regression test that occupies the metrics port so the bind fails deterministically, and asserts the error-level log line is emitted. --- .../ratelimiter/cmd/BUILD.bazel | 2 + .../ratelimiter/cmd/main.go | 2 +- .../ratelimiter/cmd/metrics_test.go | 48 +++++++++++++++++++ 3 files changed, 51 insertions(+), 1 deletion(-) create mode 100644 src/invocation-plane-services/ratelimiter/cmd/metrics_test.go diff --git a/src/invocation-plane-services/ratelimiter/cmd/BUILD.bazel b/src/invocation-plane-services/ratelimiter/cmd/BUILD.bazel index 1fd40ab86..5b1c0e4f9 100644 --- a/src/invocation-plane-services/ratelimiter/cmd/BUILD.bazel +++ b/src/invocation-plane-services/ratelimiter/cmd/BUILD.bazel @@ -85,6 +85,7 @@ go_test( srcs = [ "info_test.go", "main_test.go", + "metrics_test.go", ], embed = [":cmd_lib"], deps = [ @@ -100,5 +101,6 @@ go_test( "@org_golang_google_grpc//status", "@org_golang_x_exp//rand", "@org_uber_go_zap//:zap", + "@org_uber_go_zap//zaptest/observer", ], ) diff --git a/src/invocation-plane-services/ratelimiter/cmd/main.go b/src/invocation-plane-services/ratelimiter/cmd/main.go index 7c30b8465..dcc73e977 100644 --- a/src/invocation-plane-services/ratelimiter/cmd/main.go +++ b/src/invocation-plane-services/ratelimiter/cmd/main.go @@ -68,7 +68,7 @@ func setupMetrics() { go func() { err := http.ListenAndServe("0.0.0.0:7776", mux) if err != nil { - + zap.L().Error("metrics server failed", zap.Error(err)) } }() } diff --git a/src/invocation-plane-services/ratelimiter/cmd/metrics_test.go b/src/invocation-plane-services/ratelimiter/cmd/metrics_test.go new file mode 100644 index 000000000..32871a126 --- /dev/null +++ b/src/invocation-plane-services/ratelimiter/cmd/metrics_test.go @@ -0,0 +1,48 @@ +/* +SPDX-FileCopyrightText: Copyright (c) NVIDIA CORPORATION & AFFILIATES. All rights reserved. +SPDX-License-Identifier: Apache-2.0 + +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 main + +import ( + "net" + "testing" + "time" + + "github.com/stretchr/testify/require" + "go.uber.org/zap" + "go.uber.org/zap/zaptest/observer" +) + +// TestSetupMetricsLogsServeFailure guards against silently discarding the +// metrics server's ListenAndServe error (NVIDIA/nvcf#540). The metrics port is +// occupied up front so the bind fails deterministically, and the global logger +// is swapped for an observer so the error-level log line can be asserted. +func TestSetupMetricsLogsServeFailure(t *testing.T) { + ln, err := net.Listen("tcp", "0.0.0.0:7776") + require.NoError(t, err) + defer ln.Close() + + observed, logs := observer.New(zap.InfoLevel) + restore := zap.ReplaceGlobals(zap.New(observed)) + defer restore() + + setupMetrics() + + require.Eventually(t, func() bool { + return len(logs.FilterMessage("metrics server failed").All()) > 0 + }, 5*time.Second, 10*time.Millisecond) +}