Skip to content

Commit d84ff1b

Browse files
Enhance diagnostics logging with context-aware timeouts (#1138)
* Enhance diagnostics logging with context-aware timeouts * Add full system diagnostics logging on supervisor crash * Clear diagnostics context after logging to prevent memory leaks * remove pr.txt
1 parent 3e7542a commit d84ff1b

3 files changed

Lines changed: 29 additions & 2 deletions

File tree

cmd/main.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313

1414
"github.com/erikdubbelboer/gspt"
1515
"github.com/jetkvm/kvm"
16+
"github.com/jetkvm/kvm/internal/diagnostics"
1617
"github.com/jetkvm/kvm/internal/native"
1718
"github.com/jetkvm/kvm/internal/supervisor"
1819
)
@@ -143,6 +144,13 @@ func supervise() error {
143144
}
144145
}
145146

147+
// Log full system diagnostics
148+
diag := diagnostics.New(diagnostics.Options{Writer: logFile})
149+
diag.LogAll("supervisor_crash")
150+
151+
// Ensure all data is flushed to disk before copying
152+
_ = logFile.Sync()
153+
146154
createErrorDump(logFile)
147155
os.Exit(exiterr.ExitCode())
148156
}

internal/diagnostics/diagnostics.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@
33
package diagnostics
44

55
import (
6+
"context"
67
"io"
8+
"time"
79

810
"github.com/jetkvm/kvm/internal/logging"
911
"github.com/rs/zerolog"
@@ -39,8 +41,11 @@ type Options struct {
3941
type Diagnostics struct {
4042
logger *zerolog.Logger
4143
options Options
44+
ctx context.Context // set during LogAll for overall timeout
4245
}
4346

47+
const defaultLogAllTimeout = 10 * time.Second
48+
4449
// New creates a new Diagnostics instance using the default diagnostics logger.
4550
// If opts.Writer is set, logs are written there instead of the default logger.
4651
func New(opts Options) *Diagnostics {
@@ -61,7 +66,13 @@ func NewWithLogger(logger *zerolog.Logger, opts Options) *Diagnostics {
6166

6267
// LogAll runs all diagnostic checks and logs the results.
6368
// The phase parameter distinguishes context (e.g., "crash" vs "handshake" vs "download").
69+
// LogAll has an overall timeout of 10 seconds; individual commands have a 2-second timeout.
6470
func (d *Diagnostics) LogAll(phase string) {
71+
ctx, cancel := context.WithTimeout(context.Background(), defaultLogAllTimeout)
72+
defer cancel()
73+
d.ctx = ctx
74+
defer func() { d.ctx = nil }()
75+
6576
d.logger.Error().Str("phase", phase).Msg("=== DIAGNOSTICS ===")
6677

6778
d.LogSystemInfo()

internal/diagnostics/helpers.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,11 @@ const defaultCmdTimeout = 2 * time.Second
1212

1313
// runCmdLog runs a command and logs its output.
1414
func (d *Diagnostics) runCmdLog(label string, cmd string, args ...string) {
15-
ctx, cancel := context.WithTimeout(context.Background(), defaultCmdTimeout)
15+
parentCtx := d.ctx
16+
if parentCtx == nil {
17+
parentCtx = context.Background()
18+
}
19+
ctx, cancel := context.WithTimeout(parentCtx, defaultCmdTimeout)
1620
defer cancel()
1721

1822
output, err := exec.CommandContext(ctx, cmd, args...).CombinedOutput()
@@ -26,7 +30,11 @@ func (d *Diagnostics) runCmdLog(label string, cmd string, args ...string) {
2630

2731
// runShellLog runs a shell command (for pipelines) and logs its output.
2832
func (d *Diagnostics) runShellLog(label, script string) {
29-
ctx, cancel := context.WithTimeout(context.Background(), defaultCmdTimeout)
33+
parentCtx := d.ctx
34+
if parentCtx == nil {
35+
parentCtx = context.Background()
36+
}
37+
ctx, cancel := context.WithTimeout(parentCtx, defaultCmdTimeout)
3038
defer cancel()
3139

3240
output, err := exec.CommandContext(ctx, "sh", "-c", script).CombinedOutput()

0 commit comments

Comments
 (0)