add analytics support to the Dart MCP server #174
Conversation
PR HealthChangelog Entry ✔️
Changes to files need to be accounted for in their respective changelogs. |
In the [latest schema](https://github.com/modelcontextprotocol/modelcontextprotocol/blob/main/schema/2025-03-26/schema.json) the ClientImplementation and ServerImplementation types were merged into a single Implementation type, this updates things to match. Also closes #173 which will make #174 slightly cleaner, and matches what we do for ServerConnection as well.
| @@ -40,6 +40,7 @@ void main(List<String> args) async { | |||
| ), | |||
| forceRootsFallback: parsedArgs.flag(forceRootsFallback), | |||
| sdk: Sdk.find(dartSdkPath: dartSdkPath, flutterSdkPath: flutterSdkPath), | |||
| analytics: null, | |||
There was a problem hiding this comment.
Can we actually create the Analytics instance here?
There was a problem hiding this comment.
If we are allowed to that would simplify things, otherwise we need a custom entry point in the SDK which does create one.
I just don't know if it is kosher or not, it feels a bit weird to be putting the current tool as the dartTool, when this is actually a binary that can be ran in other ways (from source, or as a git dep).
There was a problem hiding this comment.
Another option would be a command line flag to enable analytics.
There was a problem hiding this comment.
Can we look up the resolved executable to see if the starting script was from the dart mcp-server command?
There was a problem hiding this comment.
I don't think so, we will just get the dart native runtime as the resolved executable afaik.
There was a problem hiding this comment.
Hmm. Then if we need to use the Dart CLI as the parent tool, adding a hidden flag that is set from the dart mcp-server command only could be a good workaround. We should get clarity on that first from Ander though.
Based on dart-lang/tools#2112
Adds analytics events for all tool calls as well as for the runtime errors resource.
We could consider a generic resource read event which includes the URI, but we could accidentally create URIs that have some information we don't want to capture, so I chose to instead just customize this and not provide the URI, but a field that describes the type of resource that was read.
I did not add any debouncing because I don't think it is necessary, LLMs do not invoke tools in super rapid succession generally, but let me know if you all disagree and think I should do some debouncing.