Add EPP request/response processing latency metrics by satyamg1620 · Pull Request #1658 · llm-d/llm-d-router

satyamg1620 · 2026-06-15T12:55:04Z

What type of PR is this?
/kind feature

What this PR does / why we need it:
Adds two EPP latency metrics from #1553 to expose EPP processing overhead:

llm_d_epp_request_processing_duration_seconds — from request receipt until the request body has been handled; includes admission control.
llm_d_epp_response_processing_duration_seconds — per-chunk handler sum for streaming, header-to-completion span for non-streaming; excludes model-server generation time.

Which issue(s) this PR fixes:

Fixes #1553

Release note (write NONE if no user-facing change):

Add llm_d_epp_request_processing_duration_seconds and llm_d_epp_response_processing_duration_seconds metrics measuring EPP request and response processing overhead.

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds end-to-end latency instrumentation for EPP request/response processing, including new Prometheus histograms and integration points in request orchestration and streaming response handling.

Changes:

Instrument request processing latency in Director.HandleRequest, subtracting flow-control admission time.
Accumulate and record response processing latency across response header/body handlers (streaming and non-streaming).
Register/reset new histograms and add golden-file tests for both metrics.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
pkg/epp/requestcontrol/director.go	Adds request processing latency timing and subtracts admission duration.
pkg/epp/handlers/server.go	Accumulates response handler processing time and records a histogram observation at response completion.
pkg/epp/metrics/llm_d_router_metrics.go	Defines new Prometheus histogram vectors for request/response processing durations.
pkg/epp/metrics/metrics.go	Registers/resets new histograms and exposes recording helper functions.
pkg/epp/metrics/metrics_test.go	Adds tests that compare gathered metrics to testdata fixtures.
pkg/epp/metrics/testdata/llm_d_request_processing_duration_seconds_metric	Golden output for request processing histogram.
pkg/epp/metrics/testdata/llm_d_response_processing_duration_seconds_metric	Golden output for response processing histogram.

+	start := time.Now()
+	var admissionWait time.Duration
+	// request_processing_duration captures EPP's own processing cost, so the
+	// flow-control admission wait (tracked separately by
+	// flow_control_request_queue_duration_seconds) is subtracted out.
+	defer func() { metrics.RecordRequestProcessingLatency(time.Since(start) - admissionWait) }()


+	admitStart := time.Now()
+	admitErr := d.admissionController.Admit(ctx, reqCtx, priority)
+	admissionWait = time.Since(admitStart)


+	// request_processing_duration captures EPP's own processing cost, so the
+	// flow-control admission wait (tracked separately by
+	// flow_control_request_queue_duration_seconds) is subtracted out.
+	defer func() { metrics.RecordRequestProcessingLatency(time.Since(start) - admissionWait) }()


+				reqCtx.responseProcessingDuration += time.Since(respBodyStart)
+				if endOfStream {
+					metrics.RecordResponseProcessingLatency(reqCtx.responseProcessingDuration)
+				}


satyamg1620 · 2026-06-17T06:27:39Z

@liu-cong can you please review this PR

liu-cong · 2026-06-17T17:23:16Z

@liu-cong can you please review this PR

Sorry for the delay. I am focusing on the release right now. Will try to get to this later this week if everything goes well on the release.

satyamg1620 · 2026-06-17T17:34:58Z

Sorry for the delay. I am focusing on the release right now. Will try to get to this later this week if everything goes well on the release.

Sure. Thanks @liu-cong

github-actions · 2026-07-09T01:40:44Z

This PR is marked as stale after 21d of inactivity. After an additional 14d of inactivity (7d to become rotten, then 7d more), it will be closed. To prevent this PR from being closed, add a comment or remove the lifecycle/stale label.

elevran

Two CI failures on this PR. The test job fails because the new unit tests assert the wrong subsystem constant.

The PR title and the release-note block both also advertise the wrong metric name and must be fixed.

Release-note should read:

Add `llm_d_epp_request_processing_duration_seconds` and `llm_d_epp_response_processing_duration_seconds` metrics measuring EPP request and response processing overhead.

Can be done here or in a follow up PR (please open a tracking issue if deferring):

docs/metrics.md does not list the new metrics. Since the PR description treats them as user-facing, a one-line entry under the existing llm_d_epp_* section would keep the catalog accurate.
The e2e-gaie preset in test/e2e/epp/utils_test.go:260-289 does not assert these metrics either, so they are not exercised by the e2e-gaie suite. Worth a follow-up to add them to the preset.

elevran · 2026-07-20T07:54:23Z

+		t.Fatal(err)
+	}
+	defer want.Close()
+	if err := promtestutil.GatherAndCompare(metrics.Registry, want, "llm_d_router_epp_request_processing_duration_seconds"); err != nil {


This assertion looks for the wrong metric name. LLMDRouterEndpointPickerSubsystem is "llm_d_epp" (line 28 of llm_d_router_metrics.go), so the histogram is emitted as llm_d_epp_request_processing_duration_seconds, matching the rest of the package (llm_d_epp_request_total, llm_d_epp_request_error_total, etc.). Drop the _router_ segment here and in the testdata filename + fixture # HELP/# TYPE/metric lines.

Suggested change

if err := promtestutil.GatherAndCompare(metrics.Registry, want, "llm_d_router_epp_request_processing_duration_seconds"); err != nil {

if err := promtestutil.GatherAndCompare(metrics.Registry, want, "llm_d_epp_request_processing_duration_seconds"); err != nil {

elevran · 2026-07-20T07:54:23Z

+		t.Fatal(err)
+	}
+	defer want.Close()
+	if err := promtestutil.GatherAndCompare(metrics.Registry, want, "llm_d_router_epp_response_processing_duration_seconds"); err != nil {


Same off-by-one as the previous comment: drop _router_ so the assertion matches the emitted name llm_d_epp_response_processing_duration_seconds. Also rename the testdata file and update its contents.

Suggested change

if err := promtestutil.GatherAndCompare(metrics.Registry, want, "llm_d_router_epp_response_processing_duration_seconds"); err != nil {

if err := promtestutil.GatherAndCompare(metrics.Registry, want, "llm_d_epp_response_processing_duration_seconds"); err != nil {

elevran · 2026-07-20T10:02:32Z

Please be sure to fix the all relevant fields of the new metrics (names, Help text, unit tests, and the testdata fixtures) - they all use the wrong prefix,

satyamg1620 · 2026-07-21T21:11:52Z

Thanks @liu-cong. Two questions before I rewrite this.

Request metric. Happy to move it to server.go, RequestReceivedTimestamp through the end of the RequestBody case. But that window swallows director.HandleRequest, so it swallows admission control too. Right now I subtract that (director.go#L304-L306) because under flow control Admit is EnqueueAndWait and the queue wait would dominate the histogram. Do you want it still excluded after the move, or just the raw span?

If it stays excluded, I'd like to fix what Copilot caught while I'm in there. I subtract the whole Admit call, but LegacyAdmissionController.Admit doesn't queue at all, it just runs saturation detection. Flow control is off by default, so today we subtract real EPP work out of an EPP-overhead metric. I think the cleanest fix is Admit returning its own queue wait (zero for legacy). Can you please suggest

Response metric. On #1553 you said accumulated sum makes sense for streaming, but the inline comments here describe a single elapsed span. I read that as: non-streaming becomes one span headers-to-finishResponse, streaming keeps the sum, since an elapsed span there would just re-measure generation time. Right?

liu-cong · 2026-07-21T21:27:27Z

the queue wait would dominate the histogram

This is expected by the definition of this metric. The queue wait time only applies when queuing happens.

I read that as: non-streaming becomes one span headers-to-finishResponse, streaming keeps the sum, since an elapsed span there would just re-measure generation time.

Yeah for streaming it's a bit complex, we will need to measure the sum of per-chunk EPP response processing latency.

satyamg1620 · 2026-07-21T23:44:43Z

@liu-cong @elevran can you please review this PR

Signed-off-by: satyamg1620 <Satyam.Gupta.3@ibm.com>

Copilot AI review requested due to automatic review settings June 15, 2026 12:55

satyamg1620 requested a review from a team as a code owner June 15, 2026 12:55

satyamg1620 requested review from elevran and vMaroon June 15, 2026 12:55

Copilot AI reviewed Jun 15, 2026

View reviewed changes

satyamg1620 force-pushed the epp-latency-metrics branch from 6d42111 to 0682683 Compare June 17, 2026 06:24

github-actions Bot added the lifecycle/stale label Jul 9, 2026

elevran reviewed Jul 20, 2026

View reviewed changes

elevran removed the lifecycle/stale label Jul 20, 2026

satyamg1620 force-pushed the epp-latency-metrics branch from a9ed745 to 8bde1d2 Compare July 20, 2026 13:22

elevran self-assigned this Jul 20, 2026

satyamg1620 force-pushed the epp-latency-metrics branch from 8bde1d2 to ac71d6b Compare July 20, 2026 15:40

satyamg1620 force-pushed the epp-latency-metrics branch from ac71d6b to c1011a2 Compare July 21, 2026 04:57

elevran removed their assignment Jul 21, 2026

satyamg1620 force-pushed the epp-latency-metrics branch from c1011a2 to f9bf0a8 Compare July 21, 2026 20:45

satyamg1620 force-pushed the epp-latency-metrics branch from f6c98f8 to 5401f66 Compare July 21, 2026 22:47

satyamg1620 requested review from elevran and liu-cong July 21, 2026 22:59

satyamg1620 force-pushed the epp-latency-metrics branch from 74907cc to 64d6565 Compare July 21, 2026 23:21

liu-cong approved these changes Jul 23, 2026

View reviewed changes

satyamg1620 force-pushed the epp-latency-metrics branch from 64d6565 to b078493 Compare July 23, 2026 04:49

liu-cong removed the kind/bug Categorizes issue or PR as related to a bug. label Jul 23, 2026

satyamg1620 requested a review from liu-cong July 23, 2026 06:45

satyamg1620 added 5 commits July 23, 2026 19:49

add EPP request/response processing latency metrics

dd88771

Signed-off-by: satyamg1620 <Satyam.Gupta.3@ibm.com>

address review feedback on processing latency metrics

6359ce1

Signed-off-by: satyamg1620 <Satyam.Gupta.3@ibm.com>

fix subsystem prefix in processing latency metric tests

e5cbb5c

Signed-off-by: satyamg1620 <Satyam.Gupta.3@ibm.com>

Measure EPP processing latency at the handler boundary

59a5cbf

Signed-off-by: satyamg1620 <Satyam.Gupta.3@ibm.com>

Assert EPP processing latency metrics in e2e preset

93a9bc1

Signed-off-by: satyamg1620 <Satyam.Gupta.3@ibm.com>

satyamg1620 force-pushed the epp-latency-metrics branch from b078493 to 93a9bc1 Compare July 23, 2026 14:19

liu-cong merged commit 3a31761 into llm-d:main Jul 24, 2026
29 checks passed

llm-d-router-release-notes Bot mentioned this pull request Jul 24, 2026

docs: add release-note fragment for PR #1658 #2163

Open

	if err := promtestutil.GatherAndCompare(metrics.Registry, want, "llm_d_router_epp_request_processing_duration_seconds"); err != nil {
	if err := promtestutil.GatherAndCompare(metrics.Registry, want, "llm_d_epp_request_processing_duration_seconds"); err != nil {

Uh oh!

Conversation

satyamg1620 commented Jun 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

satyamg1620 commented Jun 17, 2026

Uh oh!

liu-cong commented Jun 17, 2026

Uh oh!

satyamg1620 commented Jun 17, 2026

Uh oh!

github-actions Bot commented Jul 9, 2026

Uh oh!

elevran left a comment

Choose a reason for hiding this comment

Uh oh!

elevran Jul 20, 2026

Choose a reason for hiding this comment

Uh oh!

elevran Jul 20, 2026

Choose a reason for hiding this comment

Uh oh!

elevran commented Jul 20, 2026

Uh oh!

satyamg1620 commented Jul 21, 2026

Uh oh!

liu-cong commented Jul 21, 2026

Uh oh!

satyamg1620 commented Jul 21, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

satyamg1620 commented Jun 15, 2026 •

edited

Loading