fix: improve host system log pagination - #13395
Conversation
6c5b64d to
670ac8f
Compare
There was a problem hiding this comment.
Pull request overview
This PR improves host system log pagination by refining both the frontend table layout/initial loading behavior and the agent’s journalctl-based pagination/cursor handling, with new unit tests to lock in the expected query-argument behavior.
Changes:
- Frontend: adjust the host system log table height calculation and run initial status/log/service loads concurrently.
- Agent: refine
journalctlpagination by changing time-range handling for cursor-based pages and trimming results atstartTimeto end pagination cleanly. - Agent: add unit tests for
journalctlargument construction, filter-error handling, and start-time trimming behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| frontend/src/views/log/host-system/index.vue | Sets ComplexTable height diff and parallelizes initial data/status loads. |
| agent/app/service/logs.go | Updates journalctl query execution, cursor/time-range argument construction, filter-error handling, and start-time trimming for pagination. |
| agent/app/service/logs_test.go | Adds targeted unit tests for the new pagination and error-handling logic. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
agent/app/service/logs.go:204
- When paginating with a journal cursor (
cursor.JournalCursor != ""),buildJournalQueryArgsno longer applies the request’sstartTime/endTimebounds (only--after-cursoris used). This can cause later pages to ignore the user-selected time range (e.g., drifting outsideendTimeorstartTime). Consider keeping the time bounds for cursor-based pagination (if supported by journalctl), or explicitly filtering parsed items to[startTime, endTime]in Go so the API consistently respects the requested range across pages.
func buildJournalQueryArgs(req dto.SystemLogReq, startTime, endTime time.Time, pageSize int, cursor *systemLogCursor) []string {
args := []string{
"--no-pager", "--reverse", "--output=json",
}
if cursor != nil && cursor.JournalCursor != "" {
args = append(args, "--after-cursor="+cursor.JournalCursor)
} else {
args = append(args,
"--since", formatJournalQueryTime(startTime),
"--until", formatJournalQueryTime(endTime),
)
}
e606636 to
5f84c80
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
agent/app/service/logs.go:158
- handleJournalQueryError currently returns an empty result whenever a filtered query fails but the unfiltered probe succeeds. That also masks real failures like "unknown/unrecognized option" (e.g., if a journalctl flag is unsupported), making the UI show “no logs” instead of surfacing the error.
if hasSystemLogFilter(req) && probe != nil {
probeOutput, probeErr := probe()
if probeErr == nil {
return dto.SystemLogRes{Source: "journalctl", Items: []dto.SystemLogItem{}}, nil
}
return dto.SystemLogRes{}, newJournalQueryError(probeOutput, probeErr)
}
return dto.SystemLogRes{}, newJournalQueryError(output, queryErr)
agent/app/service/logs.go:214
- Using the journalctl flag "--case-sensitive=no" for keyword filtering can break on systemd versions that support "--grep" but not "--case-sensitive". A more portable approach is to keep using a single --grep argument and embed case-insensitivity in the regex itself.
if keyword := strings.TrimSpace(req.Keyword); keyword != "" {
args = append(args, "--grep", regexp.QuoteMeta(keyword), "--case-sensitive=no")
}
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
agent/app/service/logs.go:156
handleJournalQueryErrorreturns an empty result set when the unfiltered probe succeeds. This can silently hide real failures (e.g., unsupported filter flags) and makes it hard for API consumers/UI to distinguish "no matching logs" from "query failed". Prefer surfacing a clear error when the filtered query fails but the unfiltered probe works (at least for keyword filtering).
if hasSystemLogFilter(req) && probe != nil {
probeOutput, probeErr := probe()
if probeErr == nil {
return dto.SystemLogRes{Source: "journalctl", Items: []dto.SystemLogItem{}}, nil
}
return dto.SystemLogRes{}, newJournalQueryError(probeOutput, probeErr)
agent/app/service/logs.go:214
buildJournalQueryArgsappends--case-sensitive=nowhen a keyword is provided, butGetSystemLogStatusonly probes support for--grep. On systems where--grepexists but--case-sensitivedoes not, keyword searches will fail at runtime (and may be masked by the new probe-based error handling). Consider removing this option or gating it behind a capability check.
if keyword := strings.TrimSpace(req.Keyword); keyword != "" {
args = append(args, "--grep", regexp.QuoteMeta(keyword), "--case-sensitive=no")
}
No description provided.