standard log output and context handling by jaypipes · Pull Request #439 · jaypipes/ghw

jaypipes · 2025-12-28T21:11:27Z

Continues the work from #417 in two important ways. First, this finalizes the move towards passing only a context.Context argument to the inner InfoXXX.load() methods instead of the old pkg/option.Options struct. Using a context.Context is more modern Go idiomatic and aligned with modern Go packages like log/slog. Each InfoXXX.load() method ensures a context.Context is created to store various options and context that is passed between modules in ghw. The function signature for the main module constructors like pkg/cpu.CPU() or pkg/block.Block() have all been changed from this:

func CPU(opt ...option.Option) (*Info, error)

to

func CPU(args ...any) (*Info, error)

which is a backwards compatible API change. We then examine each of the args arguments and set various keys on the context.Context. The context.Context is examined by other modules like pkg/linuxpath or pkg/linuxdmi instead of the prior pkg/option.Options struct.

The second major part of this patch is the addition of log/output formatting to use log/slog and log/slog.Handler overrides to control the output of warning and debug log lines.

The default log output in ghw only writes WARN-level messages to stderr in a simple WARN: <msg> log record format:

$ ghwc baseboard
WARN:  Unable to read board_serial: open /sys/class/dmi/id/board_serial: permission denied
baseboard vendor=System76 version=thelio-mira-b4.1 product=Thelio Mira

You can control a number of log output options programmatically or by using environs variables.

To change the log level ghw uses, set the GHW_LOG_LEVEL environs variable:

$ GHW_LOG_LEVEL=debug ghwc baseboard
DEBUG: reading from "/sys/class/dmi/id/board_asset_tag"
DEBUG: reading from "/sys/class/dmi/id/board_serial"
WARN:  Unable to read board_serial: open /sys/class/dmi/id/board_serial: permission denied
DEBUG: reading from "/sys/class/dmi/id/board_vendor"
DEBUG: reading from "/sys/class/dmi/id/board_version"
DEBUG: reading from "/sys/class/dmi/id/board_name"
baseboard vendor=System76 version=thelio-mira-b4.1 product=Thelio Mira

Changing GHW_LOG_LEVEL to error has the same effect of setting GHW_DISABLE_WARNINGS:

$ GHW_LOG_LEVEL=error ghwc baseboard
baseboard vendor=System76 version=thelio-mira-b4.1 product=Thelio Mira

You can change the log level programmatically using the WithLogLevel modifier:

import (
    "log/slog"

	"github.com/jaypipes/ghw"
)

bb, err := ghw.Baseboard(ghw.WithLogLevel(slog.LevelDebug))

To use the logfmt standard log output format, set the GHW_LOG_LOGFMT envrions variable:

$ GHW_LOG_LOGFMT=1 ghwc baseboard
time=2025-12-28T07:31:08.614-05:00 level=WARN msg="Unable to read board_serial: open /sys/class/dmi/id/board_serial: permission denied"
baseboard vendor=System76 version=thelio-mira-b4.1 product=Thelio Mira

You can tell ghw to use logfmt standard output formatting using the WithLogLogfmt modifier:

import (
    "log/slog"

	"github.com/jaypipes/ghw"
)

bb, err := ghw.Baseboard(ghw.WithLogLogfmt())

You can now programmatically override the logger that ghw uses with the WithLogger modifier. You pass in an instance of slog.Logger, like this example that shows how to use a simple logger with colored log output:

package main

import (
    "context"
    "encoding/json"
    "io"
    "log"
    "log/slog"

    "github.com/fatih/color"
    "github.com/jaypipes/ghw"
)

type PrettyHandlerOptions struct {
    SlogOpts slog.HandlerOptions
}

type PrettyHandler struct {
    slog.Handler
    l *log.Logger
}

func (h *PrettyHandler) Handle(ctx context.Context, r slog.Record) error {
    level := r.Level.String() + ":"

    switch r.Level {
    case slog.LevelDebug:
        level = color.MagentaString(level)
    case slog.LevelInfo:
        level = color.BlueString(level)
    case slog.LevelWarn:
        level = color.YellowString(level)
    case slog.LevelError:
        level = color.RedString(level)
    }

    fields := make(map[string]interface{}, r.NumAttrs())
    r.Attrs(func(a slog.Attr) bool {
        fields[a.Key] = a.Value.Any()

        return true
    })

    b, err := json.MarshalIndent(fields, "", "  ")
    if err != nil {
        return err
    }

    timeStr := r.Time.Format("[15:05:05.000]")
    msg := color.CyanString(r.Message)

    h.l.Println(timeStr, level, msg, color.WhiteString(string(b)))

    return nil
}

func NewPrettyHandler(
    out io.Writer,
    opts PrettyHandlerOptions,
) *PrettyHandler {
    h := &PrettyHandler{
        Handler: slog.NewJSONHandler(out, &opts.SlogOpts),
        l:       log.New(out, "", 0),
    }

    return h
}

func main() {
    opts := PrettyHandlerOptions{
        SlogOpts: slog.HandlerOptions{
            Level: slog.LevelDebug,
        },
    }
    handler := NewPrettyHandler(os.Stdout, opts)
    logger := slog.New(handler)
    bb, err := ghw.Baseboard(ghw.WithLogger(logger))
    if err != nil {
        logger.Error(err.String())
    }
    fmt.Println(bb)
}

jaypipes · 2025-12-29T00:43:43Z

@ffromani happy holidays, mate :) This patch addresses a couple of the comments you'd had on #417. let me know what you think!

ffromani · 2025-12-31T20:00:06Z

Thanks for the followup! will review shortly!

ffromani

Thanks @jaypipes ! this is another massive improvement.
I left comments inside for possible improvements, overall I'm largely in favor of this change.
The only somehow grey-ish area is the fact we are still changing the signature of public functions, belonging to sub-packages.
That made me think: perhaps we can refine our stance and ensure that only the top-level exported symbols (alias.go) are stable, and this could allow to save 90% of the pain from our users, and still allow us to evolve the codebase - and maybe tighten again the types again?

Perhaps we can explore this option?

ffromani · 2025-12-31T19:58:34Z

alias.go

 // DEPRECATED: Please use Option
 type WithOption = option.Option
+
+// DEPRECATED: Please use WithXXX functions.


This is a bit puzzling because 3 lines above we instruct users to NOT use a WithXXX function

ack. removed this particular typedef.

ffromani · 2026-01-07T10:58:19Z

README.md

+        level = color.RedString(level)
+    }
+
+    fields := make(map[string]interface{}, r.NumAttrs())


nit: map[string]any (please do NOT resubmit just for this)

ffromani · 2026-01-07T11:00:56Z

pkg/block/block_linux_test.go

+	ctx := context.TODO()
+	ctx = ghwcontext.WithChroot(baseDir)(ctx)
+	paths := linuxpath.New(ctx)


in a followup PR we may want to try to add a test helper like kube did during the transition to contextual logger. I can play with this concept later on.

ffromani · 2026-01-07T11:01:43Z

pkg/context/context.go

+// See the COPYING file in the root project directory for full text.
+//
+
+package context


I'm thinking if we can avoid the name clash with the stdlib. I don't have good suggestions here.

ffromani · 2026-01-07T11:03:24Z

pkg/context/context.go

+	envKeyDisableTools    = "GHW_DISABLE_TOOLS"
+)
+
+type ContextKey string


do we want or need to export this? If exported, should it be ContextKey or just Key?
e.g.

ghwcontext.ContextKey ghwcontext.Key

ffromani · 2026-01-07T11:03:36Z

pkg/context/context.go

+)
+
+// ContextModifier sets some value on the context
+type ContextModifier func(context.Context) context.Context


this needs to be exported indeed, but maybe just Modifier ?

ffromani · 2026-01-07T11:16:35Z

pkg/context/context.go

+
+// FromArgs returns a context.Context populated with any old-style options or
+// new-style arguments.
+func FromArgs(args ...any) context.Context {


(here and everywhere else). I'm just a little off by the fact we need to use variadic args and the any type. But it's a necessary evil (and the best/only option to keep backward compat). The code is massively better with this change, so I think this is the sweet spot. In the future we will tighten the function signatures again I reckon.

ffromani · 2026-01-07T11:27:06Z

pkg/context/log.go

+// See the COPYING file in the root project directory for full text.
+//
+
+package context


minorish: does this belong in the context package? do we want or need to add a dedicated log package?
I don't have strong opinions either way but I think is worth raising the point.

certainly we can consider throwing it into an internal/log package in the future!

ffromani · 2026-01-07T12:12:46Z

pkg/context/log.go

+
+// Warn outputs an WARN-level log message to the logger configured in the
+// supplied context.
+func Warn(ctx context.Context, format string, args ...any) {


on hindsight, looking at this (and other) function callsites, I'm leaning towards adding a log package somehow. Or whatever better name we can find. WDYT?

lemme throw it into an internal/log package :)

jaypipes · 2026-02-07T12:28:18Z

@ffromani OK, I've pushed another commit that (hopefully!) gives us a good longer-term solution to the awkwardness you pointed out in comments on the previous commits around the shadowing of the stdlib context package and placing the log-writing functions in pkg/context/log.go.

I've created two packages called internal/config and internal/log that contain the configuration-getting/setting context-returning wrappers and moved the Debug/Info/Warn functions into internal/log.

Let me know if this aligns better!

ffromani · 2026-02-07T16:11:20Z

alias.go

 package ghw

 import (
+	"github.com/jaypipes/ghw/internal/config"


I'm worried this would make the top-level ghw package not-importable: https://stackoverflow.com/questions/41571946/internal-packages-in-go
I can test shortly with this change applied, but not not immediately.

I'm more than fine with postponing the config/log refactoring to a later stage to bypass this concern, so we can merge this PR quickly.

applies also to sub-packages apparently

I don't believe this would make ghw not importable. That's actually why I made the changes to some of the _test.go files that use a _test package and therefore can only import exported symbols.

I can and will push a temp fork with this change applied and try to import from other projects. Let's see how it looks. Basically this is my only concern, if this works as intended we can merge

OK, error on my side. The mistake I was making is that I was interpreting internal literally and too strictly.
How it really works: golang's internal package restriction is about direct imports, not transitive dependencies. Public facing packages are free to consume internal packages, and they're still importable just fine.

jaypipes · 2026-02-07T18:29:58Z

pkg/cpu/cpu_linux_test.go

 	"strings"
 	"testing"

+	"github.com/jaypipes/ghw"


@ffromani note that this particular test file uses the package cpu_test and therefore cannot use internal imports. Which is why I changed the "option.WithChroot to ghw.WithChroot to verify that the internal typedefs aliased as exported symbols in alias.go work properly.

right, this specific one should be fine.

ffromani

we may need a rebase? otherwise feel free to merge anytime (or ping me and I can hit the button)

Continues the work from #417 in two import ways. First, this finalizes the move towards passing only a `context.Context` argument to the inner `InfoXXX.load()` methods instead of the old `pkg/option.Options` struct. Using a `context.Context` is more modern Go idiomatic and aligned with modern Go packages like `log/slog`. Each `InfoXXX.load()` method ensures a `context.Context` is created to store various options and context that is passed between modules in `ghw`. The function signature for the main module constructors like `pkg/cpu.CPU()` or `pkg/block.Block()` have all been changed from this: ```go func CPU(opt ...option.Option) (*Info, error) ``` to ```go func CPU(args ...any) (*Info, error) ``` which is a backwards compatible API change. We then examine each of the `args` arguments and set various keys on the `context.Context`. The `context.Context` is examined by other modules like `pkg/linuxpath` or `pkg/linuxdmi` instead of the prior `pkg/option.Options` struct. The second major part of this patch is the addition of log/output formatting to use `log/slog` and `log/slog.Handler` overrides to control the output of warning and debug log lines. The default log output in `ghw` only writes WARN-level messages to `stderr` in a simple `WARN: <msg>` log record format: ``` $ ghwc baseboard WARN: Unable to read board_serial: open /sys/class/dmi/id/board_serial: permission denied baseboard vendor=System76 version=thelio-mira-b4.1 product=Thelio Mira ``` You can control a number of log output options programmatically or by using environs variables. To change the log level `ghw` uses, set the `GHW_LOG_LEVEL` environs variable: ``` $ GHW_LOG_LEVEL=debug ghwc baseboard DEBUG: reading from "/sys/class/dmi/id/board_asset_tag" DEBUG: reading from "/sys/class/dmi/id/board_serial" WARN: Unable to read board_serial: open /sys/class/dmi/id/board_serial: permission denied DEBUG: reading from "/sys/class/dmi/id/board_vendor" DEBUG: reading from "/sys/class/dmi/id/board_version" DEBUG: reading from "/sys/class/dmi/id/board_name" baseboard vendor=System76 version=thelio-mira-b4.1 product=Thelio Mira ``` Changing `GHW_LOG_LEVEL` to `error` has the same effect of setting `GHW_DISABLE_WARNINGS`: ``` $ GHW_LOG_LEVEL=error ghwc baseboard baseboard vendor=System76 version=thelio-mira-b4.1 product=Thelio Mira ``` You can change the log level programmatically using the `WithLogLevel` modifier: ```go import ( "log/slog" "github.com/jaypipes/ghw" ) bb, err := ghw.Baseboard(ghw.WithLogLevel(slog.LevelDebug)) ``` To use the [logfmt][logfmt] standard log output format, set the `GHW_LOG_LOGFMT` envrions variable: ``` $ GHW_LOG_LOGFMT=1 ghwc baseboard time=2025-12-28T07:31:08.614-05:00 level=WARN msg="Unable to read board_serial: open /sys/class/dmi/id/board_serial: permission denied" baseboard vendor=System76 version=thelio-mira-b4.1 product=Thelio Mira ``` You can tell `ghw` to use `logfmt` standard output formatting using the `WithLogLogfmt` modifier: ```go import ( "log/slog" "github.com/jaypipes/ghw" ) bb, err := ghw.Baseboard(ghw.WithLogLogfmt()) ``` [logfmt]: https://www.cloudbees.com/blog/logfmt-a-log-format-thats-easy-to-read-and-write You can now programmatically override the logger that `ghw` uses with the `WithLogger` modifier. You pass in an instance of `slog.Logger`, like this example that shows how to use a simple logger with colored log output: ```go package main import ( "context" "encoding/json" "io" "log" "log/slog" "github.com/fatih/color" "github.com/jaypipes/ghw" ) type PrettyHandlerOptions struct { SlogOpts slog.HandlerOptions } type PrettyHandler struct { slog.Handler l *log.Logger } func (h *PrettyHandler) Handle(ctx context.Context, r slog.Record) error { level := r.Level.String() + ":" switch r.Level { case slog.LevelDebug: level = color.MagentaString(level) case slog.LevelInfo: level = color.BlueString(level) case slog.LevelWarn: level = color.YellowString(level) case slog.LevelError: level = color.RedString(level) } fields := make(map[string]interface{}, r.NumAttrs()) r.Attrs(func(a slog.Attr) bool { fields[a.Key] = a.Value.Any() return true }) b, err := json.MarshalIndent(fields, "", " ") if err != nil { return err } timeStr := r.Time.Format("[15:05:05.000]") msg := color.CyanString(r.Message) h.l.Println(timeStr, level, msg, color.WhiteString(string(b))) return nil } func NewPrettyHandler( out io.Writer, opts PrettyHandlerOptions, ) *PrettyHandler { h := &PrettyHandler{ Handler: slog.NewJSONHandler(out, &opts.SlogOpts), l: log.New(out, "", 0), } return h } func main() { opts := PrettyHandlerOptions{ SlogOpts: slog.HandlerOptions{ Level: slog.LevelDebug, }, } handler := NewPrettyHandler(os.Stdout, opts) logger := slog.New(handler) bb, err := ghw.Baseboard(ghw.WithLogger(logger)) if err != nil { logger.Error(err.String()) } fmt.Println(bb) } ``` Signed-off-by: Jay Pipes <jaypipes@gmail.com>

Replaces the SetTraceFunction() functionality in `pkg/snapshot` with the `pkg/context.Debug()` function which uses the standardized `log/slog` facility. Running `ghwc snapshot --debug` now: ``` > go run cmd/ghwc/main.go snapshot --debug DEBUG: processing block device "/sys/block/nvme0n1" DEBUG: link target for block device "/sys/block/nvme0n1" is "../devices/pci0000:00/0000:00:1a.0/0000:02:00.0/nvme/nvme0/nvme0n1" DEBUG: creating device directory /tmp/ghw-snapshot840812080/sys/devices/pci0000:00/0000:00:1a.0/0000:02:00.0/nvme/nvme0/nvme0n1 DEBUG: linking device directory /tmp/ghw-snapshot840812080/sys/block/nvme0n1 to ../devices/pci0000:00/0000:00:1a.0/0000:02:00.0/nvme/nvme0/nvme0n1 DEBUG: creating device directory "/tmp/ghw-snapshot840812080/sys/devices/pci0000:00/0000:00:1a.0/0000:02:00.0/nvme/nvme0/nvme0n1" from "/sys/devices/pci0000:00/0000:00:1a.0/0000:02:00.0/nvme/nvme0/nvme0n1" DEBUG: creating /tmp/ghw-snapshot840812080/sys/devices/pci0000:00/0000:00:1a.0/0000:02:00.0/nvme/nvme0/nvme0n1/alignment_offset DEBUG: creating /tmp/ghw-snapshot840812080/sys/devices/pci0000:00/0000:00:1a.0/0000:02:00.0/nvme/nvme0/nvme0n1/capability DEBUG: creating /tmp/ghw-snapshot840812080/sys/devices/pci0000:00/0000:00:1a.0/0000:02:00.0/nvme/nvme0/nvme0n1/csi ... ``` Signed-off-by: Jay Pipes <jaypipes@gmail.com>

Moves environment and configuration-setting stuff from `pkg/context` into `internal/config`. Moves the log-writing utility functions from `pkg/context/log.go` to `internal/log`. Signed-off-by: Jay Pipes <jaypipes@gmail.com>

jaypipes assigned ffromani Dec 28, 2025

jaypipes force-pushed the log-trace-err branch 2 times, most recently from bbfb9c0 to c419aaa Compare December 28, 2025 21:55

ffromani reviewed Jan 7, 2026

View reviewed changes

jaypipes force-pushed the log-trace-err branch from c419aaa to e8d5dbb Compare February 7, 2026 12:25

jaypipes force-pushed the log-trace-err branch from e8d5dbb to 56390af Compare February 7, 2026 12:29

ffromani reviewed Feb 7, 2026

View reviewed changes

jaypipes commented Feb 7, 2026

View reviewed changes

ffromani approved these changes Feb 9, 2026

View reviewed changes

jaypipes added 2 commits February 9, 2026 07:28

jaypipes force-pushed the log-trace-err branch from 56390af to cbbbbe8 Compare February 9, 2026 12:29

hide context/config/log in internal/

61a5336

Moves environment and configuration-setting stuff from `pkg/context` into `internal/config`. Moves the log-writing utility functions from `pkg/context/log.go` to `internal/log`. Signed-off-by: Jay Pipes <jaypipes@gmail.com>

jaypipes force-pushed the log-trace-err branch from cbbbbe8 to 61a5336 Compare February 9, 2026 12:32

jaypipes merged commit 6c6d767 into main Feb 9, 2026
21 checks passed

jaypipes deleted the log-trace-err branch February 9, 2026 12:34

Conversation

jaypipes commented Dec 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jaypipes commented Dec 29, 2025

Uh oh!

ffromani commented Dec 31, 2025

Uh oh!

ffromani left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jaypipes commented Feb 7, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ffromani left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jaypipes commented Dec 28, 2025 •

edited

Loading