(WIP) feat: support output the log to file #159
Conversation
|
""" WalkthroughThis change introduces structured logging using the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant Logger
participant Backend
participant Config
User->>CLI: Run command (build/push/attach)
CLI->>Config: Load rootConfig (includes LogLevel, LogFile)
CLI->>Logger: Initialize logrus with LogLevel/LogFile
CLI->>Backend: Call Nydusify(ctx, target, rootConfig)
Backend->>Logger: Log operation start
Backend->>...: Perform backend operation (build/push/attach)
Backend->>Logger: Log operation success or error
Backend-->>CLI: Return result
CLI-->>User: Output result
Possibly related PRs
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (1.64.8)Error: you are using a configuration file for golangci-lint v2 with golangci-lint v1: please use golangci-lint v2 Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (29)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (26)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
🧹 Nitpick comments (7)
pkg/backend/processor/code.go (1)
2-2: Copyright year appears incorrectThe copyright year is listed as 2025, while other files in the same package use 2024. This should be updated for consistency.
-/* - * Copyright 2025 The CNAI Authors +/* + * Copyright 2024 The CNAI Authorspkg/backend/list.go (1)
82-82: Consider limiting the log verbosity for large artifact lists.Using
%#vfor the entire model artifacts list could produce very verbose logs if there are many items. Consider logging just the count or a more concise representation.- logrus.Infof("Listed model artifacts successfully, %#v", modelArtifacts) + logrus.Infof("Listed model artifacts successfully, count: %d", len(modelArtifacts))pkg/backend/prune.go (1)
40-40: Minor typo in success log message.The success message contains a small grammatical error.
-logrus.Info("Pruned completed successfully") +logrus.Info("Prune completed successfully")pkg/backend/processor/base.go (2)
74-74: Consider reducing verbosity when logging matched paths.While logging matched paths is useful, using
%#vformat will produce very verbose output for large arrays. Consider using a more concise representation or limiting the number of paths shown.-logrus.Infof("Processing matched paths: %#v", matchedPaths) +logrus.Infof("Processing %d matched paths", len(matchedPaths))
151-151: Consider reducing verbosity when logging descriptors.Using
%#vto log descriptors will produce very verbose output that may be difficult to read. Consider logging just the count or essential information.-logrus.Infof("Processing succeed, descriptors: %#v", descriptors) +logrus.Infof("Processing succeeded, generated %d descriptors", len(descriptors))pkg/backend/login.go (1)
76-77: Good completion log message.The success message provides confirmation of the operation completion, though there's a small grammatical issue.
Consider changing "Logined to registry" to "Logged in to registry" for better grammar:
- logrus.Infof("Logined to registry %s successfully", registry) + logrus.Infof("Logged in to registry %s successfully", registry)pkg/backend/nydusify.go (1)
62-63: Fix typo in success message.There's a small typo in the success message.
Apply this small correction:
- logrus.Infof("Nydusifyed the model artifact %s successfully", source) + logrus.Infof("Nydusified the model artifact %s successfully", source)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (29)
cmd/attach.go(1 hunks)cmd/build.go(2 hunks)cmd/push.go(1 hunks)cmd/root.go(3 hunks)pkg/archiver/archiver.go(9 hunks)pkg/backend/attach.go(7 hunks)pkg/backend/backend.go(1 hunks)pkg/backend/build.go(7 hunks)pkg/backend/build/builder.go(6 hunks)pkg/backend/extract.go(4 hunks)pkg/backend/fetch.go(5 hunks)pkg/backend/inspect.go(3 hunks)pkg/backend/list.go(3 hunks)pkg/backend/login.go(3 hunks)pkg/backend/logout.go(1 hunks)pkg/backend/nydusify.go(3 hunks)pkg/backend/processor/base.go(5 hunks)pkg/backend/processor/code.go(2 hunks)pkg/backend/processor/doc.go(2 hunks)pkg/backend/processor/model.go(2 hunks)pkg/backend/processor/model_config.go(2 hunks)pkg/backend/prune.go(1 hunks)pkg/backend/pull.go(4 hunks)pkg/backend/push.go(4 hunks)pkg/backend/rm.go(2 hunks)pkg/backend/tag.go(2 hunks)pkg/config/root.go(2 hunks)pkg/storage/distribution/distribution.go(0 hunks)test/mocks/backend/backend.go(3 hunks)
💤 Files with no reviewable changes (1)
- pkg/storage/distribution/distribution.go
🧰 Additional context used
🧬 Code Graph Analysis (9)
pkg/backend/tag.go (2)
pkg/backend/referencer.go (1)
ParseReference(36-43)pkg/backend/remote/client.go (1)
Repository(31-31)
pkg/backend/login.go (1)
pkg/config/login.go (1)
Login(21-27)
pkg/backend/rm.go (1)
pkg/backend/referencer.go (1)
ParseReference(36-43)
pkg/backend/backend.go (1)
pkg/config/root.go (1)
Root(24-30)
pkg/backend/nydusify.go (1)
pkg/config/root.go (1)
Root(24-30)
pkg/backend/extract.go (1)
pkg/backend/referencer.go (1)
ParseReference(36-43)
pkg/backend/push.go (3)
pkg/config/push.go (1)
Push(26-31)pkg/backend/referencer.go (1)
ParseReference(36-43)internal/pb/pb.go (1)
NormalizePrompt(30-32)
pkg/backend/prune.go (1)
pkg/config/prune.go (1)
Prune(19-22)
test/mocks/backend/backend.go (2)
pkg/backend/backend.go (1)
Backend(27-69)pkg/config/root.go (1)
Root(24-30)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (go)
🔇 Additional comments (87)
pkg/backend/processor/doc.go (2)
22-24: Import addition for structured logging looks goodAdded the logrus library for structured logging, which aligns with the PR objective to enhance logging capabilities across the codebase.
55-55: Good addition of structured loggingAdded clear informational logging to indicate when doc file processing begins, providing the working directory for context. This improves observability without altering the control flow.
pkg/backend/processor/model.go (2)
22-24: Import addition for structured logging looks goodAdded the logrus library for structured logging, maintaining consistency with other processor implementations.
55-55: Good addition of structured loggingAdded clear informational logging to indicate when model file processing begins, providing the working directory for context. This improves observability without altering the control flow.
cmd/push.go (1)
84-84: Updated Nydusify method call correctlyThe Nydusify method call has been updated to include rootConfig as a parameter, aligning with the updated method signature in the backend interface. This change supports the PR objective of enhancing configuration flexibility for logging.
pkg/backend/processor/code.go (2)
22-24: Import addition for structured logging looks goodAdded the logrus library for structured logging, maintaining consistency with other processor implementations.
55-55: Good addition of structured loggingAdded clear informational logging to indicate when code file processing begins, providing the working directory for context. This improves observability without altering the control flow.
pkg/backend/list.go (4)
28-28: Good addition of structured logging with logrus.The import of the logrus package aligns with the PR objective of introducing structured logging across the codebase.
47-48: LGTM: Appropriate informational logging.This log entry provides visibility into the start of the listing process.
54-55: Good error logging pattern.Error is both logged and returned with proper context, enhancing debuggability while maintaining the original error flow.
62-63: Consistent error logging implementation.The pattern of logging errors before returning them is consistent throughout the file.
Also applies to: 70-71
pkg/config/root.go (1)
28-29: LGTM: Good addition of logging configuration fields.Adding LogLevel and LogFile fields to the Root struct provides a centralized way to configure logging throughout the application.
pkg/backend/tag.go (3)
25-25: Good addition of structured logging with logrus.The import of the logrus package aligns with the PR objective of introducing structured logging across the codebase.
30-31: LGTM: Informative log statements for operation start and completion.These log entries provide clear visibility into the tagging process, including both source and target references.
Also applies to: 74-74
34-35: Consistent error logging pattern throughout the file.Errors are consistently logged before being returned, providing enhanced debuggability while maintaining the original error flow.
Also applies to: 40-41, 46-47, 52-53, 64-65, 70-71
cmd/attach.go (1)
87-87:✅ Verification successful
LGTM: Updated Nydusify method call to include rootConfig parameter.
This change correctly passes the logging configuration to the Nydusify operation, aligning with the updated method signature mentioned in the PR objectives.
🏁 Script executed:
#!/bin/bash # Verify that other similar calls to Nydusify have been updated consistently rg -A 1 -B 1 "b\.Nydusify" --glob "cmd/*.go"Length of output: 419
LGTM: All Nydusify calls include the new
rootConfigparameterI’ve verified that the
b.Nydusifyinvocation incmd/attach.go,cmd/build.go, andcmd/push.gonow consistently passesrootConfig, matching the updated method signature. No further changes needed here.pkg/backend/processor/model_config.go (2)
22-24: Good organization of importsThe imports are now properly grouped with third-party packages separate from internal packages, improving code organization.
55-55: LGTM - Added informative loggingGood addition of structured logging that provides context about the processing operation. This follows the broader pattern of enhanced logging across the codebase.
cmd/build.go (2)
27-29: Good import organizationThe imports are properly organized with standard library imports, followed by third-party packages, then internal packages.
89-89: LGTM - Updated method call to match new signatureThe
Nydusifymethod call has been properly updated to include therootConfigparameter, consistent with the interface change inpkg/backend/backend.go. This ensures logging configuration is passed down to the operation.pkg/backend/inspect.go (4)
27-27: Good addition of logrus for structured loggingAdded import for logrus to enable structured logging throughout the method.
68-69: LGTM - Added informative start loggingGood practice to log the beginning of the operation with contextual information about the target.
72-72: Good error logging implementationComprehensive error logging has been added at all error points, providing detailed context about what failed while maintaining the original error in the returned error.
Also applies to: 79-80, 85-86, 92-93, 99-100
127-127: LGTM - Added success loggingGood addition of success logging that includes the inspected artifact details for debugging purposes.
pkg/backend/backend.go (1)
68-68: LGTM - Updated interface method signatureThe
Nydusifymethod signature has been properly updated to accept a configuration parameter, allowing logging settings to be passed to the operation. This change is consistently implemented across the codebase based on the PR summary.pkg/backend/logout.go (3)
22-22: Good addition of logrus package for structured logging.The import of the logrus package enhances observability across the backend package.
28-28: Well-structured informational logging added to track operation progress.The logrus.Info statements at the beginning and end of the Logout operation provide clear visibility into the operation lifecycle, including the target registry.
Also applies to: 42-42
32-33: Effective error logging added before error returns.Adding detailed error logs before returning errors improves debuggability while maintaining the original error handling flow.
Also applies to: 38-39
pkg/backend/prune.go (4)
23-23: Good addition of logrus package for structured logging.The import of the logrus package enhances observability across the backend package.
28-29: Informative start log with parameter values.Including the dryRun and removeUntagged parameters in the initial log statement provides valuable context for understanding the operation's configuration.
31-32: Detailed error logging added before error returns.Error logging before returning errors provides better visibility into failure points while maintaining the original error handling.
36-37: Consistent error logging pattern for purge uploads.This follows the same error logging pattern established earlier, maintaining consistency in error reporting.
pkg/backend/processor/base.go (3)
27-31: Import organization improved with logrus and standard packages.The imports are now better organized, with system packages first, followed by third-party packages like logrus, and then internal packages.
61-62: Clear process start logging added.Adding a log statement at the beginning of processing provides better visibility into when processing begins and the target directory.
120-120: Effective error logging with cancellation context.Adding error logging before cancellation provides better visibility into why processing was stopped.
pkg/backend/push.go (4)
26-30: Improved import organization with logrus.The imports are now better organized, with system packages first, followed by third-party packages like logrus, and then internal packages.
39-39: Clear operation start logging added.Adding a log statement at the beginning of the push operation provides better visibility into the target artifact being pushed.
43-44: Comprehensive error logging at all failure points.Error logs have been added before all error returns, which significantly improves error traceability throughout the push operation. This consistent approach helps identify exactly where failures occur.
Also applies to: 53-54, 59-60, 65-66, 90-91, 96-97, 107-108
111-111: Operation completion log enhances observability.Adding a success log at the end of the operation completes the logging lifecycle and confirms successful completion to operators.
pkg/backend/login.go (3)
24-24: Good addition of structured logging with logrus.The addition of logrus for structured logging is a good practice for improving observability in the system.
35-35: Well-placed informational log at function entry point.This log entry appropriately captures the beginning of the login operation with relevant details (registry and username).
39-41: Good error handling pattern with logging.The changes implement a consistent pattern for error handling with appropriate logging before returning errors to the caller, which improves observability and debugging capabilities.
Also applies to: 45-47, 71-75
pkg/backend/rm.go (4)
23-23: Consistent use of logrus for structured logging.Good addition of the logrus import to maintain consistency with the logging approach across the codebase.
29-30: Well-placed informational log at function entry point.This log entry appropriately captures the beginning of the removal operation with the target artifact name.
33-35: Good error handling pattern with logging.The error handling follows a consistent pattern with informative logging before returning errors to the caller, which is good for observability.
Also applies to: 49-51
53-54: Good completion log message.The success message provides clear confirmation of the removal operation completion.
pkg/backend/nydusify.go (4)
23-26: Good organization of imports with logrus.The imports are well-organized with the addition of logrus for structured logging.
34-36: Updated method signature to accept configuration.The function signature has been updated to accept a configuration parameter, which is a good pattern for propagating configuration throughout the application.
52-56: Added logging configuration to command execution.The addition of log-level and log-file parameters to the nydusify command enhances the configurability of logging in the system.
58-60: Good error handling with logging.The error handling follows the consistent pattern with informative logging before returning errors.
pkg/backend/pull.go (4)
25-28: Well-organized imports with logrus.The imports are well-organized with the addition of logrus for structured logging, maintaining proper grouping of standard library, third-party, and internal packages.
37-38: Well-placed informational log at function entry point.This log entry appropriately captures the beginning of the pull operation with the target artifact name.
41-43: Comprehensive error logging across all failure points.The changes ensure that all potential failure points during the pull operation have appropriate error logging, which significantly improves observability and debugging capabilities.
Also applies to: 48-50, 54-56, 62-64, 98-100, 110-112, 116-118, 125-127
130-132: Good completion log message.The success message provides clear confirmation of the pull operation completion.
pkg/backend/attach.go (5)
30-30: LGTM: Added import for logrus packageAdded the logrus package import to support structured logging throughout this file.
62-63: Good addition of entry log for the Attach operationThis log statement clearly identifies the start of the attach operation with relevant context about the file and source, improving observability.
66-67: Well-implemented error loggingThese error logs appropriately capture error details before returning them to the caller, which will help with debugging and observability while maintaining the existing error handling flow.
Also applies to: 72-73, 103-104, 109-110, 119-120, 133-134, 159-160, 176-177
132-134: Good logging for early return caseThis log message helpfully explains when an artifact is unchanged after attaching a layer, providing context for the early return.
180-181: Effective completion log messageThis log clearly indicates successful completion of the operation with all relevant context information.
pkg/backend/fetch.go (5)
27-27: LGTM: Added import for logrus packageAdded the logrus package import to support structured logging throughout this file.
37-37: Good addition of entry log for the Fetch operationThis log statement clearly identifies the start of the fetch operation with relevant context about the target, output path, and patterns, improving observability.
41-42: Well-implemented error loggingThese error logs appropriately capture error details before returning them to the caller, which will help with debugging and observability while maintaining the existing error handling flow.
Also applies to: 48-49, 54-55, 62-63, 73-74, 85-86
102-105: Improved error handling for error groupThis change enhances the error handling by explicitly checking, logging, and formatting the error from the error group wait operation, which is better than the previous implementation that returned the error without logging.
107-108: Good completion log messageThis log clearly indicates successful completion of the fetch operation with relevant context.
pkg/backend/build.go (6)
26-29: LGTM: Import organization and logrus additionThe imports are properly organized, with the logrus package added to support structured logging throughout this file.
49-50: Good addition of entry log for the Build operationThis log statement clearly identifies the start of the build operation with relevant context about the modelfile path and work directory.
86-87: Informative log for nydus interceptor usageThis log provides visibility into when the nydus interceptor is enabled for the build process, which is helpful for understanding build configuration.
54-55: Well-implemented error loggingThese error logs appropriately capture error details before returning them to the caller, which will help with debugging and observability while maintaining the existing error handling flow.
Also applies to: 60-61, 71-72, 92-93, 103-104, 137-138, 154-155
142-142: Variable name improvement for readabilityChanged the variable name (likely from
manifestDesctodesc) to improve code readability, especially when used in log messages.
158-158: Detailed completion log with digest informationThis log clearly indicates successful completion of the build operation and includes the manifest digest, which is valuable for traceability.
pkg/archiver/archiver.go (5)
27-27: LGTM: Added import for logrus packageAdded the logrus package import to support structured logging throughout this file.
34-34: Good entry logs for Tar and Untar operationsThese log statements clearly identify the start of the tar and untar operations with relevant path information, improving observability.
Also applies to: 147-147
44-47: Comprehensive error logging in Tar functionThese error logs thoroughly cover all potential error cases in the tar function, including file system operations, header creation, writing, and copying. The logs provide good context and include the specific errors, which will be invaluable for debugging.
Also applies to: 60-62, 66-68, 73-75, 80-82, 86-88, 95-98, 103-106, 111-114, 120-123, 128-131, 134-137
152-154: Thorough error logging in Untar functionThese error logs comprehensively cover all potential error cases in the untar function, including directory creation, file extraction, symlink handling, and security checks. The detailed context in each log will help identify the specific point of failure during troubleshooting.
Also applies to: 162-164, 169-171, 178-180, 185-187, 196-198, 202-204, 210-212, 214-216
1-240: Overall well-implemented logging strategyThe addition of structured logging using logrus throughout the Tar and Untar functions follows a consistent pattern and provides excellent visibility into the archiving processes without altering the control flow or error handling logic. This will significantly improve debugging and monitoring capabilities.
pkg/backend/extract.go (4)
30-30: Properly added logrus dependency for structured loggingGood addition of the logrus package for structured logging throughout the file.
43-44: Well-placed start logging for extraction processAdding an informational log at the beginning of the extraction process provides good visibility into the operation.
48-49: Consistent error logging pattern applied throughout the fileYou've applied a consistent pattern of error logging before returning errors, which improves observability while maintaining the existing control flow. The error context is properly preserved.
Also applies to: 56-57, 62-63, 67-68, 85-86, 92-93, 112-113, 117-118
71-72: Added completion logging for successful operationsGood addition of success logging at the end of extraction process to indicate successful completion.
cmd/root.go (2)
27-33: Added logrus import and reorganized imports properlyThe logrus package is correctly imported and the imports are well organized.
104-105: Added appropriate CLI flags for log configurationGood addition of command-line flags for controlling log level and log file path, providing flexibility to users.
pkg/backend/build/builder.go (5)
35-35: Added logrus import for structured loggingCorrectly imported the logrus package for structured logging.
122-123: Added informative operation start/end loggingGood addition of informational logs at the beginning and end of major operations, which improves visibility into the build process.
Also applies to: 224-225, 229-230, 249-250, 254-255, 283-284
126-127: Consistent error logging pattern applied throughout the builderApplied a consistent pattern of error logging before returning errors across all builder methods, which improves observability while maintaining the existing control flow.
Also applies to: 131-132, 137-138, 145-146, 151-152, 158-159, 167-168, 178-179, 184-185, 216-217, 232-233, 238-239, 245-246, 272-273, 279-280
162-163: Added detailed progress logging during build processGood addition of detailed logs for intermediate steps like hash calculation and reader interception, which provides valuable context for debugging.
Also applies to: 170-171, 196-197
337-338: Added logging to splitReader for better traceabilityGood addition of logging in the splitReader function to track data copying operations and potential errors, which improves debugging capability.
Also applies to: 340-343, 344-345
test/mocks/backend/backend.go (1)
451-452: Updated Nydusify mock method to match interface changesThe mock's Nydusify method has been correctly updated to include the new
cfg *config.Rootparameter, keeping it in sync with the actual interface implementation. All related type assertions and method calls have been properly adjusted.These changes ensure that tests using this mock will continue to work with the updated method signature in the Backend interface.
Also applies to: 453-454, 461-466, 470-472, 487-490, 492-495, 504-504
Signed-off-by: chlins <chlins.zhang@gmail.com>
chlins
changed the title
This pull request introduces several updates to enhance logging, improve error handling, and extend functionality across multiple files. The most notable changes include adding
logrusfor structured logging, updating theNydusifymethod to accept an additional configuration parameter, and improving error reporting in various functions.Logging Enhancements:
logrusfor structured logging in multiple files, includingpkg/archiver/archiver.go,pkg/backend/attach.go, andpkg/backend/build.go. Added detailed logging for key operations such as tar/untar processes, file attachments, and model artifact builds. [1] [2] [3]cmd/root.gowith customizable log levels and log file paths. [1] [2]Functional Updates:
Nydusifymethod inpkg/backend/backend.goto accept a newcfg *config.Rootparameter, allowing for more flexible configuration handling. [1] [2] [3] [4]Error Handling Improvements:
logruserror messages in functions such asTar,Untar,Attach, andBuild, ensuring better traceability of issues. [1] [2] [3]Code Organization:
logrus. [1] [2] [3]Miscellaneous:
cmd/root.go.Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Style
Chores