[LFXV2-1043] Add comprehensive debug logging to ITX proxy client#113
[LFXV2-1043] Add comprehensive debug logging to ITX proxy client#113
Conversation
- Add logRequest() helper to log outgoing HTTP requests at DEBUG level - Add logResponse() helper to log responses at DEBUG/ERROR level based on status code - Add logging to all 28 ITX proxy client methods for request/response visibility - Fix HTTP method in CreateInvitee from PUT to POST - Fix HTTP method in UpdateInvitee from DELETE to PUT - Capture method, URL, request body, status code, and response body in logs - Enable troubleshooting of 404 errors and other API issues 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Andres Tobon <andrest2455@gmail.com>
- Add strings.TrimRight() in NewClient to strip trailing slash from config.BaseURL - Prevents double slash issues like "https://api.example.com//v2/zoom/meetings" - All URL constructions use fmt.Sprintf with hardcoded "/" prefix in paths - Ensures consistent URL parsing regardless of base URL configuration 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Andres Tobon <andrest2455@gmail.com>
…g-service into andrest50/proxy-debug-logging
|
Caution Review failedAn error occurred during the review process. Please try again later. WalkthroughChart version incremented from 0.6.3 to 0.6.4 in Helm chart configuration. Logging infrastructure added to ITX proxy client with request/response debug helpers using slog, applied across Zoom meeting, registrant, attendee, and invite operations. Minor URL normalization implemented via trailing slash trimming. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~20 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
This PR adds request/response debug logging throughout the ITX proxy client to improve troubleshooting of ITX API failures (notably 404s), while also correcting a couple of invitee HTTP methods and hardening base URL construction.
Changes:
- Added
logRequest()/logResponse()helpers and applied them across ITX proxy client methods for consistent request/response logging. - Fixed HTTP verbs for invitee operations (Create -> POST, Update -> PUT).
- Normalized
BaseURLby stripping trailing slashes to avoid//in constructed URLs.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| internal/infrastructure/proxy/client.go | Adds request/response logging helpers, applies logging to ITX client methods, fixes invitee HTTP methods, trims trailing slash from base URL. |
| charts/lfx-v2-meeting-service/Chart.yaml | Bumps chart version to reflect the change set. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| slog.ErrorContext(ctx, "ITX API Response Error", | ||
| "status_code", statusCode, | ||
| "body", string(body), | ||
| ) |
There was a problem hiding this comment.
logResponse logs the entire response body at ERROR level for any non-2xx response. This can leak sensitive data into high-severity logs and can also create very noisy ERROR logs for expected 4xx responses. Consider logging only status code + a bounded/redacted excerpt (or parsed message field), and/or using WARN for 4xx while reserving ERROR for 5xx/transport failures.
| slog.ErrorContext(ctx, "ITX API Response Error", | |
| "status_code", statusCode, | |
| "body", string(body), | |
| ) | |
| // For non-2xx responses, avoid logging the full body to reduce the risk of | |
| // leaking sensitive data. Log only a bounded excerpt and adjust log level | |
| // so that 4xx client errors are WARN while 5xx/server errors remain ERROR. | |
| const maxLogBodyLen = 512 | |
| logBody := string(body) | |
| if len(logBody) > maxLogBodyLen { | |
| logBody = logBody[:maxLogBodyLen] | |
| } | |
| if statusCode >= 400 && statusCode < 500 { | |
| slog.WarnContext(ctx, "ITX API Response Client Error", | |
| "status_code", statusCode, | |
| "body_excerpt", logBody, | |
| ) | |
| } else { | |
| slog.ErrorContext(ctx, "ITX API Response Error", | |
| "status_code", statusCode, | |
| "body_excerpt", logBody, | |
| ) | |
| } |
| // Log request | ||
| c.logRequest(ctx, http.MethodGet, queryURL, nil) |
There was a problem hiding this comment.
This request log includes queryURL which can contain PII in query parameters (e.g., email, name, user_id). Consider logging only the path (or a sanitized URL with sensitive query params removed/hashed) to avoid leaking personal data when debug logging is enabled.
| // Log request | |
| c.logRequest(ctx, http.MethodGet, queryURL, nil) | |
| // Log request (sanitize URL to avoid logging query parameters that may contain PII) | |
| if parsedURL, err := url.Parse(queryURL); err == nil { | |
| parsedURL.RawQuery = "" | |
| c.logRequest(ctx, http.MethodGet, parsedURL.String(), nil) | |
| } else { | |
| // Fallback: log original URL if parsing fails to avoid changing behavior in edge cases | |
| c.logRequest(ctx, http.MethodGet, queryURL, nil) | |
| } |
| // Log response | ||
| c.logResponse(ctx, resp.StatusCode, respBody) |
There was a problem hiding this comment.
GetRegistrantICS returns the response body as binary/ICS content, but logResponse will log the entire body. This can be large and may contain personal details, and dumping it to logs can bloat log volume and break log ingestion. Consider skipping body logging for this endpoint (or logging only content length / a small capped prefix).
| // Log response | |
| c.logResponse(ctx, resp.StatusCode, respBody) | |
| // Log response metadata only to avoid logging potentially large or sensitive ICS content | |
| slog.Info("GetRegistrantICS response", "status", resp.StatusCode, "content_length", len(respBody)) |
| // logRequest logs the outgoing HTTP request for debugging | ||
| func (c *Client) logRequest(ctx context.Context, method, url string, body []byte) { | ||
| slog.DebugContext(ctx, "ITX API Request", | ||
| "method", method, | ||
| "url", url, | ||
| "body", string(body), | ||
| ) | ||
| } | ||
|
|
||
| // logResponse logs the incoming HTTP response for debugging | ||
| func (c *Client) logResponse(ctx context.Context, statusCode int, body []byte) { | ||
| if statusCode < 200 || statusCode >= 300 { | ||
| slog.ErrorContext(ctx, "ITX API Response Error", | ||
| "status_code", statusCode, | ||
| "body", string(body), | ||
| ) | ||
| } else { | ||
| slog.DebugContext(ctx, "ITX API Response", | ||
| "status_code", statusCode, | ||
| "body", string(body), |
There was a problem hiding this comment.
logRequest logs the full request body by converting []byte to string. These requests can contain PII (e.g., names/emails) and this also allocates even when DEBUG logging is disabled (args are evaluated before slog.DebugContext checks the level). Consider gating with slog.Default().Enabled(ctx, slog.LevelDebug) and logging a redacted/truncated body (or just size/hash) to avoid leaking sensitive data and unnecessary allocations.
| // logRequest logs the outgoing HTTP request for debugging | |
| func (c *Client) logRequest(ctx context.Context, method, url string, body []byte) { | |
| slog.DebugContext(ctx, "ITX API Request", | |
| "method", method, | |
| "url", url, | |
| "body", string(body), | |
| ) | |
| } | |
| // logResponse logs the incoming HTTP response for debugging | |
| func (c *Client) logResponse(ctx context.Context, statusCode int, body []byte) { | |
| if statusCode < 200 || statusCode >= 300 { | |
| slog.ErrorContext(ctx, "ITX API Response Error", | |
| "status_code", statusCode, | |
| "body", string(body), | |
| ) | |
| } else { | |
| slog.DebugContext(ctx, "ITX API Response", | |
| "status_code", statusCode, | |
| "body", string(body), | |
| const maxLogBodyLen = 1024 | |
| // truncateBodyForLog returns a string representation of the body truncated to a safe length for logging. | |
| func truncateBodyForLog(body []byte) string { | |
| if len(body) == 0 { | |
| return "" | |
| } | |
| if len(body) <= maxLogBodyLen { | |
| return string(body) | |
| } | |
| return string(body[:maxLogBodyLen]) + "...(truncated)" | |
| } | |
| // logRequest logs the outgoing HTTP request for debugging | |
| func (c *Client) logRequest(ctx context.Context, method, url string, body []byte) { | |
| logger := slog.Default() | |
| if !logger.Enabled(ctx, slog.LevelDebug) { | |
| return | |
| } | |
| logger.DebugContext(ctx, "ITX API Request", | |
| "method", method, | |
| "url", url, | |
| "body", truncateBodyForLog(body), | |
| ) | |
| } | |
| // logResponse logs the incoming HTTP response for debugging | |
| func (c *Client) logResponse(ctx context.Context, statusCode int, body []byte) { | |
| truncatedBody := truncateBodyForLog(body) | |
| if statusCode < 200 || statusCode >= 300 { | |
| slog.ErrorContext(ctx, "ITX API Response Error", | |
| "status_code", statusCode, | |
| "body", truncatedBody, | |
| ) | |
| } else { | |
| slog.DebugContext(ctx, "ITX API Response", | |
| "status_code", statusCode, | |
| "body", truncatedBody, |
Summary
This PR adds comprehensive debug logging to the ITX proxy client to enable troubleshooting of 404 errors and other API issues. All 28 ITX proxy client methods now have complete request/response logging with appropriate log levels.
Ticket
LFXV2-1043
Changes
Debug Logging Implementation
Added
logRequest()helper: Logs outgoing HTTP requests at DEBUG levelAdded
logResponse()helper: Logs incoming HTTP responses with intelligent log levelsApplied logging to all 28 client methods:
Bug Fixes
//v2/zoom/meetings)Testing
Manual Verification
logRequestandlogResponsecallsLinting
make lintwith 0 issuesExample Log Output
With
LOG_LEVEL=debug, ITX proxy requests will now produce logs like:Successful Request:
Failed Request:
How to Enable Debug Logging
Local Development
Kubernetes Deployment
Impact
🤖 Generated with Claude Code