Skip to content

refactor(epp): unexport RequestContext protocol and response state - #2457

Merged
LukeAVanDrie merged 2 commits into
llm-d:mainfrom
LukeAVanDrie:refactor/reqctx-encapsulation
Aug 20, 2026
Merged

refactor(epp): unexport RequestContext protocol and response state#2457
LukeAVanDrie merged 2 commits into
llm-d:mainfrom
LukeAVanDrie:refactor/reqctx-encapsulation

Conversation

@LukeAVanDrie

Copy link
Copy Markdown
Contributor

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

Issue #1189 proposed splitting RequestContext into a protocol struct and a request-state struct because internal layers (director, admission control) could reach raw Envoy ext_proc machinery. Most of that machinery is already unexported: the gRPC response buffers, the streaming flag, and the processing-duration bookkeeping are package-private on main. Unexporting the remaining package-private surface enforces the boundary at compile time and makes the struct split unnecessary.

Unexported:

  • The ext_proc state machine: the RequestState field, the StreamRequestState type and its constants, and the RequestDroppedReason field.
  • Response bookkeeping with no users outside the handlers package: FirstTokenTimestamp, LastChunkReceivedTimestamp, ResponseCompleteTimestamp, ResponseSize, ResponseComplete, ResponseStatusCode, RequestRunning.

Deleted dead protocol code: the reqTrailerResp field was never assigned, so its send branch in updateStateAndSendIfNeeded could not execute, and the TrailerRequestResponsesComplete and TrailerResponseResponsesComplete constants were never referenced. The state constants are package-private and their values never leave the process, so the hand-numbered values are replaced with iota.

The struct fields are grouped by owner (request-control layers vs the handlers package), and the TODO referencing kubernetes-sigs/gateway-api-inference-extension#2082 (closed unimplemented) is replaced with a doc comment stating the visibility rule.

Exported fields with users outside the package are unchanged: Request, Response, SchedulingRequest, TargetPod, TargetEndpoint, TargetEndpointScores, the model names, ObjectiveKey, Priority, RequestReceivedTimestamp, RequestSize, Usage, ResponseBodyStarted, Parser. Out-of-tree implementations of the Director interface lose compile-time access to the newly unexported fields; no in-tree consumer reads them, and they carry ext_proc protocol state with no external use.

Observed while auditing field usage, left out of scope:

  • Priority duplicates SchedulingRequest.Objectives.Priority. Deriving it would change the priority label on error metrics for requests that fail between the two assignments.
  • ResponseBodyStarted is director-maintained state on the shared struct. Relocating it belongs with future director-state work.

Which issue(s) this PR fixes:

Fixes #1189

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

NONE

@LukeAVanDrie
LukeAVanDrie requested a review from a team as a code owner August 19, 2026 16:37
@LukeAVanDrie
LukeAVanDrie requested review from ahg-g and vMaroon August 19, 2026 16:37
The stream state machine, drop reason, and response-phase bookkeeping
fields have no users outside the handlers package. Unexporting them
gives the ext_proc protocol state a compile-time boundary, making the
struct split proposed in llm-d#1189 unnecessary. Also deletes the request
trailer response field and constants, which were never assigned.

Signed-off-by: Luke Van Drie <lukevandrie@google.com>
@LukeAVanDrie
LukeAVanDrie force-pushed the refactor/reqctx-encapsulation branch from c807938 to 60c6161 Compare August 19, 2026 16:41
@github-actions github-actions Bot added kind/cleanup size/L Denotes a PR that changes 100-499 lines, ignoring generated files. area/epp labels Aug 19, 2026
@ahg-g

ahg-g commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

/lgtm
looks good, pls rebase.

Resolves the RequestContext conflict with the StreamedEvents field added on
main: the field stays exported because the director reads it, and it joins the
request-control group next to Usage.

Signed-off-by: Luke Van Drie <lukevandrie@google.com>
@LukeAVanDrie

Copy link
Copy Markdown
Contributor Author

/lgtm looks good, pls rebase.

thanks did a merge commit, though the conflict was only a single line from another one of my PRs that merged earlier

@LukeAVanDrie
LukeAVanDrie enabled auto-merge (squash) August 20, 2026 06:54

@elevran elevran left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ahg-g approved

@LukeAVanDrie
LukeAVanDrie merged commit 227f6ca into llm-d:main Aug 20, 2026
40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/epp kind/cleanup size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Refactor] Decouple RequestContext Envoy Protocol Fields from EPP Request State

3 participants