Repository navigation
[dotnet-port-fixes] Emit execute_tool spans for auto-called tools - #446
Conversation
Port microsoft/agent-framework#6667 so toolautocall emits execute_tool spans under the existing invoke_agent span when otelprovider is active. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…06202207-fb066c86f4752aa5
There was a problem hiding this comment.
Pull request overview
Ports the OpenTelemetry tracing parity fix so that when otelprovider and toolautocall are used together, auto-called tool executions emit nested execute_tool spans under the existing invoke_agent span (using the same instrumentation scope).
Changes:
- Share the
otelprovidertracer via context to enable nested instrumentation. - Instrument
toolautocalltool execution withexecute_toolspans and record tool errors on those spans. - Add an OpenTelemetry integration test asserting
execute_toolspans are emitted and parented correctly.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| agent/harness/toolautocall/autocall.go | Starts execute_tool spans around tool execution when a tracer is present in context, and records tool errors on the span. |
| internal/otelx/context.go | Adds internal helpers to store/retrieve an OTel tracer on context.Context. |
| provider/otelprovider/otel.go | Stores the middleware’s tracer into context during Run so downstream middleware/tools can use it. |
| provider/otelprovider/otel_test.go | Adds an integration-style test that drives toolautocall and asserts execute_tool span scope + parentage under invoke_agent. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Generated by Go API Consistency Review Agent for #446 · 207.8 AIC · ⌖ 12.8 AIC · ⊞ 28.8K
|
Copilot address the parity comment on this PR -- ensuring parity with .NET framework |
…ol spans for cross-SDK parity Co-authored-by: michelle-clayton-work <262183035+michelle-clayton-work@users.noreply.github.com>
Added |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Generated by Go API Consistency Review Agent for #446 · 228.8 AIC · ⌖ 12.7 AIC · ⊞ 27.1K
|
Copilot address PR feedback |
…cross-SDK parity Co-authored-by: michelle-clayton-work <262183035+michelle-clayton-work@users.noreply.github.com>
Addressed both inline parity gaps in the latest commit:
|
|
Copilot fix lint errors |
Co-authored-by: michelle-clayton-work <262183035+michelle-clayton-work@users.noreply.github.com>
Fixed in the latest commit — removed the extra alignment spaces in the |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Generated by Go API Consistency Review Agent for #446 · 242.3 AIC · ⌖ 13 AIC · ⊞ 27.1K
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Generated by Go API Consistency Review Agent for #446 · 456.5 AIC · ⌖ 12.8 AIC · ⊞ 27.1K
Co-authored-by: michelle-clayton-work <262183035+michelle-clayton-work@users.noreply.github.com>
The remaining parity gap was in the |
|
Copilot address open parity feedback and resolve any comments that were already addressed |
…ts/result) Co-authored-by: michelle-clayton-work <262183035+michelle-clayton-work@users.noreply.github.com>
All previously addressed feedback threads are already marked resolved. The one remaining open thread — the sensitive-data span attributes ( |
This comment has been minimized.
This comment has been minimized.
…06202207-fb066c86f4752aa5
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Generated by Go API Consistency Review Agent for issue #446 · 225.8 AIC · ⌖ 11.7 AIC · ⊞ 28.4K
| }() | ||
| if err != nil { | ||
| if span != nil { | ||
| span.SetAttributes(attribute.String(attrKeyErrorType, fmt.Sprintf("%T", err))) |
There was a problem hiding this comment.
Cross-SDK parity: error.type value format diverges from Python
The upstream Python capture_exception helper sets the attribute as:
span.set_attribute(OtelAttr.ERROR_TYPE, type(exception).__name__)
# e.g. → "ValueError", "KeyError", "RuntimeError"Go here uses fmt.Sprintf("%T", err), which produces the fully-qualified type name including the package path and pointer sigil:
fmt.Sprintf("%T", err) // → "*errors.errorString", "*os.PathError"A consumer correlating error.type values across the Python and Go SDKs (e.g. in a dashboard or alert rule) would see completely different strings for the same logical error kind.
Suggested alignment — use only the short type name to match Python's type(exception).__name__:
// reflect.TypeOf(err).String() → "*errors.errorString" (still includes pointer)
// reflect.TypeOf(err).Elem().Name() is empty for non-struct types
// A simple helper that strips the package path and pointer:
func errorTypeName(err error) string {
t := fmt.Sprintf("%T", err)
if i := strings.LastIndexByte(t, '.'); i >= 0 {
t = t[i+1:]
}
return t
}
// → "errorString", "PathError"(Python's __name__ also omits the pointer sigil, so "errorString" rather than "*errorString" is the closest match.)
The same pattern is used in both autocall.go (this file) and otelprovider/otel.go; both need to be updated together.
Upstream reference: python/packages/core/agent_framework/observability.py → capture_exception
|
Copilot address PR feedback |
…xception).__name__ Co-authored-by: michelle-clayton-work <262183035+michelle-clayton-work@users.noreply.github.com>
Co-authored-by: michelle-clayton-work <262183035+michelle-clayton-work@users.noreply.github.com>
Fixed the open parity gap in the latest two commits. |
Cross-SDK Parity Review: ✅ AlignedThis PR ports Python parity (
.NET parity (
Scope: All new files are under
|
Port microsoft/agent-framework#6667 so toolautocall emits execute_tool spans under the existing invoke_agent span when otelprovider is active. Adds cross-SDK parity with Python and .NET by including the following GenAI semantic-convention attributes on every
execute_toolspan:gen_ai.tool.call.id— fromfuncCall.CallID, with"unknown"fallback when the LLM omits the call ID (matching Python'stool_call_id or "unknown"behavior)gen_ai.tool.type— always"function"gen_ai.tool.description— conditionally included when the tool has a non-empty description (matching Python'sget_function_span_attributesbehavior)error.type— set on bothexecute_toolandinvoke_agentspans when an error occurs, matching Python'scapture_exceptionpattern (type(exception).__name__→ short unqualified type name, e.g."errorString"rather than"*errors.errorString")The
error.typevalue uses a sharedotelx.ErrorTypeNamehelper (ininternal/otelx/error.go) that strips the package path and pointer sigil fromfmt.Sprintf("%T", err), aligning the Go output with Python'stype(exception).__name__format for consistent cross-SDK correlation in dashboards and alert rules.Closes #435