feat: add optional SSE middleware to obtain Grafana info from headers #21
Conversation
This PR adds a flag to the CLI which, when combined with the SSE transport, allows the client to include information about the Grafana instance to use in headers. This is a non-standard way to use the MCP server, but I'm not sure any of the other accepted auth proposals meet our use case. Unfortunately it involves changing some of the callsites because the Grafana client is no longer a singleton (since it can be changed). Instead, settings live in a contextvar and the client is recreated for each request.
This avoids the need to create a new client for each tool use, and hopefully lets us reuse connections.
|
This would definitely benefit from some tests. I think it'll be a bit complex though, we'll need to:
I would really love to have some kind of load test as well to make sure there's nothing funky going on with the settings being accidentally shared between SSE sessions... |
This commit adds a test for the new GrafanaMiddleware class. It is a bit heavy-handed due to the fact that it requires quite a lot of moving parts. There are quite a few comments in the test file which hopefully explains what is going on.
|
Alright, I've added a test which starts up some separate Grafana servers and makes requests with different headers to hit each one. It seems to work fine! One thing I noticed is that after the SSE request is set up, anyone can send a JSONRPC request to |
| @@ -7,21 +7,21 @@ async def list_datasources() -> bytes: | |||
| """ | |||
| List datasources in the Grafana instance. | |||
| """ | |||
| return await grafana_client.list_datasources() | |||
| return await grafana_client.get().list_datasources() | |||
There was a problem hiding this comment.
I would love to do away with all of these annoying .get() calls, tools definitely shouldn't be able to modify this contextvar...
sd2k
force-pushed
the
grafana-auth-middleware
branch
from
f205487 to
762c60c
Compare
There was a problem hiding this comment.
Also, cloud tests seem to have failed with
=========================== short test summary info ============================
FAILED tests/tools/incident_test.py::test_list_incidents - mcp_grafana.client.GrafanaError: {"code":"Loading","message":"Your instance is loading, and will be ready shortly."}
=================== 1 failed, 11 passed, 15 skipped in 3.38s ===================
🫠
Yeah this is really annoying, I can't figure out how to keep that cloud instance online rather than pausing all the time... I'll dig into it a bit more! |
|
There was a good suggestion by @breitreiter that we could use a custom transport for this. In reality we're just piggy-backing on the SSE transport but it's an interesting idea to make it an explicitly different transport, not just a hack on top of SSE 😅 |
|
Marking as draft while we rethink some of the details. |
|
Nevermind, you marked draft because of the alternate transport consideration. |
This is an implementation of a stateless transport over HTTP for MCP. It's not part of the official spec, but something similar will probably be added in the future. Functionally, it just presents a single HTTP endpoint (conventionally at `/mcp`) that accepts JSONRPC messages as POST bodies, and returns JSONRPC messages as responses. Each inbound message is validated and gets its own 'server' instance. The handler then takes care of initializing the server (which in a stateless protocol is the responsibility of the client), then forwards the inbound message to the server, and returns the server's response. We could easily add middleware for things like authentication, similar to in #21. This also comes with a 'client' implementation which is similar to the stdio and sse clients in the official SDK and returns a pair of read/write streams for use with a `ClientSession`. This is useful for testing or writing example MCP clients. The idea is that this transport will be compatible with the [HTTP transport in the unofficial Go SDK][go-sdk], so we can start building examples of usage. Notably this client also supports custom headers, which will be useful for authentication. [go-sdk]: https://github.com/metoro-io/mcp-golang/blob/main/examples/http_example/README.md
This PR adds a flag to the CLI which, when combined with the
SSE transport, allows the client to include information about
the Grafana instance to use in headers.
This is a non-standard way to use the MCP server, but I'm not sure any
of the other accepted auth proposals meet our use case.
Unfortunately it involves changing some of the callsites because the
Grafana client is no longer a singleton (since it can be changed).
Instead, settings live in a contextvar and the client is recreated
for each request.