Skip to content
This repository was archived by the owner on Feb 6, 2026. It is now read-only.

fix: Correct the JSON time field name #1020

Merged
losalex merged 2 commits into from
Aug 19, 2022
Merged

fix: Correct the JSON time field name #1020

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4a1557b
fix: Correct JSON time field name
Jul 30, 2022
d4ea438
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] Aug 19, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,20 +51,20 @@ If you are using Maven without BOM, add this to your dependencies:
If you are using Gradle 5.x or later, add this to your dependencies:

```Groovy
implementation platform('com.google.cloud:libraries-bom:26.0.0')
implementation platform('com.google.cloud:libraries-bom:26.1.0')

implementation 'com.google.cloud:google-cloud-logging'
```
If you are using Gradle without BOM, add this to your dependencies:

```Groovy
implementation 'com.google.cloud:google-cloud-logging:3.10.0'
implementation 'com.google.cloud:google-cloud-logging:3.10.2'
```

If you are using SBT, add this to your dependencies:

```Scala
libraryDependencies += "com.google.cloud" % "google-cloud-logging" % "3.10.0"
libraryDependencies += "com.google.cloud" % "google-cloud-logging" % "3.10.2"
```

## Authentication
Expand Down
2 changes: 1 addition & 1 deletion google-cloud-logging/src/main/java/com/google/cloud/logging/LogEntry.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -700,7 +700,7 @@ public String toStructuredJsonString() {

formatter
.appendField("severity", severity)
.appendField("timestamp", timestamp)
.appendField("time", timestamp)
Copy link
Contributor

@losalex losalex Aug 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Such change could break existing clients which use timestamp in logs. I believe to be on a safe side we should add a configuration parameter controlling this behavior with default to be working with timestamp.

Copy link
Contributor Author

@bpcreech bpcreech Aug 10, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately I think anyone using this library is already broken. I don't think this code is likely to be working for anyone as-is. Unless they implemented their own parser for some reason.

Here is the spec for these fields. Note that this code has been putting RFC3339 time (what happens when you implicitly call toString on an Instant) into the timestamp field. That is wrong.

Here is the code in the Google fluentd parser that parses this format. This is the original and primary intended parser for this format. Note that it doesn't know what to do if the timestamp field has a string in it. Instead it's defaulting to current time, meaning that everyone using this library with the Google Fluentd Parser is getting less accurate timestamps, representing when the parser sees the log as opposed to when the call site emitted it.

The other primary use case for this format (that I know of anyway) is GCP Serverless. Specifically this format works on App Engine Standard gen 2 and Cloud Run Managed. Internally, that code lives at go/time-is-broken. You'll note that if the field called timestamp is a string, it's ignored.

If we leave this broken default, it means this library will be broken by default for anyone trying to use it with the Google Fluentd agent, or GCP Serverless. :(

Copy link
Contributor

@losalex losalex Aug 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense

.appendField("httpRequest", httpRequest)
.appendField("logging.googleapis.com/insertId", insertId)
.appendField("logging.googleapis.com/labels", labels)
Expand Down
4 changes: 2 additions & 2 deletions google-cloud-logging/src/test/java/com/google/cloud/logging/LogEntryTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -368,8 +368,8 @@ private void compareLogEntry(LogEntry expected, LogEntry value, Boolean extraVal
}

private static final String[] EXPECTED_STRUCTURED_LOGS = {
"{\"severity\":\"ALERT\",\"timestamp\":\"1984-08-13T15:35:30.123Z\",\"httpRequest\":{\"requestMethod\":\"GET\",\"status\":404,\"cacheLookup\":false,\"cacheHit\":false,\"cacheValidatedWithOriginServer\":false},\"logging.googleapis.com/insertId\":\"insertId\",\"logging.googleapis.com/labels\":{\"key1\":\"value1\",\"key2\":\"value2\"},\"logging.googleapis.com/operation\":{\"id\":\"id\",\"producer\":\"producer\",\"first\":false,\"last\":false},\"logging.googleapis.com/sourceLocation\":{\"file\":\"file\",\"line\":\"42\",\"function\":\"function\"},\"logging.googleapis.com/spanId\":\"spanId\",\"logging.googleapis.com/trace\":\"trace\",\"logging.googleapis.com/trace_sampled\":true,\"message\":\"payload\"}",
"{\"severity\":\"ALERT\",\"timestamp\":\"1984-08-13T15:35:30.123Z\",\"httpRequest\":{\"requestMethod\":\"GET\",\"status\":404,\"cacheLookup\":false,\"cacheHit\":false,\"cacheValidatedWithOriginServer\":false},\"logging.googleapis.com/insertId\":\"insertId\",\"logging.googleapis.com/labels\":{\"key1\":\"value1\",\"key2\":\"value2\"},\"logging.googleapis.com/operation\":{\"id\":\"id\",\"producer\":\"producer\",\"first\":false,\"last\":false},\"logging.googleapis.com/sourceLocation\":{\"file\":\"file\",\"line\":\"42\",\"function\":\"function\"},\"logging.googleapis.com/spanId\":\"spanId\",\"logging.googleapis.com/trace\":\"trace\",\"logging.googleapis.com/trace_sampled\":true,\"key1\":\"val\",\"key2\":123.0,\"key3\":false}"
"{\"severity\":\"ALERT\",\"time\":\"1984-08-13T15:35:30.123Z\",\"httpRequest\":{\"requestMethod\":\"GET\",\"status\":404,\"cacheLookup\":false,\"cacheHit\":false,\"cacheValidatedWithOriginServer\":false},\"logging.googleapis.com/insertId\":\"insertId\",\"logging.googleapis.com/labels\":{\"key1\":\"value1\",\"key2\":\"value2\"},\"logging.googleapis.com/operation\":{\"id\":\"id\",\"producer\":\"producer\",\"first\":false,\"last\":false},\"logging.googleapis.com/sourceLocation\":{\"file\":\"file\",\"line\":\"42\",\"function\":\"function\"},\"logging.googleapis.com/spanId\":\"spanId\",\"logging.googleapis.com/trace\":\"trace\",\"logging.googleapis.com/trace_sampled\":true,\"message\":\"payload\"}",
"{\"severity\":\"ALERT\",\"time\":\"1984-08-13T15:35:30.123Z\",\"httpRequest\":{\"requestMethod\":\"GET\",\"status\":404,\"cacheLookup\":false,\"cacheHit\":false,\"cacheValidatedWithOriginServer\":false},\"logging.googleapis.com/insertId\":\"insertId\",\"logging.googleapis.com/labels\":{\"key1\":\"value1\",\"key2\":\"value2\"},\"logging.googleapis.com/operation\":{\"id\":\"id\",\"producer\":\"producer\",\"first\":false,\"last\":false},\"logging.googleapis.com/sourceLocation\":{\"file\":\"file\",\"line\":\"42\",\"function\":\"function\"},\"logging.googleapis.com/spanId\":\"spanId\",\"logging.googleapis.com/trace\":\"trace\",\"logging.googleapis.com/trace_sampled\":true,\"key1\":\"val\",\"key2\":123.0,\"key3\":false}"
};
private static final LogEntry[] TEST_LOG_ENTRIES = {STRING_ENTRY, JSON_ENTRY};

Expand Down
2 changes: 1 addition & 1 deletion google-cloud-logging/src/test/java/com/google/cloud/logging/LoggingHandlerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,7 @@ public void enhanceLogEntry(Builder builder) {
@Test
public void testEnhancedLogEntryPrintToStdout() {
final String ExpectedOutput =
"{\"severity\":\"INFO\",\"timestamp\":\"1970-01-02T10:17:36.789Z\",\"logging.googleapis.com/labels\":{\"enhanced\":\"true\"},\"logging.googleapis.com/trace_sampled\":false,\"message\":\"message\"}";
"{\"severity\":\"INFO\",\"time\":\"1970-01-02T10:17:36.789Z\",\"logging.googleapis.com/labels\":{\"enhanced\":\"true\"},\"logging.googleapis.com/trace_sampled\":false,\"message\":\"message\"}";
replay(options, logging);
ByteArrayOutputStream bout = new ByteArrayOutputStream();
PrintStream out = new PrintStream(bout);
Expand Down