From 358899c9e29051facfe6d141e0f731e53efd8d59 Mon Sep 17 00:00:00 2001 From: MegaportPhilipBrowne Date: Wed, 2 Oct 2024 09:29:53 -0400 Subject: [PATCH 01/16] feat: add option to log response body in client --- client.go | 49 ++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 40 insertions(+), 9 deletions(-) diff --git a/client.go b/client.go index 4b5f8f7..65154e4 100644 --- a/client.go +++ b/client.go @@ -75,6 +75,8 @@ type Client struct { accessToken string // Access Token for client tokenExpiry time.Time // Token Expiration + LogResponseBody bool // Log Response Body of HTTP Requests + // Optional function called after every successful request made to the API onRequestCompleted RequestCompletionCallback @@ -211,6 +213,14 @@ func WithEnvironment(e Environment) ClientOpt { } } +// WithLogResponseBody is a client option for setting the log response body flag +func WithLogResponseBody() ClientOpt { + return func(c *Client) error { + c.LogResponseBody = true + return nil + } +} + // NewRequest creates an API request. A relative URL can be provided in urlStr, which will be resolved to the // BaseURL of the Client. Relative URLS should always be specified without a preceding slash. If specified, the // value pointed to by body is JSON encoded and included in as the request body. @@ -279,14 +289,35 @@ func (c *Client) Do(ctx context.Context, req *http.Request, v interface{}) (*htt } reqTime := time.Since(reqStart) - c.Logger.DebugContext(ctx, "completed API request", - slog.Duration("duration", reqTime), - slog.Int("status_code", resp.StatusCode), - slog.String("path", req.URL.EscapedPath()), - slog.String("api_host", c.BaseURL.Host), - slog.String("method", req.Method), - slog.String("trace_id", resp.Header.Get(headerTraceId)), - ) + respBody := resp.Body + if c.LogResponseBody { + b, _ := io.ReadAll(resp.Body) + + // Base64 encode the response body + encodedBody := base64.StdEncoding.EncodeToString(b) + + // Create new reader for the later code + respBody = io.NopCloser(bytes.NewReader(b)) + + // Log With Response Body + c.Logger.DebugContext(ctx, "completed API request", + slog.Duration("duration", reqTime), + slog.Int("status_code", resp.StatusCode), + slog.String("path", req.URL.EscapedPath()), + slog.String("api_host", c.BaseURL.Host), + slog.String("method", req.Method), + slog.String("trace_id", resp.Header.Get(headerTraceId)), + slog.String("response_body_base64", encodedBody)) + } else { // Log Without Response Body + c.Logger.DebugContext(ctx, "completed API request", + slog.Duration("duration", reqTime), + slog.Int("status_code", resp.StatusCode), + slog.String("path", req.URL.EscapedPath()), + slog.String("api_host", c.BaseURL.Host), + slog.String("method", req.Method), + slog.String("trace_id", resp.Header.Get(headerTraceId)), + ) + } err = CheckResponse(resp) if err != nil { @@ -295,7 +326,7 @@ func (c *Client) Do(ctx context.Context, req *http.Request, v interface{}) (*htt if resp.StatusCode != http.StatusNoContent && v != nil { if w, ok := v.(io.Writer); ok { - _, err = io.Copy(w, resp.Body) + _, err = io.Copy(w, respBody) if err != nil { return nil, err } From 848d0adee50240ae926b9a96dc868303cceb4711 Mon Sep 17 00:00:00 2001 From: MegaportPhilipBrowne Date: Wed, 2 Oct 2024 09:31:20 -0400 Subject: [PATCH 02/16] fix: comment for response body --- client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client.go b/client.go index 65154e4..92cb135 100644 --- a/client.go +++ b/client.go @@ -307,7 +307,7 @@ func (c *Client) Do(ctx context.Context, req *http.Request, v interface{}) (*htt slog.String("api_host", c.BaseURL.Host), slog.String("method", req.Method), slog.String("trace_id", resp.Header.Get(headerTraceId)), - slog.String("response_body_base64", encodedBody)) + slog.String("response_body_base64", encodedBody)) // Response Body is Base64 Encoded } else { // Log Without Response Body c.Logger.DebugContext(ctx, "completed API request", slog.Duration("duration", reqTime), From 4b6082d76acdae8b5693349cd66b3b7d02e3f788 Mon Sep 17 00:00:00 2001 From: MegaportPhilipBrowne Date: Wed, 2 Oct 2024 10:22:05 -0400 Subject: [PATCH 03/16] fix: change message to snake case --- client.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client.go b/client.go index 92cb135..0a4c8c2 100644 --- a/client.go +++ b/client.go @@ -300,7 +300,7 @@ func (c *Client) Do(ctx context.Context, req *http.Request, v interface{}) (*htt respBody = io.NopCloser(bytes.NewReader(b)) // Log With Response Body - c.Logger.DebugContext(ctx, "completed API request", + c.Logger.DebugContext(ctx, "completed_api_request", slog.Duration("duration", reqTime), slog.Int("status_code", resp.StatusCode), slog.String("path", req.URL.EscapedPath()), @@ -309,7 +309,7 @@ func (c *Client) Do(ctx context.Context, req *http.Request, v interface{}) (*htt slog.String("trace_id", resp.Header.Get(headerTraceId)), slog.String("response_body_base64", encodedBody)) // Response Body is Base64 Encoded } else { // Log Without Response Body - c.Logger.DebugContext(ctx, "completed API request", + c.Logger.DebugContext(ctx, "completed_api_request", slog.Duration("duration", reqTime), slog.Int("status_code", resp.StatusCode), slog.String("path", req.URL.EscapedPath()), From 28ef42d1217cd4327fbe478f667e76c3188a2ddb Mon Sep 17 00:00:00 2001 From: MegaportPhilipBrowne Date: Wed, 2 Oct 2024 10:25:25 -0400 Subject: [PATCH 04/16] fix: slog linting --- client.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client.go b/client.go index 0a4c8c2..de26537 100644 --- a/client.go +++ b/client.go @@ -300,7 +300,7 @@ func (c *Client) Do(ctx context.Context, req *http.Request, v interface{}) (*htt respBody = io.NopCloser(bytes.NewReader(b)) // Log With Response Body - c.Logger.DebugContext(ctx, "completed_api_request", + c.Logger.DebugContext(ctx, "completed API request", //nolint:sloglint slog.Duration("duration", reqTime), slog.Int("status_code", resp.StatusCode), slog.String("path", req.URL.EscapedPath()), @@ -309,7 +309,7 @@ func (c *Client) Do(ctx context.Context, req *http.Request, v interface{}) (*htt slog.String("trace_id", resp.Header.Get(headerTraceId)), slog.String("response_body_base64", encodedBody)) // Response Body is Base64 Encoded } else { // Log Without Response Body - c.Logger.DebugContext(ctx, "completed_api_request", + c.Logger.DebugContext(ctx, "completed API request", //nolint:sloglint slog.Duration("duration", reqTime), slog.Int("status_code", resp.StatusCode), slog.String("path", req.URL.EscapedPath()), From 2378bfc507b9f5b8f79031c0262d32ce35493c88 Mon Sep 17 00:00:00 2001 From: MegaportPhilipBrowne Date: Wed, 2 Oct 2024 10:32:23 -0400 Subject: [PATCH 05/16] fix: add unit test for response logging --- client.go | 5 ++++- client_test.go | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/client.go b/client.go index de26537..0201121 100644 --- a/client.go +++ b/client.go @@ -291,7 +291,10 @@ func (c *Client) Do(ctx context.Context, req *http.Request, v interface{}) (*htt respBody := resp.Body if c.LogResponseBody { - b, _ := io.ReadAll(resp.Body) + b, err := io.ReadAll(resp.Body) + if err != nil { + return nil, err + } // Base64 encode the response body encodedBody := base64.StdEncoding.EncodeToString(b) diff --git a/client_test.go b/client_test.go index 83879e6..13a3355 100644 --- a/client_test.go +++ b/client_test.go @@ -2,7 +2,11 @@ package megaport import ( "context" + "encoding/base64" + "encoding/json" "fmt" + "io" + "log/slog" "net/http" "net/http/httptest" "net/http/httputil" @@ -138,6 +142,58 @@ func (suite *ClientTestSuite) TestNewRequest_withCustomUserAgent() { } } +// TestNewRequest_withResponseLogging tests if the NewRequest function returns a request with response logging. +func (suite *ClientTestSuite) TestNewRequest_withResponseLogging() { + // Mock HTTP client and server response + mockResponse := `{"message": "success"}` + mockServer := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + w.Write([]byte(mockResponse)) + }) + server := httptest.NewServer(mockServer) + defer server.Close() + + // Create a new client with the mock server URL + c := NewClient(nil, nil) + url, _ := url.Parse(server.URL) + c.BaseURL = url + + // Create a new request + req, err := c.NewRequest(ctx, http.MethodGet, "/foo", nil) + if err != nil { + suite.FailNowf("New() unexpected error: %v", err.Error()) + } + + // Perform the request + resp, err := c.Do(ctx, req, nil) + if err != nil { + suite.FailNowf("Do() unexpected error: %v", err.Error()) + } + defer resp.Body.Close() + + // Read and log the response body + body, err := io.ReadAll(resp.Body) + if err != nil { + suite.FailNowf("ReadAll() unexpected error: %v", err.Error()) + } + + // Log the response body + encodedBody := base64.StdEncoding.EncodeToString(body) + c.Logger.DebugContext(ctx, "response body", slog.String("response_body_base64", encodedBody)) + + // Check the response + var result map[string]interface{} + if err := json.Unmarshal(body, &result); err != nil { + suite.FailNowf("Unmarshal() unexpected error: %v", err.Error()) + } + + expectedMessage := "success" + resultMsg := result["message"].(string) + if result["message"] != expectedMessage { + suite.FailNowf("Response message = %s; expected %s", resultMsg, expectedMessage) + } +} + // TestNewRequest_withCustomHeaders tests if the NewRequest function returns a request with custom headers. func (suite *ClientTestSuite) TestNewRequest_withCustomHeaders() { expectedIdentity := "identity" From 619b9794ed27697a3b5a1a6fabc4c455bd59a144 Mon Sep 17 00:00:00 2001 From: MegaportPhilipBrowne Date: Wed, 2 Oct 2024 10:44:12 -0400 Subject: [PATCH 06/16] fix: log as array of attributes --- client.go | 29 +++++++++++------------------ client_test.go | 13 ++++++++++--- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/client.go b/client.go index 0201121..3526070 100644 --- a/client.go +++ b/client.go @@ -290,6 +290,14 @@ func (c *Client) Do(ctx context.Context, req *http.Request, v interface{}) (*htt reqTime := time.Since(reqStart) respBody := resp.Body + + attrs := []slog.Attr{slog.Duration("duration", reqTime), + slog.Int("status_code", resp.StatusCode), + slog.String("path", req.URL.EscapedPath()), + slog.String("api_host", c.BaseURL.Host), + slog.String("method", req.Method), + slog.String("trace_id", resp.Header.Get(headerTraceId))} + if c.LogResponseBody { b, err := io.ReadAll(resp.Body) if err != nil { @@ -302,26 +310,11 @@ func (c *Client) Do(ctx context.Context, req *http.Request, v interface{}) (*htt // Create new reader for the later code respBody = io.NopCloser(bytes.NewReader(b)) - // Log With Response Body - c.Logger.DebugContext(ctx, "completed API request", //nolint:sloglint - slog.Duration("duration", reqTime), - slog.Int("status_code", resp.StatusCode), - slog.String("path", req.URL.EscapedPath()), - slog.String("api_host", c.BaseURL.Host), - slog.String("method", req.Method), - slog.String("trace_id", resp.Header.Get(headerTraceId)), - slog.String("response_body_base64", encodedBody)) // Response Body is Base64 Encoded - } else { // Log Without Response Body - c.Logger.DebugContext(ctx, "completed API request", //nolint:sloglint - slog.Duration("duration", reqTime), - slog.Int("status_code", resp.StatusCode), - slog.String("path", req.URL.EscapedPath()), - slog.String("api_host", c.BaseURL.Host), - slog.String("method", req.Method), - slog.String("trace_id", resp.Header.Get(headerTraceId)), - ) + attrs = append(attrs, slog.String("response_body_base_64", encodedBody)) } + c.Logger.DebugContext(ctx, "completed api request", slog.Any("api_request", attrs)) + err = CheckResponse(resp) if err != nil { return nil, err diff --git a/client_test.go b/client_test.go index 13a3355..78e042e 100644 --- a/client_test.go +++ b/client_test.go @@ -148,7 +148,10 @@ func (suite *ClientTestSuite) TestNewRequest_withResponseLogging() { mockResponse := `{"message": "success"}` mockServer := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) - w.Write([]byte(mockResponse)) + _, err := w.Write([]byte(mockResponse)) + if err != nil { + suite.FailNowf("Write() unexpected error: %v", err.Error()) + } }) server := httptest.NewServer(mockServer) defer server.Close() @@ -179,7 +182,7 @@ func (suite *ClientTestSuite) TestNewRequest_withResponseLogging() { // Log the response body encodedBody := base64.StdEncoding.EncodeToString(body) - c.Logger.DebugContext(ctx, "response body", slog.String("response_body_base64", encodedBody)) + c.Logger.DebugContext(ctx, "response body", slog.String("response_body_base_64", encodedBody)) // Check the response var result map[string]interface{} @@ -188,7 +191,11 @@ func (suite *ClientTestSuite) TestNewRequest_withResponseLogging() { } expectedMessage := "success" - resultMsg := result["message"].(string) + resultMsg, ok := result["message"].(string) + if !ok { + suite.FailNow("Response message is not a string") + } + if result["message"] != expectedMessage { suite.FailNowf("Response message = %s; expected %s", resultMsg, expectedMessage) } From 8782deb7d06775be1a83a53780a73b28ac8a0aef Mon Sep 17 00:00:00 2001 From: MegaportPhilipBrowne Date: Wed, 2 Oct 2024 10:47:51 -0400 Subject: [PATCH 07/16] fix: change attrs to any array --- client.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client.go b/client.go index 3526070..57b2021 100644 --- a/client.go +++ b/client.go @@ -291,7 +291,7 @@ func (c *Client) Do(ctx context.Context, req *http.Request, v interface{}) (*htt respBody := resp.Body - attrs := []slog.Attr{slog.Duration("duration", reqTime), + attrs := []any{slog.Duration("duration", reqTime), slog.Int("status_code", resp.StatusCode), slog.String("path", req.URL.EscapedPath()), slog.String("api_host", c.BaseURL.Host), @@ -313,7 +313,7 @@ func (c *Client) Do(ctx context.Context, req *http.Request, v interface{}) (*htt attrs = append(attrs, slog.String("response_body_base_64", encodedBody)) } - c.Logger.DebugContext(ctx, "completed api request", slog.Any("api_request", attrs)) + c.Logger.DebugContext(ctx, "completed api request", attrs...) err = CheckResponse(resp) if err != nil { From 085e2a76e94a74b45d21a3de06f3a2220eeab048 Mon Sep 17 00:00:00 2001 From: MegaportPhilipBrowne Date: Wed, 2 Oct 2024 10:49:15 -0400 Subject: [PATCH 08/16] fix: snake case for response_body --- client_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client_test.go b/client_test.go index 78e042e..5d508a5 100644 --- a/client_test.go +++ b/client_test.go @@ -182,7 +182,7 @@ func (suite *ClientTestSuite) TestNewRequest_withResponseLogging() { // Log the response body encodedBody := base64.StdEncoding.EncodeToString(body) - c.Logger.DebugContext(ctx, "response body", slog.String("response_body_base_64", encodedBody)) + c.Logger.DebugContext(ctx, "response_body", slog.String("response_body_base_64", encodedBody)) // Check the response var result map[string]interface{} From 8dae13bf5cbeb7108b2f5beafec29d8f74a1d19e Mon Sep 17 00:00:00 2001 From: MegaportPhilipBrowne Date: Wed, 2 Oct 2024 10:51:22 -0400 Subject: [PATCH 09/16] fix: change to slog attrs --- client.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client.go b/client.go index 57b2021..3eaefb6 100644 --- a/client.go +++ b/client.go @@ -291,7 +291,7 @@ func (c *Client) Do(ctx context.Context, req *http.Request, v interface{}) (*htt respBody := resp.Body - attrs := []any{slog.Duration("duration", reqTime), + attrs := []slog.Attr{slog.Duration("duration", reqTime), slog.Int("status_code", resp.StatusCode), slog.String("path", req.URL.EscapedPath()), slog.String("api_host", c.BaseURL.Host), @@ -313,7 +313,7 @@ func (c *Client) Do(ctx context.Context, req *http.Request, v interface{}) (*htt attrs = append(attrs, slog.String("response_body_base_64", encodedBody)) } - c.Logger.DebugContext(ctx, "completed api request", attrs...) + c.Logger.DebugContext(ctx, "completed apirequest", slog.Any("api_request", attrs)) err = CheckResponse(resp) if err != nil { From 3d2d3021ac88a43494b98d499ada4670aac662f8 Mon Sep 17 00:00:00 2001 From: MegaportPhilipBrowne Date: Wed, 2 Oct 2024 10:54:01 -0400 Subject: [PATCH 10/16] fix: logging for test --- client_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client_test.go b/client_test.go index 5d508a5..11e6a86 100644 --- a/client_test.go +++ b/client_test.go @@ -182,7 +182,7 @@ func (suite *ClientTestSuite) TestNewRequest_withResponseLogging() { // Log the response body encodedBody := base64.StdEncoding.EncodeToString(body) - c.Logger.DebugContext(ctx, "response_body", slog.String("response_body_base_64", encodedBody)) + c.Logger.DebugContext(ctx, "response_body", slog.String("response_body", encodedBody)) // Check the response var result map[string]interface{} From 296ed7af3ea5a6096483a726399027487030cd86 Mon Sep 17 00:00:00 2001 From: MegaportPhilipBrowne Date: Fri, 4 Oct 2024 10:06:44 -0400 Subject: [PATCH 11/16] feat: add log capture (from Stdout), add client option for WithLogLevel(), check log outout from stdout, make sure it matches base64 encoded string --- client.go | 65 +++++++++++++++++++++++++++++++++++++++++++++++ client_test.go | 68 ++++++++++++++------------------------------------ 2 files changed, 84 insertions(+), 49 deletions(-) diff --git a/client.go b/client.go index 3eaefb6..9493c49 100644 --- a/client.go +++ b/client.go @@ -76,6 +76,7 @@ type Client struct { tokenExpiry time.Time // Token Expiration LogResponseBody bool // Log Response Body of HTTP Requests + LogCapture *LogCapture // Optional function called after every successful request made to the API onRequestCompleted RequestCompletionCallback @@ -95,6 +96,55 @@ type AccessTokenResponse struct { Error string `json:"error"` } +// LogCapture is a simple struct to capture logs +type LogCapture struct { + Buffer bytes.Buffer +} + +// Write writes to the buffer +func (lc *LogCapture) Write(p []byte) (n int, err error) { + return lc.Buffer.Write(p) +} + +// String returns the buffer as a string +func (lc *LogCapture) String() string { + return lc.Buffer.String() +} + +type LevelFilterHandler struct { + level slog.Level + handler slog.Handler +} + +func NewLevelFilterHandler(level slog.Level, handler slog.Handler) *LevelFilterHandler { + return &LevelFilterHandler{level: level, handler: handler} +} + +func (h *LevelFilterHandler) Handle(ctx context.Context, r slog.Record) error { + if r.Level >= h.level { + return h.handler.Handle(ctx, r) + } + return nil +} + +func (h *LevelFilterHandler) WithAttrs(attrs []slog.Attr) slog.Handler { + return &LevelFilterHandler{ + level: h.level, + handler: h.handler.WithAttrs(attrs), + } +} + +func (h *LevelFilterHandler) WithGroup(name string) slog.Handler { + return &LevelFilterHandler{ + level: h.level, + handler: h.handler.WithGroup(name), + } +} + +func (h *LevelFilterHandler) Enabled(context context.Context, level slog.Level) bool { + return level >= h.level +} + // RequestCompletionCallback defines the type of the request callback function type RequestCompletionCallback func(*http.Request, *http.Response) @@ -119,6 +169,7 @@ func NewClient(httpClient *http.Client, base *url.URL) *Client { BaseURL: baseURL, UserAgent: userAgent, Logger: logger, + LogCapture: &LogCapture{}, } c.ProductService = NewProductService(c) @@ -213,9 +264,23 @@ func WithEnvironment(e Environment) ClientOpt { } } +func WithLogLevel(level slog.Level) ClientOpt { + return func(c *Client) error { + handler := NewLevelFilterHandler(level, slog.NewJSONHandler(io.MultiWriter(os.Stdout, c.LogCapture), nil)) + c.Logger = slog.New(handler) + return nil + } +} + // WithLogResponseBody is a client option for setting the log response body flag func WithLogResponseBody() ClientOpt { return func(c *Client) error { + // for debugging - capture logs + logCapture := &LogCapture{} + multiWriter := io.MultiWriter(os.Stdout, logCapture) + logger := slog.New(slog.NewJSONHandler(multiWriter, nil)) + c.Logger = logger + c.LogCapture = logCapture c.LogResponseBody = true return nil } diff --git a/client_test.go b/client_test.go index 11e6a86..04a04db 100644 --- a/client_test.go +++ b/client_test.go @@ -2,10 +2,7 @@ package megaport import ( "context" - "encoding/base64" - "encoding/json" "fmt" - "io" "log/slog" "net/http" "net/http/httptest" @@ -144,60 +141,33 @@ func (suite *ClientTestSuite) TestNewRequest_withCustomUserAgent() { // TestNewRequest_withResponseLogging tests if the NewRequest function returns a request with response logging. func (suite *ClientTestSuite) TestNewRequest_withResponseLogging() { - // Mock HTTP client and server response - mockResponse := `{"message": "success"}` - mockServer := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(http.StatusOK) - _, err := w.Write([]byte(mockResponse)) - if err != nil { - suite.FailNowf("Write() unexpected error: %v", err.Error()) - } - }) - server := httptest.NewServer(mockServer) - defer server.Close() - - // Create a new client with the mock server URL - c := NewClient(nil, nil) - url, _ := url.Parse(server.URL) - c.BaseURL = url - - // Create a new request - req, err := c.NewRequest(ctx, http.MethodGet, "/foo", nil) + c, err := New(nil, WithLogResponseBody(), WithLogLevel(slog.LevelDebug)) if err != nil { - suite.FailNowf("New() unexpected error: %v", err.Error()) + suite.FailNowf("unexpected error", "New() unexpected error: %v", err.Error()) } + suite.client = c + url, _ := url.Parse(suite.server.URL) + suite.client.BaseURL = url - // Perform the request - resp, err := c.Do(ctx, req, nil) - if err != nil { - suite.FailNowf("Do() unexpected error: %v", err.Error()) - } - defer resp.Body.Close() + suite.mux.HandleFunc("/a", func(w http.ResponseWriter, r *http.Request) { + if m := http.MethodGet; m != r.Method { + suite.FailNowf("Request method = %v, expected %v", r.Method, m) + } + fmt.Fprint(w, `{"A":"a"}`) + }) - // Read and log the response body - body, err := io.ReadAll(resp.Body) + req, _ := suite.client.NewRequest(ctx, http.MethodGet, "/a", nil) + _, err = suite.client.Do(ctx, req, nil) if err != nil { - suite.FailNowf("ReadAll() unexpected error: %v", err.Error()) - } - - // Log the response body - encodedBody := base64.StdEncoding.EncodeToString(body) - c.Logger.DebugContext(ctx, "response_body", slog.String("response_body", encodedBody)) - - // Check the response - var result map[string]interface{} - if err := json.Unmarshal(body, &result); err != nil { - suite.FailNowf("Unmarshal() unexpected error: %v", err.Error()) + suite.FailNowf("", "Do(): %v", err.Error()) } - expectedMessage := "success" - resultMsg, ok := result["message"].(string) - if !ok { - suite.FailNow("Response message is not a string") - } + // Check the log output for the expected base64 encoded response body + expectedBase64 := "eyJBIjoiYSJ9" // base64 encoded {"A":"a"} - if result["message"] != expectedMessage { - suite.FailNowf("Response message = %s; expected %s", resultMsg, expectedMessage) + logOutput := suite.client.LogCapture.String() + if !strings.Contains(suite.client.LogCapture.String(), expectedBase64) { + suite.FailNowf("Unexpected log output", "Expected log output to contain %s, but got %s", expectedBase64, logOutput) } } From 80d8e244ca6ea82e28e1138fcf9afafc777663af Mon Sep 17 00:00:00 2001 From: MegaportPhilipBrowne Date: Fri, 4 Oct 2024 10:09:51 -0400 Subject: [PATCH 12/16] fix: better error messages --- client_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client_test.go b/client_test.go index 04a04db..174ffc8 100644 --- a/client_test.go +++ b/client_test.go @@ -151,7 +151,7 @@ func (suite *ClientTestSuite) TestNewRequest_withResponseLogging() { suite.mux.HandleFunc("/a", func(w http.ResponseWriter, r *http.Request) { if m := http.MethodGet; m != r.Method { - suite.FailNowf("Request method = %v, expected %v", r.Method, m) + suite.FailNowf("Incorrect request method", "Request method = %v, expected %v", r.Method, m) } fmt.Fprint(w, `{"A":"a"}`) }) @@ -159,7 +159,7 @@ func (suite *ClientTestSuite) TestNewRequest_withResponseLogging() { req, _ := suite.client.NewRequest(ctx, http.MethodGet, "/a", nil) _, err = suite.client.Do(ctx, req, nil) if err != nil { - suite.FailNowf("", "Do(): %v", err.Error()) + suite.FailNowf("Unexpected error: Do()", "Unexpected error: Do(): %v", err.Error()) } // Check the log output for the expected base64 encoded response body From 589fb1cddc47551478edca5f2ea0e55f1af6d75b Mon Sep 17 00:00:00 2001 From: MegaportPhilipBrowne Date: Fri, 4 Oct 2024 10:12:40 -0400 Subject: [PATCH 13/16] fix: change to ctx --- client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client.go b/client.go index 9493c49..b5c003e 100644 --- a/client.go +++ b/client.go @@ -141,7 +141,7 @@ func (h *LevelFilterHandler) WithGroup(name string) slog.Handler { } } -func (h *LevelFilterHandler) Enabled(context context.Context, level slog.Level) bool { +func (h *LevelFilterHandler) Enabled(ctx context.Context, level slog.Level) bool { return level >= h.level } From 5d7dac38048f8b743ab70217ceab248b66482b68 Mon Sep 17 00:00:00 2001 From: MegaportPhilipBrowne Date: Tue, 8 Oct 2024 12:42:15 -0400 Subject: [PATCH 14/16] fix: remove unnecessary code for log capture, WithLevelFilter, just use custom handler and bytes buffer --- client.go | 32 +------------------------------- client_test.go | 18 +++++++++++++----- 2 files changed, 14 insertions(+), 36 deletions(-) diff --git a/client.go b/client.go index b5c003e..101734a 100644 --- a/client.go +++ b/client.go @@ -76,7 +76,6 @@ type Client struct { tokenExpiry time.Time // Token Expiration LogResponseBody bool // Log Response Body of HTTP Requests - LogCapture *LogCapture // Optional function called after every successful request made to the API onRequestCompleted RequestCompletionCallback @@ -96,21 +95,7 @@ type AccessTokenResponse struct { Error string `json:"error"` } -// LogCapture is a simple struct to capture logs -type LogCapture struct { - Buffer bytes.Buffer -} - -// Write writes to the buffer -func (lc *LogCapture) Write(p []byte) (n int, err error) { - return lc.Buffer.Write(p) -} - -// String returns the buffer as a string -func (lc *LogCapture) String() string { - return lc.Buffer.String() -} - +// Custom Handler with Log Filtering type LevelFilterHandler struct { level slog.Level handler slog.Handler @@ -169,7 +154,6 @@ func NewClient(httpClient *http.Client, base *url.URL) *Client { BaseURL: baseURL, UserAgent: userAgent, Logger: logger, - LogCapture: &LogCapture{}, } c.ProductService = NewProductService(c) @@ -264,23 +248,9 @@ func WithEnvironment(e Environment) ClientOpt { } } -func WithLogLevel(level slog.Level) ClientOpt { - return func(c *Client) error { - handler := NewLevelFilterHandler(level, slog.NewJSONHandler(io.MultiWriter(os.Stdout, c.LogCapture), nil)) - c.Logger = slog.New(handler) - return nil - } -} - // WithLogResponseBody is a client option for setting the log response body flag func WithLogResponseBody() ClientOpt { return func(c *Client) error { - // for debugging - capture logs - logCapture := &LogCapture{} - multiWriter := io.MultiWriter(os.Stdout, logCapture) - logger := slog.New(slog.NewJSONHandler(multiWriter, nil)) - c.Logger = logger - c.LogCapture = logCapture c.LogResponseBody = true return nil } diff --git a/client_test.go b/client_test.go index 174ffc8..be7fbad 100644 --- a/client_test.go +++ b/client_test.go @@ -1,13 +1,16 @@ package megaport import ( + "bytes" "context" "fmt" + "io" "log/slog" "net/http" "net/http/httptest" "net/http/httputil" "net/url" + "os" "reflect" "strings" "testing" @@ -141,7 +144,13 @@ func (suite *ClientTestSuite) TestNewRequest_withCustomUserAgent() { // TestNewRequest_withResponseLogging tests if the NewRequest function returns a request with response logging. func (suite *ClientTestSuite) TestNewRequest_withResponseLogging() { - c, err := New(nil, WithLogResponseBody(), WithLogLevel(slog.LevelDebug)) + // for debugging - capture logs + logCapture := &bytes.Buffer{} + multiWriter := io.MultiWriter(os.Stdout, logCapture) + handler := slog.NewJSONHandler(multiWriter, nil) + levelFilterHandler := NewLevelFilterHandler(slog.LevelDebug, handler) + + c, err := New(nil, WithLogResponseBody(), WithLogHandler(levelFilterHandler)) if err != nil { suite.FailNowf("unexpected error", "New() unexpected error: %v", err.Error()) } @@ -164,10 +173,9 @@ func (suite *ClientTestSuite) TestNewRequest_withResponseLogging() { // Check the log output for the expected base64 encoded response body expectedBase64 := "eyJBIjoiYSJ9" // base64 encoded {"A":"a"} - - logOutput := suite.client.LogCapture.String() - if !strings.Contains(suite.client.LogCapture.String(), expectedBase64) { - suite.FailNowf("Unexpected log output", "Expected log output to contain %s, but got %s", expectedBase64, logOutput) + logOutput := logCapture.String() + if !strings.Contains(logOutput, expectedBase64) { + suite.FailNowf("Log output does not contain expected base64", "Log output: %s", logOutput) } } From bb1636e0b49a18615a95f2b432ab5d9f73485d3b Mon Sep 17 00:00:00 2001 From: MegaportPhilipBrowne Date: Tue, 8 Oct 2024 12:45:05 -0400 Subject: [PATCH 15/16] fix: cleanup test to have single writer for output --- client_test.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/client_test.go b/client_test.go index be7fbad..9d3672a 100644 --- a/client_test.go +++ b/client_test.go @@ -10,7 +10,6 @@ import ( "net/http/httptest" "net/http/httputil" "net/url" - "os" "reflect" "strings" "testing" @@ -146,9 +145,7 @@ func (suite *ClientTestSuite) TestNewRequest_withCustomUserAgent() { func (suite *ClientTestSuite) TestNewRequest_withResponseLogging() { // for debugging - capture logs logCapture := &bytes.Buffer{} - multiWriter := io.MultiWriter(os.Stdout, logCapture) - handler := slog.NewJSONHandler(multiWriter, nil) - levelFilterHandler := NewLevelFilterHandler(slog.LevelDebug, handler) + levelFilterHandler := NewLevelFilterHandler(slog.LevelDebug, slog.NewJSONHandler(io.Writer(logCapture), nil)) c, err := New(nil, WithLogResponseBody(), WithLogHandler(levelFilterHandler)) if err != nil { From 30135cfa967b8c3c2328045b97dad898e16188ca Mon Sep 17 00:00:00 2001 From: MegaportPhilipBrowne Date: Tue, 8 Oct 2024 13:00:38 -0400 Subject: [PATCH 16/16] fix: spacing for api req --- client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client.go b/client.go index 101734a..ae4be4b 100644 --- a/client.go +++ b/client.go @@ -348,7 +348,7 @@ func (c *Client) Do(ctx context.Context, req *http.Request, v interface{}) (*htt attrs = append(attrs, slog.String("response_body_base_64", encodedBody)) } - c.Logger.DebugContext(ctx, "completed apirequest", slog.Any("api_request", attrs)) + c.Logger.DebugContext(ctx, "completed api request", slog.Any("api_request", attrs)) err = CheckResponse(resp) if err != nil {