Skip to content

Commit 2e4cff4

Browse files
ashragrawalclaude
ashragrawal
and
claude
committed
Document MCP-Platform logEvaluate endpoint integration
Captures the CLI<->server contract for the new POST /agents/externalMcpServers/logEvaluate endpoint: - Empty-body POST design and the privacy rationale (no customer content in CLI telemetry payloads; identity comes from the bearer token via requestContextProvider). - Side-by-side diff from the existing logRegister handler. - End-to-end verification steps for reviewers. Companion to MCP-Platform PR https://github.com/bap-microsoft/MCP-Platform/pull/2072. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent e135876 commit 2e4cff4

1 file changed

Lines changed: 154 additions & 0 deletions

File tree

‎docs/mcp-platform-logEvaluate.md‎

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
# MCP-Platform changes: `logEvaluate` telemetry endpoint
2+
3+
CLI-side counterpart of this doc: `src/Microsoft.Agents.A365.DevTools.Cli/Services/Agent365ToolingService.cs` (`LogEvaluateUsageAsync`).
4+
5+
This endpoint is a near-clone of the existing `LogRegisterExternalMcpServer` action in `AgentController.cs` (~L1241), with one **deliberate difference**: the body is empty. The evaluated MCP server URL is customer-private content and the CLI does not transmit it as part of this telemetry call.
6+
7+
## Purpose
8+
9+
Capture a per-user marker every time the `a365 develop-mcp evaluate` pipeline runs — distinct from `logRegister` which covers the register flow. Fires at the start of `EvaluationPipelineService.RunAsync`, so any future surface that drives evaluations is also attributed.
10+
11+
## Privacy boundary (why the body is empty)
12+
13+
| Data | Where it lives | Why the CLI doesn't send it |
14+
|---|---|---|
15+
| `userId`, `tenantId` | Bearer token | Already in the JWT — server extracts via `requestContextProvider`. Sending it in the body would duplicate (and weaken) the source of truth. |
16+
| `serverUrl` (evaluated MCP endpoint) | Customer content | Identifies which third-party service the customer connects to. Not telemetry data the CLI gets to ship. |
17+
| `evalEngine` (auto/copilot/claude-code/none) | Customer config | Customer's tooling preference. Not necessary for the marker. |
18+
19+
If the server-side handler can pull useful context from `ServiceContext.Activity` ambient state (because some upstream activity in the same request chain logged it), great — but that's a server-side decision, not a CLI obligation.
20+
21+
## CLI → server contract
22+
23+
| | |
24+
|---|---|
25+
| Route | `POST /agents/externalMcpServers/logEvaluate` |
26+
| Auth | Same scheme + scope as `logRegister` (`McpScopes.AgentToolsPublishMCPServerAll`) |
27+
| Content | `application/json` |
28+
| Body | **Empty** |
29+
| Identity| Bearer token — server extracts via `requestContextProvider.GetOid()` / `GetTenantId()` |
30+
| Response| `200 OK` on success; CLI does not branch on body, swallows non-200 as `LogDebug` |
31+
32+
## CLI-side caller (already shipped)
33+
34+
```
35+
Q:\source\Agent365-devTools\src\Microsoft.Agents.A365.DevTools.Cli\
36+
├── Services\
37+
│ ├── IAgent365ToolingService.cs // Task LogEvaluateUsageAsync(ct)
38+
│ ├── Agent365ToolingService.cs // BuildLogEvaluateUrl + LogEvaluateUsageAsync
39+
│ └── Evaluate\
40+
│ └── EvaluationPipelineService.cs // calls LogEvaluateUsageAsync(ct)
41+
// at top of RunAsync — NOTHING from the
42+
// evaluate args (serverUrl, evalEngine) is
43+
// forwarded into telemetry from the CLI side
44+
```
45+
46+
Transport details the server can assume:
47+
- HTTP client is `Internal.HttpClientFactory.CreateAuthenticatedClient(token)` — same factory `logRegister` uses.
48+
- No `x-ms-correlation-id` header on telemetry calls (matches `logRegister`).
49+
- CLI swallows all non-200 responses with `LogDebug` — telemetry never blocks the user-facing command.
50+
51+
## Server-side change required
52+
53+
### Controller action
54+
55+
Add to `AgentController.cs` right next to `LogRegisterExternalMcpServer` (~L1241). The attribute set, identity extraction, and correlation context setup are **identical to logRegister**. Two deliberate omissions vs. logRegister:
56+
57+
1. **No `[FromBody]` parameter** — body is empty.
58+
2. **No body-derived custom properties** (no `ServerName`/`AuthType`/`ToolCount`-equivalent fields). If `ServiceContext.Activity` already carries useful context from earlier activities in the request chain, you can copy those forward, but do not require them.
59+
60+
```csharp
61+
/// <returns>200 OK if telemetry was logged successfully.</returns>
62+
[HttpPost]
63+
[Route("/agents/externalMcpServers/logEvaluate")]
64+
[MonitorWith(typeof(LogEvaluateExternalMcpServerActivity))]
65+
[AuditClassification(
66+
OperationCategory.CustomerFacing,
67+
OperationType.Read,
68+
AccessLevel.User,
69+
GenevaAuditPlaneType.DataPlane,
70+
[DataClassification.CustomerContent],
71+
isGatewayHostedEndpoint: true,
72+
isCustomerFacing: true,
73+
isDeprecated: false,
74+
"Agent controller log evaluate external MCP server usage API")]
75+
[GatewayAllowPassthrough]
76+
[McpRequiredScope(Constants.McpScopes.AgentToolsPublishMCPServerAll)]
77+
[ProducesResponseType(200)]
78+
[ProducesResponseType(500)]
79+
public IActionResult LogEvaluateExternalMcpServer()
80+
{
81+
try
82+
{
83+
var userId = string.Empty;
84+
var tenantId = string.Empty;
85+
try
86+
{
87+
userId = this.requestContextProvider.GetOid();
88+
tenantId = this.requestContextProvider.GetTenantId();
89+
}
90+
catch (Exception ex)
91+
{
92+
this.logger.LogWarning(ex, "Failed to extract userId or tenantId from request context");
93+
}
94+
95+
var currentClientCorrelationContext = ServiceContext.CaptureRoot()?.ClientCorrelation;
96+
using (ServiceContext.ClientCorrelation.SetClientCorrelationContext(
97+
clientPrincipalId: userId,
98+
clientTenantId: tenantId,
99+
clientSessionId: currentClientCorrelationContext?.ClientSessionId ?? string.Empty,
100+
clientRequestId: currentClientCorrelationContext?.ClientRequestId ?? string.Empty,
101+
clientAppId: currentClientCorrelationContext?.ClientAppIdId ?? string.Empty,
102+
organizationId: currentClientCorrelationContext?.OrganizationId ?? string.Empty))
103+
{
104+
ServiceContext.Activity.Current?.AddCustomProperty("StatusCode", "200");
105+
}
106+
107+
return this.Ok(new { status = "logged" });
108+
}
109+
catch (Exception ex)
110+
{
111+
ServiceContext.Activity.Current?.AddCustomProperty("StatusCode", "500");
112+
ServiceContext.Activity.Current?.AddCustomProperty("Error", ex.GetType().Name);
113+
return this.StatusCode(500, new { error = "Failed to log evaluate telemetry" });
114+
}
115+
}
116+
```
117+
118+
### Activity class
119+
120+
Add `LogEvaluateExternalMcpServerActivity` alongside `LogRegisterExternalMcpServerActivity` (same file/folder, same base class — whatever the existing convention is). No new request DTO is needed.
121+
122+
### Diff from `LogRegisterExternalMcpServer` (every change explained)
123+
124+
| Line region | Change | Reason |
125+
|---|---|---|
126+
| `[Route(...)]` | `logRegister` → `logEvaluate` | New endpoint |
127+
| `[MonitorWith(...)]` | `LogRegisterExternalMcpServerActivity` → `LogEvaluateExternalMcpServerActivity` | New activity class for separate dashboards |
128+
| `AuditClassification` description | `"... log register ..."` → `"... log evaluate ..."` | Cosmetic, matches the activity |
129+
| `[ProducesResponseType(400)]` | **Removed** | No body validation, so no `400 BadRequest` path |
130+
| `[FromBody]` parameter | **Removed** | Empty body — no DTO |
131+
| Body validation block | **Removed** | No fields to validate |
132+
| `AddCustomProperty("ServerName"...)`, `"AuthType"`, `"ToolCount"` | **Removed** | None of these are valid for this endpoint — `serverUrl`/`evalEngine` are customer content and the CLI doesn't ship them |
133+
| 500 error message | `"... registration telemetry"` → `"... evaluate telemetry"` | Cosmetic |
134+
135+
Everything else — `requestContextProvider` usage, `ServiceContext.ClientCorrelation.SetClientCorrelationContext` block, the rest of the attribute set, the scope, the audit classification flags, the outer try/catch shape — is **identical**.
136+
137+
## Reviewer notes
138+
139+
- **No persistence** — telemetry-only, like `logRegister`.
140+
- **Identity via `requestContextProvider`** — same path as logRegister so dashboards join cleanly.
141+
- **No body, no validation** — if the CLI starts sending a body in the future, that is a privacy-review-worthy change and should be discussed before merging.
142+
- **Authorization scope** — `AgentToolsPublishMCPServerAll` (same as logRegister). If evaluate needs a distinct scope, that's a CLI-side change too — coordinate first.
143+
144+
## Out of scope for this PR
145+
146+
- No completion/outcome marker. If you want one later, add a second endpoint (`/logEvaluateComplete`) — do not bolt outcome onto this start-of-workflow marker.
147+
- No retry/backoff in the CLI. Telemetry failure is silently dropped by design.
148+
149+
## How to verify the integration end-to-end
150+
151+
1. Build the CLI from `users/ashragrawal/evaluate` branch with the telemetry changes staged.
152+
2. Run `a365 develop-mcp evaluate --server-url <test-mcp-server> --output-dir /tmp/eval-out`.
153+
3. On the MCP-Platform side, query the Geneva activity sink for `LogEvaluateExternalMcpServerActivity` events. Each event carries `userId` (Entra OID) and `tenantId` from the bearer token. The evaluated server URL is **not** in the event by design.
154+
4. Confirm CLI exit code is unaffected even when the server endpoint returns 500 — telemetry must not block the user's evaluation run.

0 commit comments

Comments
 (0)