Skip to content

Use SetTag instead of AddTag when updating field values.#837

Merged
conniey merged 5 commits intomicrosoft:mainfrom
conniey:telemetry-set-tag-instead-of-add
Oct 16, 2025
Merged

Use SetTag instead of AddTag when updating field values.#837
conniey merged 5 commits intomicrosoft:mainfrom
conniey:telemetry-set-tag-instead-of-add

Conversation

@conniey
Copy link
Copy Markdown
Member

@conniey conniey commented Oct 16, 2025

What does this PR do?

Problem All telemetry items appear as "IsServerCommandInvoked" = "true".

The issue was that using AddTag would append a new KVP to the telemetry Tags rather than replace an existing KVP of the same name. Only the first matching KVP of the same name was being pulled in.

Problem Before, invoking a child tool in namespace mode would spawn a new process. This new process would create its own telemetry service. This new telemetry service would add another telemetry item with the tool name "blob_list".

Since we no longer spawn a child process, we need to associate the "command" in the NamespaceToolLoader with what the ToolName.

GitHub issue number?

[Link to the GitHub issue this PR addresses]

Pre-merge Checklist

  • Required for All PRs
    • Read contribution guidelines
    • PR title clearly describes the change
    • Commit history is clean with descriptive messages (cleanup guide)
    • Added comprehensive tests for new/modified functionality
    • Updated servers/Azure.Mcp.Server/CHANGELOG.md and/or servers/Fabric.Mcp.Server/CHANGELOG.md for product changes (features, bug fixes, UI/UX, updated dependencies)
  • For MCP tool changes:
    • One tool per PR: This PR adds or modifies only one MCP tool for faster review cycles
    • Updated servers/Azure.Mcp.Server/README.md and/or servers/Fabric.Mcp.Server/README.md documentation
    • Validate README.md changes using script at eng/scripts/Process-PackageReadMe.ps1. See Package README
    • Updated command list in /servers/Azure.Mcp.Server/docs/azmcp-commands.md and/or /docs/fabric-commands.md
    • For new or modified tool descriptions, ran ToolDescriptionEvaluator and obtained a score of 0.4 or more and a top 3 ranking for all related test prompts
    • For new tools associated with Azure services or publicly available tools/APIs/products, add URL to documentation in the PR description
  • Extra steps for Azure MCP Server tool changes:
    • Updated test prompts in /servers/Azure.Mcp.Server/docs/e2eTestPrompts.md
    • 👉 For Community (non-Microsoft team member) PRs:
      • Security review: Reviewed code for security vulnerabilities, malicious code, or suspicious activities before running tests (crypto mining, spam, data exfiltration, etc.)
      • Manual tests run: added comment /azp run mcp - pullrequest - live to run Live Test Pipeline

@conniey conniey requested a review from a team as a code owner October 16, 2025 15:47
Copilot AI review requested due to automatic review settings October 16, 2025 15:47
Copy link
Copy Markdown
Contributor

Copilot AI left a 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 fixes a telemetry bug where all telemetry items incorrectly appeared as "IsServerCommandInvoked" = "true" and updates the tool name tracking for namespace mode invocations.

Key Changes:

  • Replaced AddTag with SetTag to properly update telemetry tag values instead of appending duplicates
  • Added ToolName tag update in NamespaceToolLoader to correctly track child tool invocations when not spawning new processes

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
SingleProxyToolLoader.cs Changed AddTag to SetTag for IsServerCommandInvoked tag to fix duplicate tag issue
ServerToolLoader.cs Changed AddTag to SetTag for IsServerCommandInvoked tag to fix duplicate tag issue
NamespaceToolLoader.cs Changed AddTag to SetTag and added ToolName tag update to properly track child tool names in namespace mode

Comment thread core/Azure.Mcp.Core/src/Areas/Server/Commands/ToolLoading/NamespaceToolLoader.cs Outdated
@conniey conniey force-pushed the telemetry-set-tag-instead-of-add branch from cdcd671 to 83e1dbf Compare October 16, 2025 15:53
@github-project-automation github-project-automation Bot moved this from Untriaged to In Progress in Azure MCP Server Oct 16, 2025
@conniey conniey enabled auto-merge (squash) October 16, 2025 18:13
@conniey conniey merged commit 091b9b5 into microsoft:main Oct 16, 2025
24 checks passed
@github-project-automation github-project-automation Bot moved this from In Progress to Done in Azure MCP Server Oct 16, 2025
@conniey conniey deleted the telemetry-set-tag-instead-of-add branch October 16, 2025 18:39
colbytimm pushed a commit to colbytimm/microsoft-mcp that referenced this pull request Dec 8, 2025
* Update to use SetTag rather than AddTag

* Add documentation

* Add CHANGELOG

* Fix spelling error
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants