From a696f0ded6e559384dc72fe8211928183ce6db04 Mon Sep 17 00:00:00 2001 From: Brian Picciano Date: Tue, 19 Nov 2024 14:12:17 +0100 Subject: [PATCH] Add debug logging to all HTTP requests --- go/cmd/entrypoint/client.go | 7 +- go/cmd/entrypoint/sub_cmd.go | 13 +++- go/daemon/jsonrpc2/client_http.go | 50 +++++---------- go/daemon/jsonrpc2/jsonrpc2_test.go | 13 +++- go/garage/admin_client.go | 46 +++----------- go/toolkit/http.go | 99 +++++++++++++++++++++++++++++ 6 files changed, 147 insertions(+), 81 deletions(-) create mode 100644 go/toolkit/http.go diff --git a/go/cmd/entrypoint/client.go b/go/cmd/entrypoint/client.go index a8406dd..8d65b79 100644 --- a/go/cmd/entrypoint/client.go +++ b/go/cmd/entrypoint/client.go @@ -1,14 +1,9 @@ package main import ( - "fmt" "isle/bootstrap" ) func (ctx subCmdCtx) getHosts() ([]bootstrap.Host, error) { - res, err := ctx.getDaemonRPC().GetHosts(ctx) - if err != nil { - return nil, fmt.Errorf("calling GetHosts: %w", err) - } - return res, nil + return ctx.getDaemonRPC().GetHosts(ctx) } diff --git a/go/cmd/entrypoint/sub_cmd.go b/go/cmd/entrypoint/sub_cmd.go index 011b649..c5f8070 100644 --- a/go/cmd/entrypoint/sub_cmd.go +++ b/go/cmd/entrypoint/sub_cmd.go @@ -8,6 +8,7 @@ import ( "isle/daemon" "isle/daemon/jsonrpc2" "isle/jsonutil" + "isle/toolkit" "os" "strings" @@ -95,10 +96,16 @@ func usagePrefix(subCmdNames []string) string { func (ctx subCmdCtx) getDaemonRPC() daemon.RPC { if ctx.opts.daemonRPC == nil { + // TODO Close is not being called on the HTTPClient + httpClient, baseURL := toolkit.NewUnixHTTPClient( + ctx.logger.WithNamespace("http-client"), + daemon.HTTPSocketPath(), + ) + + baseURL.Path = daemonHTTPRPCPath + ctx.opts.daemonRPC = daemon.RPCFromClient( - jsonrpc2.NewUnixHTTPClient( - daemon.HTTPSocketPath(), daemonHTTPRPCPath, - ), + jsonrpc2.NewHTTPClient(httpClient, baseURL.String()), ) } return ctx.opts.daemonRPC diff --git a/go/daemon/jsonrpc2/client_http.go b/go/daemon/jsonrpc2/client_http.go index 5e5529d..b76bbff 100644 --- a/go/daemon/jsonrpc2/client_http.go +++ b/go/daemon/jsonrpc2/client_http.go @@ -6,50 +6,28 @@ import ( "encoding/json" "errors" "fmt" + "isle/toolkit" "net/http" - "net/url" - - "github.com/tv42/httpunix" ) -type httpClient struct { - c *http.Client +// HTTPClient makes JSONRPC2 requests over an HTTP endpoint. +// +// Close should be called once the HTTPClient is not longer needed. +type HTTPClient struct { + c toolkit.HTTPClient url string } -// NewHTTPClient returns a Client which will use HTTP POST requests against the -// given URL as a transport for JSONRPC2 method calls. -func NewHTTPClient(urlStr string) Client { - return &httpClient{ - c: &http.Client{ - Transport: http.DefaultTransport.(*http.Transport).Clone(), - }, +// NewHTTPClient returns an HTTPClient which will use HTTP POST requests against +// the given URL as a transport for JSONRPC2 method calls. +func NewHTTPClient(httpClient toolkit.HTTPClient, urlStr string) *HTTPClient { + return &HTTPClient{ + c: httpClient, url: urlStr, } } -// NewUnixHTTPClient returns a Client which will use HTTP POST requests against -// the given unix socket as a transport for JSONRPC2 method calls. The given -// path will be used as the path portion of the HTTP request. -func NewUnixHTTPClient(unixSocketPath, reqPath string) Client { - const host = "uds" - - u := &url.URL{ - Scheme: httpunix.Scheme, - Host: host, - Path: reqPath, - } - - transport := new(httpunix.Transport) - transport.RegisterLocation(host, unixSocketPath) - - return &httpClient{ - c: &http.Client{Transport: transport}, - url: u.String(), - } -} - -func (c *httpClient) Call( +func (c *HTTPClient) Call( ctx context.Context, rcv any, method string, args ...any, ) error { var ( @@ -88,3 +66,7 @@ func (c *httpClient) Call( return nil } + +func (c *HTTPClient) Close() error { + return c.c.Close() +} diff --git a/go/daemon/jsonrpc2/jsonrpc2_test.go b/go/daemon/jsonrpc2/jsonrpc2_test.go index 53ee67b..fa0e8ae 100644 --- a/go/daemon/jsonrpc2/jsonrpc2_test.go +++ b/go/daemon/jsonrpc2/jsonrpc2_test.go @@ -12,6 +12,8 @@ import ( "sync" "testing" "time" + + "github.com/stretchr/testify/assert" ) type DivideParams struct { @@ -216,13 +218,17 @@ func TestReadWriter(t *testing.T) { } func TestHTTP(t *testing.T) { + logger := toolkit.NewTestLogger(t) server := httptest.NewServer(NewHTTPHandler(testHandler(t))) t.Cleanup(server.Close) - testClient(t, NewHTTPClient(server.URL)) + httpClient := toolkit.NewHTTPClient(logger) + t.Cleanup(func() { assert.NoError(t, httpClient.Close()) }) + testClient(t, NewHTTPClient(httpClient, server.URL)) } func TestUnixHTTP(t *testing.T) { var ( + logger = toolkit.NewTestLogger(t) unixSocketPath = filepath.Join(t.TempDir(), "test.sock") server = httptest.NewUnstartedServer(NewHTTPHandler(testHandler(t))) ) @@ -235,5 +241,8 @@ func TestUnixHTTP(t *testing.T) { server.Start() t.Cleanup(server.Close) - testClient(t, NewUnixHTTPClient(unixSocketPath, "/")) + httpClient, baseURL := toolkit.NewUnixHTTPClient(logger, unixSocketPath) + t.Cleanup(func() { assert.NoError(t, httpClient.Close()) }) + + testClient(t, NewHTTPClient(httpClient, baseURL.String())) } diff --git a/go/garage/admin_client.go b/go/garage/admin_client.go index 322483d..84cb77c 100644 --- a/go/garage/admin_client.go +++ b/go/garage/admin_client.go @@ -6,8 +6,8 @@ import ( "encoding/json" "fmt" "io" + "isle/toolkit" "net/http" - "net/http/httputil" "net/netip" "time" @@ -51,35 +51,31 @@ func (e AdminClientError) Error() string { // interface. type AdminClient struct { logger *mlog.Logger - c *http.Client + c toolkit.HTTPClient addr string adminToken string - - transport *http.Transport } // NewAdminClient initializes and returns an AdminClient which will use the // given address and adminToken for all requests made. // // If Logger is nil then logs will be suppressed. -func NewAdminClient(logger *mlog.Logger, addr, adminToken string) *AdminClient { - transport := http.DefaultTransport.(*http.Transport).Clone() - +func NewAdminClient( + logger *mlog.Logger, + addr, adminToken string, +) *AdminClient { + httpClient := toolkit.NewHTTPClient(logger.WithNamespace("http")) return &AdminClient{ - logger: logger, - c: &http.Client{ - Transport: transport, - }, + logger: logger, + c: httpClient, addr: addr, adminToken: adminToken, - transport: transport, } } // Close cleans up all resources held by the lient. func (c *AdminClient) Close() error { - c.transport.CloseIdleConnections() - return nil + return c.c.Close() } // do performs an HTTP request with the given method (GET, POST) and path, and @@ -88,7 +84,6 @@ func (c *AdminClient) Close() error { func (c *AdminClient) do( ctx context.Context, rcv any, method, path string, body any, ) error { - var bodyR io.Reader if body != nil { @@ -109,31 +104,10 @@ func (c *AdminClient) do( req.Header.Set("Authorization", "Bearer "+c.adminToken) - if c.logger.MaxLevel() >= mlog.LevelDebug.Int() { - - reqB, err := httputil.DumpRequestOut(req, true) - if err != nil { - c.logger.Error(ctx, "failed to dump http request", err) - } else { - c.logger.Debug(ctx, "------ request ------\n"+string(reqB)+"\n") - } - } - res, err := c.c.Do(req) if err != nil { return fmt.Errorf("performing http request: %w", err) } - - if c.logger.MaxLevel() >= mlog.LevelDebug.Int() { - - resB, err := httputil.DumpResponse(res, true) - if err != nil { - c.logger.Error(ctx, "failed to dump http response", err) - } else { - c.logger.Debug(ctx, "------ response ------\n"+string(resB)+"\n") - } - } - defer res.Body.Close() if res.StatusCode >= 300 { diff --git a/go/toolkit/http.go b/go/toolkit/http.go new file mode 100644 index 0000000..e5fca05 --- /dev/null +++ b/go/toolkit/http.go @@ -0,0 +1,99 @@ +package toolkit + +import ( + "net/http" + "net/http/httputil" + "net/url" + + "dev.mediocregopher.com/mediocre-go-lib.git/mlog" + "github.com/tv42/httpunix" +) + +// HTTPClient is an interface around the default http.Client type. +type HTTPClient interface { + Do(*http.Request) (*http.Response, error) + + // Close cleans up any resources being held by the HTTPClient. + Close() error +} + +type httpClient struct { + logger *mlog.Logger + *http.Client +} + +// NewHTTPClient returns a new HTTPClient using a clone of +// [http.DefaultTransport]. +func NewHTTPClient(logger *mlog.Logger) HTTPClient { + return &httpClient{ + logger, + &http.Client{ + Transport: http.DefaultTransport.(*http.Transport).Clone(), + }, + } +} + +// NewUnixHTTPClient returns an HTTPClient which will use the given +// unixSocketPath to serve requests. The base URL (scheme and host) which should +// be used for requests is returned as well. +func NewUnixHTTPClient( + logger *mlog.Logger, unixSocketPath string, +) ( + HTTPClient, *url.URL, +) { + const host = "uds" + + u := &url.URL{ + Scheme: httpunix.Scheme, + Host: host, + } + + transport := new(httpunix.Transport) + transport.RegisterLocation(host, unixSocketPath) + + return &httpClient{logger, &http.Client{Transport: transport}}, u +} + +func (c *httpClient) Do(req *http.Request) (*http.Response, error) { + if c.logger.MaxLevel() < mlog.LevelDebug.Int() { + return c.Client.Do(req) + } + + var ( + ctx = req.Context() + origScheme = req.URL.Scheme + ) + + // httputil.DumpRequestOut doesn't like the httpunix.Scheme, so we + // temporarily switch it. + if req.URL.Scheme == httpunix.Scheme { + req.URL.Scheme = "http" + } + + reqB, err := httputil.DumpRequestOut(req, true) + if err != nil { + c.logger.Error(ctx, "failed to dump http request", err) + } else { + c.logger.Debug(ctx, "------ request ------\n"+string(reqB)+"\n") + } + + req.URL.Scheme = origScheme + + res, err := c.Client.Do(req) + + if res != nil { + resB, err := httputil.DumpResponse(res, true) + if err != nil { + c.logger.Error(ctx, "failed to dump http response", err) + } else { + c.logger.Debug(ctx, "------ response ------\n"+string(resB)+"\n") + } + } + + return res, err +} + +func (c *httpClient) Close() error { + c.CloseIdleConnections() + return nil +}