main.go: leftovers from PR #242 by stefanb · Pull Request #289 · grpc-ecosystem/grpc-health-probe

stefanb · 2026-01-06T22:30:30Z

Reviewable leftovers from overloaded PR

chore: Update dependencies and do a small refactor #242

Copilot

Pull request overview

This PR updates the gRPC-related code to use newer, non-deprecated APIs and addresses leftover changes from PR #242. The changes modernize the connection handling and credential setup.

Key changes:

Replaces deprecated grpc.DialContext with grpc.NewClient
Updates to use grpc.WithTransportCredentials(insecure.NewCredentials()) instead of deprecated grpc.WithInsecure()
Fixes context leak by properly canceling the SPIFFE context

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

Copilot · 2026-01-06T22:31:29Z

main.go

+	// Note: GZIP compression is now handled automatically by gRPC when available
+	// The deprecated WithCompressor/WithDecompressor options are no longer needed
+	// The -gzip flag is kept for backward compatibility but has no effect
+	_ = flGZIP // Suppress unused variable warning


The flGZIP flag is now silently ignored, which could confuse users who expect it to enable compression. Consider either: (1) logging a deprecation warning when the flag is used, or (2) documenting this behavior change in the PR description or changelog so users understand the flag no longer has an effect.

Suggested change

_ = flGZIP // Suppress unused variable warning

if flGZIP {

log.Printf("warning: the -gzip flag is deprecated and no longer has any effect; gRPC compression is negotiated automatically")

}

Ported over to

Deprecate gzip cli flag #290

Copilot · 2026-01-06T22:31:30Z

main.go

-	conn, err := grpc.DialContext(dialCtx, flAddr, opts...)
+
+	// Use the new grpc.NewClient instead of deprecated DialContext
+	conn, err := grpc.NewClient(flAddr, opts...)


grpc.NewClient creates a client without immediately establishing a connection, but the code measures connDuration and logs 'connection established'. This is misleading since the actual connection happens lazily on the first RPC. The connection timing logic should be removed or updated to reflect that NewClient doesn't block to establish a connection.

I'm still thinking we do some form of blocking dial and still let people configure dial timeout. What do you think?

Indeed, current (on master) blocking while immediately establishing the connection may be deprecated for real-world applications, but for explicitly testing the grpc services health it may still be ok to do so. We just need to find how to elegantly do similar with the new NewClient() instead of deprecated grpc.DialContext().

highly doubt this method will be removed anytime soon since it's a major breaking change for a lot of legacy code out there.

The deprecation notices of both DialContext and WithBlock say:

Will be supported throughout 1.x.

(this is also clearly shown by linter)

So, perhaps we should just add a comment why is this still being used and instruct linter to ignore it?

Copilot · 2026-01-06T22:31:30Z

main.go

+	defer func() {
+		if closeErr := conn.Close(); closeErr != nil {
+			log.Printf("warning: failed to close connection: %v", closeErr)
+		}
+	}()


The error handling for Close() is now more verbose than necessary. Since conn.Close() errors are typically not actionable and the program is already exiting, the simpler 'defer conn.Close()' pattern would be sufficient and more readable.

Suggested change

defer func() {

if closeErr := conn.Close(); closeErr != nil {

log.Printf("warning: failed to close connection: %v", closeErr)

}

}()

defer conn.Close()

This suggestion reverts this defer block addition and re-introduces the linter waning.

stefanb · 2026-01-06T22:35:20Z

This PR fixes all remaining golangci-lint linter warnings from master branch:

$ golangci-lint run
main.go:325:18: Error return value of `conn.Close` is not checked (errcheck)
        defer conn.Close()
                        ^
main.go:281:14: lostcancel: the cancel function returned by context.WithTimeout should be called, not discarded, to avoid a context leak (govet)
                spiffeCtx, _ := context.WithTimeout(ctx, flRPCTimeout)
                           ^
main.go:262:3: SA1019: grpc.WithBlock is deprecated: this DialOption is not supported by NewClient. Will be supported throughout 1.x. (staticcheck)
                grpc.WithBlock(),
                ^
main.go:298:23: SA1019: grpc.WithInsecure is deprecated: use WithTransportCredentials and insecure.NewCredentials() instead. Will be supported throughout 1.x. (staticcheck)
                opts = append(opts, grpc.WithInsecure())
                                    ^
main.go:303:4: SA1019: grpc.WithCompressor is deprecated: use UseCompressor instead.  Will be supported throughout 1.x. (staticcheck)
                        grpc.WithCompressor(grpc.NewGZIPCompressor()),
                        ^
main.go:304:4: SA1019: grpc.WithDecompressor is deprecated: use encoding.RegisterCompressor instead.  Will be supported throughout 1.x. (staticcheck)
                        grpc.WithDecompressor(grpc.NewGZIPDecompressor()),
                        ^
main.go:314:15: SA1019: grpc.DialContext is deprecated: use NewClient instead.  Will be supported throughout 1.x. (staticcheck)
        conn, err := grpc.DialContext(dialCtx, flAddr, opts...)
                     ^
7 issues:
* errcheck: 1
* govet: 1
* staticcheck: 5

...some may be needed and are detected as false positives.

stefanb · 2026-01-07T08:36:00Z

main.go

-		opts = append(opts,
-			grpc.WithCompressor(grpc.NewGZIPCompressor()),
-			grpc.WithDecompressor(grpc.NewGZIPDecompressor()),
-		)


Possibly related:

Health checks no longer works with legacy compression options. grpc/grpc-go#8162

Ported to

Deprecate gzip cli flag #290

Ported over from * #289 Co-Authored-By: Erazem Kokot <mail@erazemk.com>

main.go: leftovers from PR #242

493ef99

stefanb requested a review from Copilot January 6, 2026 22:30

Copilot AI reviewed Jan 6, 2026

View reviewed changes

stefanb requested a review from ahmetb January 6, 2026 22:33

stefanb mentioned this pull request Jan 7, 2026

chore: Update dependencies and do a small refactor #242

Closed

stefanb commented Jan 7, 2026

View reviewed changes

stefanb added a commit that referenced this pull request Jan 12, 2026

deprecate gzip cli flag

49c91b9

Ported over from * #289 Co-Authored-By: Erazem Kokot <mail@erazemk.com>

This was referenced Jan 12, 2026

Deprecate gzip cli flag #290

Draft

refactor: Use non-deprecated insecure credentials #291

Merged

Merge branch 'master' into PR-242-leftovers

21bf459

-	_ = flGZIP // Suppress unused variable warning
+	if flGZIP {
+		log.Printf("warning: the -gzip flag is deprecated and no longer has any effect; gRPC compression is negotiated automatically")
+	}

Conversation

stefanb commented Jan 6, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

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!

stefanb Jan 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stefanb commented Jan 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

stefanb Jan 7, 2026 •

edited

Loading

stefanb commented Jan 6, 2026 •

edited

Loading