feat: add log download #1582
feat: add log download #1582
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughAdds server endpoints to download DAG-run and step logs, implements corresponding frontend download buttons, extends generated API types and routing for the new endpoints, and adjusts file-audit day-iteration to use timezone-aware day boundaries. Also expands frontend audit-log date filtering logic and UI. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as Browser UI
participant API as Frontend API Handler
participant MGR as dagRunMgr / Store
participant FS as File System
User->>UI: Click "Download" for DAG/step log
UI->>API: GET /dag-runs/{name}/{id}/.../log/download?remoteNode=...
API->>MGR: Validate DAG-run / (sub)step existence
alt not found
MGR-->>API: NotFound
API-->>UI: 404 JSON
else found
API->>FS: Read log file path
FS-->>API: File bytes
API-->>UI: 200 with Content-Disposition (attachment)
UI->>UI: Create blob & save file
UI-->>User: File download starts
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@internal/persis/fileaudit/store.go`:
- Around line 114-123: Replace the Truncate-based day boundaries with explicit
date construction using time.Date so boundaries are midnight in the original
timestamp's location: compute fileStartDate := time.Date(startDate.Year(),
startDate.Month(), startDate.Day(), 0, 0, 0, 0, startDate.Location()) and
fileEndDate := time.Date(endDate.Year(), endDate.Month(), endDate.Day(), 0, 0,
0, 0, endDate.Location()), then iterate from fileStartDate to fileEndDate (as
currently done) and continue to use s.auditFilePath(d) and readEntriesFromFile
for per-entry filtering.
In `@ui/src/pages/audit-logs/index.tsx`:
- Around line 103-126: getSpecificPeriodDates currently formats start/end with
'YYYY-MM-DDTHH:mm', which truncates endOf(unit) milliseconds to minutes and then
later code (formatDateForApi) appends ":00", dropping the final seconds and
excluding up to ~59s of data; update getSpecificPeriodDates (the fallback branch
and the return values) to format with seconds, e.g. 'YYYY-MM-DDTHH:mm:ss' for
both from and to so endOf(unit) precision is preserved (keep the period -> unit
mapping and tzOffset handling the same).
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
internal/persis/fileaudit/store.goui/src/pages/audit-logs/index.tsx
🧰 Additional context used
📓 Path-based instructions (4)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Backend entrypoint incmd/orchestrates the scheduler and CLI; runtime, persistence, and service layers sit underinternal/*(for exampleinternal/runtime,internal/persistence)
Keep Go filesgofmt/goimportsclean; use tabs, PascalCase for exported symbols (SchedulerClient), lowerCamelCase for locals, andErr...names for package-level errors
Repository linting relies ongolangci-lint; prefer idiomatic Go patterns, minimal global state, and structured logging helpers ininternal/common
Files:
internal/persis/fileaudit/store.go
ui/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
ui/**/*.{ts,tsx}: The React + TypeScript frontend resides inui/, with production bundles copied tointernal/service/frontend/assetsbymake ui
UI code follows ESLint + Prettier (2-space indent) and Tailwind utilities; name React components in PascalCase (JobList.tsx) and hooks withuse*(useJobs.ts)
Files:
ui/src/pages/audit-logs/index.tsx
ui/**/*.{tsx,ts,jsx,js}
📄 CodeRabbit inference engine (ui/CLAUDE.md)
Avoid full-page loading overlays and LoadingIndicator components that hide content - show stale data while fetching updates instead
Files:
ui/src/pages/audit-logs/index.tsx
ui/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (ui/CLAUDE.md)
ui/**/*.{tsx,jsx}: Keep modal headers small and information-dense with minimal padding (e.g.,p-2orp-3instead ofp-4orp-6)
Use compact form element heights: select boxes withh-7or smaller, buttons withh-7orh-8, inputs with compact padding (py-0.5orpy-1)
Minimize table and list row heights while maintaining readability, merge related columns to save space, and handle long text withwhitespace-normal break-words
Use consistent metadata styling with uniform backgrounds (e.g.,bg-slate-200 dark:bg-slate-700) and text hierarchy using size/weight over color variation
Use flexbox-first layouts withmin-h-0andoverflow-hiddento prevent layout breaks, and account for fixed elements when setting heights
Support keyboard navigation in all interactive components including modals with arrow keys, enter, and escape keys
Avoid auto-focusing first items in modals unless it makes sense for the specific use case
Maintain sufficient color contrast in both light and dark modes, use proper ARIA labels, and ensure text remains readable at smaller sizes
Use transparent backgrounds in navigation elements and keep navigation components small and unobtrusive
Avoid two-line displays for single metadata items, excessive whitespace between elements, decorative elements without purpose, and modals that take up excessive screen space
Files:
ui/src/pages/audit-logs/index.tsx
🧠 Learnings (2)
📚 Learning: 2026-01-12T16:59:14.752Z
Learnt from: CR
Repo: dagu-org/dagu PR: 0
File: ui/CLAUDE.md:0-0
Timestamp: 2026-01-12T16:59:14.752Z
Learning: Applies to ui/**/*.{tsx,jsx} : Support keyboard navigation in all interactive components including modals with arrow keys, enter, and escape keys
Applied to files:
ui/src/pages/audit-logs/index.tsx
📚 Learning: 2026-01-12T16:59:14.752Z
Learnt from: CR
Repo: dagu-org/dagu PR: 0
File: ui/CLAUDE.md:0-0
Timestamp: 2026-01-12T16:59:14.752Z
Learning: Applies to ui/**/*.{tsx,jsx} : Use compact form element heights: select boxes with `h-7` or smaller, buttons with `h-7` or `h-8`, inputs with compact padding (`py-0.5` or `py-1`)
Applied to files:
ui/src/pages/audit-logs/index.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test on ubuntu-latest
- GitHub Check: Build
🔇 Additional comments (2)
ui/src/pages/audit-logs/index.tsx (2)
350-452: Verify keyboard navigation for the new date filter controls.Please confirm that ToggleGroup/ToggleButton and DateRangePicker support arrow keys, Enter, and Escape, and that focus order is sensible when switching modes. Based on learnings, ensure keyboard navigation is fully supported for these new interactive elements.
Based on learnings, please verify keyboard accessibility for the added controls.
129-138: No fix needed—the potential date-only input bug does not exist.The
DateRangePickercomponent always outputs dates inYYYY-MM-DDTHH:mmformat (with aTseparator), as seen inCustomDateTimeInput.parseDisplayValue(line 72) and all itsonChangehandlers. TheformatDateForApifunction receives these properly formatted strings and correctly appends:00for missing seconds, producing valid ISO timestamps like2024-01-15T14:30:00. Date-only inputs (withoutT) are not possible with the current implementation, so the guard againstYYYY-MM-DD:00is unnecessary.Likely an incorrect or invalid review comment.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@internal/service/frontend/api/v2/dagruns.go`:
- Around line 633-679: The Content-Disposition filename is constructed from
unvalidated components (dagName, dagRunId, request.StepName) in
DownloadDAGRunStepLog (and other similar handlers) and may allow header
injection via quotes/CRLF in step names; before building filename and setting
DownloadDAGRunStepLog200ResponseHeaders.ContentDisposition, sanitize each
filename component (at least request.StepName, and preferably dagName/dagRunId)
by removing or escaping CR/LF and quoting characters and replacing unsafe
characters with safe substitutes (e.g., allow only alphanumerics, hyphen,
underscore, dot or percent-encode others, or use RFC5987-style encoding), then
use the sanitized values to form filename and Content-Disposition for all
occurrences referenced (the variable filename and the ContentDisposition
assignments in DownloadDAGRunStepLog and the other three Content-Disposition
locations).
In `@ui/src/features/dags/components/dag-execution/ExecutionLog.tsx`:
- Around line 271-304: The download routine in handleDownload currently builds
the fetch URL with window.location.origin which ignores the app's configurable
API base; import and use useConfig (from ConfigContext) inside the component to
get config.apiURL and construct the endpoint base from config.apiURL (e.g. new
URL(endpoint, config.apiURL)), falling back to window.location.origin if
config.apiURL is falsy; update the handleDownload dependency list to include
config.apiURL (or config) so the hook updates when the base changes and keep
existing token/auth and blob handling logic intact.
In `@ui/src/features/dags/components/dag-execution/StepLog.tsx`:
- Around line 284-318: The download handler handleDownload currently builds the
fetch URL against window.location.origin which breaks non-same-origin
deployments; use the app config API base instead by calling useConfig() to get
config.apiURL and build the URL with new URL(endpoint, config.apiURL) (same
approach used elsewhere like ExecutionLog), replace window.location.origin with
config.apiURL, and add config (or the apiURL) to the hook dependency array so
handleDownload updates if the configured API base changes.
♻️ Duplicate comments (3)
internal/persis/fileaudit/store.go (1)
114-123: Convert endDate to the chosen timezone before extracting date components.When
endDatedefaults totime.Now().UTC()butstartDatehas a different location, extractingendDate's Y/M/D without normalization toloccauses the final day boundary to shift. This can skip entries on the last day of the query range. ConvertendDatetolocbefore creatingfileEndDate:loc := startDate.Location() + endInLoc := endDate.In(loc) fileStartDate := time.Date(startDate.Year(), startDate.Month(), startDate.Day(), 0, 0, 0, 0, loc) - fileEndDate := time.Date(endDate.Year(), endDate.Month(), endDate.Day(), 0, 0, 0, 0, loc) + fileEndDate := time.Date(endInLoc.Year(), endInLoc.Month(), endInLoc.Day(), 0, 0, 0, 0, loc)internal/service/frontend/api/v2/dagruns.go (2)
1214-1243: Same memory/size concern as DownloadDAGRunLog.
os.ReadFile+string(content)can cause large memory spikes for big sub-DAG logs; consider streaming or enforcing a size cap as noted earlier.
1299-1343: Same filename sanitization concern as DownloadDAGRunStepLog.Sanitize DAG/step identifiers before embedding in
Content-Dispositionto avoid malformed headers or unsafe filenames.
🧹 Nitpick comments (3)
ui/src/pages/audit-logs/index.tsx (1)
350-455: Add arrow-key navigation for the date mode toggle.The ToggleGroup is operable via Tab/Enter, but adding ArrowLeft/ArrowRight roving focus would meet the keyboard-navigation requirement for grouped controls. Based on learnings, please ensure arrow-key navigation for this control.
internal/service/frontend/api/v2/dagruns.go (1)
477-509: Avoid loading full log into memory for downloads.
os.ReadFile+string(content)reads the entire log and duplicates it in memory, which can spike usage for large logs and stall request handlers. Consider streaming (e.g.,os.Open+io.Copyin a custom response writer) or enforcing a max download size.api/v2/api.gen.go (1)
7822-8191: Stream logs to avoid buffering full payloads in memory.The current OpenAPI definitions for
DownloadDAGRunLog,DownloadDAGRunStepLog,DownloadSubDAGRunLog, andDownloadSubDAGRunStepLogusetext/plainwithtype: string, which generates response types with aBody stringfield. This forces the entire log to be materialized into a string before sending.Update
api/v2/api.yamlto define these responses asapplication/octet-streamwithtype: string, format: binary. oapi-codegen will then generate response helpers acceptingio.Reader, allowing logs to be streamed directly without buffering.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
api/v2/api.gen.goapi/v2/api.yamlinternal/persis/fileaudit/store.gointernal/service/frontend/api/v2/dagruns.goui/src/features/dags/components/dag-execution/ExecutionLog.tsxui/src/features/dags/components/dag-execution/StepLog.tsxui/src/pages/audit-logs/index.tsx
🧰 Additional context used
📓 Path-based instructions (5)
ui/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
ui/**/*.{ts,tsx}: The React + TypeScript frontend resides inui/, with production bundles copied tointernal/service/frontend/assetsbymake ui
UI code follows ESLint + Prettier (2-space indent) and Tailwind utilities; name React components in PascalCase (JobList.tsx) and hooks withuse*(useJobs.ts)
Files:
ui/src/features/dags/components/dag-execution/ExecutionLog.tsxui/src/pages/audit-logs/index.tsxui/src/features/dags/components/dag-execution/StepLog.tsx
ui/**/*.{tsx,ts,jsx,js}
📄 CodeRabbit inference engine (ui/CLAUDE.md)
Avoid full-page loading overlays and LoadingIndicator components that hide content - show stale data while fetching updates instead
Files:
ui/src/features/dags/components/dag-execution/ExecutionLog.tsxui/src/pages/audit-logs/index.tsxui/src/features/dags/components/dag-execution/StepLog.tsx
ui/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (ui/CLAUDE.md)
ui/**/*.{tsx,jsx}: Keep modal headers small and information-dense with minimal padding (e.g.,p-2orp-3instead ofp-4orp-6)
Use compact form element heights: select boxes withh-7or smaller, buttons withh-7orh-8, inputs with compact padding (py-0.5orpy-1)
Minimize table and list row heights while maintaining readability, merge related columns to save space, and handle long text withwhitespace-normal break-words
Use consistent metadata styling with uniform backgrounds (e.g.,bg-slate-200 dark:bg-slate-700) and text hierarchy using size/weight over color variation
Use flexbox-first layouts withmin-h-0andoverflow-hiddento prevent layout breaks, and account for fixed elements when setting heights
Support keyboard navigation in all interactive components including modals with arrow keys, enter, and escape keys
Avoid auto-focusing first items in modals unless it makes sense for the specific use case
Maintain sufficient color contrast in both light and dark modes, use proper ARIA labels, and ensure text remains readable at smaller sizes
Use transparent backgrounds in navigation elements and keep navigation components small and unobtrusive
Avoid two-line displays for single metadata items, excessive whitespace between elements, decorative elements without purpose, and modals that take up excessive screen space
Files:
ui/src/features/dags/components/dag-execution/ExecutionLog.tsxui/src/pages/audit-logs/index.tsxui/src/features/dags/components/dag-execution/StepLog.tsx
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Backend entrypoint incmd/orchestrates the scheduler and CLI; runtime, persistence, and service layers sit underinternal/*(for exampleinternal/runtime,internal/persistence)
Keep Go filesgofmt/goimportsclean; use tabs, PascalCase for exported symbols (SchedulerClient), lowerCamelCase for locals, andErr...names for package-level errors
Repository linting relies ongolangci-lint; prefer idiomatic Go patterns, minimal global state, and structured logging helpers ininternal/common
Files:
internal/persis/fileaudit/store.gointernal/service/frontend/api/v2/dagruns.goapi/v2/api.gen.go
{api/v1,api/v2}/**/*.{proto,yaml,yml,json}
📄 CodeRabbit inference engine (AGENTS.md)
API definitions live in
api/v1andapi/v2; generated server stubs land ininternal/service, while matching TypeScript clients flow intoui/src/api
Files:
api/v2/api.yaml
🧠 Learnings (2)
📚 Learning: 2026-01-12T16:59:14.752Z
Learnt from: CR
Repo: dagu-org/dagu PR: 0
File: ui/CLAUDE.md:0-0
Timestamp: 2026-01-12T16:59:14.752Z
Learning: Applies to ui/**/*.{tsx,jsx} : Support keyboard navigation in all interactive components including modals with arrow keys, enter, and escape keys
Applied to files:
ui/src/pages/audit-logs/index.tsx
📚 Learning: 2026-01-12T16:59:14.752Z
Learnt from: CR
Repo: dagu-org/dagu PR: 0
File: ui/CLAUDE.md:0-0
Timestamp: 2026-01-12T16:59:14.752Z
Learning: Applies to ui/**/*.{tsx,jsx} : Use compact form element heights: select boxes with `h-7` or smaller, buttons with `h-7` or `h-8`, inputs with compact padding (`py-0.5` or `py-1`)
Applied to files:
ui/src/pages/audit-logs/index.tsx
🧬 Code graph analysis (4)
ui/src/features/dags/components/dag-execution/ExecutionLog.tsx (1)
ui/src/contexts/AuthContext.tsx (1)
TOKEN_KEY(24-24)
ui/src/pages/audit-logs/index.tsx (4)
ui/src/components/ui/toggle-group.tsx (2)
ToggleGroup(12-31)ToggleButton(43-80)ui/src/components/ui/select.tsx (5)
Select(175-175)SelectTrigger(183-183)SelectValue(184-184)SelectContent(176-176)SelectItem(178-178)ui/src/components/ui/date-range-picker.tsx (1)
DateRangePicker(200-245)ui/src/components/ui/button.tsx (1)
Button(57-57)
ui/src/features/dags/components/dag-execution/StepLog.tsx (1)
ui/src/contexts/AuthContext.tsx (1)
TOKEN_KEY(24-24)
internal/service/frontend/api/v2/dagruns.go (4)
api/v2/api.gen.go (8)
DownloadDAGRunLogRequestObject(7822-7826)DownloadDAGRunLogResponseObject(7828-7830)ErrorCodeNotFound(57-57)Log(751-766)DownloadDAGRunLog200TextResponse(7836-7839)DownloadDAGRunLog200ResponseHeaders(7832-7834)Stream(1213-1213)StreamStderr(143-143)internal/core/exec/dagrun.go (1)
NewDAGRunRef(149-154)api/v1/api.gen.go (1)
ErrorCodeNotFound(58-58)internal/core/params.go (1)
Params(36-45)
🔇 Additional comments (18)
ui/src/pages/audit-logs/index.tsx (3)
1-168: Date filter state + helper wiring looks solid.
176-235: Filter-change detection and pagination reset are clear.
247-287: Handlers keep UI and API date values aligned.ui/src/features/dags/components/dag-execution/ExecutionLog.tsx (2)
7-15: Imports for download support look good.
422-431: Download control wiring looks good.api/v2/api.yaml (2)
1923-1958: DAG-run log download endpoint is well documented.
2085-2499: Step and sub-DAG download endpoints are consistent with existing log APIs.ui/src/features/dags/components/dag-execution/StepLog.tsx (2)
7-15: Imports for download support look good.
401-446: Page-size extension + download action integrate cleanly.api/v2/api.gen.go (9)
1625-1630: Download log parameter structs mirror existing query patterns.
Remote node + stream selection matches the existing log endpoints.Also applies to: 1697-1704, 1778-1783, 1811-1818
2398-2401: ServerInterface additions for download endpoints are consistent.
Signatures align with existing log handlers and naming conventions.Also applies to: 2419-2422, 2443-2454
2677-2681: Unimplemented download handlers follow the standard NotImplemented pattern.
Consistent with other autogenerated stubs.Also applies to: 2719-2723, 2767-2771, 2785-2789
3744-3795: Download wrappers bind params and middleware consistently.
No deviations from existing wrapper behavior.Also applies to: 4173-4241, 4699-4759, 4943-5020
4908-4941: Sub-DAG step log query params now match DAG step log options.
Offset/limit/stream parity looks good.
6886-6942: Download routes registered with expected paths.
Routing additions align with the new endpoint patterns.
10426-10482: StrictServerInterface updated for download endpoints.
Matches the non-strict interface additions.
11089-11115: Strict handler download methods follow standard response dispatch.
Consistent middleware wiring and response visitation.Also applies to: 11308-11335, 11552-11579, 11648-11676
12849-13129: Generated swaggerSpec blob update (no review needed).
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1582 +/- ##
=======================================
Coverage 65.09% 65.10%
=======================================
Files 258 258
Lines 28645 28653 +8
=======================================
+ Hits 18647 18655 +8
Misses 8327 8327
Partials 1671 1671
... and 4 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.