docs(logging): improve write log entry sample#12974
docs(logging): improve write log entry sample#12974MukundaKatta wants to merge 1 commit intogoogleapis:mainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the WriteLogEntry sample to demonstrate both single and batch log writing by introducing a createLogEntries helper method. It also enhances the integration tests to validate the specific attributes of the generated log entries. Review feedback suggests avoiding duplicate log writes by using a sublist for the batch operation and renaming a variable that shadows the JsonPayload class to improve clarity.
| logging.write(Collections.singleton(entries.get(0))); | ||
|
|
||
| // Writes a batch of text and structured log entries. | ||
| logging.write(entries); |
There was a problem hiding this comment.
The current implementation writes the first entry (entries.get(0)) twice: once individually at line 44 and again as part of the batch at line 47. This will result in duplicate log entries in Cloud Logging. While this demonstrates both APIs, it might be confusing for users. Consider using distinct entries or a sublist for the batch write to avoid duplication.
| logging.write(Collections.singleton(entries.get(0))); | |
| // Writes a batch of text and structured log entries. | |
| logging.write(entries); | |
| // Writes one text log entry. | |
| logging.write(Collections.singleton(entries.get(0))); | |
| // Writes a batch of structured log entries. | |
| logging.write(entries.subList(1, entries.size())); |
| Map<String, Object> jsonPayload = | ||
| ImmutableMap.of( | ||
| "message", "Structured log entry written from Java.", "component", "sample"); | ||
| LogEntry structEntry = | ||
| LogEntry.newBuilder(JsonPayload.of(jsonPayload)) |
There was a problem hiding this comment.
The local variable jsonPayload shadows the JsonPayload class name (differing only by case), which is imported and used in the same scope (line 71). This can lead to confusion and is generally discouraged in Java. Consider renaming the variable to payload or payloadMap.
| Map<String, Object> jsonPayload = | |
| ImmutableMap.of( | |
| "message", "Structured log entry written from Java.", "component", "sample"); | |
| LogEntry structEntry = | |
| LogEntry.newBuilder(JsonPayload.of(jsonPayload)) | |
| Map<String, Object> payload = | |
| ImmutableMap.of( | |
| "message", "Structured log entry written from Java.", "component", "sample"); | |
| LogEntry structEntry = | |
| LogEntry.newBuilder(JsonPayload.of(payload)) |
Fixes #11923.
Updates the Java Cloud Logging write log entry sample to demonstrate:
The accompanying sample test now asserts the constructed entry metadata and payload types before exercising the sample.
Verification:
Not run locally: