Python: Refine DevUI request logging#7083
Conversation
There was a problem hiding this comment.
Automated Code Review
Reviewers: 5 | Confidence: 94%
✓ Correctness
The PR cleanly removes verbose request-content logging (raw body and parsed metadata) and adds a focused regression test verifying that private input/metadata markers don't appear in logs. The remaining log statement on line 842 only emits the entity_id value (None when not provided), which doesn't leak request content. The test correctly expects a 400 status (no entity_id in metadata) and validates log absence at DEBUG level. No correctness issues found.
✓ Security Reliability
This PR removes verbose request-content logging (raw body and parsed metadata) from the /v1/responses endpoint and adds a regression test confirming that user input and metadata are not written to server logs at any level. The change is a clear security improvement — it eliminates a potential PII/secret leakage vector through logs. The remaining log statement only outputs the entity_id routing identifier, not user content. No security or reliability concerns found.
✓ Test Coverage
The PR cleanly removes verbose request-content logging (raw body and parsed metadata) from the /v1/responses endpoint and adds a focused regression test that verifies neither input nor metadata values leak into server logs. The test is well-designed: it captures all log levels (DEBUG and above) from the correct logger, uses unique marker strings, and exercises the exact code path where the logging was removed. The test's reliance on a 400 response (missing entity_id) is appropriate since the removed logging occurred before entity_id validation, meaning it exercises the relevant lines without requiring complex executor setup.
✓ Failure Modes
This PR cleanly removes verbose request-content logging (raw body and parsed metadata) from the DevUI server's /v1/responses endpoint and adds a focused regression test verifying that request input and metadata are not written to server logs. The remaining logger.info call logs only the extracted entity_id (an operational routing identifier), which is appropriate. No silent failures, swallowed exceptions, or operational failure modes are introduced or preserved by this change.
✗ Design Approach
The change removes request-body logging from
/v1/responses, but it does not fully address the stated goal of refining DevUI request logging: another request path in the same module still logs caller-suplied metadata verbatim, so sensitive request content can still reach server logs.
Flagged Issues
-
python/packages/devui/agent_framework_devui/_server.py:942-943still logsrequest_data.get("metadata")increate_conversation, so private metadata sent to/v1/conversationswill continue to be written to logs even after this PR removes the analogous/v1/responseslogging.
Automated review by moonbox3's agents
|
Flagged issue
Source: automated DevFlow PR review |
There was a problem hiding this comment.
Pull request overview
This PR refines Python DevUI server logging to avoid emitting verbose request content, and adds a regression test to prevent accidental reintroduction of request-input/metadata logging.
Changes:
- Removed logging of raw request bodies and parsed request metadata from the
/v1/responseshandler. - Added a test ensuring request
inputand non-routingmetadatavalues are not written to DevUI server logs.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| python/packages/devui/agent_framework_devui/_server.py | Removes request-body/metadata diagnostics from the responses endpoint logging path. |
| python/packages/devui/tests/devui/test_server.py | Adds regression coverage to ensure request content is not emitted in server logs. |
|
Addressed in e5d5b18. Removed the remaining conversation-metadata diagnostic and kept the upstream request behavior unchanged. |
Motivation & Context
Keep DevUI request logging focused on operational signals.
Description & Review Guide
Related Issue
Tracked separately.
Contribution Checklist
breaking changelabel (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.