From d49e740821493948ae8b15cb287d283ae7cd00a2 Mon Sep 17 00:00:00 2001
From: Sergey Kozyrenko <sirozha_blr@tut.by>
Date: Sat, 11 Jul 2026 09:09:38 +0700
Subject: [PATCH] test(observability): cover dial-timeout + ErrNotConfigured;
 close conn on exporter error
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Unit coverage for NewTelemetryClient: an unset OTEL_HOST yields ErrNotConfigured,
and an unreachable collector returns within the dial timeout instead of hanging
(a TCP-accepting, silent listener drives the WithBlock path).

Also close the grpc.ClientConn on the exporter-creation error paths — a
successful dial followed by a failed exporter New() previously leaked it.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---
 backend/pkg/observability/otelclient.go      |  3 ++
 backend/pkg/observability/otelclient_test.go | 56 ++++++++++++++++++++
 2 files changed, 59 insertions(+)
 create mode 100644 backend/pkg/observability/otelclient_test.go

diff --git a/backend/pkg/observability/otelclient.go b/backend/pkg/observability/otelclient.go
index e159bccc..599787b9 100644
--- a/backend/pkg/observability/otelclient.go
+++ b/backend/pkg/observability/otelclient.go
@@ -117,6 +117,7 @@ func NewTelemetryClient(ctx context.Context, cfg *config.Config) (TelemetryClien
 
 	logExporter, err := otlploggrpc.New(ctx, otlploggrpc.WithGRPCConn(conn))
 	if err != nil {
+		_ = conn.Close()
 		return nil, fmt.Errorf("failed to create log exporter: %w", err)
 	}
 
@@ -132,6 +133,7 @@ func NewTelemetryClient(ctx context.Context, cfg *config.Config) (TelemetryClien
 
 	metricExporter, err := otlpmetricgrpc.New(ctx, otlpmetricgrpc.WithGRPCConn(conn))
 	if err != nil {
+		_ = conn.Close()
 		return nil, fmt.Errorf("failed to create metric exporter: %w", err)
 	}
 
@@ -148,6 +150,7 @@ func NewTelemetryClient(ctx context.Context, cfg *config.Config) (TelemetryClien
 
 	spanExporter, err := otlptracegrpc.New(ctx, otlptracegrpc.WithGRPCConn(conn))
 	if err != nil {
+		_ = conn.Close()
 		return nil, fmt.Errorf("failed to create tracer exporter: %w", err)
 	}
 
diff --git a/backend/pkg/observability/otelclient_test.go b/backend/pkg/observability/otelclient_test.go
new file mode 100644
index 00000000..c01c8c42
--- /dev/null
+++ b/backend/pkg/observability/otelclient_test.go
@@ -0,0 +1,56 @@
+package observability
+
+import (
+	"context"
+	"errors"
+	"net"
+	"testing"
+	"time"
+
+	"pentagi/pkg/config"
+)
+
+func TestNewTelemetryClient_EmptyEndpointReturnsErrNotConfigured(t *testing.T) {
+	_, err := NewTelemetryClient(context.Background(), &config.Config{TelemetryEndpoint: ""})
+	if !errors.Is(err, ErrNotConfigured) {
+		t.Fatalf("want ErrNotConfigured, got %v", err)
+	}
+}
+
+// H4 guard: a set-but-unreachable collector must not hang startup. The endpoint
+// accepts the TCP connection but never completes the gRPC handshake, so a
+// WithBlock dial would wait forever without the internal DefaultDialTimeout that
+// this bounds — the caller's context has no deadline.
+func TestNewTelemetryClient_UnreachableReturnsWithinDialTimeout(t *testing.T) {
+	ln, err := net.Listen("tcp", "127.0.0.1:0")
+	if err != nil {
+		t.Fatalf("listen: %v", err)
+	}
+	defer ln.Close()
+	go func() {
+		for {
+			conn, err := ln.Accept()
+			if err != nil {
+				return
+			}
+			// hold the connection open and stay silent (no HTTP/2 handshake)
+			defer conn.Close()
+		}
+	}()
+
+	cfg := &config.Config{TelemetryEndpoint: ln.Addr().String()}
+	done := make(chan error, 1)
+	go func() {
+		_, e := NewTelemetryClient(context.Background(), cfg)
+		done <- e
+	}()
+
+	select {
+	case err := <-done:
+		if err == nil {
+			t.Fatal("expected an error for an unreachable collector")
+		}
+	case <-time.After(DefaultDialTimeout + 10*time.Second):
+		t.Fatal("NewTelemetryClient hung past the dial timeout (H4 regression)")
+	}
+}