egress: log donor country not IP, make deploys verifiable, bump to v2.3.14

egress/otellogs.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"log/slog"
 	"os"
+	"strings"
 	"time"
 
 	"go.opentelemetry.io/contrib/bridges/otelslog"
@@ -53,7 +54,15 @@ func enableOTELLogs(ctx context.Context) func(context.Context) error {
 	// Same shape as telemetry.EnableOTELTracing returning a no-op when its
 	// sampler variables are absent: absent configuration means the feature is
 	// off, not misconfigured.
-	if !otlpLogsConfigured() {
+	endpointVar, endpoint := otlpLogsEndpoint()
+	if endpointVar == "" {
+		// Warn rather than returning quietly. Whether export is on is not
+		// otherwise observable: the answer lives in the absence of logs in the
+		// collector, which is indistinguishable from a healthy egress that
+		// simply had nothing to say. Someone deploying this needs to be able
+		// to confirm it from the journal.
+		slog.Warn("Log export disabled: no OTLP logs endpoint configured",
+			"checked", strings.Join(otlpLogsEndpointVars, ", "))
 		return func(context.Context) error { return nil }
 	}
 
@@ -81,6 +90,11 @@ func enableOTELLogs(ctx context.Context) func(context.Context) error {
 	// Wrap whatever the binary installed rather than replacing it — each
 	// egress/cmd main sets a stderr TextHandler at Debug, and that is still
 	// the only place the high-volume lines are readable.
+	// Logged before the handler is swapped, so it appears on stderr whether or
+	// not the exporter itself turns out to work.
+	slog.Info("Log export enabled",
+		"endpoint", endpoint, "from", endpointVar, "min_level", otelLogLevel)
+
 	local := slog.Default().Handler()
 	remote := otelslog.NewHandler("github.com/getlantern/broflake/egress",
 		otelslog.WithLoggerProvider(lp))
@@ -106,19 +120,27 @@ func enableOTELLogs(ctx context.Context) func(context.Context) error {
 // hold up process exit.
 const logShutdownTimeout = 5 * time.Second
 
-// otlpLogsConfigured reports whether an OTLP endpoint is configured for logs.
-// Checks the signal-specific variable first, matching OTEL's own precedence,
-// then the shared one.
-func otlpLogsConfigured() bool {
-	for _, k := range []string{
-		"OTEL_EXPORTER_OTLP_LOGS_ENDPOINT",
-		"OTEL_EXPORTER_OTLP_ENDPOINT",
-	} {
-		if os.Getenv(k) != "" {
-			return true
+// otlpLogsEndpointVars are the variables that can supply a logs endpoint, in
+// OTEL's own precedence order: signal-specific first, then shared.
+//
+// Deliberately not the metrics or traces variables. A host that sets only
+// OTEL_EXPORTER_OTLP_METRICS_ENDPOINT has a collector, but says nothing about
+// where logs should go — otlploghttp would fall back to localhost:4318 and
+// queue records for something that is not there.
+var otlpLogsEndpointVars = []string{
+	"OTEL_EXPORTER_OTLP_LOGS_ENDPOINT",
+	"OTEL_EXPORTER_OTLP_ENDPOINT",
+}
+
+// otlpLogsEndpoint returns the variable that supplied a logs endpoint and its
+// value, or two empty strings when none is configured.
+func otlpLogsEndpoint() (name, value string) {
+	for _, k := range otlpLogsEndpointVars {
+		if v := os.Getenv(k); v != "" {
+			return k, v
 		}
 	}
-	return false
+	return "", ""
 }
 
 // teeHandler writes each record to both destinations. Not a general-purpose

egress/otellogs_test.go

@@ -229,7 +229,7 @@ func TestTeeHandler_LocalLegStillFormatsNormally(t *testing.T) {
 // every Info record, and blocks on shutdown flushing to nothing — which hung
 // NewListener's own shutdown test for the full 600s timeout and would do the
 // same to a production egress on a host with no collector.
-func TestOTLPLogsConfigured_RequiresAnEndpoint(t *testing.T) {
+func TestOTLPLogsEndpoint_RequiresALogsEndpoint(t *testing.T) {
 	for _, tc := range []struct {
 		name, generic, logs string
 		want                bool
@@ -243,8 +243,9 @@ func TestOTLPLogsConfigured_RequiresAnEndpoint(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			t.Setenv("OTEL_EXPORTER_OTLP_ENDPOINT", tc.generic)
 			t.Setenv("OTEL_EXPORTER_OTLP_LOGS_ENDPOINT", tc.logs)
-			if got := otlpLogsConfigured(); got != tc.want {
-				t.Errorf("otlpLogsConfigured() = %v, want %v", got, tc.want)
+			name, _ := otlpLogsEndpoint()
+			if got := name != ""; got != tc.want {
+				t.Errorf("otlpLogsEndpoint() name = %q (configured=%v), want configured=%v", name, got, tc.want)
 			}
 		})
 	}
@@ -275,3 +276,59 @@ func TestEnableOTELLogs_NoEndpointLeavesLoggingUntouched(t *testing.T) {
 		t.Fatal("shutdown blocked with no exporter configured")
 	}
 }
+
+// Whether export is on must be visible on stderr. Otherwise the only evidence
+// is the absence of logs in the collector, which looks identical to a healthy
+// egress that had nothing to say — so a misconfigured deploy is unfalsifiable.
+func TestEnableOTELLogs_SaysWhyItIsDisabled(t *testing.T) {
+	t.Setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "")
+	t.Setenv("OTEL_EXPORTER_OTLP_LOGS_ENDPOINT", "")
+
+	var buf bytes.Buffer
+	prev := slog.Default()
+	slog.SetDefault(slog.New(slog.NewTextHandler(&buf, &slog.HandlerOptions{Level: slog.LevelDebug})))
+	t.Cleanup(func() { slog.SetDefault(prev) })
+
+	_ = enableOTELLogs(context.Background())
+
+	out := buf.String()
+	if !strings.Contains(out, "Log export disabled") {
+		t.Errorf("no line explaining that export is off: %q", out)
+	}
+	// Naming the variables it looked at is the actionable part — otherwise the
+	// operator knows it is off but not what to set.
+	for _, v := range otlpLogsEndpointVars {
+		if !strings.Contains(out, v) {
+			t.Errorf("the disabled line does not name %s, so it is not actionable: %q", v, out)
+		}
+	}
+}
+
+// A metrics-only collector configuration must not be mistaken for a logs
+// endpoint. otlploghttp would fall back to localhost:4318 and queue records for
+// something that is not listening.
+func TestOTLPLogsEndpoint_IgnoresTheMetricsEndpoint(t *testing.T) {
+	t.Setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "")
+	t.Setenv("OTEL_EXPORTER_OTLP_LOGS_ENDPOINT", "")
+	t.Setenv("OTEL_EXPORTER_OTLP_METRICS_ENDPOINT", "http://collector:4318/v1/metrics")
+
+	if name, _ := otlpLogsEndpoint(); name != "" {
+		t.Errorf("treated %s as a logs endpoint", name)
+	}
+}
+
+// And when one is configured, the enabled line has to say so, with the source
+// variable — that is what makes a deploy verifiable from the journal.
+func TestOTLPLogsEndpoint_ReportsWhichVariableSuppliedIt(t *testing.T) {
+	t.Setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "http://shared:4318")
+	t.Setenv("OTEL_EXPORTER_OTLP_LOGS_ENDPOINT", "")
+	if name, val := otlpLogsEndpoint(); name != "OTEL_EXPORTER_OTLP_ENDPOINT" || val != "http://shared:4318" {
+		t.Errorf("got (%q, %q), want the shared variable", name, val)
+	}
+
+	// Signal-specific wins, matching OTEL's precedence.
+	t.Setenv("OTEL_EXPORTER_OTLP_LOGS_ENDPOINT", "http://logs:4318/v1/logs")
+	if name, val := otlpLogsEndpoint(); name != "OTEL_EXPORTER_OTLP_LOGS_ENDPOINT" || val != "http://logs:4318/v1/logs" {
+		t.Errorf("got (%q, %q), want the logs-specific variable to win", name, val)
+	}
+}