Improve error logging #629
Conversation
improve logs from stderr to zap
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| @@ -66,7 +63,7 @@ | |||
| WithCreator(). | |||
| All(ctx) | |||
| if err != nil { | |||
| log.Println("Error when getting team API keys: ", err) | |||
| zap.L().Warn("error when getting team API keys", zap.Error(err)) | |||
Check failure
Code scanning / CodeQL
Clear-text logging of sensitive information High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 7 days ago
To fix the issue, we should ensure that sensitive information is not logged in plaintext. Instead of logging the raw error object, we can log a generic error message that does not include sensitive details. If additional context is needed for debugging, we can log a sanitized or obfuscated version of the error message. This approach ensures that sensitive data is not exposed while still providing useful information for troubleshooting.
| @@ -65,3 +65,3 @@ | ||
| if err != nil { | ||
| zap.L().Warn("error when getting team API keys", zap.Error(err)) | ||
| zap.L().Warn("error when getting team API keys", zap.String("error", "an error occurred while querying team API keys")) | ||
| c.String(http.StatusInternalServerError, "Error when getting team API keys") |
| if OTELTracingPrint { | ||
| var msg string | ||
| debugID := getDebugID(ctx) | ||
| zap.L().Error(message, zap.Stringp("debug_id", debugID), zap.Error(err), zap.Any("attrs", attrs)) |
Check failure
Code scanning / CodeQL
Clear-text logging of sensitive information High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 7 days ago
To fix the issue, we need to ensure that sensitive information is not logged in plaintext. This can be achieved by sanitizing the err object before logging it. Specifically, we can replace sensitive details in the error message with a generic placeholder or remove them entirely. Additionally, we can implement a utility function to sanitize error messages consistently across the codebase.
In this case, we will modify the ReportCriticalError and ReportError functions in packages/shared/pkg/telemetry/tracing.go to sanitize the err object before passing it to the logging call. This ensures that sensitive data is not exposed in the logs.
| @@ -79,3 +79,4 @@ | ||
| debugID := getDebugID(ctx) | ||
| zap.L().Error(message, zap.Stringp("debug_id", debugID), zap.Error(err), zap.Any("attrs", attrs)) | ||
| sanitizedErr := sanitizeError(err) | ||
| zap.L().Error(message, zap.Stringp("debug_id", debugID), zap.Error(sanitizedErr), zap.Any("attrs", attrs)) | ||
|
|
||
| @@ -97,3 +98,4 @@ | ||
| debugID := getDebugID(ctx) | ||
| zap.L().Warn(message, zap.Stringp("debug_id", debugID), zap.Error(err), zap.Any("attrs", attrs)) | ||
| sanitizedErr := sanitizeError(err) | ||
| zap.L().Warn(message, zap.Stringp("debug_id", debugID), zap.Error(sanitizedErr), zap.Any("attrs", attrs)) | ||
|
|
Change stderr to zap logs. Add logging for all critical and non-critical telemetry errors