fix(byoo-otel-collector): log otelcol-contrib exit error on shutdown

src/compute-plane-services/byoo-otel-collector/internal/cli/BUILD.bazel

@@ -1,4 +1,4 @@
-load("@rules_go//go:def.bzl", "go_library")
+load("@rules_go//go:def.bzl", "go_library", "go_test")
 
 go_library(
     name = "cli",
@@ -21,3 +21,15 @@ alias(
     actual = ":cli",
     visibility = ["//:__subpackages__"],
 )
+
+go_test(
+    name = "cli_test",
+    srcs = ["cli_test.go"],
+    embed = [":cli"],
+    deps = [
+        "//internal/logger",
+        "@org_uber_go_zap//:zap",
+        "@org_uber_go_zap//zapcore",
+        "@org_uber_go_zap//zaptest/observer",
+    ],
+)

src/compute-plane-services/byoo-otel-collector/internal/cli/cli.go

@@ -37,6 +37,20 @@ import (
 	"github.com/NVIDIA/nvcf/src/compute-plane-services/byoo-otel-collector/internal/secrets"
 )
 
+// processWaiter is the subset of *os.Process used by runSecretsCheckLoop, extracted so tests
+// can exercise the wait-error logging without spawning a real OS process.
+type processWaiter interface {
+	Wait() (*os.ProcessState, error)
+}
+
+// waitAndLogProcessExit waits for otelcol-contrib to exit and logs a non-nil error instead of
+// discarding it.
+func waitAndLogProcessExit(proc processWaiter) {
+	if _, waitErr := proc.Wait(); waitErr != nil {
+		logger.Logger.Errorf("error waiting for otelcol-contrib to exit: %v", waitErr)
+	}
+}
+
 func runSecretsCheckLoop(ctx context.Context, otelCollectorProc *os.Process, args []string, accountsSecrets, secretsFolder string, lastContent []byte, lastModTime time.Time) error {
 	restartCh := make(chan struct{}, 1)
 
@@ -72,7 +86,7 @@ func runSecretsCheckLoop(ctx context.Context, otelCollectorProc *os.Process, arg
 			logger.Logger.Info("Received interrupt signal, terminating otelcol-contrib and exiting.")
 			otelcollector.GracefulShutdown(otelCollectorProc)
 			// Wait for the process to actually exit
-			otelCollectorProc.Wait()
+			waitAndLogProcessExit(otelCollectorProc)
 			return nil
 		case <-restartCh:
 			logger.Logger.Info("Regenerating the secret files and restarting otelcol-contrib due to the secret file changes.")
@@ -88,7 +102,7 @@ func runSecretsCheckLoop(ctx context.Context, otelCollectorProc *os.Process, arg
 			// shutdown the current otelcol-contrib process
 			otelcollector.GracefulShutdown(otelCollectorProc)
 			// Wait for it to exit
-			otelCollectorProc.Wait()
+			waitAndLogProcessExit(otelCollectorProc)
 
 			// run the otelcol-contrib process again
 			if otelCollectorProc, err = otelcollector.RunOtelCollector(args); err != nil {

src/compute-plane-services/byoo-otel-collector/internal/cli/cli_test.go

@@ -0,0 +1,80 @@
+/*
+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 cli
+
+import (
+	"errors"
+	"os"
+	"strings"
+	"testing"
+
+	"go.uber.org/zap"
+	"go.uber.org/zap/zapcore"
+	"go.uber.org/zap/zaptest/observer"
+
+	"github.com/NVIDIA/nvcf/src/compute-plane-services/byoo-otel-collector/internal/logger"
+)
+
+type fakeProcess struct {
+	waitErr error
+}
+
+func (f *fakeProcess) Wait() (*os.ProcessState, error) {
+	return nil, f.waitErr
+}
+
+// TestWaitAndLogProcessExit covers waitAndLogProcessExit directly: both runSecretsCheckLoop
+// call sites (interrupt shutdown and secret-triggered restart) delegate to this same helper, so
+// exercising it once here covers both.
+func TestWaitAndLogProcessExit(t *testing.T) {
+	tests := []struct {
+		name    string
+		waitErr error
+		wantLog bool
+	}{
+		{name: "wait error is logged with its message", waitErr: errors.New("wait: no child processes"), wantLog: true},
+		{name: "a different wait error message is preserved verbatim", waitErr: errors.New("signal: killed"), wantLog: true},
+		{name: "clean exit logs nothing", waitErr: nil, wantLog: false},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			orig := logger.Logger
+			t.Cleanup(func() { logger.Logger = orig })
+
+			core, logs := observer.New(zapcore.DebugLevel)
+			logger.Logger = zap.New(core).Sugar()
+
+			waitAndLogProcessExit(&fakeProcess{waitErr: tt.waitErr})
+
+			entries := logs.TakeAll()
+			if !tt.wantLog {
+				if len(entries) != 0 {
+					t.Fatalf("expected no log entries, got %d", len(entries))
+				}
+				return
+			}
+			if len(entries) != 1 {
+				t.Fatalf("expected 1 log entry, got %d", len(entries))
+			}
+			if !strings.Contains(entries[0].Message, tt.waitErr.Error()) {
+				t.Fatalf("expected log message to contain %q, got %q", tt.waitErr.Error(), entries[0].Message)
+			}
+		})
+	}
+}