Skip to content

Commit 4e86b02

Browse files
committed
Refactor logging implementation and add GetLogLevel method
1 parent 003b713 commit 4e86b02

File tree

4 files changed

+149
-112
lines changed

4 files changed

+149
-112
lines changed
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
package httpclient
2+
3+
import (
4+
"net/http"
5+
)
6+
7+
// LogHTTPHeaders logs the HTTP headers of an HTTP request or response, with an option to hide sensitive information like the token in secure mode.
8+
func LogHTTPHeaders(logger Logger, headers http.Header, secureMode bool) {
9+
var keysAndValues []interface{}
10+
if secureMode {
11+
for key, values := range headers {
12+
if key != "Authorization" { // Exclude the token header
13+
// Assuming each header has a single value for simplicity
14+
keysAndValues = append(keysAndValues, key, values[0])
15+
}
16+
}
17+
} else {
18+
for key, values := range headers {
19+
// Assuming each header has a single value for simplicity
20+
keysAndValues = append(keysAndValues, key, values[0])
21+
}
22+
}
23+
24+
// Log the headers using the logger from the httpclient package
25+
if len(keysAndValues) > 0 {
26+
logger.Debug("HTTP Headers", keysAndValues...)
27+
}
28+
}

internal/httpclient/http_logger.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ type Logger interface {
2525
Error(msg string, keysAndValues ...interface{})
2626
Panic(msg string, keysAndValues ...interface{})
2727
Fatal(msg string, keysAndValues ...interface{})
28+
GetLogLevel() LogLevel
2829
}
2930

3031
// defaultLogger is an implementation of the Logger interface using Uber's zap logging library.
@@ -108,3 +109,8 @@ func (d *defaultLogger) Fatal(msg string, keysAndValues ...interface{}) {
108109
d.logger.Fatal(msg, toZapFields(keysAndValues...)...)
109110
}
110111
}
112+
113+
// GetLevel returns the current logging level
114+
func (d *defaultLogger) GetLogLevel() LogLevel {
115+
return d.logLevel
116+
}

internal/httpclient/http_logging.go.refactor

Lines changed: 0 additions & 83 deletions
This file was deleted.

internal/httpclient/http_request.go

Lines changed: 115 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -129,13 +129,16 @@ func (c *Client) DoRequest(method, endpoint string, body, out interface{}) (*htt
129129
// Start response time measurement
130130
responseTimeStart := time.Now()
131131

132-
// Execute Request with context
132+
// Execute the request
133133
resp, err := c.httpClient.Do(req)
134134
if err != nil {
135-
c.logger.Error("Failed to send request", "method", method, "endpoint", endpoint, "error", err)
135+
c.logger.Error("Failed to send multipart request",
136+
"method", method,
137+
"endpoint", endpoint,
138+
"error", err.Error(),
139+
)
136140
return nil, err
137141
}
138-
139142
// After each request, compute and update response time
140143
responseDuration := time.Since(responseTimeStart)
141144
c.PerfMetrics.lock.Lock()
@@ -147,45 +150,86 @@ func (c *Client) DoRequest(method, endpoint string, body, out interface{}) (*htt
147150
CheckDeprecationHeader(resp, c.logger)
148151
}
149152

150-
// Handle (unmarshall) response with API Handler
153+
// Handle (unmarshal) response with API Handler
151154
if err := handler.UnmarshalResponse(resp, out); err != nil {
152155
switch e := err.(type) {
153-
case *APIError:
154-
c.logger.Error("Received an API error", "status_code", e.StatusCode, "message", e.Message)
156+
case *APIError: // Assuming APIError is a type that includes StatusCode and Message
157+
// Log the API error with structured logging
158+
c.logger.Error("Received an API error",
159+
"method", method,
160+
"endpoint", endpoint,
161+
"status_code", e.StatusCode,
162+
"message", e.Message,
163+
)
155164
return resp, e
156165
default:
157-
// Existing error handling logic
158-
c.logger.Error("Failed to unmarshal HTTP response", "method", method, "endpoint", endpoint, "error", err)
166+
// Log the default error with structured logging
167+
c.logger.Error("Failed to unmarshal HTTP response",
168+
"method", method,
169+
"endpoint", endpoint,
170+
"error", err.Error(), // Convert error to string to log as a value
171+
)
159172
return resp, err
160173
}
161174
}
162175

163176
if resp.StatusCode >= 200 && resp.StatusCode < 300 {
164-
c.logger.Info("HTTP request succeeded", "method", method, "endpoint", endpoint, "status_code", resp.StatusCode)
177+
// Log successful HTTP request
178+
c.logger.Info("HTTP request succeeded",
179+
"method", method,
180+
"endpoint", endpoint,
181+
"status_code", resp.StatusCode,
182+
)
165183
return resp, nil
166184
} else if resp.StatusCode == http.StatusNotFound {
167-
c.logger.Warn("Resource not found", "method", method, "endpoint", endpoint)
185+
// Log when resource is not found
186+
c.logger.Warn("Resource not found",
187+
"method", method,
188+
"endpoint", endpoint,
189+
"status_code", resp.StatusCode,
190+
)
168191
return resp, fmt.Errorf("resource not found: %s", endpoint)
169192
}
170193

171194
// Retry Logic
172195
if IsNonRetryableError(resp) {
173-
c.logger.Warn("Encountered a non-retryable error", "status", resp.StatusCode, "description", TranslateStatusCode(resp.StatusCode))
196+
c.logger.Warn("Encountered a non-retryable error",
197+
"method", method,
198+
"endpoint", endpoint,
199+
"status_code", resp.StatusCode,
200+
"description", TranslateStatusCode(resp.StatusCode),
201+
)
174202
return resp, c.HandleAPIError(resp)
175203
} else if IsRateLimitError(resp) {
176-
waitDuration := parseRateLimitHeaders(resp) // Checks for the Retry-After, X-RateLimit-Remaining and X-RateLimit-Reset headers
177-
c.logger.Warn("Encountered a rate limit error. Retrying after wait duration.", "wait_duration", waitDuration)
204+
waitDuration := parseRateLimitHeaders(resp) // Parses headers to determine wait duration
205+
c.logger.Warn("Encountered a rate limit error. Retrying after wait duration.",
206+
"method", method,
207+
"endpoint", endpoint,
208+
"status_code", resp.StatusCode,
209+
"wait_duration", waitDuration,
210+
)
178211
time.Sleep(waitDuration)
179212
i++
180213
continue // This will restart the loop, effectively "retrying" the request
181214
} else if IsTransientError(resp) {
182-
waitDuration := calculateBackoff(i) //uses exponential backoff (with jitter)
183-
c.logger.Warn("Encountered a transient error. Retrying after backoff.", "wait_duration", waitDuration)
215+
waitDuration := calculateBackoff(i) // Calculates backoff duration
216+
c.logger.Warn("Encountered a transient error. Retrying after backoff.",
217+
"method", method,
218+
"endpoint", endpoint,
219+
"status_code", resp.StatusCode,
220+
"wait_duration", waitDuration,
221+
"attempt", i,
222+
)
184223
time.Sleep(waitDuration)
185224
i++
186225
continue // This will restart the loop, effectively "retrying" the request
187226
} else {
188-
c.logger.Error("Received unexpected error status from HTTP request", "method", method, "endpoint", endpoint, "status_code", resp.StatusCode, "description", TranslateStatusCode(resp.StatusCode))
227+
c.logger.Error("Received unexpected error status from HTTP request",
228+
"method", method,
229+
"endpoint", endpoint,
230+
"status_code", resp.StatusCode,
231+
"description", TranslateStatusCode(resp.StatusCode),
232+
)
189233
return resp, c.HandleAPIError(resp)
190234
}
191235
}
@@ -194,10 +238,14 @@ func (c *Client) DoRequest(method, endpoint string, body, out interface{}) (*htt
194238
responseTimeStart := time.Now()
195239
// For non-retryable HTTP Methods (POST - Create)
196240
req = req.WithContext(ctx)
241+
// Execute the request
197242
resp, err := c.httpClient.Do(req)
198-
199243
if err != nil {
200-
c.logger.Error("Failed to send request", "method", method, "endpoint", endpoint, "error", err)
244+
c.logger.Error("Failed to send multipart request",
245+
"method", method,
246+
"endpoint", endpoint,
247+
"error", err.Error(),
248+
)
201249
return nil, err
202250
}
203251

@@ -209,25 +257,46 @@ func (c *Client) DoRequest(method, endpoint string, body, out interface{}) (*htt
209257

210258
CheckDeprecationHeader(resp, c.logger)
211259

212-
// Unmarshal the response with the determined API Handler
260+
// Handle (unmarshal) response with API Handler
213261
if err := handler.UnmarshalResponse(resp, out); err != nil {
214262
switch e := err.(type) {
215-
case *APIError:
216-
c.logger.Error("Received an API error", "status_code", e.StatusCode, "message", e.Message)
263+
case *APIError: // Assuming APIError is a type that includes StatusCode and Message
264+
// Log the API error with structured logging
265+
c.logger.Error("Received an API error",
266+
"method", method,
267+
"endpoint", endpoint,
268+
"status_code", e.StatusCode,
269+
"message", e.Message,
270+
)
217271
return resp, e
218272
default:
219-
// Existing error handling logic
220-
c.logger.Error("Failed to unmarshal HTTP response", "method", method, "endpoint", endpoint, "error", err)
273+
// Log the default error with structured logging
274+
c.logger.Error("Failed to unmarshal HTTP response",
275+
"method", method,
276+
"endpoint", endpoint,
277+
"error", err.Error(), // Convert error to string to log as a value
278+
)
221279
return resp, err
222280
}
223281
}
224282

225283
// Check if the response status code is within the success range
226284
if resp.StatusCode >= 200 && resp.StatusCode < 300 {
285+
// Success, no need for logging
227286
return resp, nil
228287
} else {
288+
// Translate the status code to a human-readable description
229289
statusDescription := TranslateStatusCode(resp.StatusCode)
230-
c.logger.Error("Received non-success status code from HTTP request", "method", method, "endpoint", endpoint, "status_code", resp.StatusCode, "description", statusDescription)
290+
291+
// Log the error with structured context
292+
c.logger.Error("Received non-success status code from HTTP request",
293+
"method", method,
294+
"endpoint", endpoint,
295+
"status_code", resp.StatusCode,
296+
"description", statusDescription,
297+
)
298+
299+
// Return an error with the status code and its description
231300
return resp, fmt.Errorf("Error status code: %d - %s", resp.StatusCode, statusDescription)
232301
}
233302
}
@@ -294,24 +363,41 @@ func (c *Client) DoMultipartRequest(method, endpoint string, fields map[string]s
294363

295364
// Debug: Print request headers if in debug mode
296365

297-
c.logger.Debug("HTTP Multipart Request Headers:", req.Header)
366+
// Check if logging level is DEBUG or higher before logging headers
367+
if c.logger.GetLogLevel() <= LogLevelDebug {
368+
// Debug: Print request headers without hiding sensitive information
369+
LogHTTPHeaders(c.logger, req.Header, false) // Use false to display all headers
370+
}
298371

299372
// Execute the request
300373
resp, err := c.httpClient.Do(req)
301374
if err != nil {
302-
c.logger.Error("Failed to send multipart request", "method", method, "endpoint", endpoint, "error", err)
375+
c.logger.Error("Failed to send multipart request",
376+
"method", method,
377+
"endpoint", endpoint,
378+
"error", err.Error(),
379+
)
303380
return nil, err
304381
}
305382

306383
// Check for successful status code
307384
if resp.StatusCode < 200 || resp.StatusCode >= 300 {
308-
c.logger.Error("Received non-success status code from multipart request", "status_code", resp.StatusCode)
309-
return resp, fmt.Errorf("received non-success status code: %d", resp.StatusCode)
385+
c.logger.Error("Received non-success status code from multipart request",
386+
"method", method,
387+
"endpoint", endpoint,
388+
"status_code", resp.StatusCode,
389+
"status_text", http.StatusText(resp.StatusCode),
390+
)
391+
return resp, fmt.Errorf("received non-success status code: %d - %s", resp.StatusCode, http.StatusText(resp.StatusCode))
310392
}
311393

312394
// Unmarshal the response
313395
if err := handler.UnmarshalResponse(resp, out); err != nil {
314-
c.logger.Error("Failed to unmarshal HTTP response", "method", method, "endpoint", endpoint, "error", err)
396+
c.logger.Error("Failed to unmarshal HTTP response",
397+
"method", method,
398+
"endpoint", endpoint,
399+
"error", err.Error(),
400+
)
315401
return resp, err
316402
}
317403

0 commit comments

Comments
 (0)