[serve] haproxy ingress request router metrics

[akyang-anyscale]

Adds observability into HAProxy ingress request router metrics. Adds the following:

Metrics:

• serve_haproxy_ingress_router_latency_ms | Histogram | Wall-clock time HAProxy spent consulting the ingress request router (measured around the Lua socket call) for all consultations only. Buckets cover 0.5 ms to 1 s. Categorized by outcome "success" and "failure".
• serve_haproxy_ingress_router_truncations | Counter | Number of requests whose body was clipped by HAProxy's tune.bufsize before being forwarded to the router. The router still gets a prefix plus an X-Body-Truncated: <have>/<full> header. Non-zero values indicate a body-aware policy may be missing context; consider raising RAY_SERVE_HAPROXY_INGRESS_REQUEST_ROUTER_BUFSIZE.
• serve_haproxy_ingress_router_server_mismatch | Counter | Number of requests where HAProxy ultimately routed to a different replica than the router returned. This happens when the named replica is DOWN and option redispatch falls through to load balancing. Non-zero values indicate the router's view of replica health is stale, or replicas are flapping.
• serve_haproxy_ingress_router_failures | Counter | Number of router consultations that failed to pin a replica. Each failure causes HAProxy to return 503 to the client. The reason tag is one of router_unreachable (socket connect/send/recv failed), router_non_200 (router returned a non-200 status), unparseable_replica_id (router 200 but body didn't contain a string replica_id), or unknown_replica_id (router returned a replica_id not in HAProxy's current replica map).
• serve_haproxy_ingress_router_requests_total Total number of requests handled by the ingress router.

For metrics, haproxy is emitting logs with request + ingress router metadata (target server, actual server, router latency, failure reason, etc.) to a metrics socket. This socket is scraped by HAProxyManager, which parses the log line, and ultimately increments/adds an observation to the appropriate counters.

Logs:

• controller log saying if ingress request router exists for an ingress deployment
• ingress request router targets are added to haproxy's long poll receive log

[gemini-code-assist]

Code Review

This pull request introduces per-request metrics for the HAProxy ingress request router by adding configuration options, updating Lua and HAProxy templates to capture latency and truncation data, and implementing a socket-based collection mechanism in the HAProxyManager. Feedback highlights a missing module import that would lead to runtime errors and suggests explicitly cancelling the metrics attachment task during shutdown to prevent dangling event loop tasks.

[eicherseiji]

Some metric naming convention nits. Could we also add serve_haproxy_ingress_router_requests_total tagged with {application} to get an easy denominator for rate?

[eicherseiji]

Suggested change

	| `serve_haproxy_ingress_router_latency_ms` | Histogram | `application` | Wall-clock time HAProxy spent consulting the ingress request router (measured around the Lua socket call) for **successful** consultations only. Buckets cover 0.5 ms to 1 s. Use to detect router-side slowdowns before they show up in end-to-end p99. |
	| `serve_haproxy_ingress_router_duration_seconds` | Histogram | `application` | Wall-clock time HAProxy spent consulting the ingress request router (measured around the Lua socket call) for **successful** consultations only. Buckets cover 0.5 ms to 1 s. Use to detect router-side slowdowns before they show up in end-to-end p99. |

[eicherseiji]

Could we also include failures in this histogram with the option to split by outcome="success|failure"?

[akyang-anyscale]

I think the latency_ms suffix fits better with the naming convention of other metrics, but I can apply the suggestion if you feel strongly about it.

[eicherseiji]

Nah not blocking. The _seconds part is the most relevant best practice https://prometheus.io/docs/practices/naming/

[akyang-anyscale]

latency by outcome

histogram_quantile(0.9, sum(rate(ray_serve_haproxy_ingress_router_latency_ms_bucket{route=~".*",route!~"/-/.*",application=~".*",deployment=~".*",replica=~".*",ray_io_cluster=~".*",ClusterId=~"ses_fpdiazhqrzpeakwf1aw4zmrdrs", outcome=~".+"}[5m])) by (application, outcome, le))

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

^{Reviewed by Cursor Bugbot for commit ded3226. Configure here.}

[eicherseiji]

🙌

[akyang-anyscale]

total request counter + other metrics

sum(rate(ray_serve_haproxy_ingress_router_requests_total{route=~".*",route!~"/-/.*",application=~".*",deployment=~".*",replica=~".*",ray_io_cluster=~".*",ClusterId=~"ses_fpdiazhqrzpeakwf1aw4zmrdrs"}[5m])) by (application, deployment, replica)

sum(rate(ray_serve_haproxy_ingress_router_truncations_total{route=~".*",route!~"/-/.*",application=~".*",deployment=~".*",replica=~".*",ray_io_cluster=~".*",ClusterId=~"ses_fpdiazhqrzpeakwf1aw4zmrdrs"}[5m])) by (application, deployment, replica)

sum(rate(ray_serve_haproxy_ingress_router_failures_total{route=~".*",route!~"/-/.*",application=~".*",deployment=~".*",replica=~".*",ray_io_cluster=~".*",ClusterId=~"ses_fpdiazhqrzpeakwf1aw4zmrdrs"}[5m])) by (application, reason, replica)

sum(rate(ray_serve_haproxy_ingress_router_server_mismatch_total{route=~".*",route!~"/-/.*",application=~".*",deployment=~".*",replica=~".*",ray_io_cluster=~".*",ClusterId=~"ses_fpdiazhqrzpeakwf1aw4zmrdrs"}[5m])) by (application, replica)

[kouroshHakha]

Overall this is clean work — the syslog-over-dgram approach is lightweight, the collector cleanly separates socket lifecycle from metric ownership, and the metrics-disabled path genuinely has zero overhead (no log target, no Lua timing calls, no socket). Two things below worth addressing before merge.

Note

This review was co-written with AI assistance (Claude Code).

[kouroshHakha]

HAProxy log directive in a section overrides inherited log global, silently dropping standard access logs when metrics are enabled.

HAProxy docs: "If at least one log directive appears in a proxy section, it takes precedence over and replaces all inherited log directives." The defaults section has log global, but once frontend http_frontend gains its own log {{ metrics_socket_path }} ... line, it no longer inherits log global — all standard syslog logging (to /dev/log and 127.0.0.1:{{ config.syslog_port }}) stops for this frontend.

The comment above says "the inherited rfc3164 targets do not include the SD section, so their byte stream is unchanged" — but the problem is the rfc3164 targets won't be reached at all when metrics are enabled.

Fix: add log global in the metrics block so both targets are active:

    {%- if ingress_request_router_metrics_enabled and has_ingress_request_router %}
+   log global
    log {{ metrics_socket_path }} len 8192 format rfc5424 local1 info
    log-format-sd "..."
    {%- endif %}

[kouroshHakha]

_metrics_collector.close() is skipped when _haproxy.stop() raises.

If stop() throws, the collector's dgram transport and socket file are leaked. Moving collector cleanup to a finally block fixes it:

try:
    await self._haproxy.stop()
finally:
    if self._metrics_collector is not None:
        self._metrics_collector.close()
        self._metrics_collector = None