cert-checker: fix logging & push metrics by lenaunderwood22 · Pull Request #8763 · letsencrypt/boulder

lenaunderwood22 · 2026-05-26T21:25:12Z

github-actions · 2026-05-26T21:56:29Z

@lenaunderwood22, this PR appears to contain configuration and/or SQL schema changes. Please ensure that a corresponding deployment ticket has been filed with the new values.

Co-authored-by: Aaron Gable <aaron@letsencrypt.org>

…l nit fixes

Copilot

Pull request overview

This PR updates cert-checker to emit structured audit logs (instead of printing a large multi-line JSON blob to stdout) and to publish run metrics (good/bad cert counts, latency, last-run timestamp) to a Prometheus Pushgateway discovered via DNS SRV (intended to replace an external wrapper script per issue #8753).

Changes:

Replace stdout JSON report output with per-certificate audit error logs plus a final audit summary.
Add Prometheus metrics for cert-checker results and push them to a Pushgateway (incl. SRV discovery + shared cmd.PushMetrics helper).
Update tests/config fixtures and vendor the Prometheus push subpackage.

Reviewed changes

Copilot reviewed 5 out of 7 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
`cmd/cert-checker/main.go`	Switch to audit logging, add metrics, add SRV-based Pushgateway discovery and push behavior.
`cmd/cert-checker/main_test.go`	Update tests to assert audit log emission rather than JSON report contents.
`cmd/shell.go`	Add `PushMetrics` helper using Prometheus push client.
`test/config/cert-checker.json`	Remove `unexpiredOnly` field from fixture config.
`test/config-next/cert-checker.json`	Remove `unexpiredOnly` field from fixture config.
`vendor/modules.txt`	Add vendored package path for `prometheus/push`.
`vendor/github.com/prometheus/client_golang/prometheus/push/push.go`	Vendor Prometheus Pushgateway client implementation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

aarongable

LGTM with some nits / test improvements.

aarongable · 2026-06-05T19:14:16Z

+	checkerBadCount  prometheus.Gauge
+}
+
+func NewCertCheckerMetrics(stats prometheus.Registerer) *certCheckerMetrics {


This constructor is only called from inside this package, so it doesn't need to be Exported.

Suggested change

func NewCertCheckerMetrics(stats prometheus.Registerer) *certCheckerMetrics {

func newCertCheckerMetrics(stats prometheus.Registerer) *certCheckerMetrics {

aarongable · 2026-06-05T19:22:31Z

-			checker.processCerts(context.TODO(), wg, config.CertChecker.BadResultsOnly)
-			checkerLatency.Observe(checker.clock.Since(s).Seconds())
-		}()
+			checker.processCerts(context.TODO())


Suggested change

checker.processCerts(context.TODO())

checker.processCerts(context.Background())

aarongable · 2026-06-05T19:25:13Z

-	wg := new(sync.WaitGroup)
-	wg.Add(1)
-	checker.processCerts(context.Background(), wg, false)
+	checker.processCerts(context.Background())


In unit tests, always use the context provided by the test harness:

Suggested change

checker.processCerts(context.Background())

checker.processCerts(t.Context())

aarongable · 2026-06-05T19:25:31Z

 }
+
+func TestGetPushgatewayURL(t *testing.T) {
+	ctx := context.Background()


Same comment here. In fact, probably leave this line out entirely, and just use t.Context() in the three places below.

aarongable · 2026-06-05T19:28:43Z

+	t.Run("happy path", func(t *testing.T) {
+		gotURL, err := getPushgatewayURL(ctx, "consul.service.consul:53",
+			cmd.ServiceDomain{Service: "redisratelimits", Domain: "service.consul"})
+		test.AssertNotError(t, err, "")


For a new test, we prefer to follow the Google Go style guide. That means using traditional error checking, and not using the assertion helper library.

Suggested change

test.AssertNotError(t, err, "")

if err != nil {

t.Fatalf("getPushgatewayURL(consul.service.consul:53) = %s, but want success", err)

}

The same applies for the assertions below. Use t.Fatalf for places where test execution should stop, and t.Errorf for places where the test should fail but we should maybe also see the results of the next thing as well.

aarongable · 2026-06-05T19:32:11Z

+	t.Run("SRV not found", func(t *testing.T) {
+		_, err := getPushgatewayURL(ctx, "consul.service.consul:53",
+			cmd.ServiceDomain{Service: "doesnotexist", Domain: "service.consul"})
+		test.AssertError(t, err, "")


In addition to the comment above about not using the test assertion helper, these tests should probably also ensure that they're getting the right error:

Suggested change

test.AssertError(t, err, "")

if err != nil {

t.Fatalf("getPushgatewayURL(consul.service.consul:53) = %s, but want success", err)

}

if !strings.Contains(err.Error(), "SRV lookup of doesnotexist._tcp.service.consul failed") {

t.Errorf("getPushgatewayURL(doesnotexist) = %q, but want 'SRV lookup failed'"), err)

}

or something like that

lenaunderwood22 added 2 commits May 26, 2026 14:20

cert-checker: fix logging & push metrics

fae0b03

add pushgatewayURL to test configs

1bbc8e4

lenaunderwood22 requested a review from a team as a code owner May 26, 2026 21:25

lenaunderwood22 requested a review from ezekiel May 26, 2026 21:25

lenaunderwood22 and others added 2 commits May 26, 2026 16:27

Merge branch 'main' into cert-checker-logs

600b6c3

lena can't syntax in the merge conflict editor

7d26e04

ezekiel previously approved these changes May 26, 2026

View reviewed changes

ezekiel requested review from a team and aarongable and removed request for a team May 26, 2026 21:56

ezekiel requested review from a team and removed request for aarongable May 26, 2026 21:56

aarongable reviewed May 28, 2026

View reviewed changes

Update cmd/cert-checker/main.go

a26009e

Co-authored-by: Aaron Gable <aaron@letsencrypt.org>

lenaunderwood22 dismissed ezekiel’s stale review via a26009e May 28, 2026 21:10

lenaunderwood22 and others added 7 commits May 28, 2026 14:27

Update cmd/cert-checker/main.go

37ec383

Co-authored-by: Aaron Gable <aaron@letsencrypt.org>

rename certCheckerMetrics to metrics; remove unexpiredOnly conf; smal…

bd919b8

…l nit fixes

remove 'entries' from report struct

e02607b

drop reportEntry too

be9ce40

remove badresultsonly references

c5e0cf6

one more

96eb1c2

more cleanup

a09d4b1

aarongable previously approved these changes May 28, 2026

View reviewed changes

aarongable requested a review from ezekiel May 28, 2026 23:33

ezekiel previously approved these changes May 28, 2026

View reviewed changes

rename metrics back to certCheckerMetrics

15ce586

lenaunderwood22 dismissed stale reviews from ezekiel and aarongable via 15ce586 May 29, 2026 14:50

lookup pushgatewayurl

02e95e5

lenaunderwood22 added 2 commits May 29, 2026 11:49

update key validations

90f7d8f

fix nil pointer ref

c50dce2

mcpherrinm requested a review from Copilot May 29, 2026 21:54

Copilot started reviewing on behalf of mcpherrinm May 29, 2026 21:55 View session

Copilot AI reviewed May 29, 2026

View reviewed changes

Comment thread cmd/cert-checker/main.go Outdated

Comment thread cmd/cert-checker/main.go Outdated

Comment thread cmd/cert-checker/main.go Outdated

Comment thread cmd/shell.go

Comment thread cmd/cert-checker/main.go Outdated

Comment thread cmd/cert-checker/main.go Outdated

lenaunderwood22 and others added 5 commits May 29, 2026 15:34

Potential fix for pull request finding

4b4d70e

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

context with 10s timeout for lookups & push

2a03d31

hardcode http

dad8b8c

add comments to getPushgatewayURL, add tests for it

ad64b08

test fixes

6c75a9c

aarongable requested a review from beautifulentropy June 1, 2026 23:11

beautifulentropy requested changes Jun 3, 2026

View reviewed changes

Comment thread cmd/cert-checker/main.go Outdated

don't need rmu anymore

72fc6a7

aarongable approved these changes Jun 5, 2026

View reviewed changes

	func NewCertCheckerMetrics(stats prometheus.Registerer) *certCheckerMetrics {
	func newCertCheckerMetrics(stats prometheus.Registerer) *certCheckerMetrics {

	checker.processCerts(context.TODO())
	checker.processCerts(context.Background())

	checker.processCerts(context.Background())
	checker.processCerts(t.Context())

-		test.AssertNotError(t, err, "")
+		if err != nil {
+			t.Fatalf("getPushgatewayURL(consul.service.consul:53) = %s, but want success", err)
+		}

Uh oh!

Conversation

lenaunderwood22 commented May 26, 2026

Uh oh!

github-actions Bot commented May 26, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aarongable left a comment

Choose a reason for hiding this comment

Uh oh!

aarongable Jun 5, 2026

Choose a reason for hiding this comment

Uh oh!

aarongable Jun 5, 2026

Choose a reason for hiding this comment

Uh oh!

aarongable Jun 5, 2026

Choose a reason for hiding this comment

Uh oh!

aarongable Jun 5, 2026

Choose a reason for hiding this comment

Uh oh!

aarongable Jun 5, 2026

Choose a reason for hiding this comment

Uh oh!

aarongable Jun 5, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants