Skip to content

add analytics support to the Dart MCP server #174

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Jun 24, 2025
Merged

add analytics support to the Dart MCP server #174

merged 8 commits into from
Jun 24, 2025

Conversation

jakemac53
Copy link
Contributor

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.

Copy link

PR Health

Changelog Entry ✔️
Package Changed Files

Changes to files need to be accounted for in their respective changelogs.

jakemac53 added a commit that referenced this pull request Jun 18, 2025
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,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we actually create the Analytics instance here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another option would be a command line flag to enable analytics.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we look up the resolved executable to see if the starting script was from the dart mcp-server command?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so, we will just get the dart native runtime as the resolved executable afaik.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It sounds like we would have to support showing the message to the user etc in that case, which we have no way of doing as an MCP server, so I think we will need a custom entry point in the SDK.

@jakemac53 jakemac53 marked this pull request as ready for review June 23, 2025 14:50
@jakemac53 jakemac53 requested a review from kenzieschmoll June 24, 2025 17:47
@jakemac53 jakemac53 merged commit 5f9c50f into main Jun 24, 2025
15 checks passed
@jakemac53 jakemac53 deleted the analytics branch June 24, 2025 18:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants