Conversation
Everything this library does on a host is a vendor CLI invocation behind a commandrunner.CommandRunner, so that interface is the one place where all executed commands can be observed. Logging goes there as a decorator rather than into each adapter: commandrunner.Logging wraps any runner, records the invocation on an slog.Logger, and returns the wrapped output and error unchanged. It is opt-in -- a consumer injects the decorated runner where it would have injected the concrete one -- and it covers the runners and adapters added later for free, since they all sit behind the same port. One record per invocation carries the binary, the arguments, the duration and, on failure, the error. Successful commands are recorded at slog.LevelInfo, which WithLevel lowers, and failures at slog.LevelError. ErrNoLogicalDrives reports an empty inventory rather than a failure, so it stays at the success level instead of logging a controller with no volume as an error. The output itself is never logged, only its size: vendor payloads carry drive serials and other identifying data. The arguments are the ones the adapter asked for, so a runner that appends flags of its own does so after the decorator has seen them and the storcli2/perccli2 JSON output flag is absent from the record; both are documented where a reader would look for them. Each runner now reports the binary it invokes through CommandPath(), so a record names the tool that ran. The decorator forwards its wrapped runner's path, which also keeps it transparent to another decorator, and falls back to the runner's type for a runner without one, such as a test mock. The legacy megaraid.Runner parses its own output instead of returning bytes, so it cannot share the decorator. megaraid.LoggingRunner decorates it the same way and emits through commandrunner.LogCommand, keeping one log shape across the library; it records no payload size, since that runner exposes none. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hello,
thank you for creating this project.
I found it very useful, but it would be nice to include command logging for debug purposes.
This is a PR to enable that.
Everything this library does on a host is a vendor CLI invocation behind a commandrunner.CommandRunner, so that interface is the one place where all executed commands can be observed. Logging goes there as a decorator rather than into each adapter: commandrunner.Logging wraps any runner, records the invocation on an slog.Logger, and returns the wrapped output and error unchanged. It is opt-in -- a consumer injects the decorated runner where it would have injected the concrete one -- and it covers the runners and adapters added later for free, since they all sit behind the same port.
One record per invocation carries the binary, the arguments, the duration and, on failure, the error. Successful commands are recorded at slog.LevelInfo, which WithLevel lowers, and failures at slog.LevelError. ErrNoLogicalDrives reports an empty inventory rather than a failure, so it stays at the success level instead of logging a controller with no volume as an error.
The output itself is never logged, only its size: vendor payloads carry drive serials and other identifying data. The arguments are the ones the adapter asked for, so a runner that appends flags of its own does so after the decorator has seen them and the storcli2/perccli2 JSON output flag is absent from the record; both are documented where a reader would look for them.
Each runner now reports the binary it invokes through CommandPath(), so a record names the tool that ran. The decorator forwards its wrapped runner's path, which also keeps it transparent to another decorator, and falls back to the runner's type for a runner without one, such as a test mock.
The legacy megaraid.Runner parses its own output instead of returning bytes, so it cannot share the decorator. megaraid.LoggingRunner decorates it the same way and emits through commandrunner.LogCommand, keeping one log shape across the library; it records no payload size, since that runner exposes none.