DYN-9695: Add analytics support for external components #16597
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds two new analytics categories to track events for MCP (Model Context Protocol) Extension/Server operations and Dynamo Assistant operations. This enables better monitoring and analytics collection for these newer Dynamo features.
- Adds
MCPOperationscategory for MCP-related analytics events - Adds
AssistantOperationscategory for DynamoAssistant analytics events
src/NodeServices/IAnalyticsClient.cs
Outdated
| /// <summary> | ||
| /// Events Category related to DynamoAssistant operations | ||
| /// </summary> | ||
| AssistantOperations |
There was a problem hiding this comment.
Hmm… having Dynamo services be aware of external components like MCP and Assistant does seem a bit unusual. But since Analytics.TrackEvent is strongly typed with the Categories enum, there’s not much we can do about it right now (unless we introduce something like IAnalyticsClient.TrackEvent(Actions action, string category, ...), since it ultimately gets converted to a string anyway).
Just some food for thought.
There was a problem hiding this comment.
That's a good idea!
There was a problem hiding this comment.
On second thought, @kalunkuo, I’m not sure if this was a finalized decision. Instead of doing this:
Analytics.TrackEvent(Actions.Run, Categories.MCPOperations, $"MCP.ToolCalled_{name}", success ? 1 : 0);
// Equivalent: TrackEvent("Run", "MCPOperations", "MCP.ToolCalled_createNodes", 1);
Would it make more sense to do this instead (by introducing Actions.CallTool, etc. and Categories.MCP)?
Analytics.TrackEvent(Actions.CallTool, Categories.MCP, name, 1); // New action 'CallTool'
Analytics.TrackEvent(Actions.Handle, Categories.MCP, method); // New action 'Handle'
Analytics.TrackEvent(Actions.Start, Categories.MCP, "ServerInitialized", port); // Existing action 'Start'
This might make data segmentation and grouping more intuitive -- we wouldn’t need to parse out createNodes from ToolCalled_createNodes in the analytics portal. Just curious what the team’s thoughts are; if a decision’s already been made, that’s totally fine by me 🙂
I would agree, and prefer this approach, also I see the last argument takes in an integer, we can use it to record how many nodes were created/deleted/connected, since these tools can process batch request. |
There was a problem hiding this comment.
See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-9695
johnpierson
left a comment
There was a problem hiding this comment.
very awesome. thanks for the comments @benglin and @zeusongit. I think this implementation is clean. 👍
aparajit-pratap
left a comment
There was a problem hiding this comment.
@kalunkuo, I think that the DaaS team was also planning on adding analytics to their service. Did you check with @mjkkirschner for any of their requirements?
Let me confirm with him! |
Purpose
Add analytics support for external components like MCP and Dynamo Assistant operations
Declarations
Check these if you believe they are true
Release Notes
This PR introduces a new
TrackEventoverload that accepts a string-based category, enabling external components to log analytics events without relying on the predefined Categories enum.Reviewers
@DynamoDS/synapse
@DynamoDS/eidos
FYIs
(FILL ME IN, Optional) Names of anyone else you wish to be notified of