Skip to content

Replace log package with fully slog-based system #8606

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
aarongable wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

Replace log package with fully slog-based system #8606

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7a00963
WIP: create blog package
aarongable Mar 13, 2026
81a8ce7
Update cmd/shell and grpc/ to use blog
aarongable Mar 14, 2026
bff319e
Update SA
aarongable Mar 14, 2026
9f3392c
Update RA and its dependencies
aarongable Mar 14, 2026
ecf5a7f
Update VA and bdns
aarongable Mar 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions bdns/dns.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"errors"
"fmt"
"io"
"log/slog"
"net"
"net/http"
"strconv"
Expand All @@ -17,7 +18,7 @@ import (
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/promauto"

blog "github.com/letsencrypt/boulder/log"
"github.com/letsencrypt/boulder/blog"
"github.com/letsencrypt/boulder/metrics"
)

Expand Down Expand Up @@ -67,7 +68,6 @@ type impl struct {
servers ServerProvider
maxTries int
clk clock.Clock
log blog.Logger

queryTime *prometheus.HistogramVec
totalLookupTime *prometheus.HistogramVec
Expand All @@ -86,7 +86,6 @@ func New(
clk clock.Clock,
maxTries int,
userAgent string,
log blog.Logger,
tlsConfig *tls.Config,
) Client {
// Clone the default transport because it comes with various settings that we
Expand Down Expand Up @@ -143,7 +142,6 @@ func New(
queryTime: queryTime,
totalLookupTime: totalLookupTime,
timeoutCounter: timeoutCounter,
log: log,
}
}

Expand Down Expand Up @@ -226,7 +224,12 @@ func (c *impl) exchangeOne(ctx context.Context, hostname string, qtype uint16) (
}).Observe(rtt.Seconds())

if err != nil {
c.log.Infof("logDNSError chosenServer=[%s] hostname=[%s] queryType=[%s] err=[%s]", chosenServer, hostname, qtypeStr, err)
blog.Info(ctx, "logDNSError",
slog.String("chosenServer", chosenServer),
slog.String("hostname", hostname),
slog.String("qtype", qtypeStr),
blog.ErrAttr(err),
)

// Check if the error is a network timeout, rather than a local context
// timeout. If it is, retry instead of giving up.
Expand Down
84 changes: 42 additions & 42 deletions bdns/dns_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import (
"github.com/miekg/dns"
"github.com/prometheus/client_golang/prometheus"

blog "github.com/letsencrypt/boulder/log"
"github.com/letsencrypt/boulder/blog"
"github.com/letsencrypt/boulder/metrics"
"github.com/letsencrypt/boulder/test"
)
Expand Down Expand Up @@ -283,21 +283,21 @@ func TestDNSNoServers(t *testing.T) {
staticProvider, err := NewStaticProvider([]string{})
test.AssertNotError(t, err, "Got error creating StaticProvider")

obj := New(time.Hour, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, "", blog.UseMock(), tlsConfig)
obj := New(time.Hour, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, "", tlsConfig)

_, resolver, err := obj.LookupA(context.Background(), "letsencrypt.org")
_, resolver, err := obj.LookupA(blog.MockContext(t), "letsencrypt.org")
test.AssertEquals(t, resolver, "")
test.AssertError(t, err, "No servers")

_, resolver, err = obj.LookupAAAA(context.Background(), "letsencrypt.org")
_, resolver, err = obj.LookupAAAA(blog.MockContext(t), "letsencrypt.org")
test.AssertEquals(t, resolver, "")
test.AssertError(t, err, "No servers")

_, resolver, err = obj.LookupTXT(context.Background(), "letsencrypt.org")
_, resolver, err = obj.LookupTXT(blog.MockContext(t), "letsencrypt.org")
test.AssertEquals(t, resolver, "")
test.AssertError(t, err, "No servers")

_, resolver, err = obj.LookupCAA(context.Background(), "letsencrypt.org")
_, resolver, err = obj.LookupCAA(blog.MockContext(t), "letsencrypt.org")
test.AssertEquals(t, resolver, "")
test.AssertError(t, err, "No servers")
}
Expand All @@ -306,9 +306,9 @@ func TestDNSOneServer(t *testing.T) {
staticProvider, err := NewStaticProvider([]string{dnsLoopbackAddr})
test.AssertNotError(t, err, "Got error creating StaticProvider")

obj := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, "", blog.UseMock(), tlsConfig)
obj := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, "", tlsConfig)

_, resolver, err := obj.LookupA(context.Background(), "letsencrypt.org")
_, resolver, err := obj.LookupA(blog.MockContext(t), "letsencrypt.org")
test.AssertNotError(t, err, "No message")
test.AssertEquals(t, resolver, "127.0.0.1:4053")
}
Expand All @@ -317,9 +317,9 @@ func TestDNSDuplicateServers(t *testing.T) {
staticProvider, err := NewStaticProvider([]string{dnsLoopbackAddr, dnsLoopbackAddr})
test.AssertNotError(t, err, "Got error creating StaticProvider")

obj := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, "", blog.UseMock(), tlsConfig)
obj := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, "", tlsConfig)

_, resolver, err := obj.LookupA(context.Background(), "letsencrypt.org")
_, resolver, err := obj.LookupA(blog.MockContext(t), "letsencrypt.org")
test.AssertNotError(t, err, "No message")
test.AssertEquals(t, resolver, "127.0.0.1:4053")
}
Expand All @@ -328,32 +328,32 @@ func TestDNSServFail(t *testing.T) {
staticProvider, err := NewStaticProvider([]string{dnsLoopbackAddr})
test.AssertNotError(t, err, "Got error creating StaticProvider")

obj := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, "", blog.UseMock(), tlsConfig)
obj := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, "", tlsConfig)
bad := "servfail.com"

_, _, err = obj.LookupTXT(context.Background(), "servfail.com")
_, _, err = obj.LookupTXT(blog.MockContext(t), "servfail.com")
test.AssertError(t, err, "LookupTXT didn't return an error")

_, _, err = obj.LookupA(context.Background(), bad)
_, _, err = obj.LookupA(blog.MockContext(t), bad)
test.AssertError(t, err, "LookupA didn't return an error")

_, _, err = obj.LookupAAAA(context.Background(), bad)
_, _, err = obj.LookupAAAA(blog.MockContext(t), bad)
test.AssertError(t, err, "LookupAAAA didn't return an error")

_, _, err = obj.LookupCAA(context.Background(), bad)
_, _, err = obj.LookupCAA(blog.MockContext(t), bad)
test.AssertError(t, err, "LookupCAA didn't return an error")
}

func TestDNSLookupTXT(t *testing.T) {
staticProvider, err := NewStaticProvider([]string{dnsLoopbackAddr})
test.AssertNotError(t, err, "Got error creating StaticProvider")

obj := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, "", blog.UseMock(), tlsConfig)
obj := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, "", tlsConfig)

_, _, err = obj.LookupTXT(context.Background(), "letsencrypt.org")
_, _, err = obj.LookupTXT(blog.MockContext(t), "letsencrypt.org")
test.AssertNotError(t, err, "No message")

txt, _, err := obj.LookupTXT(context.Background(), "split-txt.letsencrypt.org")
txt, _, err := obj.LookupTXT(blog.MockContext(t), "split-txt.letsencrypt.org")
test.AssertNotError(t, err, "No message")
test.AssertEquals(t, len(txt.Final), 1)
test.AssertEquals(t, strings.Join(txt.Final[0].Txt, ""), "abc")
Expand All @@ -363,7 +363,7 @@ func TestDNSLookupA(t *testing.T) {
staticProvider, err := NewStaticProvider([]string{dnsLoopbackAddr})
test.AssertNotError(t, err, "Got error creating StaticProvider")

obj := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, "", blog.UseMock(), tlsConfig)
obj := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, "", tlsConfig)

for _, tc := range []struct {
name string
Expand Down Expand Up @@ -413,7 +413,7 @@ func TestDNSLookupA(t *testing.T) {
},
} {
t.Run(tc.name, func(t *testing.T) {
res, resolver, err := obj.LookupA(context.Background(), tc.hostname)
res, resolver, err := obj.LookupA(blog.MockContext(t), tc.hostname)

wantResolver := "127.0.0.1:4053"
if resolver != wantResolver {
Expand Down Expand Up @@ -448,7 +448,7 @@ func TestDNSLookupAAAA(t *testing.T) {
staticProvider, err := NewStaticProvider([]string{dnsLoopbackAddr})
test.AssertNotError(t, err, "Got error creating StaticProvider")

obj := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, "", blog.UseMock(), tlsConfig)
obj := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, "", tlsConfig)

for _, tc := range []struct {
name string
Expand Down Expand Up @@ -498,7 +498,7 @@ func TestDNSLookupAAAA(t *testing.T) {
},
} {
t.Run(tc.name, func(t *testing.T) {
res, resolver, err := obj.LookupAAAA(context.Background(), tc.hostname)
res, resolver, err := obj.LookupAAAA(blog.MockContext(t), tc.hostname)

wantResolver := "127.0.0.1:4053"
if resolver != wantResolver {
Expand Down Expand Up @@ -533,16 +533,16 @@ func TestDNSNXDOMAIN(t *testing.T) {
staticProvider, err := NewStaticProvider([]string{dnsLoopbackAddr})
test.AssertNotError(t, err, "Got error creating StaticProvider")

obj := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, "", blog.UseMock(), tlsConfig)
obj := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, "", tlsConfig)
hostname := "nxdomain.letsencrypt.org"

_, _, err = obj.LookupA(context.Background(), hostname)
_, _, err = obj.LookupA(blog.MockContext(t), hostname)
test.AssertContains(t, err.Error(), "NXDOMAIN looking up A for")

_, _, err = obj.LookupAAAA(context.Background(), hostname)
_, _, err = obj.LookupAAAA(blog.MockContext(t), hostname)
test.AssertContains(t, err.Error(), "NXDOMAIN looking up AAAA for")

_, _, err = obj.LookupTXT(context.Background(), hostname)
_, _, err = obj.LookupTXT(blog.MockContext(t), hostname)
expected := Error{dns.TypeTXT, hostname, nil, dns.RcodeNameError, nil}
test.AssertDeepEquals(t, err, expected)
}
Expand All @@ -551,10 +551,10 @@ func TestDNSLookupCAA(t *testing.T) {
staticProvider, err := NewStaticProvider([]string{dnsLoopbackAddr})
test.AssertNotError(t, err, "Got error creating StaticProvider")

obj := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, "", blog.UseMock(), tlsConfig)
obj := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, "", tlsConfig)
removeIDExp := regexp.MustCompile(" id: [[:digit:]]+")

caas, resolver, err := obj.LookupCAA(context.Background(), "bracewel.net")
caas, resolver, err := obj.LookupCAA(blog.MockContext(t), "bracewel.net")
test.AssertNotError(t, err, "CAA lookup failed")
test.Assert(t, len(caas.Final) > 0, "Should have CAA records")
test.AssertEquals(t, resolver, "127.0.0.1:4053")
Expand All @@ -569,17 +569,17 @@ bracewel.net. 0 IN CAA 1 issue "letsencrypt.org"
`
test.AssertEquals(t, removeIDExp.ReplaceAllString(caas.String(), " id: XXXX"), expectedResp)

caas, resolver, err = obj.LookupCAA(context.Background(), "nonexistent.letsencrypt.org")
caas, resolver, err = obj.LookupCAA(blog.MockContext(t), "nonexistent.letsencrypt.org")
test.AssertNotError(t, err, "CAA lookup failed")
test.Assert(t, len(caas.Final) == 0, "Shouldn't have CAA records")
test.AssertEquals(t, resolver, "127.0.0.1:4053")

caas, resolver, err = obj.LookupCAA(context.Background(), "nxdomain.letsencrypt.org")
caas, resolver, err = obj.LookupCAA(blog.MockContext(t), "nxdomain.letsencrypt.org")
test.AssertNotError(t, err, "CAA lookup failed")
test.Assert(t, len(caas.Final) == 0, "Shouldn't have CAA records")
test.AssertEquals(t, resolver, "127.0.0.1:4053")

caas, resolver, err = obj.LookupCAA(context.Background(), "cname.example.com")
caas, resolver, err = obj.LookupCAA(blog.MockContext(t), "cname.example.com")
test.AssertNotError(t, err, "CAA lookup failed")
test.Assert(t, len(caas.Final) > 0, "Should follow CNAME to find CAA")
test.AssertEquals(t, resolver, "127.0.0.1:4053")
Expand All @@ -594,7 +594,7 @@ caa.example.com. 0 IN CAA 1 issue "letsencrypt.org"
`
test.AssertEquals(t, removeIDExp.ReplaceAllString(caas.String(), " id: XXXX"), expectedResp)

_, resolver, err = obj.LookupCAA(context.Background(), "gonetld")
_, resolver, err = obj.LookupCAA(blog.MockContext(t), "gonetld")
test.AssertError(t, err, "should fail for TLD NXDOMAIN")
test.AssertContains(t, err.Error(), "NXDOMAIN")
test.AssertEquals(t, resolver, "127.0.0.1:4053")
Expand Down Expand Up @@ -759,10 +759,10 @@ func TestRetry(t *testing.T) {
staticProvider, err := NewStaticProvider([]string{dnsLoopbackAddr})
test.AssertNotError(t, err, "Got error creating StaticProvider")

testClient := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), tc.maxTries, "", blog.UseMock(), tlsConfig)
testClient := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), tc.maxTries, "", tlsConfig)
dr := testClient.(*impl)
dr.exchanger = tc.te
_, _, err = dr.LookupTXT(context.Background(), "example.com")
_, _, err = dr.LookupTXT(blog.MockContext(t), "example.com")
if err == errTooManyRequests {
t.Errorf("#%d, sent more requests than the test case handles", i)
}
Expand Down Expand Up @@ -796,10 +796,10 @@ func TestRetryMetrics(t *testing.T) {
// context itself being cancelled. It should never see the error in the
// testExchanger, because the fake exchanger (like the real http package)
// checks for cancellation before doing any work.
testClient := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 3, "", blog.UseMock(), tlsConfig)
testClient := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 3, "", tlsConfig)
dr := testClient.(*impl)
dr.exchanger = &testExchanger{errs: []error{errors.New("oops")}}
ctx, cancel := context.WithCancel(t.Context())
ctx, cancel := context.WithCancel(blog.MockContext(t))
cancel()
_, _, err = dr.LookupTXT(ctx, "example.com")
if err == nil ||
Expand All @@ -815,10 +815,10 @@ func TestRetryMetrics(t *testing.T) {

// Same as above, except rather than cancelling the context ourselves, we
// let the go runtime cancel it as a result of a deadline in the past.
testClient = New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 3, "", blog.UseMock(), tlsConfig)
testClient = New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 3, "", tlsConfig)
dr = testClient.(*impl)
dr.exchanger = &testExchanger{errs: []error{errors.New("oops")}}
ctx, cancel = context.WithTimeout(t.Context(), -10*time.Hour)
ctx, cancel = context.WithTimeout(blog.MockContext(t), -10*time.Hour)
defer cancel()
_, _, err = dr.LookupTXT(ctx, "example.com")
if err == nil ||
Expand Down Expand Up @@ -883,7 +883,7 @@ func TestRotateServerOnErr(t *testing.T) {
test.AssertNotError(t, err, "Got error creating StaticProvider")

maxTries := 5
client := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), maxTries, "", blog.UseMock(), tlsConfig)
client := New(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), maxTries, "", tlsConfig)

// Configure a mock exchanger that will always return a retryable error for
// servers A and B. This will force server "[2606:4700:4700::1111]:53" to do
Expand All @@ -903,7 +903,7 @@ func TestRotateServerOnErr(t *testing.T) {
// servers *all* queries should eventually succeed by being retried against
// server "[2606:4700:4700::1111]:53".
for range maxTries * 2 {
_, resolver, err := client.LookupTXT(context.Background(), "example.com")
_, resolver, err := client.LookupTXT(blog.MockContext(t), "example.com")
test.AssertEquals(t, resolver, "[2606:4700:4700::1111]:53")
// Any errors are unexpected - server "[2606:4700:4700::1111]:53" should
// have responded without error.
Expand Down Expand Up @@ -948,15 +948,15 @@ func TestDOHMetric(t *testing.T) {
staticProvider, err := NewStaticProvider([]string{dnsLoopbackAddr})
test.AssertNotError(t, err, "Got error creating StaticProvider")

testClient := New(time.Second*11, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 0, "", blog.UseMock(), tlsConfig)
testClient := New(time.Second*11, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 0, "", tlsConfig)
resolver := testClient.(*impl)
resolver.exchanger = &dohAlwaysRetryExchanger{err: &url.Error{Op: "read", Err: testTimeoutError(true)}}

// Starting out, we should count 0 "out of retries" errors.
test.AssertMetricWithLabelsEquals(t, resolver.timeoutCounter, prometheus.Labels{"qtype": "None", "type": "out of retries", "resolver": "127.0.0.1", "isTLD": "false"}, 0)

// Trigger the error.
_, _, _ = resolver.exchangeOne(context.Background(), "example.com", 0)
_, _, _ = resolver.exchangeOne(blog.MockContext(t), "example.com", 0)

// Now, we should count 1 "out of retries" errors.
test.AssertMetricWithLabelsEquals(t, resolver.timeoutCounter, prometheus.Labels{"qtype": "None", "type": "out of retries", "resolver": "127.0.0.1", "isTLD": "false"}, 1)
Expand Down
Loading
Loading