Repository navigation
Count the requests the fleet gateway has open to each model - #548
dennis-upbound wants to merge 1 commit into
Conversation
|
Docs preview: https://modelplane-docs-pr-548.vercel.app (ready once the site's Content workflow finishes) |
Nothing in the modelplane_* surface showed what callers were waiting on. An engine whose callers disconnected without it being told kept decoding: on one deployment the engine held 29 requests while the gateway had 2 open, and new requests queued behind output nobody read. Both engine gauges were there; nothing showed the gap. Envoy counts the requests it has open upstream in envoy_cluster_upstream_rq_active, per cluster it built, and the collector already scraped it from every gateway and dropped it. It's now modelplane_frontend_requests_active, kept only where it means what callers are waiting on: on the fleet's inference-gateway, not each inference cluster's cluster-gateway, which counts the same requests again, and only for the cluster of a ModelService's route, not Envoy's own clusters or routes such as Grafana's. Envoy names that cluster httproute/<namespace>/<service>/rule/0, so the series carries namespace, the same one an engine's series carry, and service. Relabeling at scrape time does this, because both drops are on labels a MetricMapping can't select on. The telemetry docs show how to compare it with an engine's running and waiting requests. Envoy's count drops when a caller leaves, which the 2 against 29 shows, so the comparison catches the case it's for. Towards modelplaneai#476. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Dennis Ramdass <dennis@upbound.io>
86c98cf to
03ebf1c
Compare
haarchri
left a comment
There was a problem hiding this comment.
overall LGTM, foreign Envoy series can leak into the new metric please check my comments rest are nits
| # collector keeps for the fleet gateway's model routes only (see | ||
| # collector._IN_FLIGHT). The engines' running and waiting requests well above | ||
| # it, for minutes, is work no caller is waiting for. | ||
| _GATEWAY_PROXY = { |
There was a problem hiding this comment.
This mapping isn’t scoped to a job, but the filtering only runs in the modelplane-gateway job. Any other pod that exposes envoy_cluster_upstream_rq_active is picked up by modelplane-substrate ?
That includes an Istio sidecar or other non-Envoy-Gateway proxy. Its series will be renamed to modelplane_frontend_requests_active with its raw envoy_cluster_name and the pod’s namespace, and none of the drops apply.
Before this PR, filter/modelplane dropped those series. Now they leave the cluster under a modelplane name. Could we drop envoy_cluster_upstream_rq_active in the other jobs metric_relabel_configs (at least substrate)? Another option is to have the gateway job rename the kept series to a name only it produces, so the mapping can’t match anything else. A test case for a substrate pod emitting this metric would pin it.
| # AIGatewayRoute, named for the service in the team's mirrored namespace, and | ||
| # rule 0, the one rule compose_route gives it. The namespace is the one an | ||
| # engine's series carry. | ||
| _MODEL_ROUTE = r"httproute/(mp-[^/]+)/([^/]+)/rule/0" |
There was a problem hiding this comment.
rule/0 ties this to compose_route in compose-model-route emitting exactly one rule. If someone adds a second rule there (for example a fallback match), this silently stops counting part of the traffic, and nothing fails. Can we add a comment in compose_route pointing back here, so the coupling is visible from both sides?
| ] | ||
|
|
||
|
|
||
| def _in_flight_relabels() -> list[dict[str, Any]]: |
There was a problem hiding this comment.
Nit: the ordering matters here.
The drop has to run before the namespace/service rules, and the cleanup has to run last. A one-line comment would help the next person who edits this. Also, it’s worth stating explicitly that __tmp_model has to be cleared by hand because metric relabeling doesn’t strip __-prefixed labels the way target relabeling does?
| # series - the ones an SLO is written against - never arrive. | ||
| *_annotated_path(), | ||
| { | ||
| "source_labels": ["__meta_kubernetes_pod_label_gateway_envoyproxy_io_owning_gateway_name"], |
There was a problem hiding this comment.
The gateway target label now lands on every series from this job, not only on rq_active. That’s fine today, because everything else from Envoy’s admin port is dropped by filter/modelplane. But if someone later maps another Envoy stat, it will carry gateway unexpectedly. Maybe add a short note, or clear gateway for all series at the end of the metric relabels instead of only for _IN_FLIGHT?
| assert got == case.want, case.reason | ||
|
|
||
|
|
||
| IN_FLIGHT_CASES = [ |
There was a problem hiding this comment.
Please add a case for httproute/mp-ml-team-51733/qwen/rule/1 on inference-gateway with want=None. That pins the end anchor and the rule-0 assumption.
| for r in j["metric_relabel_configs"] | ||
| if r.get("target_label") == "__tmp_model" and r.get("replacement") == "true" | ||
| ) | ||
| matched = re.fullmatch(rule["regex"], f"envoy_cluster_upstream_rq_active;{case.cluster};{case.gateway}") |
There was a problem hiding this comment.
Nit: This only exercises the first rule’s regex. The drop, the $1/$2 extraction, and the cleanup are only checked by comparing config literals, which doesn’t prove the series comes out with exactly {namespace, service}. A small helper that applies the relabel list to a label dict would test the whole chain.
| carry the model instead: `gen_ai_original_model` is the model the caller sent, | ||
| `<namespace>/<service>`, and `gen_ai_request_model` is the deployment the gateway | ||
| sent the request to, `<namespace>/<deployment>`. | ||
| `modelplane_frontend_requests_active` is counted per `ModelService` instead. It |
There was a problem hiding this comment.
can we add here a bit more info? this namespace is the mirrored mp-… namespace, while the namespace inside gen_ai_original_model on the other modelplane_frontend_* series is the team’s own namespace. Someone trying to join this metric with the latency series by namespace will hit that mismatch.
| `ModelService` of the same name fronts, and that serves each request on one engine: | ||
|
|
||
| ```promql | ||
| sum by (namespace) (modelplane_requests_running{deployment="qwen3-8b"} + modelplane_requests_waiting{deployment="qwen3-8b"}) |
There was a problem hiding this comment.
running + waiting inside the sum uses one-to-one vector matching. If one engine is missing either series for a scrape (for example right after a restart), it drops out of the result. sum by (namespace) (running) + sum by (namespace) (waiting) avoids that.
| - sum by (namespace) (modelplane_frontend_requests_active{service="qwen3-8b"}) | ||
| ``` | ||
|
|
||
| If the difference is well above 0 for minutes, the engine is doing work no caller |
There was a problem hiding this comment.
is a bit vague for someone writing an alert. Can we suggest a concrete starting point, like > 5 for 5m, and mention that the two sides are scraped at different moments, so small transient differences are expected?
Description of your changes
Towards #476.
Nothing in the
modelplane_*surface showed what callers are waiting on. When callers disconnect and the engine isn't told, it keeps decoding. On one deployment the engine held 29 requests while the gateway had 2 open, and new requests queued behind output nobody read. Both engine gauges were there, and nothing showed the gap. The direction is in #476 (comment).Envoy counts the requests it has open upstream in
envoy_cluster_upstream_rq_active, per cluster it builds, and the collector already scraped it from every gateway pod and dropped it. It's nowmodelplane_frontend_requests_active, kept only where it counts what callers are waiting on:inference-gateway. Each inference cluster'scluster-gatewaycounts the same requests again, so its copy is dropped, and a sum counts each request once.ModelService's route. Envoy's own clusters, and routes such as Grafana's and the OTLP receiver's, are dropped.Envoy names that cluster
httproute/<namespace>/<service>/rule/0, after compose-model-route's AIGatewayRoute. The series carriesnamespace, the same label an engine's series carry, andservice. The work is done bymetric_relabel_configson the gateway job, at scrape time, because both drops are on labels aMetricMappingcan't select on.Envoy's count drops when a caller leaves, as the 2 against 29 shows. So comparing it with an engine's running and waiting requests catches the case this is for. The telemetry docs show the query.
Run through the collector with a real
/stats/prometheusfrom an Envoy Gateway v1.8.4 fleet gateway, the version Modelplane pins, served once as the fleet gateway and once as a cluster gateway. Only the fleet gateway's three model routes came out, each withnamespaceandserviceand no Envoy labels.I have:
nix flake check(or./nix.sh flake check) and made sure it passes.git commit -s.🤖 Generated with Claude Code