-
Notifications
You must be signed in to change notification settings - Fork 75
feat(gax): add utility for logging actionable errors #4144
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -31,15 +31,34 @@ | |||||||||||||||||||||||||||||||||||||
| package com.google.api.gax.logging; | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| import com.google.api.core.InternalApi; | ||||||||||||||||||||||||||||||||||||||
| import com.google.common.annotations.VisibleForTesting; | ||||||||||||||||||||||||||||||||||||||
| import java.util.Map; | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| @InternalApi | ||||||||||||||||||||||||||||||||||||||
| public class LoggingUtils { | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| private static boolean loggingEnabled = isLoggingEnabled(); | ||||||||||||||||||||||||||||||||||||||
| private static boolean loggingEnabled = checkLoggingEnabled(); | ||||||||||||||||||||||||||||||||||||||
| static final String GOOGLE_SDK_JAVA_LOGGING = "GOOGLE_SDK_JAVA_LOGGING"; | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| static boolean isLoggingEnabled() { | ||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||
| * Returns whether client-side logging is enabled. | ||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||
| * @return true if logging is enabled, false otherwise. | ||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||
| public static boolean isLoggingEnabled() { | ||||||||||||||||||||||||||||||||||||||
| return loggingEnabled; | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||
| * Sets whether client-side logging is enabled. Visible for testing. | ||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||
| * @param enabled true to enable logging, false to disable. | ||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||
| public static void setLoggingEnabled(boolean enabled) { | ||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can be package private and annotated with a |
||||||||||||||||||||||||||||||||||||||
| loggingEnabled = enabled; | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| private static boolean checkLoggingEnabled() { | ||||||||||||||||||||||||||||||||||||||
| String enableLogging = System.getenv(GOOGLE_SDK_JAVA_LOGGING); | ||||||||||||||||||||||||||||||||||||||
| return "true".equalsIgnoreCase(enableLogging); | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
@@ -126,6 +145,27 @@ public static <RespT> void logRequest( | |||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||
| * Logs an actionable error message with structured context. | ||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||
| * @param logContext A map containing the structured logging context (e.g., RPC service, method, | ||||||||||||||||||||||||||||||||||||||
| * error details). | ||||||||||||||||||||||||||||||||||||||
| * @param loggerProvider The provider used to obtain the logger. | ||||||||||||||||||||||||||||||||||||||
| * @param message The human-readable error message. | ||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||
| public static void logActionableError( | ||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. According to the repository's testing guidelines, new logic in the References
|
||||||||||||||||||||||||||||||||||||||
| Map<String, Object> logContext, LoggerProvider loggerProvider, String message) { | ||||||||||||||||||||||||||||||||||||||
| if (loggingEnabled) { | ||||||||||||||||||||||||||||||||||||||
| org.slf4j.Logger logger = loggerProvider.getLogger(); | ||||||||||||||||||||||||||||||||||||||
| // Actionable errors are logged at the INFO level because transport errors | ||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Do we have a cross-language requirements for this? |
||||||||||||||||||||||||||||||||||||||
| // might be retryable and self-healing. Logging at ERROR would trigger | ||||||||||||||||||||||||||||||||||||||
| // unintended production alerts for transient issues. | ||||||||||||||||||||||||||||||||||||||
| if (logger.isInfoEnabled()) { | ||||||||||||||||||||||||||||||||||||||
| Slf4jUtils.log(logger, org.slf4j.event.Level.INFO, logContext, message); | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+156
to
+167
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The method
Suggested change
|
||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| public static void executeWithTryCatch(ThrowingRunnable action) { | ||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||
| action.run(); | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this method remain package private?