L4 passthrough: failed tunnels are invisible to metrics, and error log conflates peek vs. tunnel failures #8

New issue

Open

opened 2026-07-22 15:07:22 +00:00 by james.oates · 0 comments

james.oates commented

2026-07-22 15:07:22 +00:00

Owner

Found during the final whole-branch review of the L4 SNI Passthrough (Phase A) feature.

Problem 1: failed tunnels don't record metrics

handle_connection (src/l4.rs) bails via ? on a run_tunnel error, which skips the call to record_l4_route_connection. This means total_connections/total_bytes_sent/total_bytes_received only count cleanly completed tunnels — a route whose backend is down (dial failure) produces no metric at all, rather than an incrementing failure counter. During an incident, the dashboard would show a route with zero recent connections even though clients are actively trying (and failing) to reach it.

Problem 2: misleading error log label

main.rs's accept-loop error arm logs any handle_connection error as "L4 passthrough peek failed", even when the actual failure occurred later — in the backend dial or the copy_bidirectional relay inside run_tunnel, not in the ClientHello peek itself. This is confusing when debugging a real incident (a dead backend looks like a peek/parsing problem in the logs).

Suggested fix

Distinguish peek failures from tunnel failures in the error path (e.g. a richer error enum or separate log call sites), so log messages accurately name where the failure occurred.
Consider recording a failed-connection counter (or at least incrementing total_connections with a separate failure tally) even when run_tunnel errors, so the dashboard reflects connection attempts, not just successes.

Minor — operational/observability polish, not a correctness bug. Not blocking, logged for awareness.

Found during the final whole-branch review of the L4 SNI Passthrough (Phase A) feature. ## Problem 1: failed tunnels don't record metrics `handle_connection` (`src/l4.rs`) bails via `?` on a `run_tunnel` error, which skips the call to `record_l4_route_connection`. This means `total_connections`/`total_bytes_sent`/`total_bytes_received` only count *cleanly completed* tunnels — a route whose backend is down (dial failure) produces no metric at all, rather than an incrementing failure counter. During an incident, the dashboard would show a route with zero recent connections even though clients are actively trying (and failing) to reach it. ## Problem 2: misleading error log label `main.rs`'s accept-loop error arm logs any `handle_connection` error as `"L4 passthrough peek failed"`, even when the actual failure occurred later — in the backend dial or the `copy_bidirectional` relay inside `run_tunnel`, not in the ClientHello peek itself. This is confusing when debugging a real incident (a dead backend looks like a peek/parsing problem in the logs). ## Suggested fix - Distinguish peek failures from tunnel failures in the error path (e.g. a richer error enum or separate log call sites), so log messages accurately name where the failure occurred. - Consider recording a failed-connection counter (or at least incrementing `total_connections` with a separate failure tally) even when `run_tunnel` errors, so the dashboard reflects connection attempts, not just successes. Minor — operational/observability polish, not a correctness bug. Not blocking, logged for awareness.

No labels

No milestone

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

IsoHex/edge-router#8

No description provided.

Rows
Columns