Implement rate-limiting based on session ID #30
Conversation
nimishamehta5
force-pushed
the
rate-limit-session-id
branch
from
1773958 to
f972cff
Compare
nimishamehta5
changed the title
nimishamehta5
force-pushed
the
rate-limit-session-id
branch
from
f972cff to
153edfb
Compare
nimishamehta5
changed the title
pkg/ratelimit/config.go
Outdated
| "delete_resource": 30, // 30 requests per minute (0.5 per second) | ||
|
|
||
| // Default for any other tool | ||
| "default": defaultLimit, |
There was a problem hiding this comment.
it's fine for this PR but, could we us constants for these? And we could re-use those whend eclaring tools
There was a problem hiding this comment.
The above +1, I'd also wonder if we could make these configurable. So that users can decide how much they want their API servers to be hit
There was a problem hiding this comment.
@JAORMX - not sure if you meant to extract the tool names into consts, I saw that it had been done in another commit, but event after rebasing I extracted those into a common types/ package, PTAL
@ChrisJBurns - good suggestion, will address that in a future PR - added a TODO comment for now
ChrisJBurns
left a comment
ChrisJBurns
left a comment
There was a problem hiding this comment.
LGTM! Thanks for all the contributions @nimishamehta5 !! 🚀
Possibly one for the future (unless it's easy to slot into this PR), but could we also make the limits configurable someway so users can control how much their API servers get hit?
That and @JAORMX constant changes would be great!
Fixes: StacklokLabs#22 Signed-off-by: Nimisha Mehta <nimishamehta5@gmail.com>
Signed-off-by: Nimisha Mehta <nimishamehta5@gmail.com>
Signed-off-by: Nimisha Mehta <nimishamehta5@gmail.com>
Signed-off-by: Nimisha Mehta <nimishamehta5@gmail.com>
Signed-off-by: Nimisha Mehta <nimishamehta5@gmail.com>
nimishamehta5
force-pushed
the
rate-limit-session-id
branch
from
7e725f3 to
4defaa1
Compare
Signed-off-by: Nimisha Mehta <nimishamehta5@gmail.com>
nimishamehta5
force-pushed
the
rate-limit-session-id
branch
from
37da173 to
990eac7
Compare
3 participants
Fixes: #22
This implements a rate-limiter based on the session ID that is automatically set in the context by
mark3labs/mcp-go.We use the Tool Middleware functionality to implement this. An example of a middleware that's built already in the
mcp-golibrary is theWithRecoveryhandler.We set per-tool rate limits, and we use the Fixed Window algorithm to implement the rate limiter.
I had to modify the
WithTimeoutContextto save the session ID as the new background context overwrites the session ID otherwise. And if we do not create a new context, we get ctx deadline exceeded errors from the k8s apiserver (at least when testing locally).