chore: include tool args for perf advisor telemetry MCP-277 #706
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
Pull Request Overview
This PR refactors telemetry event handling to allow tools to include arbitrary key-value pairs in telemetry metadata, and disables telemetry in accuracy tests. The refactoring renames metadata fields to match the actual property names being sent (e.g., projectId → project_id), enabling direct spreading of metadata objects instead of manual property mapping.
Key changes:
- Extended
TelemetryToolMetadatatype to support arbitrary string/number/array properties via intersection type - Renamed fixed telemetry metadata fields to match transmitted property names (
projectId→project_id, etc.) - Modified
resolveTelemetryMetadatato acceptresultandargsparameters for context-aware metadata - Disabled telemetry in accuracy tests via
DO_NOT_TRACK=1environment variable
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/tools/tool.ts | Extended TelemetryToolMetadata type and simplified event emission by spreading metadata directly instead of manual property assignment |
src/tools/atlas/atlasTool.ts | Updated field names from projectId/orgId to project_id/org_id to match new naming convention |
src/tools/mongodb/mongodbTool.ts | Updated projectId to project_id in metadata assignment |
src/tools/atlasLocal/atlasLocalTool.ts | Updated atlasLocaldeploymentId to atlas_local_deployment_id in metadata assignment |
src/tools/atlas/read/getPerformanceAdvisor.ts | Implemented resolveTelemetryMetadata override to include operations array in telemetry events |
src/telemetry/types.ts | Extended TelemetryEvent properties with index signature to support arbitrary metadata fields |
tests/unit/toolBase.test.ts | Added comprehensive tests for resolveTelemetryMetadata functionality with mock telemetry emission verification |
tests/integration/tools/atlas/performanceAdvisor.test.ts | Added telemetry emission tests for operations metadata and reorganized test structure with proper mock setup |
tests/integration/elicitation.test.ts | Removed redundant telemetry disable configuration (now handled globally) |
tests/accuracy/sdk/accuracyTestingClient.ts | Disabled telemetry in accuracy tests using DO_NOT_TRACK environment variable |
blva reviewed Oct 29, 2025
| component: "tool", | ||
| duration_ms: duration, | ||
| result: result.isError ? "failure" : "success", | ||
| ...metadata, |
nirinchev force-pushed the ni/perf-advisor-telemetry branch from e8d664d to 2271809 Compare 
Pull Request Test Coverage Report for Build 19066434028Details
💛 - Coveralls |
nirinchev changed the title 
| note: let's make sure to trigger the perf advisor tests here via GH actions |
blva reviewed Oct 29, 2025
| extra: RequestHandlerExtra<ServerRequest, ServerNotification> | ||
| ): TelemetryToolMetadata { | ||
| const baseMetadata = super.resolveTelemetryMetadata(result, args, extra); | ||
| baseMetadata.operations = args.operations; |
There was a problem hiding this comment.
[q] why not capture this new field in TelemetryToolMetadata? keeping track of the data we populate there is handy - makes it easy to get a list of properties in the long term
There was a problem hiding this comment.
That's fair - I think it's a bit annoying if it becomes unwieldy as we add more and more stuff. But you're right that by being overly permissive, we lose oversight into what's being sent to telemetry. I'll try to find a better solution.
kylelai1 approved these changes Oct 29, 2025
blva approved these changes Nov 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
Proposed changes
Reworks telemetry events a bit - now we can provide arbitrary kvp data from
resolveTelemetryMetadatain tools. This allows us to include the operations as additional field in properties. Additionally, renamed the fixed fields inTelemetryToolMetadatato match the names we're sending so we can just spread the object instead of the custom handling we had previously.