Unify logging with slog#161
Open
dmcgowan wants to merge 1 commit intocontainerd:mainfrom
Open
Conversation
There was a problem hiding this comment.
Pull request overview
This PR migrates shim/vminitd/libkrun console output toward a unified slog-based structured logging flow, forwarding VM console lines into structured records and configuring slog as the default logger in both shim and vminitd.
Changes:
- Add
internal/loggingutilities to configure shim slog output and to forward VM console output as structuredslogrecords. - Update
vminitdto emit JSONsloglogs to/dev/console(to avoid polluting kmsg) and support debug level via aslog.LevelVar. - Adjust console log forwarding in libkrun and update file-dumping to log file contents as structured fields; bump
github.com/containerd/logdependency.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/vm/libkrun/instance.go | Switch console forwarding to the new logging.ForwardConsoleLogs helper. |
| internal/systools/dump.go | Change file dumping to structured debug logging with file content included. |
| internal/logging/shim.go | Add shim-specific slog setup using containerd’s log FIFO. |
| internal/logging/logging.go | Add console-line forwarding and JSON-log re-emission utilities. |
| internal/logging/logging_test.go | Add benchmarks and correctness tests for the new logging utilities. |
| go.mod | Bump github.com/containerd/log to a newer pseudo-version. |
| go.sum | Add checksums for the updated github.com/containerd/log version. |
| cmd/vminitd/main.go | Configure slog JSON logging to /dev/console and wire debug level. |
| cmd/containerd-shim-nerdbox-v1/main.go | Initialize shim slog logging and disable shim’s default logger setup. |
💡 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 9 out of 10 changed files in this pull request and generated 2 comments.
💡 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 9 out of 10 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+122
to
+133
| // If raw is non-nil, every line is also written there verbatim (useful for | ||
| // tests that need the unprocessed console output). | ||
| func ForwardConsoleLogs(r io.Reader, raw io.Writer) { | ||
| scanner := bufio.NewScanner(r) | ||
| scanner.Buffer(make([]byte, 0, bufio.MaxScanTokenSize), 1<<20) // 1 MiB max line | ||
| for scanner.Scan() { | ||
| line := scanner.Text() | ||
|
|
||
| if raw != nil { | ||
| raw.Write([]byte(line)) | ||
| raw.Write([]byte("\n")) | ||
| } |
Comment on lines
+1
to
+26
| /* | ||
| Copyright The containerd Authors. | ||
|
|
||
| Licensed under the Apache License, Version 2.0 (the "License"); | ||
| you may not use this file except in compliance with the License. | ||
| You may obtain a copy of the License at | ||
|
|
||
| http://www.apache.org/licenses/LICENSE-2.0 | ||
|
|
||
| Unless required by applicable law or agreed to in writing, software | ||
| distributed under the License is distributed on an "AS IS" BASIS, | ||
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| See the License for the specific language governing permissions and | ||
| limitations under the License. | ||
| */ | ||
|
|
||
| package logging | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "io" | ||
| "os" | ||
|
|
||
| "github.com/Microsoft/go-winio" | ||
| "golang.org/x/sys/windows" | ||
| ) |
Signed-off-by: Derek McGowan <derek@mcg.dev>
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.
Output all shim, kernel, and vminit logs as slot entries, using the default text output of slog by default.