feat: include only Grafana-connected tools#595
feat: include only Grafana-connected tools#595Shaik-Sirajuddin wants to merge 7 commits intografana:mainfrom
Conversation
|
|
Shaik-Sirajuddin
changed the title
Shaik-Sirajuddin
changed the title
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| } | ||
| slog.Info("Registered Tools of discovered datasources for session ", "sessionId", sessionID, "count", len(tools)) | ||
| }) | ||
| } |
There was a problem hiding this comment.
Connected-only doesn’t verify datasource health
Medium Severity
connected-only discovery treats grafana.Datasources.GetDataSources() results as “connected” and registers tools based only on datasource.Type presence, without checking datasource connectivity/health. This can expose tools for misconfigured or disconnected datasources, contradicting the connected-only flag’s documented behavior.
Additional Locations (1)
Shaik-Sirajuddin
changed the title
| mayBeEnabledTool(tools.CloudWatchDatasourceType, dt.cloudwatch, &enabledMap, enabledTools) | ||
| mayBeEnabledTool(tools.ClickHouseDatasourceType, dt.clickhouse, &enabledMap, enabledTools) | ||
|
|
||
| return enabledMap |
There was a problem hiding this comment.
Datasource type mismatch breaks Pyroscope and ClickHouse discovery
High Severity
toolsState uses datasource type constants like tools.PyroscopeDataSourceType ("grafana-pyroscope-datasource") and tools.ClickHouseDatasourceType ("grafana-clickhouse-datasource") as the category argument to mayBeEnabledTool, which checks slices.Contains(enabled, category). However, the enabled-tools flag uses short names like "pyroscope" and "clickhouse". Since "grafana-pyroscope-datasource" will never match "pyroscope" in the enabled list, these datasources are never added to the enabledTools map and will always be skipped during discovery, even when they are connected and explicitly enabled.
Additional Locations (1)
| obs.NetworkTransport = mcpconv.NetworkTransportTCP | ||
| } | ||
|
|
||
| if err := run(transport, *addr, *basePath, *endpointPath, parseLevel(*logLevel), dt, grafanaConfig, tls, obs); err != nil { |
There was a problem hiding this comment.
Stateless streamable-http breaks connected-only session tools
Medium Severity
WithStateLess(dt.proxied) makes the streamable-http server stateless when --disable-proxied is set. But the hook registration condition was changed to !dt.proxied || tc.onlyConnected, enabling hooks even when proxied is disabled. When both --disable-proxied and --connected-only are set, the server is stateless yet DiscoverAndRegisterToolsSession attempts to use AddSessionTools, which requires session tracking. The connected-only discovery silently fails on this transport combination.
Additional Locations (1)
2 participants
Initialize data source tools connected to grafana instance , skips non connected tools
Addresses #572
Updated Functionality :
connected-only flag to enable auto discovery
Test Strategy :
Pass pyroscope ( or any other datasource ) via enabled-tools which is not used in grafana instance
Start mcpserver normally
Verify the pyroscope tools are present by calling /list_tools
Start mcp server with connected-only flag
Verify pyroscope tools are now not visible
Verify Promethues , loki , e.t.c connected tools are visible
Note
Medium Risk
Changes when and how tools are registered (startup vs per-session, conditional on datasource discovery), which can affect tool visibility and session behavior across transports; scope is contained to tool registration paths.
Overview
Adds a new
--connected-onlyflag that switches datasource tool registration from static startup registration to runtime discovery against the Grafana datasource list, so only tools for datasource types that are both enabled via flags and present in the Grafana instance are broadcast.Implements discovery/registration in
ToolManagerfor both stdio (server-wideInitDiscoverAndRegister) and HTTP/SSE (per-sessionDiscoverAndRegisterToolsSessionvia hooks), and refactors datasource tool packages to exposeGet*Tools()factories plus datasource-type constants so discovery can map datasource types to tool sets. Includes unit tests validatingsync.Oncebehavior for discovery initialization under concurrent/sequential access.Written by Cursor Bugbot for commit 1839fc3. This will update automatically on new commits. Configure here.