fix/http: add logging middleware to default exported external http clients

[ggilmore]

Closes https://linear.app/sourcegraph/issue/SRC-507/make-sure-that-the-http-retries-log-some-info-about-requests-that-are

The core change in this PR is the following:

diff --git a/internal/httpcli/client.go b/internal/httpcli/client.go
index 0f1fef2df81..e464afd5a53 100644
--- a/internal/httpcli/client.go
+++ b/internal/httpcli/client.go
@@ -100,7 +100,9 @@ var CachedTransportOpt = NewCachedTransportOpt(redisCache, true)
 // WARN: Clients from this factory cache entire responses for etag matching. Do not
 // use them for one-off requests if possible, and definitely not for larger payloads,
 // like downloading arbitrarily sized files! See UncachedExternalClientFactory instead.
-var ExternalClientFactory = NewExternalClientFactory()
+func ExternalClientFactory(logger log.Logger) *Factory {
+	return NewExternalClientFactory(NewLoggingMiddleware(logger))
+}
 
 // UncachedExternalClientFactory is a httpcli.Factory with common options
 // and middleware pre-set for communicating with external services, but with caching
@@ -175,7 +177,10 @@ func newExternalClientFactory(cache bool, testOpt bool, middleware ...Middleware
 // WARN: This client caches entire responses for etag matching. Do not use it for
 // one-off requests if possible, and definitely not for larger payloads, like
 // downloading arbitrarily sized files! See UncachedExternalDoer instead.
-var ExternalDoer, _ = ExternalClientFactory.Doer()
+func ExternalDoer(logger log.Logger) Doer {
+	d, _ := ExternalClientFactory(logger).Doer()
+	return d
+}
 
 // UncachedExternalDoer is a shared client for external communication. This is a
 // convenience for existing uses of http.DefaultClient.
@@ -204,7 +209,10 @@ var TestExternalDoer, _ = TestExternalClientFactory.Doer()
 // WARN: This client caches entire responses for etag matching. Do not use it for
 // one-off requests if possible, and definitely not for larger payloads, like
 // downloading arbitrarily sized files! See UncachedExternalClient instead.
-var ExternalClient, _ = ExternalClientFactory.Client()
+func ExternalClient(logger log.Logger) *http.Client {
+	c, _ := ExternalClientFactory(logger).Client()
+	return c
+}
 
 // UncachedExternalClient returns a shared client for external communication. This is
 // a convenience for existing uses of http.DefaultClient.

We already had existing HTTP middleware that logs retries, but many of the default exports (default Doer, client, etc.) didn't include this middleware by default:

https://github.com/sourcegraph/sourcegraph/blob/6136978b18306b5e41c77b5704a71da135db5ea9/internal/httpcli/client.go#L363-L412

This PR does all the grunt work of changing these default exports to be a function that accepts a logger (so that we can include the logging middleware by default). The vast majority of the changes are just passing new loggers as appropriate and fixing the resulting type errors.

Test plan

• Existing unit tests
• Ran env SRC_LOG_LEVEL=debug sg start and saw debug log lines from the middleware. Ex:

[         worker] DEBUG authz.BitbucketServerAuthzProvider.BitbucketServerClient.BitbucketServerClient.httpcli httpcli/client.go:407 request {"TraceId": "81ae8c7072115329d94228b0887e8776", "SpanId": "444efd3fe6a2e0a8", "host": "bitbucket.sgdev.org", "path": "/rest/api/1.0/repos", "duration": "1.679952667s", "code": 200}

Changelog

[ggilmore]

• fix/http: add logging middleware to default exported external http clients #64402 👈
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @ggilmore and the rest of your teammates on Graphite