fix(cmd): disable klog in STDIO mode #331
Conversation
Prevents protocol issues with some clients. Kubernetes tooling uses klog to log messages. Some of these messages end up logged in the stderr or stdout which breaks some of the clients that expect jsonrpc messages. Signed-off-by: Marc Nuri <[email protected]>
| func (m *MCPServerOptions) initializeLogging() { | ||
| flagSet := flag.NewFlagSet("klog", flag.ContinueOnError) | ||
| klog.InitFlags(flagSet) | ||
| if m.StaticConfig.Port == "" { |
There was a problem hiding this comment.
Why don't we simply by pass klog.InitFlags if m.StaticConfig.Port is empty?
There was a problem hiding this comment.
because we need to set the other flags (logtostderr, alsologtostderr, stderrthreshold) which have defaults that enable logging of some messages
There was a problem hiding this comment.
the fix is not for our logs (which there aren't many at the moment) but rather for the upstream kubernetes logs which aren't controlled easily.
There was a problem hiding this comment.
Does that mean our logs do not break the jsonrpc?
There was a problem hiding this comment.
Yes, they would break the jsonrpc too.
This statement also disables them (as asserted in the provided tests).
There was a problem hiding this comment.
Just for clarification; by running klog.InitFlags in SDTIO, which logs that do not break jsonrpc will be logged?
There was a problem hiding this comment.
Ok, let me try to summarize.
If you run kubernetes-mcp-server in STDIO mode (i.e. no --port flag) with an unauthorized cluster you can immediately trigger an error that will be logged to STDERR:
E0917 16:47:01.678632 85672 memcache.go:265] "Unhandled Error" err="couldn't get current server API group list: ...
I understand that there might be other scenarios where this might happen.
I've started to see that some MCP Hosts(->Clients) start failing because they've become ore strict and will mark the MCP server as not working.
The goal here is to disable all logging either to stderr or stdout in order not to break the protocol for these more strict clients.
By running klog.InitFlags and then providing the three log configuration flags we prevent klog from logging to stderr which is what happens by default.
Not sure if it's clearer now.
There was a problem hiding this comment.
Thank you for explanation. It makes sense.
LGTM
2 participants
Prevents protocol issues with some clients.
Kubernetes tooling uses klog to log messages.
Some of these messages end up logged in the stderr or stdout which breaks some of the clients that expect jsonrpc messages.
/cc @ardaguclu