feat: Add pending requests count to the main dashboard#516
feat: Add pending requests count to the main dashboard#516mostlygeek merged 10 commits intomostlygeek:mainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds in-flight request tracking: backend event/type, an InflightCounter and middleware that emits InFlightRequestsEvent, SSE emission of totals, frontend types and store, and UI changes to display completed vs waiting requests. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
author-brian-mendonca
changed the title
|
Neat feature. Integrating it into the events system is the right place. I think a gin middleware in the ProxyManager would be a better place to implement it. Wrap the handlers that proxy a request. Also, I prefer to use int instead of int32 for counters. |
Move "Waiting" tracking into a Gin wrapper in ProxyManager, emitting total in‑flight events from there. Switch in‑flight counters to int and simplify SSE payload to total only.
Move "Waiting" tracking into a Gin wrapper in ProxyManager, emitting total in‑flight events from there. Switch in‑flight counters to int and simplify SSE payload to total only.
|
Sure thing, I've pushed up some additional changes that should take into account your suggestions. Please let me know if you have any other change requests. Go isn't my primary language, but this has been a fun project to work on. |
proxy/proxymanager.go
Outdated
|
|
||
| processGroups map[string]*ProcessGroup | ||
|
|
||
| inFlightMu sync.Mutex |
There was a problem hiding this comment.
Having an InflightCounter struct to encapsulate the variables, locking, etc would be a better design. See metrics_monitor.go as an example.
There was a problem hiding this comment.
I think I've got this done how you've intended; if not, please let me know.
|
Converting to draft PR for now as I follow up on the latest change requests... |
mostlygeek
left a comment
mostlygeek
left a comment
There was a problem hiding this comment.
Just a couple of small changes and I think it'll be good to land.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
proxy/proxymanager.go (1)
31-34:InflightCounteris exported but its constructornewInflightCounteris unexported.If this type is only used within the
proxypackage, consider making it unexported (inflightCounter) for consistency. If external access is intended, export the constructor as well.Also, this whole struct could be replaced with
atomic.Int64from the standard library, which would eliminate the mutex and simplify the code. The floor-at-zero check inDecrementwould need a CAS loop, though, so the current approach is reasonable if you prefer the explicit guard.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@proxy/proxymanager.go`:
- Around line 430-436: The in-flight counter in ProxyManager.trackInflight
currently increments before calling c.Next() but calls
pm.inFlightCounter.Decrement() after c.Next(), which will be skipped if a
downstream handler panics; change the handler to call
pm.inFlightCounter.Decrement() (and emit the corresponding
InFlightRequestsEvent) in a defer immediately after incrementing so the
decrement always runs (even on panic/recovery) while still emitting the updated
totals around c.Next().
🧹 Nitpick comments (1)
proxy/proxymanager.go (1)
31-63:InflightCounteris exported but only used internally.The type is exported (
InflightCounter) while its constructornewInflightCounteris unexported, suggesting this is internal-only. Consider making the type unexported (inflightCounter) for consistency, or export the constructor if external use is intended.
|
Thank you! 👍 |
2 participants
This pull requests adds a "Waiting" count to the models dashboard, which I have found useful for tracking how many requests are pending to be processed if there is a lot of traffic incoming to the local server. This does not change how multiple requests are managed as that should be done in configs, but it can be useful at showing backlog versus completed requests and be a helpful metric for the overall system health. Tested compiling and usability on macOS Tahoe.
Summary by CodeRabbit