Add logging for arm mcp server testing - #124
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds structured logging to support Arm MCP server testing by recording tool invocation context and (for selected tools) pairing invocations with their results in a JSONL traffic log.
Changes:
- Extend
log_invocation_reasonto always emit a JSONL “call” entry (and return a stable entry ID for correlation), while keeping YAML logging for non-empty reasons. - Add
log_tool_resultto append a paired JSONL “result” entry. - Wire paired call/result logging into
knowledge_base_searchand add a new unit test module for the logger.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| mcp-local/utils/invocation_logger.py | Adds JSONL traffic logging, returns invocation IDs, and introduces result logging. |
| mcp-local/tests/test_invocation_logger.py | Adds tests validating paired call/result entries and the “no reason” behavior. |
| mcp-local/server.py | Captures invocation ID and logs knowledge_base_search results to the traffic log. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Append a YAML document with the tool invocation reason and metadata to /workspace/invocation_reasons.yaml. | ||
| Also append a JSONL call entry to MCP_LOG_FILE. | ||
|
|
||
| Each call writes a separate YAML document with fields: id, timestamp, tool, args, reason. | ||
| Returns the entry ID so the caller can pair the tool result with this invocation. | ||
| Errors are swallowed to avoid impacting tool execution. |
| import json | ||
|
|
||
| from utils import invocation_logger |
| from utils import invocation_logger | ||
|
|
||
|
|
||
| def test_logs_paired_call_and_result(tmp_path, monkeypatch): |
|
|
||
| LOG_FILE_NAME = "invocation_reasons.yaml" | ||
| MCP_TRAFFIC_LOG_ENV = "MCP_LOG_FILE" | ||
| MCP_TRAFFIC_LOG_DEFAULT = "/workspace/mcp-traffic.jsonl" |
There was a problem hiding this comment.
Should rich JSONL logging be opt-in through MCP_LOG_FILE rather than enabled by default? The current default creates mcp-traffic.jsonl in the host project directory mounted at /workspace. Default-on is more convenient, so I could see either approach, but it seems worth considering and documenting explicitly.
| try: | ||
| return arm_kb_search.search(query, SEARCH_RESOURCES) | ||
| results = arm_kb_search.search(query, SEARCH_RESOURCES) | ||
| log_tool_result(entry_id, "knowledge_base_search", results) |
There was a problem hiding this comment.
This currently writes JSONL call entries for all tools, but writes a corresponding result entry only for successful knowledge_base_search calls. Is that intentional? It seems we should either log results for all tools or scope JSONL logging entirely to knowledge_base_search.
No description provided.