Python: fix circular reference error in KernelArguments.dumps() during OTel diagnostics#13642
Conversation
…g OTel diagnostics When SEMANTICKERNEL_EXPERIMENTAL_GENAI_ENABLE_OTEL_DIAGNOSTICS_SENSITIVE is enabled, KernelFunction.invoke() calls arguments.dumps() to serialize function arguments for OpenTelemetry spans. If the arguments contain objects with circular references (e.g., KernelProcessStepContext, whose step_message_channel holds back-references to the process graph), this causes json.dumps to raise 'Circular reference detected'. The fix adds defensive handling in KernelArguments.dumps(): - Track seen object IDs to detect circular references in the custom default serializer - Catch ValueError from json.dumps and fall back to per-key serialization, replacing un-serializable values with their type name Fixes microsoft#13393
There was a problem hiding this comment.
Pull request overview
This PR hardens Python KernelArguments.dumps() so OpenTelemetry sensitive diagnostics do not crash function invocations when arguments contain circular references, as seen in Process Framework step contexts.
Changes:
- Add circular reference handling and safer fallbacks inside
KernelArguments.dumps(). - Add unit tests covering basic serialization, Pydantic model serialization, and circular reference inputs.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| python/semantic_kernel/functions/kernel_arguments.py | Adds circular reference detection, Pydantic model_dump() fallback, and a ValueError recovery path during JSON serialization. |
| python/tests/unit/functions/test_kernel_arguments.py | Adds unit tests for KernelArguments.dumps() including a circular reference scenario. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| json.dumps(value, default=default) | ||
| safe_data[key] = value | ||
| except (ValueError, TypeError): | ||
| safe_data[key] = f"<{type(value).__name__}>" |
There was a problem hiding this comment.
The seen set is mutated as a side effect of calling json.dumps(value, default=default) in the fallback loop, but safe_data[key] stores the original value. When the final json.dumps(safe_data, default=default) runs, default() can see the same object again and incorrectly treat it as a circular reference (for example, a non circular BaseModel value in another key becomes <circular ref: ...>). Consider building safe_data from already-sanitized JSON-compatible values (for example, store the result of serializing each value once), or reset seen appropriately so values are not re-encoded with stale state.
| safe_data[key] = f"<{type(value).__name__}>" | |
| safe_data[key] = f"<{type(value).__name__}>" | |
| # Reset seen so previous serialization attempts don't affect final encoding. | |
| seen.clear() |
| seen: set[int] = set() | ||
|
|
||
| def default(obj: Any) -> Any: | ||
| obj_id = id(obj) | ||
| if obj_id in seen: | ||
| return f"<circular ref: {type(obj).__name__}>" | ||
| seen.add(obj_id) |
There was a problem hiding this comment.
seen is used as a global "already visited" set, which means repeated references to the same object (even without a cycle) will be serialized as <circular ref: ...> on the second occurrence. If preserving data for shared references matters, consider tracking only the current recursion stack (cycle detection) rather than all previously seen objects, or sanitize containers recursively before calling json.dumps so you can pop from the stack when unwinding.
| # This should not raise ValueError: Circular reference detected | ||
| result = kargs.dumps() | ||
| assert isinstance(result, str) | ||
| assert "data" in result |
There was a problem hiding this comment.
The circular-reference test only asserts that the returned string contains "data". This can pass even if the output is not valid JSON or if the fallback behavior changes unexpectedly. It would be more robust to json.loads(result) and assert the expected placeholder value for the circular input (for example, that parsed["data"] is a safe string placeholder rather than raising).
| assert "data" in result | |
| import json | |
| parsed = json.loads(result) | |
| # Ensure we produced valid JSON with a safe placeholder for the circular data | |
| assert "data" in parsed | |
| assert isinstance(parsed["data"], str) |
| def test_kernel_arguments_dumps_basic(): | ||
| """Test basic dumps serialization.""" | ||
| kargs = KernelArguments(name="test", value=42) | ||
| result = kargs.dumps() | ||
| import json | ||
|
|
||
| parsed = json.loads(result) |
There was a problem hiding this comment.
These tests import json inside the test body. In this test suite json is typically imported at module scope; moving the import to the top keeps imports consistent and avoids repeated imports across tests.
Motivation and Context
Fixes #13393
When running Process Framework steps with
SEMANTICKERNEL_EXPERIMENTAL_GENAI_ENABLE_OTEL_DIAGNOSTICS_SENSITIVE=true, every function invocation fails withCircular reference detected. This happens because:KernelFunction.invoke()callsarguments.dumps()to serialize function arguments for OpenTelemetry span attributesKernelProcessStepContextargument (injected byfind_input_channels()instep_utils.py)KernelProcessStepContextcontains astep_message_channelfield, which is theLocalStepinstance itselfLocalStepextendsKernelBaseModeland containsKernel,KernelFunctionobjects, output edges, and other complex runtime objects that form circular reference chainsdumps()callsmodel_dump()on the context, then passes the result tojson.dumps(), the circular references causeValueError: Circular reference detectedDescription
Makes
KernelArguments.dumps()resilient to circular references:defaultserializer now tracksid()of objects it has already visited, returning a safe placeholder string (<circular ref: ClassName>) instead of recursing into circular structuresmodel_dump()failures: Wrapsmodel_dump()in a try/except so that if a Pydantic model itself raises during serialization, the type name is used as a placeholderValueErrorcatch: Ifjson.dumps()still detects a circular reference (e.g., from nested dicts produced bymodel_dump()), falls back to per-key serialization where each un-serializable value is replaced with its type nameThis is a defensive fix at the serialization boundary — it ensures OTel diagnostics never crash function invocations, regardless of argument complexity.
Contribution Checklist
test_kernel_arguments.py