#994·pingora

ConnectionFilter has no end-of-connection hook, so downstream connections can be counted starting but never ending

Author: ShanireZCreated Sep 4, 2026Updated Sep 4, 2026

What is the problem your feature solves, or the need it fulfills?

A process that uses Pingora as its entry point cannot report how many downstream connections it currently has. A connection starting can be observed today; a connection ending cannot be observed at all.

ConnectionFilter::should_accept (added by #671, behind the connection_filter feature) is called after accept() and before the TLS handshake, which is the right moment to see a connection start. Nothing is called when one ends. run_endpoint spawns one task per connection, and that task has three exits — handshake timeout, handshake error, and handle_event returning — and none of them notifies anything. So a number built on should_accept can only ever go up.

This has been asked for more than once:

  • #118, "on connect phase for incoming connections" — closed as completed by #671, which covers the accept moment only.
  • #295 — asked specifically for the number of active connections (increment on connect, decrement on disconnect). The suggested answer was a userland Drop guard; the issue was closed by the stale bot, the reporter's last comment being "still relevant".
  • #337 — asked how to track client connect/disconnect (for WebSocket); the answer was to use Tracer, and the issue was closed as completed.

Pingora already has exactly this shape, on the other side: upstreams::peer::Tracing provides on_connected / on_disconnected, and Tracer is the answer people are pointed at on #245, #295 and #337. But a Tracer is a field on PeerOptions and counts connections to upstreams; there is no equivalent for connections from downstream. The alternatives section below says why that is a different question rather than a smaller one.

Worth noting from the other side: pingora-prometheus and pingora-foundations already give a place to publish such a number and an endpoint to serve it from. What is missing is anything in core that reports it.

One caveat we are not asking you to change, mentioned so a count is not oversold: in ListenerEndpoint::accept(), if the stream has no socket digest or the digest has no peer address, should_accept is not called at all and the connection is accepted by default. For a filter that is a safe default; it does mean a count of starts is not exhaustive.

Describe the solution you'd like

Extend the existing ConnectionFilter seam rather than introduce a second one. It already carries the plumbing (a per-endpoint field, a builder method, Listeners::set_connection_filter) and is already gated behind connection_filter, which is off by default.

One addition: a defaulted fn connection_closed(&self), called once for each connection should_accept was consulted about, when that connection is gone. No signature changes, no new data crossing the boundary, and nothing for existing implementors to do.

The pairing is the part worth pinning down: as noted above, should_accept is skipped entirely when there is no peer address. A close hook that fired for every accepted connection would therefore not pair with it, and the obvious +1 / -1 implementation would underflow. Firing it exactly where should_accept ran keeps the two in step.

rust
#[async_trait]
impl ConnectionFilter for MyConnectionCount {
    async fn should_accept(&self, _addr: Option<&SocketAddr>) -> bool {
        self.live.fetch_add(1, Relaxed);
        true
    }

    // new
    fn connection_closed(&self) { self.live.fetch_sub(1, Relaxed); }
}

On attributing connections to a particular listener: we are deliberately not asking for that here. Today Listeners::set_connection_filter installs one instance across every endpoint (and add_endpoint clones that same one), so the hook above yields a process-wide live count — which is more than is obtainable now. Per-listener attribution follows from #941 by construction, since a filter instance attached to one address already knows its address. We would rather wait for that than ask for a second mechanism.

Two things we learned the hard way carrying this in a fork, offered for what they are worth rather than as a prescription:

  • The end hook wants to be synchronous. The only way we found to guarantee it fires is a value owned by the spawned task that calls the hook from Drop — and Drop cannot await. An async close hook therefore forces either a spawn per closing connection or a blocking bridge, both worse than a plain fn. should_accept can stay async.
  • Drive it from that guard, not from explicit calls at each exit. The task has three exits; an explicit call has to be written three times, and the failure mode of missing one is silent — a live-connection number that never comes back down, sitting next to a total that looks perfectly healthy.

Backwards compatibility: a defaulted method is source-compatible for existing implementors, and the feature is opt-in, so nothing changes for anyone who does not implement it.

One thing worth knowing before shaping this: the trait exists twice. The real one is in listeners/connection_filter.rs under the feature; listeners/mod.rs defines a stub for when the feature is off, described as being there "for API compatibility". Their signatures already differ — the stub's should_accept is synchronous and takes &SocketAddr, the real one is async and takes Option<&SocketAddr> — so any addition here needs a decision about the stub too. We mention it only because it shapes the change, not as a separate report.

On what this would add to pingora-core: we are aware that core deliberately no longer carries metrics. #560 and #822 asked for prometheus to be made optional, and 842ddd9 answered by moving the Prometheus HTTP app out into its own pingora-prometheus crate, which now builds entirely on core's public API. What we are asking for sits on the other side of that same line: one notification, with no metrics dependency, no counter/gauge distinction and no label vocabulary. The counting stays in the caller, exactly as pingora-prometheus now sits outside core.

A scope question we would rather ask than assume: connection_filter is named for filtering, and observing when a connection ends is a different concern. If you would prefer it to live under a differently named feature, or on a sibling trait, we are happy to shape a PR that way. What we would like to avoid is a second, parallel wiring path alongside the one ConnectionFilter already has.

Describe alternatives you've considered

  • upstreams::peer::Tracer — the answer given on #245, #295 and #337, so it is worth addressing up front rather than after a round trip. A Tracer is placed in PeerOptions and its on_connected / on_disconnected fire for connections Pingora opens to an upstream, counting active plus pooled ones. That is a different population from the connections a listener is currently holding: a downstream connection that never opens an upstream connection produces no tracer events at all — one rejected by a filter, one that fails or times out in the TLS handshake, a request served from cache or by a local handler, or any L4 application that never dials out. Nothing in listeners/ or services/listening.rs references it.
  • ServerApp::cleanup / HttpServerApp::http_cleanup — suggested on #118, so worth ruling out explicitly. These are per-service, not per-connection: the doc comment on cleanup says it is "called once after the service stops listening to its endpoints", and the call site in run_endpoint is after the accept loop has exited. They cannot see individual connections at all.
  • Counting in userland with a Drop guard (also suggested on #295). It cannot cover the window this is about: connections that time out or fail during io.handshake() never reach the application, because handle_event is only called on the successful branch. For an entry point those are a population you specifically want to see.
  • Polling the operating system (netstat-style, also considered on #295): it is a sampling answer to a question about state transitions, and attributing sockets back to a particular listener of a particular process is awkward at best.
  • A separate observer trait alongside ConnectionFilter: this duplicates wiring that already exists (per-endpoint field, builder method, Listeners::set_*), which is why we are proposing to extend the existing seam. See the scope question above if you disagree.
  • Carrying it in a fork — what we do today. It works, but it is a permanent rebase cost for something that looks generally useful rather than specific to us, which is why we are asking here instead of keeping it.

Additional context

  • Related, on the seam itself: #671 (the ConnectionFilter trait), #118, #295, #337.
  • Related, on listener identity, which we are deliberately leaving alone: #941 (open) attaches filters per address, which is how we would expect per-listener attribution to arrive; #988, with PR #991 reporting the addresses a service actually bound and PR #990 fixing the fd-table collision it needs (all open).
  • Related, on end-of-something hooks: #751 (open since 2025-11) adds finish_downstream_session to ProxyHttp. That is a per-session hook in pingora-proxy; what is asked for above is a per-connection notification in pingora-core, so it also covers L4 applications and connections that never complete a handshake.
  • We maintain a fork carrying this capability and would be glad to send the PR, in whatever shape you prefer.

Pingora version: main @ 09696b5