fix(telemetry): report tracesUrl and explain trace disconnects

The agent never populated tracesUrl, and never said why traces were down.

tracesUrl: the legacy sink passed clickhouse_url (CLICKHOUSE_HOST) into
get_trace_url(); the Go port hardcoded that path to "" on the premise that we
no longer run a local ClickHouse. main.go does read and probe CLICKHOUSE_HOST,
so the premise was stale and the field shipped empty for every otel_clickhouse
cluster. That left agent_service.go's `TracesUrl != nil` gate permanently
closed, so the trace-table config was never written and Last9 clusters resolved
`otel_traces` instead of `otel.traces`. Add isClickHouseEnabled as the
otel_clickhouse counterpart of isJaegerEnabled, and report the host.

ClickHouse is checked last in traceURL rather than first as in the legacy: the
legacy order returns the ClickHouse host even when TRACE_TABLE makes the
provider bigquery, and the backend then quotes that host as a BigQuery table.
It is also deliberately not used by traceStatus — gating tracesEnabled on the
URL would turn traces off for TRACES_ENABLED=true-without-host, a config the
agent supports for external ClickHouse it cannot probe.

tracesConnectionError: the field was declared (agent_service.go) and rendered
(agentHealth.jsx renderReason) but emitted by nobody, so a Disconnected Traces
pill never showed a reason. probeClickhouse now returns one alongside the
status; httpProbe is split over a new httpProbeErr so the failure survives,
leaving its other callers untouched. Credentials in a URL-form CLICKHOUSE_HOST
are stripped before the reason ships, since it renders verbatim in the UI.

The field intentionally omits `omitempty`: the collector merges activity_stats
into connection_status with jsonb `||`, so an omitted key leaves the previous
value in place. A recovered ClickHouse must post an explicit "" to clear the
stale reason.

Fixes #34231

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: 2 people

runner/cmd/agent/main.go

@@ -15,6 +15,7 @@ import (
 	"maps"
 	"net/http"
 	"net/http/pprof"
+	"net/url"
 	"os"
 	"os/signal"
 	"runtime"
@@ -992,7 +993,7 @@ func run(ctx context.Context, logger *slog.Logger, cfg *config.Config) error {
 				probeClient := &http.Client{Timeout: 5 * time.Second}
 				logsProvider, logsURL, logsOK, logCfg := probeLogsProvider(probeCtx, cfg)
 				as := telemetry.DetectAutoScaler(probeCtx, typedKube, providerInfo.Provider, logger)
-				clickhouseStatus := probeClickhouse(probeCtx, probeClient, clickhouseHost, clickhousePort)
+				clickhouseStatus, clickhouseErr := probeClickhouse(probeCtx, probeClient, clickhouseHost, clickhousePort)
 				return telemetry.Datasources{
 					PrometheusURL:              cfg.PrometheusURL,
 					AlertManagerURL:            cfg.AlertManagerURL,
@@ -1013,6 +1014,8 @@ func run(ctx context.Context, logger *slog.Logger, cfg *config.Config) error {
 					ChronosphereTracesURL:      chronosphereURL,
 					ChronosphereURL:            cfg.ChronosphereURL,
 					ClickHouseStatus:           clickhouseStatus,
+					ClickHouseURL:              clickhouseHost,
+					ClickHouseError:            clickhouseErr,
 					AgentURL:                   agentURL,
 					GrafanaEnabled:             grafanaURL != "" && httpProbe(probeCtx, probeClient, grafanaURL+"/api/health"),
 					AutoScalerEnabled:          as.Enabled,
@@ -1293,16 +1296,50 @@ func probeLogsProvider(ctx context.Context, cfg *config.Config) (provider, url s
 // the host is set, hit `/ping` on the HTTP port and reflect the result.
 // TRACES_ENABLED=true|false acts as an explicit override (some users run
 // an external clickhouse the agent can't reach).
-func probeClickhouse(ctx context.Context, c *http.Client, host, port string) bool {
+//
+// The second return is the reason traces are down, shipped as
+// tracesConnectionError and rendered verbatim by the UI. It is empty whenever
+// ClickHouse is healthy, and also when traces are off deliberately — an
+// operator disabling a backend isn't a failure to explain.
+func probeClickhouse(ctx context.Context, c *http.Client, host, port string) (bool, string) {
 	if v := os.Getenv("TRACES_ENABLED"); v == "true" {
-		return true
+		return true, ""
 	} else if v == "false" {
-		return false
+		return false, ""
 	}
 	if host == "" {
-		return false
+		return false, "CLICKHOUSE_HOST is not set: no traces backend is configured"
+	}
+	if err := httpProbeErr(ctx, c, fmt.Sprintf("http://%s:%s/ping", host, port)); err != nil {
+		return false, fmt.Sprintf("ClickHouse ping failed at %s:%s: %v", redactUserinfo(host), port, probeCause(err))
+	}
+	return true, ""
+}
+
+// probeCause unwraps the *url.Error net/http wraps around transport failures.
+// The wrapper stringifies the whole request URL; the inner cause ("connection
+// refused", "i/o timeout") is the half worth showing and carries no address.
+func probeCause(err error) error {
+	var uerr *url.Error
+	if errors.As(err, &uerr) {
+		return uerr.Err
+	}
+	return err
+}
+
+// redactUserinfo strips a `user:pass@` prefix from a host. CLICKHOUSE_HOST may
+// be a full URL rather than a bare host (pkg/clickhouse normalizes both forms),
+// and the reason string built above lands in the agent's connection_status
+// JSON, which the UI renders as-is — credentials must not ride along.
+func redactUserinfo(host string) string {
+	at := strings.LastIndex(host, "@")
+	if at < 0 {
+		return host
 	}
-	return httpProbe(ctx, c, fmt.Sprintf("http://%s:%s/ping", host, port))
+	if scheme := strings.Index(host, "://"); scheme >= 0 && scheme+3 <= at {
+		return host[:scheme+3] + host[at+1:]
+	}
+	return host[at+1:]
 }
 
 // fetchSignozVersion GETs Signoz's /api/v1/version and returns the reported
@@ -1336,9 +1373,15 @@ func fetchSignozVersion(ctx context.Context, c *http.Client, baseURL string) str
 // URL is sourced from operator-provided config (PROMETHEUS_URL, LOKI_URL,
 // etc.), not request-derived — taint flow is operator → probe by design.
 func httpProbe(ctx context.Context, c *http.Client, url string, headers ...map[string]string) bool {
+	return httpProbeErr(ctx, c, url, headers...) == nil
+}
+
+// httpProbeErr is httpProbe with the failure preserved, for the callers that
+// report *why* a datasource is unreachable rather than just that it is.
+func httpProbeErr(ctx context.Context, c *http.Client, url string, headers ...map[string]string) error {
 	req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) //nolint:gosec // operator-provided URL
 	if err != nil {
-		return false
+		return err
 	}
 	for _, h := range headers {
 		for k, v := range h {
@@ -1347,10 +1390,13 @@ func httpProbe(ctx context.Context, c *http.Client, url string, headers ...map[s
 	}
 	resp, err := c.Do(req) //nolint:gosec // operator-provided URL
 	if err != nil {
-		return false
+		return err
 	}
 	defer func() { _ = resp.Body.Close() }()
-	return resp.StatusCode >= 200 && resp.StatusCode < 300
+	if resp.StatusCode < 200 || resp.StatusCode >= 300 {
+		return fmt.Errorf("HTTP %d", resp.StatusCode)
+	}
+	return nil
 }
 
 // esHTTPClient returns the HTTP client the ES query client uses. When

runner/cmd/agent/probe_test.go

@@ -4,9 +4,12 @@ import (
 	"context"
 	"encoding/base64"
 	"log/slog"
+	"net"
 	"net/http"
 	"net/http/httptest"
+	"strings"
 	"testing"
+	"time"
 
 	"github.com/nudgebee/nudgebee-agent/pkg/config"
 	"github.com/nudgebee/nudgebee-agent/pkg/observability/prometheus"
@@ -185,3 +188,83 @@ func TestSelectedLogsProvider_Precedence(t *testing.T) {
 		})
 	}
 }
+
+// A ClickHouse that answers /ping is healthy and has no reason to report.
+func TestProbeClickhouse_HealthyReportsNoReason(t *testing.T) {
+	t.Setenv("TRACES_ENABLED", "")
+	ch := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusOK)
+	}))
+	defer ch.Close()
+
+	host, port, _ := net.SplitHostPort(strings.TrimPrefix(ch.URL, "http://"))
+	ok, reason := probeClickhouse(context.Background(), ch.Client(), host, port)
+	if !ok {
+		t.Errorf("probeClickhouse ok = false; want true")
+	}
+	if reason != "" {
+		t.Errorf("reason = %q; want empty for a healthy ClickHouse", reason)
+	}
+}
+
+// An unreachable ClickHouse must explain itself — this is the string the UI
+// renders under the Traces "Disconnected" pill.
+func TestProbeClickhouse_UnreachableReportsReason(t *testing.T) {
+	t.Setenv("TRACES_ENABLED", "")
+	// Bind and immediately close, so the port is dead but well-formed.
+	dead := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}))
+	host, port, _ := net.SplitHostPort(strings.TrimPrefix(dead.URL, "http://"))
+	dead.Close()
+
+	ok, reason := probeClickhouse(context.Background(), &http.Client{Timeout: 2 * time.Second}, host, port)
+	if ok {
+		t.Errorf("probeClickhouse ok = true; want false for a dead ClickHouse")
+	}
+	if reason == "" {
+		t.Fatal("reason = empty; want a failure explanation")
+	}
+	if !strings.Contains(reason, "ClickHouse ping failed") {
+		t.Errorf("reason = %q; want it to name the failing probe", reason)
+	}
+}
+
+// Traces off by operator choice is not a failure, so there's nothing to explain.
+func TestProbeClickhouse_ExplicitlyDisabledReportsNoReason(t *testing.T) {
+	t.Setenv("TRACES_ENABLED", "false")
+	ok, reason := probeClickhouse(context.Background(), http.DefaultClient, "ch.example", "8123")
+	if ok {
+		t.Errorf("probeClickhouse ok = true; want false under TRACES_ENABLED=false")
+	}
+	if reason != "" {
+		t.Errorf("reason = %q; want empty — disabled on purpose isn't a failure", reason)
+	}
+}
+
+// No host means traces were never wired up; say so rather than going silent.
+func TestProbeClickhouse_UnconfiguredExplainsItself(t *testing.T) {
+	t.Setenv("TRACES_ENABLED", "")
+	ok, reason := probeClickhouse(context.Background(), http.DefaultClient, "", "8123")
+	if ok {
+		t.Errorf("probeClickhouse ok = true; want false with no CLICKHOUSE_HOST")
+	}
+	if !strings.Contains(reason, "CLICKHOUSE_HOST") {
+		t.Errorf("reason = %q; want it to name the missing env var", reason)
+	}
+}
+
+// The reason string ships to the backend and renders in the UI, so a URL-form
+// CLICKHOUSE_HOST must not leak its credentials into it.
+func TestRedactUserinfo(t *testing.T) {
+	cases := []struct{ in, want string }{
+		{"clickhouse.svc:8123", "clickhouse.svc:8123"},
+		{"https://otel.last9.io:443", "https://otel.last9.io:443"},
+		{"https://admin:hunter2@otel.last9.io:443", "https://otel.last9.io:443"},
+		{"admin:hunter2@clickhouse.svc:8123", "clickhouse.svc:8123"},
+		{"", ""},
+	}
+	for _, tc := range cases {
+		if got := redactUserinfo(tc.in); got != tc.want {
+			t.Errorf("redactUserinfo(%q) = %q; want %q", tc.in, got, tc.want)
+		}
+	}
+}

runner/pkg/telemetry/service.go

@@ -66,6 +66,15 @@ type ActivityStats struct {
 	HealthCheckDuration        float64           `json:"healthCheckDuration,omitempty"`
 	TraceProvider              string            `json:"traceProvider,omitempty"`
 	TraceProviderConfig        map[string]any    `json:"traceProviderConfig,omitempty"`
+	// TracesConnectionError is the reason traces are disconnected, rendered by
+	// the UI's Agent Health card under the Traces pill ("Reason - ...").
+	//
+	// Deliberately no `omitempty`: the collector merges activity_stats into
+	// `agent.connection_status` with the jsonb `||` operator, so an omitted key
+	// leaves the previous value in place. Once ClickHouse recovers we must post
+	// an explicit "" to clear the stale reason — dropping the key would strand
+	// it in the DB forever.
+	TracesConnectionError string `json:"tracesConnectionError"`
 }
 
 // ClusterStatus is the wire payload posted to /v1/k8s/telemetry.
@@ -142,6 +151,15 @@ type Datasources struct {
 	// ClickHouseStatus is the `clickhouse_status` flag — used only as the
 	// fallback for tracesEnabled when no other provider matches.
 	ClickHouseStatus bool
+	// ClickHouseURL is CLICKHOUSE_HOST verbatim (bare host, no scheme/port) —
+	// same value the legacy checker put on `clickhouse_url`. Reported as
+	// tracesUrl for the otel_clickhouse provider; the backend substring-matches
+	// it to pick the trace table (`otel.traces` for Last9, else `otel_traces`).
+	ClickHouseURL string
+	// ClickHouseError is why the probe failed, already stripped of credentials
+	// by the caller. Empty when ClickHouse is healthy or was never probed
+	// (unconfigured / disabled). Surfaced as tracesConnectionError.
+	ClickHouseError string
 
 	// Node-agent: count of `up{job=~"...nudgebee(-.*)?-node-agent"}` from
 	// Prometheus, computed by the caller.
@@ -359,6 +377,13 @@ func (s *Service) probe(ctx context.Context, ds Datasources) ActivityStats {
 	out.TracesEnabled = traceStatus(ds)
 	out.TraceProvider = traceProvider(ds)
 	out.TracesURL = traceURL(ds)
+	// The reason slot only makes sense while traces are down, and ClickHouse is
+	// the only trace backend the agent actually probes — the others (bigquery,
+	// chronosphere, jaeger) are env-configured and force TracesEnabled true
+	// without a health check, so they never have a failure to report.
+	if !out.TracesEnabled {
+		out.TracesConnectionError = ds.ClickHouseError
+	}
 	// traceProviderConfig: the legacy code queries ClickHouse for the
 	// otel_traces materialized-column flag. The agent doesn't run a
 	// local ClickHouse anymore; the backend computes this. Emit an
@@ -398,9 +423,12 @@ func traceProvider(ds Datasources) string {
 	return "otel_clickhouse"
 }
 
-// traceURL mirrors get_trace_url. Note the first
-// argument `url_from_prometheus` is what the legacy passes as
-// `clickhouse_url` — we don't run a local ClickHouse, so it's always "".
+// traceURL mirrors get_trace_url. The legacy's first argument
+// `url_from_prometheus` is what it passes as `clickhouse_url` (CLICKHOUSE_HOST)
+// — reported here via isClickHouseEnabled, but checked last rather than first:
+// the legacy order returns the ClickHouse host even when TRACE_TABLE makes the
+// provider `bigquery`, and the backend then quotes that host as a BigQuery
+// table. Checking it last keeps the URL consistent with the reported provider.
 func traceURL(ds Datasources) string {
 	if ds.TraceTable != "" {
 		return ds.TraceTable
@@ -417,6 +445,9 @@ func traceURL(ds Datasources) string {
 		}
 		return ds.ChronosphereURL
 	}
+	if isClickHouseEnabled(ds) {
+		return ds.ClickHouseURL
+	}
 	return ""
 }
 
@@ -436,6 +467,18 @@ func isJaegerEnabled(ds Datasources) bool {
 	return ds.JaegerEnabled && ds.JaegerQueryURL != ""
 }
 
+// isClickHouseEnabled is the otel_clickhouse counterpart of isJaegerEnabled:
+// a reachable ClickHouse we have an address for. The status flag alone isn't
+// enough to report a URL — TRACES_ENABLED=true forces the flag on without
+// probing, so it can be true with CLICKHOUSE_HOST unset.
+//
+// Deliberately not used by traceStatus: gating tracesEnabled on the URL would
+// turn traces off for exactly that TRACES_ENABLED=true-without-host config,
+// which the agent supports for external ClickHouse it can't probe.
+func isClickHouseEnabled(ds Datasources) bool {
+	return ds.ClickHouseStatus && ds.ClickHouseURL != ""
+}
+
 // httpHealth returns true iff GET <url> returns 2xx within 5s.
 func httpHealth(ctx context.Context, c *http.Client, url string) bool {
 	cctx, cancel := context.WithTimeout(ctx, 5*time.Second)