Skip to content

Commit eecac8d

Browse files
Copilotstephentoub
Copilot
and
stephentoub
committed
Address PR feedback: fix span naming, add mcp.protocol.version, detect outer GenAI activity
- Fix span name to follow format "{mcp.method.name} {target}" per semantic conventions - Add mcp.protocol.version attribute to telemetry after initialization - Add network.protocol.name attribute for HTTP transports - Implement outer GenAI activity detection for tool calls (avoid duplicate spans) - Add NegotiatedProtocolVersion property to McpSessionHandler for telemetry - Propagate protocol version from client/server impl to session handler - Update tests to verify mcp.protocol.version attribute Co-authored-by: stephentoub <[email protected]>
1 parent 6c0d9ae commit eecac8d

File tree

5 files changed

+177
-42
lines changed

5 files changed

+177
-42
lines changed

‎src/ModelContextProtocol.Core/Client/McpClientImpl.cs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,9 @@ public async Task ConnectAsync(CancellationToken cancellationToken = default)
187187

188188
_negotiatedProtocolVersion = initializeResponse.ProtocolVersion;
189189

190+
// Update session handler with the negotiated protocol version for telemetry
191+
_sessionHandler.NegotiatedProtocolVersion = _negotiatedProtocolVersion;
192+
190193
// Send initialized notification
191194
await this.SendNotificationAsync(
192195
NotificationMethods.InitializedNotification,
@@ -230,6 +233,9 @@ internal void ResumeSession(ResumeClientSessionOptions resumeOptions)
230233
?? _options.ProtocolVersion
231234
?? McpSessionHandler.LatestProtocolVersion;
232235

236+
// Update session handler with the negotiated protocol version for telemetry
237+
_sessionHandler.NegotiatedProtocolVersion = _negotiatedProtocolVersion;
238+
233239
LogClientSessionResumed(_endpointName);
234240
}
235241

‎src/ModelContextProtocol.Core/Diagnostics.cs‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
using ModelContextProtocol.Protocol;
22
using System.Diagnostics;
3+
using System.Diagnostics.CodeAnalysis;
34
using System.Diagnostics.Metrics;
45
using System.Text.Json;
56
using System.Text.Json.Nodes;
@@ -103,5 +104,33 @@ internal static bool ShouldInstrumentMessage(JsonRpcMessage message) =>
103104
_ => false
104105
};
105106

107+
/// <summary>
108+
/// Per MCP semantic conventions: If outer GenAI instrumentation is already tracing the tool execution,
109+
/// MCP instrumentation SHOULD add MCP-specific attributes to the existing tool execution span instead
110+
/// of creating a new one.
111+
/// </summary>
112+
/// <param name="activity">The outer activity with gen_ai.operation.name = execute_tool, if found.</param>
113+
/// <returns>true if an outer tool execution activity was found and can be reused; false otherwise.</returns>
114+
internal static bool TryGetOuterToolExecutionActivity([NotNullWhen(true)] out Activity? activity)
115+
{
116+
activity = Activity.Current;
117+
if (activity is null)
118+
{
119+
return false;
120+
}
121+
122+
// Check if the current activity has gen_ai.operation.name = execute_tool
123+
foreach (var tag in activity.Tags)
124+
{
125+
if (tag.Key == "gen_ai.operation.name" && tag.Value == "execute_tool")
126+
{
127+
return true;
128+
}
129+
}
130+
131+
activity = null;
132+
return false;
133+
}
134+
106135
internal static ActivityLink[] ActivityLinkFromCurrent() => Activity.Current is null ? [] : [new ActivityLink(Activity.Current.Context)];
107136
}

‎src/ModelContextProtocol.Core/McpSessionHandler.cs‎

Lines changed: 133 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,11 @@ internal sealed partial class McpSessionHandler : IAsyncDisposable
6161

6262
// This _sessionId is solely used to identify the session in telemetry and logs.
6363
private readonly string _sessionId = Guid.NewGuid().ToString("N");
64+
65+
// The negotiated MCP protocol version (set after initialization).
66+
// Note: This is exposed via property for telemetry purposes.
67+
private string? _negotiatedProtocolVersion;
68+
6469
private long _lastRequestId;
6570

6671
private CancellationTokenSource? _messageProcessingCts;
@@ -110,6 +115,15 @@ public McpSessionHandler(
110115
/// </summary>
111116
public string EndpointName { get; set; }
112117

118+
/// <summary>
119+
/// Gets or sets the negotiated MCP protocol version for telemetry.
120+
/// </summary>
121+
public string? NegotiatedProtocolVersion
122+
{
123+
get => _negotiatedProtocolVersion;
124+
set => _negotiatedProtocolVersion = value;
125+
}
126+
113127
/// <summary>
114128
/// Starts processing messages from the transport. This method will block until the transport is disconnected.
115129
/// This is generally started in a background task or thread from the initialization logic of the derived class.
@@ -261,24 +275,40 @@ private async Task HandleMessageAsync(JsonRpcMessage message, CancellationToken
261275
{
262276
Histogram<double> durationMetric = _isServer ? s_serverOperationDuration : s_clientOperationDuration;
263277
string method = GetMethodName(message);
278+
string? target = ExtractTargetFromMessage(message, method);
264279

265280
long? startingTimestamp = durationMetric.Enabled ? Stopwatch.GetTimestamp() : null;
266281

267-
Activity? activity = Diagnostics.ShouldInstrumentMessage(message) ?
268-
Diagnostics.ActivitySource.StartActivity(
269-
CreateActivityName(method),
270-
ActivityKind.Server,
271-
parentContext: _propagator.ExtractActivityContext(message),
272-
links: Diagnostics.ActivityLinkFromCurrent()) :
273-
null;
282+
// Per MCP semantic conventions: If outer GenAI instrumentation is already tracing the tool execution
283+
// (i.e., Activity.Current has gen_ai.operation.name = execute_tool), we should add MCP attributes
284+
// to that activity instead of creating a new one.
285+
Activity? activity = null;
286+
bool usingOuterActivity = false;
287+
if (Diagnostics.ShouldInstrumentMessage(message))
288+
{
289+
if (method == RequestMethods.ToolsCall && Diagnostics.TryGetOuterToolExecutionActivity(out var outerActivity))
290+
{
291+
// Add MCP-specific attributes to the existing tool execution span
292+
activity = outerActivity;
293+
usingOuterActivity = true;
294+
}
295+
else
296+
{
297+
activity = Diagnostics.ActivitySource.StartActivity(
298+
CreateActivityName(method, target),
299+
ActivityKind.Server,
300+
parentContext: _propagator.ExtractActivityContext(message),
301+
links: Diagnostics.ActivityLinkFromCurrent());
302+
}
303+
}
274304

275305
TagList tags = default;
276306
bool addTags = activity is { IsAllDataRequested: true } || startingTimestamp is not null;
277307
try
278308
{
279309
if (addTags)
280310
{
281-
AddTags(ref tags, activity, message, method);
311+
AddTags(ref tags, activity, message, method, target, usingOuterActivity);
282312
}
283313

284314
switch (message)
@@ -319,7 +349,7 @@ private async Task HandleMessageAsync(JsonRpcMessage message, CancellationToken
319349
}
320350
finally
321351
{
322-
FinalizeDiagnostics(activity, startingTimestamp, durationMetric, ref tags);
352+
FinalizeDiagnostics(activity, startingTimestamp, durationMetric, ref tags, disposeActivity: !usingOuterActivity);
323353
}
324354
}
325355

@@ -422,11 +452,27 @@ public async Task<JsonRpcResponse> SendRequestAsync(JsonRpcRequest request, Canc
422452

423453
Histogram<double> durationMetric = _isServer ? s_serverOperationDuration : s_clientOperationDuration;
424454
string method = request.Method;
455+
string? target = ExtractTargetFromMessage(request, method);
425456

426457
long? startingTimestamp = durationMetric.Enabled ? Stopwatch.GetTimestamp() : null;
427-
using Activity? activity = Diagnostics.ShouldInstrumentMessage(request) ?
428-
Diagnostics.ActivitySource.StartActivity(McpSessionHandler.CreateActivityName(method), ActivityKind.Client) :
429-
null;
458+
459+
// Per MCP semantic conventions: If outer GenAI instrumentation is already tracing the tool execution
460+
// (i.e., Activity.Current has gen_ai.operation.name = execute_tool), we should add MCP attributes
461+
// to that activity instead of creating a new one.
462+
Activity? activity = null;
463+
bool usingOuterActivity = false;
464+
if (Diagnostics.ShouldInstrumentMessage(request))
465+
{
466+
if (method == RequestMethods.ToolsCall && Diagnostics.TryGetOuterToolExecutionActivity(out var outerActivity))
467+
{
468+
activity = outerActivity;
469+
usingOuterActivity = true;
470+
}
471+
else
472+
{
473+
activity = Diagnostics.ActivitySource.StartActivity(CreateActivityName(method, target), ActivityKind.Client);
474+
}
475+
}
430476

431477
// Set request ID
432478
if (request.Id.Id is null)
@@ -445,7 +491,7 @@ public async Task<JsonRpcResponse> SendRequestAsync(JsonRpcRequest request, Canc
445491
{
446492
if (addTags)
447493
{
448-
AddTags(ref tags, activity, request, method);
494+
AddTags(ref tags, activity, request, method, target, usingOuterActivity);
449495
}
450496

451497
if (_logger.IsEnabled(LogLevel.Trace))
@@ -506,7 +552,7 @@ public async Task<JsonRpcResponse> SendRequestAsync(JsonRpcRequest request, Canc
506552
finally
507553
{
508554
_pendingRequests.TryRemove(request.Id, out _);
509-
FinalizeDiagnostics(activity, startingTimestamp, durationMetric, ref tags);
555+
FinalizeDiagnostics(activity, startingTimestamp, durationMetric, ref tags, disposeActivity: !usingOuterActivity);
510556
}
511557
}
512558

@@ -518,10 +564,11 @@ public async Task SendMessageAsync(JsonRpcMessage message, CancellationToken can
518564

519565
Histogram<double> durationMetric = _isServer ? s_serverOperationDuration : s_clientOperationDuration;
520566
string method = GetMethodName(message);
567+
string? target = ExtractTargetFromMessage(message, method);
521568

522569
long? startingTimestamp = durationMetric.Enabled ? Stopwatch.GetTimestamp() : null;
523570
using Activity? activity = Diagnostics.ShouldInstrumentMessage(message) ?
524-
Diagnostics.ActivitySource.StartActivity(McpSessionHandler.CreateActivityName(method), ActivityKind.Client) :
571+
Diagnostics.ActivitySource.StartActivity(CreateActivityName(method, target), ActivityKind.Client) :
525572
null;
526573

527574
TagList tags = default;
@@ -534,7 +581,7 @@ public async Task SendMessageAsync(JsonRpcMessage message, CancellationToken can
534581
{
535582
if (addTags)
536583
{
537-
AddTags(ref tags, activity, message, method);
584+
AddTags(ref tags, activity, message, method, target, usingOuterActivity: false);
538585
}
539586

540587
if (_logger.IsEnabled(LogLevel.Trace))
@@ -565,7 +612,7 @@ public async Task SendMessageAsync(JsonRpcMessage message, CancellationToken can
565612
}
566613
finally
567614
{
568-
FinalizeDiagnostics(activity, startingTimestamp, durationMetric, ref tags);
615+
FinalizeDiagnostics(activity, startingTimestamp, durationMetric, ref tags, disposeActivity: true);
569616
}
570617
}
571618

@@ -589,6 +636,38 @@ private Task SendToRelatedTransportAsync(JsonRpcMessage message, CancellationTok
589636

590637
private static string CreateActivityName(string method) => method;
591638

639+
/// <summary>
640+
/// Creates a span name according to semantic conventions: "{mcp.method.name} {target}" where
641+
/// target is the tool name, prompt name, or resource URI when applicable.
642+
/// </summary>
643+
private static string CreateActivityName(string method, string? target) =>
644+
target is null ? method : $"{method} {target}";
645+
646+
/// <summary>
647+
/// Extracts the target (tool name, prompt name, or resource URI) from a message for use in span naming.
648+
/// </summary>
649+
private static string? ExtractTargetFromMessage(JsonRpcMessage message, string method)
650+
{
651+
JsonObject? paramsObj = message switch
652+
{
653+
JsonRpcRequest request => request.Params as JsonObject,
654+
JsonRpcNotification notification => notification.Params as JsonObject,
655+
_ => null
656+
};
657+
658+
if (paramsObj is null)
659+
{
660+
return null;
661+
}
662+
663+
return method switch
664+
{
665+
RequestMethods.ToolsCall or RequestMethods.PromptsGet => GetStringProperty(paramsObj, "name"),
666+
// Note: resource URI is not included in span name by default due to high cardinality per semantic conventions
667+
_ => null
668+
};
669+
}
670+
592671
private static string GetMethodName(JsonRpcMessage message) =>
593672
message switch
594673
{
@@ -597,11 +676,23 @@ private static string GetMethodName(JsonRpcMessage message) =>
597676
_ => "unknownMethod"
598677
};
599678

600-
private void AddTags(ref TagList tags, Activity? activity, JsonRpcMessage message, string method)
679+
private void AddTags(ref TagList tags, Activity? activity, JsonRpcMessage message, string method, string? target, bool usingOuterActivity)
601680
{
602681
tags.Add("mcp.method.name", method);
603682
tags.Add("network.transport", _transportKind);
604683

684+
// Per semantic conventions: network.protocol.name when applicable (HTTP transports)
685+
if (_transportKind == "tcp")
686+
{
687+
tags.Add("network.protocol.name", "http");
688+
}
689+
690+
// Per semantic conventions: mcp.protocol.version is Recommended
691+
if (_negotiatedProtocolVersion is not null)
692+
{
693+
tags.Add("mcp.protocol.version", _negotiatedProtocolVersion);
694+
}
695+
605696
// TODO: When using HTTP transport, add:
606697
// - server.address and server.port on client spans and metrics
607698
// - client.address and client.port on server spans (not metrics because of cardinality)
@@ -615,25 +706,18 @@ private void AddTags(ref TagList tags, Activity? activity, JsonRpcMessage messag
615706
{
616707
activity.AddTag("jsonrpc.request.id", withId.Id.Id?.ToString());
617708
}
618-
}
619-
620-
JsonObject? paramsObj = message switch
621-
{
622-
JsonRpcRequest request => request.Params as JsonObject,
623-
JsonRpcNotification notification => notification.Params as JsonObject,
624-
_ => null
625-
};
626709

627-
if (paramsObj == null)
628-
{
629-
return;
710+
// If we're adding tags to an outer activity, we don't need to set DisplayName as it's already set
711+
if (!usingOuterActivity && target is not null)
712+
{
713+
activity.DisplayName = $"{method} {target}";
714+
}
630715
}
631716

632-
string? target = null;
717+
// Add target-specific tags based on method
633718
switch (method)
634719
{
635720
case RequestMethods.ToolsCall:
636-
target = GetStringProperty(paramsObj, "name");
637721
if (target is not null)
638722
{
639723
// Per semantic conventions: gen_ai.tool.name for tool operations
@@ -644,7 +728,6 @@ private void AddTags(ref TagList tags, Activity? activity, JsonRpcMessage messag
644728
break;
645729

646730
case RequestMethods.PromptsGet:
647-
target = GetStringProperty(paramsObj, "name");
648731
if (target is not null)
649732
{
650733
// Per semantic conventions: gen_ai.prompt.name for prompt operations
@@ -656,18 +739,22 @@ private void AddTags(ref TagList tags, Activity? activity, JsonRpcMessage messag
656739
case RequestMethods.ResourcesSubscribe:
657740
case RequestMethods.ResourcesUnsubscribe:
658741
case NotificationMethods.ResourceUpdatedNotification:
659-
target = GetStringProperty(paramsObj, "uri");
660-
if (target is not null)
661742
{
662-
tags.Add("mcp.resource.uri", target);
743+
// Get resource URI from params (not included in span name due to high cardinality)
744+
JsonObject? paramsObj = message switch
745+
{
746+
JsonRpcRequest request => request.Params as JsonObject,
747+
JsonRpcNotification notification => notification.Params as JsonObject,
748+
_ => null
749+
};
750+
string? uri = paramsObj is not null ? GetStringProperty(paramsObj, "uri") : null;
751+
if (uri is not null)
752+
{
753+
tags.Add("mcp.resource.uri", uri);
754+
}
663755
}
664756
break;
665757
}
666-
667-
if (activity is { IsAllDataRequested: true })
668-
{
669-
activity.DisplayName = target == null ? method : $"{method} {target}";
670-
}
671758
}
672759

673760
private static void AddExceptionTags(ref TagList tags, Activity? activity, Exception e)
@@ -718,7 +805,7 @@ private static void AddResponseTags(ref TagList tags, Activity? activity, JsonNo
718805
}
719806

720807
private static void FinalizeDiagnostics(
721-
Activity? activity, long? startingTimestamp, Histogram<double> durationMetric, ref TagList tags)
808+
Activity? activity, long? startingTimestamp, Histogram<double> durationMetric, ref TagList tags, bool disposeActivity = true)
722809
{
723810
try
724811
{
@@ -737,7 +824,11 @@ private static void FinalizeDiagnostics(
737824
}
738825
finally
739826
{
740-
activity?.Dispose();
827+
// Only dispose the activity if we created it (not when reusing an outer GenAI activity)
828+
if (disposeActivity)
829+
{
830+
activity?.Dispose();
831+
}
741832
}
742833
}
743834

‎src/ModelContextProtocol.Core/Server/McpServerImpl.cs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,9 @@ private void ConfigureInitialize(McpServerOptions options)
213213

214214
_negotiatedProtocolVersion = protocolVersion;
215215

216+
// Update session handler with the negotiated protocol version for telemetry
217+
_sessionHandler.NegotiatedProtocolVersion = protocolVersion;
218+
216219
return new InitializeResult
217220
{
218221
ProtocolVersion = protocolVersion,

0 commit comments

Comments
 (0)