feat: ability to fetch logs from instances #564
Conversation
WalkthroughThe pull request introduces enhanced logging functionality and corresponding tests within the Knuu framework. It adds methods for retrieving log streams from both instances and Kubernetes containers, along with tests to validate the logging behavior of standalone instances and those with sidecars. These changes improve the monitoring capabilities of the system by allowing flexible log retrieval based on instance types. Changes
Poem
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (1)
Additional comments not posted (6)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 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: 0
Outside diff range and nitpick comments (4)
e2e/basic/logs_test.go (4)
49-49: Consider using a more reliable waiting mechanism.The test currently waits for a fixed duration of 5 seconds to allow log generation. This may not be reliable in all scenarios, especially if log generation takes longer than expected.
Consider using a more dynamic waiting mechanism, such as polling for the presence of the expected log message or waiting for a specific log event, to make the test more resilient to timing issues.
55-56: Consider using a more efficient way to read the log stream.The test currently uses
io.ReadAllto read the entire log stream into memory. This may cause performance issues for large log outputs.Consider using a more efficient approach, such as reading the log stream in chunks or using a scanner to read the logs line by line, to handle large log outputs more effectively.
86-86: Consider using a more reliable waiting mechanism.The test currently waits for a fixed duration of 5 seconds to allow log generation. This may not be reliable in all scenarios, especially if log generation takes longer than expected.
Consider using a more dynamic waiting mechanism, such as polling for the presence of the expected log message or waiting for a specific log event, to make the test more resilient to timing issues.
92-93: Consider using a more efficient way to read the log stream.The test currently uses
io.ReadAllto read the entire log stream into memory. This may cause performance issues for large log outputs.Consider using a more efficient approach, such as reading the log stream in chunks or using a scanner to read the logs line by line, to handle large log outputs more effectively.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- e2e/basic/logs_test.go (1 hunks)
- pkg/instance/monitoring.go (2 hunks)
- pkg/k8s/logs.go (1 hunks)
- pkg/k8s/types.go (2 hunks)
Additional comments not posted (4)
pkg/k8s/logs.go (1)
10-23: LGTM!The
GetLogStreamfunction is well-implemented and follows best practices. It enhances the client's capabilities by enabling log retrieval for specific containers within ReplicaSets. The function signature, parameter handling, error handling, and resource management are all properly done.Some key points:
- The function takes appropriate parameters (context, ReplicaSet name, and container name) to identify the specific container for log retrieval.
- It handles the case when the container name is not provided by setting an empty container name in the
PodLogOptions.- It retrieves the first pod associated with the specified ReplicaSet using
GetFirstPodFromReplicaSetand performs proper error handling.- It uses the Kubernetes client to create a log request and stream the logs efficiently.
- The function returns the log stream as an
io.ReadCloser, allowing the caller to read the logs and close the stream when done, promoting proper resource management.Overall, this function is a valuable addition to the client and provides a convenient way to access logs for debugging and monitoring purposes.
pkg/instance/monitoring.go (1)
22-27: LGTM!The
Logsfunction is a great addition to the monitoring system. It enhances the flexibility and usability of the logging feature by handling log retrieval differently based on the instance type. The implementation follows good practices by:
- Correctly determining the parameters for
GetLogStreambased on whether the instance is a sidecar or not.- Returning the log stream as an
io.ReadCloserwhich is suitable for streaming data.- Propagating errors from
GetLogStreamto the caller for appropriate error handling.Overall, this is a well-designed and implemented function.
pkg/k8s/types.go (2)
5-5: LGTM!The import statement for the
iopackage is correctly added. This is a standard library package in Go that provides basic interfaces to I/O primitives.
54-54: Approve the new method addition to the interface.The
GetLogStreammethod is a valuable addition to theKubeManagerinterface. It is well-defined and follows the Go conventions. The method parameters and return types are appropriately specified.This method will enable retrieving log streams from specified containers within pods, which enhances the monitoring and debugging capabilities in Kubernetes environments.
Note that this is a breaking change for any existing implementations of the
KubeManagerinterface. They will need to be updated to implement this new method.
mojtaba-esk
deleted the
mojtaba/feat-ability-to-fetch-logs-from-instances
branch
Closes #496
Summary by CodeRabbit
New Features
Bug Fixes