Adopt recent updates in the Go runtime #4031
Open
Conversation
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
This commit replaces all uses of [dlog](telepresenceio/dlib/dlog) with the new [clog](telepresenceio/clog) package, built on top of the recent (as of Go 1.21) runtime package `log/slog`, which provides standardized structured logging for Golang. This is a first step. Telepresence still uses some datawire and other telepresenceio packages that use `dlog`. Signed-off-by: Thomas Hallgren <[email protected]>
The `caarlos0/env` is actively maintained, widely used, fits the bill perfectly, and has no external dependencies. Signed-off-by: Thomas Hallgren <[email protected]>
Signed-off-by: Thomas Hallgren <[email protected]>
Signed-off-by: Thomas Hallgren <[email protected]>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR removes dependencies on the datawire/dlib package and adopts equivalent functionality from the Go standard library and a new clog package. The changes align with recent Go runtime updates that now provide built-in alternatives to features previously offered by dlib.
Changes:
- Replaced dlog with clog for context-aware structured logging
- Replaced dgroup with a new log.Group wrapper around errgroup for goroutine management
- Replaced dtime.FakeTime/SleepWithContext with standard time package
- Replaced dhttp server with standard net/http server
- Replaced dcontext utilities with standard context package
- Updated dependency versions (golang.org/x, k8s.io, google.golang.org)
Reviewed changes
Copilot reviewed 261 out of 268 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tools/src/test-report/* | Updated logging and progress reporting with new BUILD_FAIL constant |
| pkg/vif/* | Replaced dlog with clog for VIF device logging |
| pkg/workload/informers.go | Updated watcher error handlers to use clog |
| pkg/tunnel/* | Replaced dlog with clog throughout tunnel package |
| pkg/types/mountpolicy.go | Added ParseMountPolicy and UnmarshalText methods |
| pkg/sigctx/context.go | New package for signal context handling |
| pkg/routing/* | Updated logging to clog |
| pkg/restapi/api.go | Replaced dhttp server with standard http.Server |
| pkg/proc/* | Updated command execution logging to clog |
| pkg/pprof/pprof.go | Replaced dhttp with standard http.Server |
| pkg/log/* | Removed old logging utilities, added new Group wrapper |
| pkg/json/marshal.go | Added slog.Level unmarshaling support |
| pkg/grpc/server/* | Updated gRPC server logging and context handling |
| pkg/forwarder/* | Updated forwarder logging to clog |
| pkg/dpipe/* | Updated pipe logging to clog |
| pkg/dnsproxy/* | Updated DNS proxy logging to clog |
| pkg/dos/filesystem_test.go | Updated test context creation |
| pkg/client/* | Extensive updates to user daemon, root daemon, and session management |
| pkg/client/logging/* | Major refactor replacing logrus with slog-based logging |
| rpc/go.* | Updated dependency versions |
Comments suppressed due to low confidence (1)
tools/src/test-report/logger.go:52
- File handle may be writable as a result of data flow from a call to OpenFile and closing it may result in data loss upon failure, which is not handled explicitly.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
thallgren
added
the
ok to test
github-actions
bot
removed
the
ok to test
thallgren
force-pushed
the
thallgren/modernize
branch
from
57a70b9 to
813e2ef
Compare
The dgroup uses dlog and hard/soft context semantics. This functionality is not needed so this commit replaces all use of the `dgroup.Group` with a much simpler embedded `log.Group` which just combines a context and an `errgroup.Group`. Signed-off-by: Thomas Hallgren <[email protected]>
The `errors.Join` function introduced in Go 1.20 is the idiomatic way of combining errors. Signed-off-by: Thomas Hallgren <[email protected]>
The synctest.Test released in Go 1.25 makes it possible to write tests that simulate the time moving forward so we no longer have any use for the "fake time" that the dlib/dtime package provides. The dtime package also provided a `dtime.SleepWithContext` but it's not very relevant for short sleep periods where context cancellation is unlikely, which turned out to be always the case. Signed-off-by: Thomas Hallgren <[email protected]>
The dlib/dhttp package is not maintained, and Telepresence doesn't use anything that it provides, so this commit replaces all uses of it with the runtime http package. Signed-off-by: Thomas Hallgren <[email protected]>
Copies of code creating a `common.VersionInfo` were scattered all over the place. This commit replaces that with one function. Signed-off-by: Thomas Hallgren <[email protected]>
Signed-off-by: Thomas Hallgren <[email protected]>
thallgren
force-pushed
the
thallgren/modernize
branch
from
813e2ef to
fc6dfb7
Compare
Signed-off-by: Thomas Hallgren <[email protected]>
Signed-off-by: Thomas Hallgren <[email protected]>
thallgren
force-pushed
the
thallgren/modernize
branch
from
fc6dfb7 to
940263c
Compare
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.
The datawire/dlib component introduced a lot of great features which has now been adopted, albeit in a different form, by the Go runtime. Example:
log/slog.dcontext.WithoutCancelhas an exactcontext.WithoutCancelequivalent.derrors.MultiErrorcan instead useerrors.Join.dexec.CommandContextcan be replaced withexec.CommandContexttesting/synctestpackage instead ofdtime.FakeTime.net/httppackage and google.golang.org/grpc can fully replacedhttp.This, in addition to the fact that the datawire/dlib isn't actively maintained, are the main factors that motivates this PR, which removes all dependencies to the dlib package.