Skip to content

fix(plugins): export real tool args when payload.args is a CopyOnWriteDict - #5911

Open
Sanjays2402 wants to merge 1 commit into
IBM:mainfrom
Sanjays2402:fix/telemetry-exporter-copyonwrite-args
Open

fix(plugins): export real tool args when payload.args is a CopyOnWriteDict#5911
Sanjays2402 wants to merge 1 commit into
IBM:mainfrom
Sanjays2402:fix/telemetry-exporter-copyonwrite-args

Conversation

@Sanjays2402

Copy link
Copy Markdown

Closes #5699

📌 Summary

ToolsTelemetryExporter always exported tool.invocation.args as {} on the tool.pre_invoke span, even when the tool was invoked with real arguments, so tool telemetry lost all input data.

🔁 Reproduction Steps

Issue #5699. Offline reproduction of the root cause:

>>> import orjson
>>> from cpex.framework.memory import CopyOnWriteDict
>>> orjson.dumps(CopyOnWriteDict({"sysid": "PRD", "client": "999"}), default=str)
b'{}'
>>> orjson.dumps(dict(CopyOnWriteDict({"sysid": "PRD", "client": "999"})), default=str)
b'{"sysid":"PRD","client":"999"}'

🐞 Root Cause

tool_pre_invoke passes payload.args straight to orjson.dumps (plugins/tools_telemetry_exporter/telemetry_exporter.py:171). When the plugin framework hands over a cpex.framework.memory.CopyOnWriteDict, orjson serializes a dict subclass by reading its internal C-level storage and bypasses the Python-level overrides the copy-on-write wrapper relies on. The backing shell is empty, so the export is {} while repr() and normal access show the real arguments.

💡 Fix Description

Convert to a plain dict before dumping. One line, no behaviour change for the plain-dict case (nested values are already plain containers).

📏 Reviewability

  • This PR has one clear purpose
  • The linked issue is not labeled triage
  • Unrelated bugs or improvements are tracked in separate issues/PRs
  • Tests are included with the code they validate
  • If AI-assisted, I understand and can explain the generated changes

🧪 Verification

New test_pre_invoke_exports_copy_on_write_args in the plugin's existing test module. Verified fail-without-fix / pass-with-fix by stashing the source change:

Check Command Status
Plugin unit tests pytest tests/unit/mcpgateway/plugins/plugins/tools_telemetry_exporter/ 6 passed
New test without the source fix same, -k copy_on_write 1 failed (expected)
Lint / format ruff check, ruff format --check on both changed files clean

📐 MCP Compliance (if relevant)

  • Matches current MCP spec
  • No breaking change to MCP clients

✅ Checklist

  • Code formatted (make black isort pre-commit)
  • No secrets/credentials committed

…eDict

ToolsTelemetryExporter serialized payload.args directly with orjson.
orjson reads a dict subclass's internal C-level storage rather than its
Python-level accessors, so a cpex CopyOnWriteDict (whose real contents
live behind overridden methods, with an empty backing shell) serialized
to "{}". Every tool.pre_invoke span exported an empty
tool.invocation.args even when the tool was called with real arguments.

Converting to a plain dict before dumping restores the real arguments and
is a no-op for the plain-dict case.

Adds a regression test in the plugin's existing test module asserting the
exported args for a CopyOnWriteDict payload.

Signed-off-by: Sanjay Santhanam <51058514+Sanjays2402@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG]: ToolsTelemetryExporter plugin exports empty tool.invocation.args due to orjson not serializing CopyOnWriteDict correctly

1 participant