-
Notifications
You must be signed in to change notification settings - Fork 302
Add ToolMetadataExporter #992
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
base: main
Are you sure you want to change the base?
Conversation
|
|
||
| public virtual Task<ListToolsResult?> LoadToolsDynamicallyAsync() | ||
| { | ||
| return Utility.LoadToolsDynamicallyAsync(_toolDirectory, false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are currently utility methods but the logic is from ToolDescriptionEvaluator where I think the logic can be shared.
|
|
||
| namespace ToolMetadataExporter; | ||
|
|
||
| internal class Utility |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Taken from ToolDescriptionEvaluator and modified. Need to update logic together.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds a new ToolMetadataExporter tool that exports and tracks metadata changes from the Azure MCP server (azmcp). The tool compares current tool definitions against historical data stored in Azure Data Explorer (Kusto) and records creation, update, and deletion events.
Key changes:
- New
ToolMetadataExporterconsole application that dynamically loads tools from azmcp and compares them against a Kusto datastore - Integration with Azure Data Explorer for querying existing tools and ingesting change events
- Support for tracking tool lifecycle events (Created, Updated, Deleted) with version information
Reviewed Changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| eng/tools/ToolMetadataExporter/Program.cs | Entry point configuring DI, Azure authentication, and Kusto clients |
| eng/tools/ToolMetadataExporter/ToolAnalyzer.cs | Core analysis logic comparing current tools against datastore and detecting changes |
| eng/tools/ToolMetadataExporter/Utility.cs | Helper methods for executing azmcp commands and parsing tool metadata |
| eng/tools/ToolMetadataExporter/Services/AzmcpProgram.cs | Service wrapper for interacting with the azmcp executable |
| eng/tools/ToolMetadataExporter/Services/AzureMcpKustoDatastore.cs | Kusto datastore implementation for querying and ingesting tool events |
| eng/tools/ToolMetadataExporter/Services/IAzureMcpDatastore.cs | Interface defining datastore operations |
| eng/tools/ToolMetadataExporter/Models/Kusto/McpToolEvent.cs | Model representing tool lifecycle events with Kusto column mappings |
| eng/tools/ToolMetadataExporter/Models/Kusto/McpToolEventType.cs | Enum defining event types (Created, Updated, Deleted) |
| eng/tools/ToolMetadataExporter/Models/ModelsSerializationContext.cs | JSON source generation context for AOT compatibility |
| eng/tools/ToolMetadataExporter/Models/AzureMcpTool.cs | Record representing a tool with ID, name, and area |
| eng/tools/ToolMetadataExporter/AppConfiguration.cs | Configuration model for Kusto endpoints and settings |
| eng/tools/ToolMetadataExporter/appsettings.json | Production configuration template |
| eng/tools/ToolMetadataExporter/appsettings.Development.json | Development environment configuration |
| eng/tools/ToolMetadataExporter/Resources/queries/GetAvailableTools.kql | KQL query to retrieve latest tool states |
| eng/tools/ToolMetadataExporter/Resources/queries/CreateTable.kql | KQL script to create the McpToolEvents table |
| eng/tools/ToolMetadataExporter/ToolMetadataExporter.csproj | Project file with dependencies on Azure.Identity and Kusto SDKs |
| eng/tools/ToolMetadataExporter.UnitTests/ToolAnalyzerTests.cs | Empty test class placeholder for unit tests |
| eng/tools/ToolMetadataExporter.UnitTests/ToolMetadataExporter.UnitTests.csproj | Test project configuration |
| eng/tools/ToolDescriptionEvaluator/Models/McpModels.cs | Added Id property to Tool model |
| Directory.Packages.props | Updated Kusto package versions to 14.0.1 and added Ingest package |
| AzureMcp.sln | Added ToolMetadataExporter projects to solution |
| if (string.IsNullOrEmpty(latestEvent.ToolId)) | ||
| { | ||
| throw new InvalidOperationException( | ||
| $"Cannot have an event with no id. Name: {latestEvent.ToolArea}, Area: {latestEvent.ToolArea}"); |
Copilot
AI
Nov 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error message incorrectly displays ToolArea for the Name parameter. It should display latestEvent.ToolName instead of latestEvent.ToolArea for the name field.
Fix:
$"Cannot have an event with no id. Name: {latestEvent.ToolName}, Area: {latestEvent.ToolArea}");| $"Cannot have an event with no id. Name: {latestEvent.ToolArea}, Area: {latestEvent.ToolArea}"); | |
| $"Cannot have an event with no id. Name: {latestEvent.ToolName}, Area: {latestEvent.ToolArea}"); |
| private static string EscapeCharacters(string text) | ||
| { | ||
| if (string.IsNullOrEmpty(text)) | ||
| return text; | ||
|
|
||
| // Normalize only the fancy “curly” quotes to straight ASCII. Identity replacements were removed. | ||
| return text.Replace(UnicodeChars.LeftSingleQuote, "'") | ||
| .Replace(UnicodeChars.RightSingleQuote, "'") |
Copilot
AI
Nov 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable dir can be null after the while loop exits if no repo root is found. The null check should occur before dereferencing dir.Parent in the loop.
Consider using a null-conditional operator:
dir = dir?.Parent;Or check for null before the iteration:
while (dir != null)
{
if (File.Exists(Path.Combine(dir.FullName, "AzureMcp.sln")) ||
Directory.Exists(Path.Combine(dir.FullName, ".git")))
{
return dir.FullName;
}
dir = dir.Parent;
}| if (existing.WorkDirectory == null) | ||
| { | ||
| string exeDir = AppContext.BaseDirectory; | ||
| var repoRoot = Utility.FindRepoRoot(exeDir); | ||
| existing.WorkDirectory = Path.Combine(repoRoot, ".work"); | ||
| } |
Copilot
AI
Nov 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The PostConfigure block sets a default WorkDirectory if it's null. However, the QueriesFolder property also has a default value ("Resources/queries"), but the AzureMcpKustoDatastore constructor validates this path existence. If QueriesFolder is not explicitly set, it will use the default relative path, which may fail validation. Consider also handling QueriesFolder in PostConfigure to ensure it resolves to an absolute path.
| if (existing.WorkDirectory == null) | |
| { | |
| string exeDir = AppContext.BaseDirectory; | |
| var repoRoot = Utility.FindRepoRoot(exeDir); | |
| existing.WorkDirectory = Path.Combine(repoRoot, ".work"); | |
| } | |
| string exeDir = AppContext.BaseDirectory; | |
| var repoRoot = Utility.FindRepoRoot(exeDir); | |
| if (existing.WorkDirectory == null) | |
| { | |
| existing.WorkDirectory = Path.Combine(repoRoot, ".work"); | |
| } | |
| if (string.IsNullOrEmpty(existing.QueriesFolder) || !Path.IsPathRooted(existing.QueriesFolder)) | |
| { | |
| existing.QueriesFolder = Path.Combine(repoRoot, existing.QueriesFolder ?? "Resources/queries"); | |
| } |
| } | ||
|
|
||
| if (cliArtifact != null) | ||
| { |
Copilot
AI
Nov 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The argumentsToUse variable is incorrectly assigned when isDll is true. It should append arguments to the DLL path, but currently it only includes the DLL path without the actual arguments.
Fix:
var argumentsToUse = isDll ? $"{cliArtifact.FullName} {arguments}" : arguments;| _logger = logger; | ||
|
|
||
| _workingDirectory = configuration.Value.WorkDirectory ?? throw new ArgumentNullException(nameof(AppConfiguration.WorkDirectory)); | ||
| ; |
Copilot
AI
Nov 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extraneous semicolon on an empty line. This should be removed.
| ; |
|
|
||
| private static async Task SaveToolsToJsonAsync(ListToolsResult toolsResult, string filePath) | ||
| { | ||
| try | ||
| { | ||
| // Normalize only tool and option descriptions instead of escaping the entire JSON document | ||
| if (toolsResult.Tools != null) |
Copilot
AI
Nov 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This foreach loop implicitly filters its target sequence - consider filtering the sequence explicitly using '.Where(...)'.
|
|
||
| internal static async Task<string> ExecuteAzmcpAsync(string arguments, bool isCiMode = false, bool checkErrorCode = true) | ||
| { | ||
| // Locate azmcp artifact across common build outputs (servers/core, Debug/Release) | ||
| var exeDir = AppContext.BaseDirectory; | ||
| var repoRoot = FindRepoRoot(exeDir); | ||
| var searchRoots = new List<string> | ||
| { | ||
| Path.Combine(repoRoot, "servers", "Azure.Mcp.Server", "src", "bin", "Debug"), | ||
| Path.Combine(repoRoot, "servers", "Azure.Mcp.Server", "src", "bin", "Release") | ||
| }; |
Copilot
AI
Nov 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This foreach loop immediately maps its iteration variable to another variable - consider mapping the sequence explicitly using '.Select(...)'.
| } | ||
| } | ||
|
|
||
| if (cliArtifact == null) | ||
| { | ||
| if (isCiMode) | ||
| { | ||
| return string.Empty; // Graceful fallback in CI | ||
| } | ||
|
|
||
| throw new FileNotFoundException("Could not locate azmcp CLI artifact in Debug/Release outputs under servers."); | ||
| } |
Copilot
AI
Nov 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Disposable 'Process' is created but not disposed.
| catch (Exception ex) | ||
| { | ||
| _logger.LogError(ex, "Error writing to {FileName}", outputFile); | ||
| } |
Copilot
AI
Nov 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generic catch clause.
| catch (Exception ex) | |
| { | |
| _logger.LogError(ex, "Error writing to {FileName}", outputFile); | |
| } | |
| catch (IOException ex) | |
| { | |
| _logger.LogError(ex, "IO error writing to {FileName}", outputFile); | |
| } | |
| catch (UnauthorizedAccessException ex) | |
| { | |
| _logger.LogError(ex, "Unauthorized access writing to {FileName}", outputFile); | |
| } |
| } | ||
| } | ||
|
|
||
| var writerOptions = new JsonWriterOptions | ||
| { |
Copilot
AI
Nov 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generic catch clause.
What does this PR do?
Adds tool to export current metadata from azmcp.
GitHub issue number?
[Link to the GitHub issue this PR addresses]https://github.com/microsoft/mcp-pr/issues/123
Pre-merge Checklist
servers/Azure.Mcp.Server/CHANGELOG.mdand/orservers/Fabric.Mcp.Server/CHANGELOG.mdfor product changes (features, bug fixes, UI/UX, updated dependencies)servers/Azure.Mcp.Server/README.mdand/orservers/Fabric.Mcp.Server/README.mddocumentationeng/scripts/Process-PackageReadMe.ps1. See Package README/servers/Azure.Mcp.Server/docs/azmcp-commands.mdand/or/docs/fabric-commands.md.\eng\scripts\Update-AzCommandsMetadata.ps1to update tool metadata in azmcp-commands.md (required for CI)ToolDescriptionEvaluatorand obtained a score of0.4or more and a top 3 ranking for all related test prompts/servers/Azure.Mcp.Server/docs/e2eTestPrompts.mdcrypto mining, spam, data exfiltration, etc.)/azp run mcp - pullrequest - liveto run Live Test Pipeline