-
Notifications
You must be signed in to change notification settings - Fork 209
in memory request logging and observability #503
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -40,6 +40,7 @@ type ProxyManager struct { | |
| muxLogger *LogMonitor | ||
|
|
||
| metricsMonitor *metricsMonitor | ||
| requestMonitor *requestMonitor | ||
|
|
||
| processGroups map[string]*ProcessGroup | ||
|
|
||
|
|
@@ -152,6 +153,7 @@ func New(proxyConfig config.Config) *ProxyManager { | |
| upstreamLogger: upstreamLogger, | ||
|
|
||
| metricsMonitor: newMetricsMonitor(proxyLogger, maxMetrics), | ||
| requestMonitor: newRequestMonitor(maxMetrics), | ||
|
|
||
| processGroups: make(map[string]*ProcessGroup), | ||
|
|
||
|
|
@@ -598,15 +600,29 @@ func (pm *ProxyManager) proxyToUpstream(c *gin.Context) { | |
| originalPath := c.Request.URL.Path | ||
| c.Request.URL.Path = remainingPath | ||
|
|
||
| var requestBody string | ||
| if c.Request.ContentLength > 0 && c.Request.ContentLength < 1024*1024 { // Only capture small bodies | ||
| bodyBytes, err := io.ReadAll(c.Request.Body) | ||
| if err != nil { | ||
| pm.proxyLogger.Errorf("Error reading request body for recording: %v", err) | ||
| } else { | ||
| c.Request.Body = io.NopCloser(bytes.NewBuffer(bodyBytes)) | ||
| requestBody = string(bodyBytes) | ||
| } | ||
| } | ||
|
|
||
| recorder, done := pm.recordRequest(c, modelID, requestBody) | ||
| defer done() | ||
|
|
||
| // attempt to record metrics if it is a POST request | ||
| if pm.metricsMonitor != nil && c.Request.Method == "POST" { | ||
| if err := pm.metricsMonitor.wrapHandler(modelID, c.Writer, c.Request, processGroup.ProxyRequest); err != nil { | ||
| if err := pm.metricsMonitor.wrapHandler(modelID, recorder, c.Request, processGroup.ProxyRequest); err != nil { | ||
| pm.sendErrorResponse(c, http.StatusInternalServerError, fmt.Sprintf("error proxying metrics wrapped request: %s", err.Error())) | ||
| pm.proxyLogger.Errorf("Error proxying wrapped upstream request for model %s, path=%s", modelID, originalPath) | ||
| pm.proxyLogger.Errorf("Error proxying metrics wrapped request for model %s, path=%s", modelID, originalPath) | ||
| return | ||
| } | ||
| } else { | ||
| if err := processGroup.ProxyRequest(modelID, c.Writer, c.Request); err != nil { | ||
| if err := processGroup.ProxyRequest(modelID, recorder, c.Request); err != nil { | ||
| pm.sendErrorResponse(c, http.StatusInternalServerError, fmt.Sprintf("error proxying request: %s", err.Error())) | ||
| pm.proxyLogger.Errorf("Error proxying upstream request for model %s, path=%s", modelID, originalPath) | ||
| return | ||
|
|
@@ -720,20 +736,23 @@ func (pm *ProxyManager) proxyInferenceHandler(c *gin.Context) { | |
| c.Request.Header.Set("content-length", strconv.Itoa(len(bodyBytes))) | ||
| c.Request.ContentLength = int64(len(bodyBytes)) | ||
|
|
||
| recorder, done := pm.recordRequest(c, modelID, string(bodyBytes)) | ||
| defer done() | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| // issue #366 extract values that downstream handlers may need | ||
| isStreaming := gjson.GetBytes(bodyBytes, "stream").Bool() | ||
| ctx := context.WithValue(c.Request.Context(), proxyCtxKey("streaming"), isStreaming) | ||
| ctx = context.WithValue(ctx, proxyCtxKey("model"), modelID) | ||
| c.Request = c.Request.WithContext(ctx) | ||
|
|
||
| if pm.metricsMonitor != nil && c.Request.Method == "POST" { | ||
| if err := pm.metricsMonitor.wrapHandler(modelID, c.Writer, c.Request, nextHandler); err != nil { | ||
| if err := pm.metricsMonitor.wrapHandler(modelID, recorder, c.Request, nextHandler); err != nil { | ||
| pm.sendErrorResponse(c, http.StatusInternalServerError, fmt.Sprintf("error proxying metrics wrapped request: %s", err.Error())) | ||
| pm.proxyLogger.Errorf("Error Proxying Metrics Wrapped Request model %s", modelID) | ||
| return | ||
| } | ||
| } else { | ||
| if err := nextHandler(modelID, c.Writer, c.Request); err != nil { | ||
| if err := nextHandler(modelID, recorder, c.Request); err != nil { | ||
| pm.sendErrorResponse(c, http.StatusInternalServerError, fmt.Sprintf("error proxying request: %s", err.Error())) | ||
| pm.proxyLogger.Errorf("Error Proxying Request for model %s", modelID) | ||
| return | ||
|
|
@@ -862,7 +881,10 @@ func (pm *ProxyManager) proxyOAIPostFormHandler(c *gin.Context) { | |
| modifiedReq.ContentLength = int64(requestBuffer.Len()) | ||
|
|
||
| // Use the modified request for proxying | ||
| if err := nextHandler(modelID, c.Writer, modifiedReq); err != nil { | ||
| recorder, done := pm.recordRequest(c, modelID, "") | ||
| defer done() | ||
|
|
||
| if err := nextHandler(modelID, recorder, modifiedReq); err != nil { | ||
| pm.sendErrorResponse(c, http.StatusInternalServerError, fmt.Sprintf("error proxying request: %s", err.Error())) | ||
| pm.proxyLogger.Errorf("Error Proxying Request for model %s", modelID) | ||
| return | ||
|
|
@@ -899,7 +921,10 @@ func (pm *ProxyManager) proxyGETModelHandler(c *gin.Context) { | |
| return | ||
| } | ||
|
|
||
| if err := nextHandler(modelID, c.Writer, c.Request); err != nil { | ||
| recorder, done := pm.recordRequest(c, modelID, "") | ||
| defer done() | ||
|
|
||
| if err := nextHandler(modelID, recorder, c.Request); err != nil { | ||
| pm.sendErrorResponse(c, http.StatusInternalServerError, fmt.Sprintf("error proxying request: %s", err.Error())) | ||
| pm.proxyLogger.Errorf("Error Proxying GET Request for model %s", modelID) | ||
| return | ||
|
|
@@ -1026,3 +1051,29 @@ func (pm *ProxyManager) SetVersion(buildDate string, commit string, version stri | |
| pm.commit = commit | ||
| pm.version = version | ||
| } | ||
|
|
||
| func (pm *ProxyManager) recordRequest(c *gin.Context, modelID string, requestBody string) (*responseBodyCopier, func()) { | ||
| startTime := time.Now() | ||
| requestID := pm.requestMonitor.Add(&RequestEntry{ | ||
| Timestamp: startTime, | ||
| Method: c.Request.Method, | ||
| Path: c.Request.URL.Path, | ||
| Model: modelID, | ||
| RequestBody: requestBody, | ||
| }) | ||
|
|
||
| recorder := newBodyCopier(c.Writer) | ||
| recorder.onWrite = func(b []byte) { | ||
| pm.requestMonitor.AppendResponse(requestID, string(b)) | ||
| } | ||
|
|
||
| return recorder, func() { | ||
| duration := time.Since(startTime) | ||
| respBody := "" | ||
| isStreaming := strings.Contains(recorder.Header().Get("Content-Type"), "text/event-stream") | ||
| if !isStreaming { | ||
| respBody = recorder.body.String() | ||
| } | ||
| pm.requestMonitor.Update(requestID, recorder.Status(), duration, respBody) | ||
| } | ||
|
Comment on lines
+1055
to
+1078
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# First, let's locate the responseBodyCopier implementation
cd /tmp/repo 2>/dev/null || cd .
git ls-files | head -20
rg -l "responseBodyCopier|newBodyCopier" --type goRepository: mostlygeek/llama-swap Length of output: 686 🏁 Script executed: #!/bin/bash
# Find the responseBodyCopier type definition and newBodyCopier function
rg "type responseBodyCopier|newBodyCopier" -A 5 --type goRepository: mostlygeek/llama-swap Length of output: 2756 🏁 Script executed: #!/bin/bash
# Find all methods implemented by responseBodyCopier
rg "func \(.*\*?responseBodyCopier\)" --type goRepository: mostlygeek/llama-swap Length of output: 388 🏁 Script executed: #!/bin/bash
# Check how sendErrorResponse is implemented
rg "func.*sendErrorResponse" -A 20 --type goRepository: mostlygeek/llama-swap Length of output: 1185 🏁 Script executed: #!/bin/bash
# Find where recordRequest is called
rg "recordRequest" -B 3 -A 10 --type goRepository: mostlygeek/llama-swap Length of output: 4222 🏁 Script executed: #!/bin/bash
# Find the handler/middleware where recordRequest is used
rg "func.*gin.HandlerFunc|func.*gin.Context" -B 2 -A 25 --type go | head -200Repository: mostlygeek/llama-swap Length of output: 10675 🏁 Script executed: #!/bin/bash
# Check if wrapHandler or other handlers assign c.Writer = recorder
rg "c.Writer\s*=" -B 5 -A 5 --type go proxy/Repository: mostlygeek/llama-swap Length of output: 47 🏁 Script executed: #!/bin/bash
# Check the wrapHandler signature and implementation
rg "func.*wrapHandler" -A 30 --type goRepository: mostlygeek/llama-swap Length of output: 1814 Assign the recorder to
The fix is valid: Suggested fix recorder := newBodyCopier(c.Writer)
+c.Writer = recorder
recorder.onWrite = func(b []byte) {🤖 Prompt for AI Agents |
||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -30,6 +30,8 @@ func addApiHandlers(pm *ProxyManager) { | |||||||||||||||||||||||||||||||||||||||||
| apiGroup.POST("/models/unload/*model", pm.apiUnloadSingleModelHandler) | ||||||||||||||||||||||||||||||||||||||||||
| apiGroup.GET("/events", pm.apiSendEvents) | ||||||||||||||||||||||||||||||||||||||||||
| apiGroup.GET("/metrics", pm.apiGetMetrics) | ||||||||||||||||||||||||||||||||||||||||||
| apiGroup.GET("/requests", pm.apiGetRequests) | ||||||||||||||||||||||||||||||||||||||||||
| apiGroup.GET("/requests/:id", pm.apiGetRequest) | ||||||||||||||||||||||||||||||||||||||||||
| apiGroup.GET("/version", pm.apiGetVersion) | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -105,6 +107,7 @@ const ( | |||||||||||||||||||||||||||||||||||||||||
| msgTypeModelStatus messageType = "modelStatus" | ||||||||||||||||||||||||||||||||||||||||||
| msgTypeLogData messageType = "logData" | ||||||||||||||||||||||||||||||||||||||||||
| msgTypeMetrics messageType = "metrics" | ||||||||||||||||||||||||||||||||||||||||||
| msgTypeRequest messageType = "request" | ||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| type messageEnvelope struct { | ||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -121,7 +124,7 @@ func (pm *ProxyManager) apiSendEvents(c *gin.Context) { | |||||||||||||||||||||||||||||||||||||||||
| // prevent nginx from buffering SSE | ||||||||||||||||||||||||||||||||||||||||||
| c.Header("X-Accel-Buffering", "no") | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| sendBuffer := make(chan messageEnvelope, 25) | ||||||||||||||||||||||||||||||||||||||||||
| sendBuffer := make(chan messageEnvelope, 100) | ||||||||||||||||||||||||||||||||||||||||||
| ctx, cancel := context.WithCancel(c.Request.Context()) | ||||||||||||||||||||||||||||||||||||||||||
| sendModels := func() { | ||||||||||||||||||||||||||||||||||||||||||
| data, err := json.Marshal(pm.getModelStatus()) | ||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -164,6 +167,18 @@ func (pm *ProxyManager) apiSendEvents(c *gin.Context) { | |||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| sendRequest := func(req RequestEntry) { | ||||||||||||||||||||||||||||||||||||||||||
| jsonData, err := json.Marshal(req) | ||||||||||||||||||||||||||||||||||||||||||
| if err == nil { | ||||||||||||||||||||||||||||||||||||||||||
| select { | ||||||||||||||||||||||||||||||||||||||||||
| case sendBuffer <- messageEnvelope{Type: msgTypeRequest, Data: string(jsonData)}: | ||||||||||||||||||||||||||||||||||||||||||
| case <-ctx.Done(): | ||||||||||||||||||||||||||||||||||||||||||
| return | ||||||||||||||||||||||||||||||||||||||||||
| default: | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||
| * Send updated models list | ||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -191,12 +206,30 @@ func (pm *ProxyManager) apiSendEvents(c *gin.Context) { | |||||||||||||||||||||||||||||||||||||||||
| sendMetrics([]TokenMetrics{e.Metrics}) | ||||||||||||||||||||||||||||||||||||||||||
| })() | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||
| * Send Request data | ||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||
| defer event.On(func(e RequestEvent) { | ||||||||||||||||||||||||||||||||||||||||||
| sendRequest(e.Entry) | ||||||||||||||||||||||||||||||||||||||||||
| })() | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| // send initial batch of data | ||||||||||||||||||||||||||||||||||||||||||
| sendLogData("proxy", pm.proxyLogger.GetHistory()) | ||||||||||||||||||||||||||||||||||||||||||
| sendLogData("upstream", pm.upstreamLogger.GetHistory()) | ||||||||||||||||||||||||||||||||||||||||||
| sendModels() | ||||||||||||||||||||||||||||||||||||||||||
| sendMetrics(pm.metricsMonitor.getMetrics()) | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| // Send recent requests (without bodies to keep initial sync light) | ||||||||||||||||||||||||||||||||||||||||||
| requests := pm.requestMonitor.GetEntries() | ||||||||||||||||||||||||||||||||||||||||||
| if len(requests) > 20 { | ||||||||||||||||||||||||||||||||||||||||||
| requests = requests[len(requests)-20:] | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| for _, r := range requests { | ||||||||||||||||||||||||||||||||||||||||||
| r.RequestBody = "" | ||||||||||||||||||||||||||||||||||||||||||
| r.ResponseBody = "" | ||||||||||||||||||||||||||||||||||||||||||
| sendRequest(r) | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| for { | ||||||||||||||||||||||||||||||||||||||||||
| select { | ||||||||||||||||||||||||||||||||||||||||||
| case <-c.Request.Context().Done(): | ||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -221,6 +254,33 @@ func (pm *ProxyManager) apiGetMetrics(c *gin.Context) { | |||||||||||||||||||||||||||||||||||||||||
| c.Data(http.StatusOK, "application/json", jsonData) | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| func (pm *ProxyManager) apiGetRequests(c *gin.Context) { | ||||||||||||||||||||||||||||||||||||||||||
| entries := pm.requestMonitor.GetEntries() | ||||||||||||||||||||||||||||||||||||||||||
| // Strip bodies for list view | ||||||||||||||||||||||||||||||||||||||||||
| for i := range entries { | ||||||||||||||||||||||||||||||||||||||||||
| entries[i].RequestBody = "" | ||||||||||||||||||||||||||||||||||||||||||
| entries[i].ResponseBody = "" | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| c.JSON(http.StatusOK, entries) | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+257
to
+265
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same mutation issue — modifying returned slice elements. Similar to the SSE initial sync, this modifies Suggested fix func (pm *ProxyManager) apiGetRequests(c *gin.Context) {
entries := pm.requestMonitor.GetEntries()
- // Strip bodies for list view
- for i := range entries {
- entries[i].RequestBody = ""
- entries[i].ResponseBody = ""
- }
- c.JSON(http.StatusOK, entries)
+ // Strip bodies for list view - create copies to avoid mutating stored data
+ stripped := make([]RequestEntry, len(entries))
+ for i, e := range entries {
+ stripped[i] = e
+ stripped[i].RequestBody = ""
+ stripped[i].ResponseBody = ""
+ }
+ c.JSON(http.StatusOK, stripped)
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| func (pm *ProxyManager) apiGetRequest(c *gin.Context) { | ||||||||||||||||||||||||||||||||||||||||||
| idStr := c.Param("id") | ||||||||||||||||||||||||||||||||||||||||||
| var id int | ||||||||||||||||||||||||||||||||||||||||||
| if _, err := fmt.Sscanf(idStr, "%d", &id); err != nil { | ||||||||||||||||||||||||||||||||||||||||||
| c.JSON(http.StatusBadRequest, gin.H{"error": "invalid request id"}) | ||||||||||||||||||||||||||||||||||||||||||
| return | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| entry, found := pm.requestMonitor.GetEntry(id) | ||||||||||||||||||||||||||||||||||||||||||
| if !found { | ||||||||||||||||||||||||||||||||||||||||||
| c.JSON(http.StatusNotFound, gin.H{"error": "request not found"}) | ||||||||||||||||||||||||||||||||||||||||||
| return | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| c.JSON(http.StatusOK, entry) | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| func (pm *ProxyManager) apiUnloadSingleModelHandler(c *gin.Context) { | ||||||||||||||||||||||||||||||||||||||||||
| requestedModel := strings.TrimPrefix(c.Param("model"), "/") | ||||||||||||||||||||||||||||||||||||||||||
| realModelName, found := pm.config.RealModelName(requestedModel) | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don’t continue proxying after a failed body read.
If
io.ReadAllfails, the request body may be partially consumed, and the upstream call will see a truncated or empty body. Bail out early and return an error response.🔧 Suggested fix
🤖 Prompt for AI Agents