[azd ai agents] Glharper/monitor compact logs#7790
Conversation
…er/monitor-compact-logs
glharper
requested review from
JeffreyCA,
hemarina,
jongio,
rajeshkamal5050,
tg-msft,
therealjohn,
trangevi,
trrwilson,
vhvb1989,
wbreza and
weikanglim
as code owners
There was a problem hiding this comment.
Pull request overview
This PR improves azd ai agent monitor (azure.ai.agents extension) log readability by formatting the hosted agent SSE logstream into compact, single-line entries (with special handling for session metadata), while still supporting a raw passthrough mode for scripting/compatibility.
Changes:
- Add a dedicated SSE log formatter with compact rendering, per-stream coloring, UTC/local timestamp support, and raw passthrough.
- Wire the formatter into
azd ai agent monitorand add--utc/--rawflags. - Add unit tests for SSE formatting behavior and bump extension version + changelog.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| cli/azd/extensions/azure.ai.agents/version.txt | Bumps extension version to 0.1.24-preview. |
| cli/azd/extensions/azure.ai.agents/extension.yaml | Keeps extension manifest version in sync with version.txt. |
| cli/azd/extensions/azure.ai.agents/CHANGELOG.md | Documents the monitor output change and new flags. |
| cli/azd/extensions/azure.ai.agents/internal/cmd/monitor.go | Uses the new formatter and adds --utc/--raw flags. |
| cli/azd/extensions/azure.ai.agents/internal/cmd/monitor_format.go | Implements compact SSE log formatting and parsing logic. |
| cli/azd/extensions/azure.ai.agents/internal/cmd/monitor_format_test.go | Adds formatter test coverage for multiple SSE cases. |
| if len(dataLines) == 0 { | ||
| return | ||
| } | ||
| data := strings.Join(dataLines, "\n") | ||
| f.renderEvent(eventName, data) |
There was a problem hiding this comment.
In formatStream's flush() helper, when an event has an event: line but no data: lines, flush() returns early and leaves eventName unchanged. That can leak the previous event name into the next event (especially for data-only events) and misclassify payloads. Consider always resetting eventName (and clearing dataLines) on blank-line flush, even when len(dataLines)==0.
| if len(dataLines) == 0 { | |
| return | |
| } | |
| data := strings.Join(dataLines, "\n") | |
| f.renderEvent(eventName, data) | |
| if len(dataLines) != 0 { | |
| data := strings.Join(dataLines, "\n") | |
| f.renderEvent(eventName, data) | |
| } |
| func (f *logFormatter) renderEvent(eventName, data string) { | ||
| // Accept events with no "event:" line (data-only) and "event: log". | ||
| // Anything else we don't recognize is passed through raw. | ||
| if eventName != "" && eventName != "log" { | ||
| fmt.Fprintf(f.writer, "event: %s\ndata: %s\n\n", eventName, data) | ||
| return |
There was a problem hiding this comment.
renderEvent claims to pass through unknown event names as raw SSE, but it re-emits the payload as a single data: line. If the original SSE event had multiple data: lines (or if data contains embedded newlines after concatenation), the output is no longer valid SSE because the continuation lines won't be prefixed with data:. Consider preserving the original SSE framing by writing one data: line per original data line (and a blank line terminator), or otherwise emitting a clearly non-SSE raw representation.
| func init() { | ||
| // Disable color globally for stable assertions in this test file. | ||
| color.NoColor = true | ||
| } |
There was a problem hiding this comment.
This test file sets color.NoColor = true in an init() function, which mutates global state for the entire cmd package test run and can affect unrelated tests that rely on color behavior. Prefer a per-test approach (e.g., t.Setenv("NO_COLOR", "1")) or saving/restoring the previous color.NoColor value via t.Cleanup/TestMain to keep tests isolated.
jongio
left a comment
jongio
left a comment
There was a problem hiding this comment.
A couple of low-severity nits below. Solid PR overall, tests cover the interesting cases.
|
@copilot apply changes based on the comments in this thread |
Agent-Logs-Url: https://github.com/Azure/azure-dev/sessions/c595290e-6f3f-4820-9570-e1563fe9cb59 Co-authored-by: glharper <64209257+glharper@users.noreply.github.com>
Applied all three changes from that review thread in 9f5ab2e:
|
Fixes #7209