-
Notifications
You must be signed in to change notification settings - Fork 39
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
feat: ingest structured logs using stdout in LoggingHandler #812
Conversation
resolve warnings change type for trace an spanId to be strings create more realistic and visual test data: - timestamp with more recent date - json payload with various data types
refactor setAutoPopulateMetadata() to avoid setting flag to null
implement populateMetadata() API. change write() to use the new API. refactor SourceLocation.fromCurrentContext() to use a list of exclusion prefixes instead of depth level as parameter. change relevant unit tests to support the refactored changes.
modify publish() method to print structured logging if configured instead of calling to Logging.write(). populate created LogEntry instance with metadata separately instead of adding WriteOption.AUTO_POPULATE_METADATA. refactor LoggingHandler to remove getLogging() method. refactor unit tests to reflect the change in LoggingHandler.publish() implementation.
refactor LoggingHandlerTest.
fixes testFromCurrentContextWithVeryLargeLevel by excluding classes from OS JDK
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.
Thanks for your effort - good job! I left some comments, PTAL and let me know if those makes sense
google-cloud-logging/src/main/java/com/google/cloud/logging/LogEntry.java
Show resolved
Hide resolved
google-cloud-logging/src/main/java/com/google/cloud/logging/LogEntry.java
Show resolved
Hide resolved
google-cloud-logging/src/main/java/com/google/cloud/logging/LogEntry.java
Show resolved
Hide resolved
google-cloud-logging/src/main/java/com/google/cloud/logging/LogEntry.java
Show resolved
Hide resolved
google-cloud-logging/src/main/java/com/google/cloud/logging/LogEntry.java
Show resolved
Hide resolved
google-cloud-logging/src/main/java/com/google/cloud/logging/LogEntry.java
Outdated
Show resolved
Hide resolved
google-cloud-logging/src/main/java/com/google/cloud/logging/LoggingHandler.java
Outdated
Show resolved
Hide resolved
google-cloud-logging/src/main/java/com/google/cloud/logging/LoggingImpl.java
Show resolved
Hide resolved
google-cloud-logging/src/main/java/com/google/cloud/logging/LoggingImpl.java
Outdated
Show resolved
Hide resolved
google-cloud-logging/src/main/java/com/google/cloud/logging/LoggingImpl.java
Outdated
Show resolved
Hide resolved
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.
Overall looks good - just fix whatever we agreed in comments. Also left some clarification regarding incomplete comment, hope it make sense
google-cloud-logging/src/main/java/com/google/cloud/logging/LogEntry.java
Show resolved
Hide resolved
} | ||
|
||
/** | ||
* Adds a collection of Json fields that {@code value} parameter is serialized to to the current |
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.
Yes, I think name change like appendDictionary
will be fine then
} else if (payload.getType() == Type.JSON) { | ||
Payload.JsonPayload jsonPayload = (Payload.JsonPayload) payload; | ||
formatter.appendJson(jsonPayload.getDataAsMap(), true); | ||
} else if (payload.getType() == Type.PROTO) { |
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.
Thank you!
#812) Implements populateMetadata() API. Changes Logging.write() API to populate the provided list of log entries using the new API. Refactors SourceLocation.fromCurrentContext() to use a list of exclusion instead of the call stack depth level as parameter. Adds configuration `populateToStdout` to `LoggingConfig`. Use the new configuration within `LoggingHandler` to print to STDOUT instead of ingesting the log by calling Logging.write(). Refactor LoggingImpl, LoggingHandler and unit tests.
#812) Implements populateMetadata() API. Changes Logging.write() API to populate the provided list of log entries using the new API. Refactors SourceLocation.fromCurrentContext() to use a list of exclusion instead of the call stack depth level as parameter. Adds configuration `populateToStdout` to `LoggingConfig`. Use the new configuration within `LoggingHandler` to print to STDOUT instead of ingesting the log by calling Logging.write(). Refactor LoggingImpl, LoggingHandler and unit tests.
…ogs redirection to stdout in JUL handler (#808) Aggregates the following work: - #821 - #812 - #807 - #803 - #798 Fixes #689, #691, #799 and #800 * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Add configuration (edited:)
redirectoToStdout
touseStructuredLogging
LoggingHandler
class to support an option to print logs to STDOUT formatted as a one line Json and following the structured logging guidelines.Implement Jsonification of the
LogEntry
object following the structured logging format.Move metadata population to a standalone API to be used by
LoggingHandler
explicitly.