feat: add streaming endpoint for inference requests by doringeman · Pull Request #157 · docker/model-runner

doringeman · 2025-09-12T13:35:25Z

Add a streaming endpoint for the requests+responses recorded by the OpenAIRecorder.

MODEL_RUNNER_PORT=8080 make run

$ curl -N http://localhost:8080/engines/requests/stream\?include_existing\=true
event: existing_request
data: {"id":"sha256:354bf30d0aa3af413d2aa5ae4f23c66d78980072d1e07a5b0d776e9606a2f0b9_1757684038418214000","model":"ai/smollm2","method":"POST","url":"/engines/v1/chat/completions","request":"{\"model\":\"ai/smollm2\",\"messages\":[{\"role\":\"user\",\"content\":\"hi\"}],\"stream\":true}","response":"{\"choices\":[{\"finish_reason\":\"stop\",\"index\":0,\"message\":{\"content\":\"Hello, I am a helpful assistant. How can I assist you today?\",\"role\":\"assistant\"}}],\"created\":1757684038,\"id\":\"chatcmpl-o6igq8UbWNbQG21cJloEbdd1TVaBqIOU\",\"model\":\"ai/smollm2\",\"object\":\"chat.completion\",\"system_fingerprint\":\"b1-c610b6c\",\"timings\":{\"predicted_ms\":84.945,\"predicted_n\":16,\"predicted_per_second\":188.35717228795104,\"predicted_per_token_ms\":5.3090625,\"prompt_ms\":35.378,\"prompt_n\":31,\"prompt_per_second\":876.250777319238,\"prompt_per_token_ms\":1.141225806451613},\"usage\":{\"completion_tokens\":16,\"prompt_tokens\":31,\"total_tokens\":47}}","timestamp":1757684038,"status_code":200,"user_agent":"docker-model-cli/dev"}

event: connected
data: {"status": "connected"}

event: new_request
data: {"id":"sha256:354bf30d0aa3af413d2aa5ae4f23c66d78980072d1e07a5b0d776e9606a2f0b9_1757684048095779000","model":"ai/smollm2","method":"POST","url":"/engines/v1/chat/completions","request":"{\"model\":\"ai/smollm2\",\"messages\":[{\"role\":\"user\",\"content\":\"hi\"}],\"stream\":true}","response":"{\"choices\":[{\"finish_reason\":\"stop\",\"index\":0,\"message\":{\"content\":\"Hello, hello! Hi there!\",\"role\":\"assistant\"}}],\"created\":1757684048,\"id\":\"chatcmpl-UV3PfQhZRAvhfsefXuaHsAMqM3sm0EZM\",\"model\":\"ai/smollm2\",\"object\":\"chat.completion\",\"system_fingerprint\":\"b1-c610b6c\",\"timings\":{\"predicted_ms\":40.469,\"predicted_n\":8,\"predicted_per_second\":197.68217648076305,\"predicted_per_token_ms\":5.058625,\"prompt_ms\":43.48,\"prompt_n\":1,\"prompt_per_second\":22.999080036798528,\"prompt_per_token_ms\":43.48},\"usage\":{\"completion_tokens\":8,\"prompt_tokens\":31,\"total_tokens\":39}}","timestamp":1757684048,"status_code":200,"user_agent":"docker-model-cli/dev"}

You can also interact with it using docker-archive-public/docker.model-cli#151.

Signed-off-by: Dorin Geman <dorin.geman@docker.com>

ilopezluna · 2025-09-12T15:10:28Z

 	m["POST "+inference.InferencePrefix+"/{backend}/_configure"] = s.Configure
 	m["POST "+inference.InferencePrefix+"/_configure"] = s.Configure
 	m["GET "+inference.InferencePrefix+"/requests"] = s.openAIRecorder.GetRecordsHandler()
+	m["GET "+inference.InferencePrefix+"/requests/stream"] = s.openAIRecorder.StreamRequestsHandler()


have you considered using same /requests endpoint?and return streams in case of "Content-Type": "text/event-stream",?

Right, using an Accept: text/event-stream in the GET request might provide an easy differentiator (with no header defaulting to one-shot application/json.

Ah, it looks like you're already sending that in the CLI, so yeah, that might keep the API a bit cleaner.

Good idea! Implemented in 6ad7d25. Thanks!

Copilot

Pull Request Overview

This PR adds a streaming endpoint for inference requests using Server-Sent Events (SSE) to the OpenAI recorder functionality. The change enables real-time monitoring of requests and responses recorded by the OpenAIRecorder.

Key changes:

Added SSE streaming support to the existing /engines/requests endpoint
Implemented subscriber management for broadcasting new request/response pairs
Added support for including existing records when subscribing to the stream

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-09-15T14:13:05Z

+
+	// Create subscriber channel.
+	subscriberID := fmt.Sprintf("sub_%d", time.Now().UnixNano())
+	ch := make(chan *RequestResponsePair, 100)


The buffer size of 100 is a magic number. Consider defining this as a named constant (e.g., subscriberChannelBufferSize) to make the code more maintainable and allow for easier configuration.

Differentiate regular and streaming based on the Accept Header. Signed-off-by: Dorin Geman <dorin.geman@docker.com>

Signed-off-by: Dorin Geman <dorin.geman@docker.com>

Copilot

Pull Request Overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

…tion Explains that modelRecords is expected to have size 1 due to how broadcastToSubscribers is called, avoiding the need for a second model config query. Signed-off-by: Dorin Geman <dorin.geman@docker.com>

Signed-off-by: Dorin Geman <dorin.geman@docker.com>

ericcurtin · 2025-09-16T12:36:16Z

@doringeman do you think some of the review comments are worth addressing? If not please resolve.

Signed-off-by: Dorin Geman <dorin.geman@docker.com>

doringeman · 2025-09-16T12:41:54Z

@ericcurtin addressed them in the last 3 commits. Resolved.

ilopezluna

I added a question, not to change the code, but just to understand the convention.

ilopezluna · 2025-09-16T12:59:19Z

+		allRecords := r.getAllRecords()
+		if allRecords == nil {
+			// No records found.
+			http.Error(w, "No records found", http.StatusNotFound)
+			return
+		}


There’s no need to change it, this is actually the current behavior. I’m just curious whether this is considered idiomatic in Go.
In Java, for example, a getAllRecords() method would return an empty array if no records are found, and the request would still return a 200 status code, which makes sense since the request itself succeeded.
In our case, though, the HTTP status code is being used to reflect business logic: a 404 means "no records found."
This morning, I got confused while debugging because I interpreted the 404 Not Found as if the endpoint itself didn’t exist, rather than simply meaning there were no records.

Actually you're right, it's more REST idiomatic to return an empty list and 200.
The 404 doesn't really make sense here as the endpoint and the resource exist (although it's empty).

Addressed in 296301f. Thank you! 🙌

ilopezluna · 2025-09-16T13:00:17Z

+
+	// Optional: Send existing records first.
+	model := req.URL.Query().Get("model")
+	if includeExisting := req.URL.Query().Get("include_existing"); includeExisting == "true" {


Signed-off-by: Dorin Geman <dorin.geman@docker.com>

Copilot

Pull Request Overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

feat: add streaming endpoint for inference requests

033cf83

Signed-off-by: Dorin Geman <dorin.geman@docker.com>

doringeman mentioned this pull request Sep 12, 2025

Add requests monitoring docker-archive-public/docker.model-cli#151

Merged

ericcurtin approved these changes Sep 12, 2025

View reviewed changes

ilopezluna reviewed Sep 12, 2025

View reviewed changes

ilopezluna approved these changes Sep 15, 2025

View reviewed changes

ericcurtin requested a review from Copilot September 15, 2025 14:12

Copilot AI reviewed Sep 15, 2025

View reviewed changes

refactor: combine the requests endpoints

a0ddf2e

Differentiate regular and streaming based on the Accept Header. Signed-off-by: Dorin Geman <dorin.geman@docker.com>

doringeman force-pushed the requests-sse branch from 6ad7d25 to a0ddf2e Compare September 15, 2025 14:27

refactor: on streaming return the same struct as on regular

d61cffb

Signed-off-by: Dorin Geman <dorin.geman@docker.com>

doringeman requested a review from Copilot September 16, 2025 12:19

Copilot AI reviewed Sep 16, 2025

View reviewed changes

Comment thread pkg/metrics/openai_recorder.go

Comment thread pkg/metrics/openai_recorder.go

Comment thread pkg/metrics/openai_recorder.go Outdated

doringeman added 2 commits September 16, 2025 15:29

OpenAIRecorder: add clarifying comment about modelRecords size assump…

98361c1

…tion Explains that modelRecords is expected to have size 1 due to how broadcastToSubscribers is called, avoiding the need for a second model config query. Signed-off-by: Dorin Geman <dorin.geman@docker.com>

OpenAIRecorder: send error event for JSON marshaling failure

78f30f9

Signed-off-by: Dorin Geman <dorin.geman@docker.com>

OpenAIRecorder: add error handling and logging for SSE response writes

2fca1f5

Signed-off-by: Dorin Geman <dorin.geman@docker.com>

ilopezluna approved these changes Sep 16, 2025

View reviewed changes

OpenAIRecorder: return an empty list instead of 404

296301f

Signed-off-by: Dorin Geman <dorin.geman@docker.com>

Copilot AI review requested due to automatic review settings September 16, 2025 13:14

Copilot AI reviewed Sep 16, 2025

View reviewed changes

Comment thread pkg/metrics/openai_recorder.go

Comment thread pkg/metrics/openai_recorder.go

Comment thread pkg/metrics/openai_recorder.go

doringeman merged commit 14e9c6c into docker:main Sep 16, 2025
1 check passed

Conversation

doringeman commented Sep 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI Sep 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ericcurtin commented Sep 16, 2025

Uh oh!

doringeman commented Sep 16, 2025

Uh oh!

ilopezluna left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

doringeman commented Sep 12, 2025 •

edited

Loading