From 7ae153789268629539cf1ea5246597aae7aa8d19 Mon Sep 17 00:00:00 2001 From: minherz Date: Thu, 2 Sep 2021 08:43:58 +0300 Subject: [PATCH] fix: Change timestamp type to support nanosecond resolution (#654) Changes timestamp type to java.time.Instant to support nanosec resolution. Keeps current get/set methods to operate with milliseconds. Updates unit tests to reflect type change and additional API. Fixes #598 --- .../cloud/logging/JavaTimeConversions.java | 80 +++++++++++++ .../com/google/cloud/logging/LogEntry.java | 109 ++++++++++++------ .../google/cloud/logging/LogEntryTest.java | 36 ++++-- 3 files changed, 180 insertions(+), 45 deletions(-) create mode 100644 google-cloud-logging/src/main/java/com/google/cloud/logging/JavaTimeConversions.java diff --git a/google-cloud-logging/src/main/java/com/google/cloud/logging/JavaTimeConversions.java b/google-cloud-logging/src/main/java/com/google/cloud/logging/JavaTimeConversions.java new file mode 100644 index 000000000..26d42791c --- /dev/null +++ b/google-cloud-logging/src/main/java/com/google/cloud/logging/JavaTimeConversions.java @@ -0,0 +1,80 @@ +/* + * Copyright 2021 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.cloud.logging; + +import static com.google.common.math.LongMath.checkedAdd; +import static com.google.common.math.LongMath.checkedSubtract; + +import com.google.protobuf.Timestamp; +import com.google.protobuf.util.Timestamps; +import java.time.Instant; + +/** + * A utility class that provides conversions between: + * + * + * + * The class complements convertion methods that are currently not supported in the published + * protobuf-java-util. After migrating protobuf-java-util to Java 8 this class can be removed. + * + * @see protobuf-java-util + */ +class JavaTimeConversions { + static final long NANOS_PER_SECOND = 1000000000; + + private JavaTimeConversions() {} + + /** + * Converts a protobuf {@link Timestamp} to a {@link java.time.Instant}. + * + * @throws IllegalArgumentException if the given {@link Timestamp} is not valid. See {@link + * Timestamps#isValid}. + */ + public static Instant toJavaInstant(Timestamp timestamp) { + timestamp = normalizedTimestamp(timestamp.getSeconds(), timestamp.getNanos()); + return Instant.ofEpochSecond(timestamp.getSeconds(), timestamp.getNanos()); + } + + /** + * Converts a {@link java.time.Instant} to a protobuf {@link Timestamp}. + * + * @throws IllegalArgumentException if the given {@link java.time.Instant} cannot legally fit into + * a {@link Timestamp}. See {@link Timestamps#isValid}. + */ + public static Timestamp toProtoTimestamp(Instant instant) { + return normalizedTimestamp(instant.getEpochSecond(), instant.getNano()); + } + + /** Exposes {@link Timestamps#normalizedTimestamp} internal method. */ + static Timestamp normalizedTimestamp(long seconds, int nanos) { + if (nanos <= -NANOS_PER_SECOND || nanos >= NANOS_PER_SECOND) { + seconds = checkedAdd(seconds, nanos / NANOS_PER_SECOND); + nanos = (int) (nanos % NANOS_PER_SECOND); + } + if (nanos < 0) { + nanos = + (int) + (nanos + NANOS_PER_SECOND); // no overflow since nanos is negative (and we're adding) + seconds = checkedSubtract(seconds, 1); + } + Timestamp timestamp = Timestamp.newBuilder().setSeconds(seconds).setNanos(nanos).build(); + return Timestamps.checkValid(timestamp); + } +} diff --git a/google-cloud-logging/src/main/java/com/google/cloud/logging/LogEntry.java b/google-cloud-logging/src/main/java/com/google/cloud/logging/LogEntry.java index cf0848f02..026838e7b 100644 --- a/google-cloud-logging/src/main/java/com/google/cloud/logging/LogEntry.java +++ b/google-cloud-logging/src/main/java/com/google/cloud/logging/LogEntry.java @@ -25,8 +25,8 @@ import com.google.logging.v2.LogEntryOperation; import com.google.logging.v2.LogEntrySourceLocation; import com.google.logging.v2.LogName; -import com.google.protobuf.Timestamp; import java.io.Serializable; +import java.time.Instant; import java.util.HashMap; import java.util.Map; import java.util.Objects; @@ -44,8 +44,6 @@ public class LogEntry implements Serializable { private static final long serialVersionUID = -944788159728228219L; - private static final long NANOS_PER_MILLISECOND = 1000000; - private static final long MILLIS_PER_SECOND = 1000; static final Function FROM_PB_FUNCTION = new Function() { @Override @@ -56,8 +54,8 @@ public LogEntry apply(com.google.logging.v2.LogEntry pb) { private final String logName; private final MonitoredResource resource; - private final Long timestamp; - private final Long receiveTimestamp; + private final Instant timestamp; + private final Instant receiveTimestamp; private final Severity severity; private final String insertId; private final HttpRequest httpRequest; @@ -74,8 +72,8 @@ public static class Builder { private String logName; private MonitoredResource resource; - private Long timestamp; - private Long receiveTimestamp; + private Instant timestamp; + private Instant receiveTimestamp; private Severity severity = Severity.DEFAULT; private String insertId; private HttpRequest httpRequest; @@ -133,14 +131,44 @@ public Builder setResource(MonitoredResource resource) { /** * Sets the time at which the event described by the log entry occurred, in milliseconds. If * omitted, the Logging service will use the time at which the log entry is received. + * + * @deprecated This method is no longer recommended to setup the entry timestamp. + *

Use {@link setTimeStamp(Instant)} instead. */ - public Builder setTimestamp(long timestamp) { + @Deprecated + public Builder setTimestamp(long milliseconds) { + this.timestamp = Instant.ofEpochMilli(milliseconds); + return this; + } + + /** + * Sets the time at which the event described by the log entry occurred. If omitted, the Logging + * service will use the time at which the log entry is received. + */ + public Builder setTimestamp(Instant timestamp) { this.timestamp = timestamp; return this; } - /** Sets the time the log entry was received by Cloud Logging. */ - public Builder setReceiveTimestamp(long receiveTimestamp) { + /** + * Sets the time the log entry was received by Cloud Logging, in milliseconds. If omitted, the + * Logging service will set the time at which the log entry is received. + * + * @deprecated This method is no longer recommended to setup the receive time timestamp. + *

Use {@link setReceiveTimestamp(Instant)} instead. + */ + @Deprecated + public Builder setReceiveTimestamp(long milliseconds) { + this.receiveTimestamp = Instant.ofEpochMilli(milliseconds); + ; + return this; + } + + /** + * Sets the time the log entry was received by Cloud Logging. If omitted, the Logging service + * will set the time at which the log entry is received. + */ + public Builder setReceiveTimestamp(Instant receiveTimestamp) { this.receiveTimestamp = receiveTimestamp; return this; } @@ -298,15 +326,44 @@ public MonitoredResource getResource() { } /** - * Returns the time at which the event described by the log entry occurred, in milliseconds. If - * omitted, the Logging service will use the time at which the log entry is received. + * Returns the time at which the event described by the log entry occurred, in milliseconds. + * + * @deprecated This method is no longer recommended to get the entry timestamp. + *

Use {@link getInstantTimestamp()} instead. + * @return timestamp in milliseconds */ + @Deprecated public Long getTimestamp() { + return timestamp != null ? timestamp.toEpochMilli() : null; + } + + /** + * Returns the time at which the event described by the log entry occurred. + * + * @return timestamp as {@link Instant} + */ + public Instant getInstantTimestamp() { return timestamp; } - /** Returns the time the log entry was received by Cloud Logging. */ + /** + * Returns the time the log entry was received by Cloud Logging, in milliseconds. + * + * @deprecated This method is no longer recommended to get the received time timestamp. + *

Use {@link getInstantReceiveTimestamp()} instead. + * @return timestamp in milliseconds + */ + @Deprecated public Long getReceiveTimestamp() { + return receiveTimestamp != null ? receiveTimestamp.toEpochMilli() : null; + } + + /** + * Returns the time the log entry was received by Cloud Logging, in milliseconds. + * + * @return timestamp as {@link Instant} + */ + public Instant getInstantReceiveTimestamp() { return receiveTimestamp; } @@ -450,18 +507,6 @@ public Builder toBuilder() { return new Builder(this); } - private static Timestamp timestampFromMillis(Long millis) { - Timestamp.Builder tsBuilder = Timestamp.newBuilder(); - tsBuilder.setSeconds(millis / MILLIS_PER_SECOND); - tsBuilder.setNanos((int) (millis % MILLIS_PER_SECOND * NANOS_PER_MILLISECOND)); - return tsBuilder.build(); - } - - private static Long millisFromTimestamp(Timestamp timestamp) { - return timestamp.getSeconds() * MILLIS_PER_SECOND - + timestamp.getNanos() / NANOS_PER_MILLISECOND; - } - com.google.logging.v2.LogEntry toPb(String projectId) { com.google.logging.v2.LogEntry.Builder builder = payload.toPb(); builder.putAllLabels(labels); @@ -472,10 +517,10 @@ com.google.logging.v2.LogEntry toPb(String projectId) { builder.setResource(resource.toPb()); } if (timestamp != null) { - builder.setTimestamp(timestampFromMillis(timestamp)); + builder.setTimestamp(JavaTimeConversions.toProtoTimestamp(timestamp)); } if (receiveTimestamp != null) { - builder.setReceiveTimestamp(timestampFromMillis(receiveTimestamp)); + builder.setReceiveTimestamp(JavaTimeConversions.toProtoTimestamp(receiveTimestamp)); } if (severity != null) { builder.setSeverity(severity.toPb()); @@ -531,16 +576,10 @@ static LogEntry fromPb(com.google.logging.v2.LogEntry entryPb) { builder.setResource(MonitoredResource.fromPb(entryPb.getResource())); } if (entryPb.hasTimestamp()) { - Long millis = millisFromTimestamp(entryPb.getTimestamp()); - if (millis != 0) { - builder.setTimestamp(millis); - } + builder.setTimestamp(JavaTimeConversions.toJavaInstant(entryPb.getTimestamp())); } if (entryPb.hasReceiveTimestamp()) { - Long millis = millisFromTimestamp(entryPb.getReceiveTimestamp()); - if (millis != 0) { - builder.setReceiveTimestamp(millis); - } + builder.setReceiveTimestamp(JavaTimeConversions.toJavaInstant(entryPb.getReceiveTimestamp())); } if (!entryPb.getInsertId().equals("")) { builder.setInsertId(entryPb.getInsertId()); diff --git a/google-cloud-logging/src/test/java/com/google/cloud/logging/LogEntryTest.java b/google-cloud-logging/src/test/java/com/google/cloud/logging/LogEntryTest.java index 0342dc99d..8adc1765f 100644 --- a/google-cloud-logging/src/test/java/com/google/cloud/logging/LogEntryTest.java +++ b/google-cloud-logging/src/test/java/com/google/cloud/logging/LogEntryTest.java @@ -27,6 +27,7 @@ import com.google.common.collect.ImmutableMap; import com.google.protobuf.Any; import com.google.protobuf.Empty; +import java.time.Instant; import java.util.Map; import org.junit.Test; @@ -37,8 +38,8 @@ public class LogEntryTest { MonitoredResource.newBuilder("cloudsql_database") .setLabels(ImmutableMap.of("datasetId", "myDataset", "zone", "myZone")) .build(); - private static final long TIMESTAMP = 42; - private static final long RECEIVE_TIMESTAMP = 24; + private static final Instant TIMESTAMP = Instant.ofEpochMilli(42); + private static final Instant RECEIVE_TIMESTAMP = Instant.ofEpochMilli(24); private static final Severity SEVERITY = Severity.ALERT; private static final String INSERT_ID = "insertId"; private static final HttpRequest HTTP_REQUEST = @@ -131,7 +132,9 @@ public void testOf() { assertNull(logEntry.getLogName()); assertNull(logEntry.getResource()); assertNull(logEntry.getTimestamp()); + assertNull(logEntry.getInstantTimestamp()); assertNull(logEntry.getReceiveTimestamp()); + assertNull(logEntry.getInstantReceiveTimestamp()); assertNull(logEntry.getInsertId()); assertNull(logEntry.getHttpRequest()); assertNull(logEntry.getOperation()); @@ -147,7 +150,9 @@ public void testOf() { assertEquals(ImmutableMap.of(), logEntry.getLabels()); assertEquals(ImmutableMap.of(), logEntry.getLabels()); assertNull(logEntry.getTimestamp()); + assertNull(logEntry.getInstantTimestamp()); assertNull(logEntry.getReceiveTimestamp()); + assertNull(logEntry.getInstantReceiveTimestamp()); assertNull(logEntry.getInsertId()); assertNull(logEntry.getHttpRequest()); assertNull(logEntry.getOperation()); @@ -161,8 +166,10 @@ public void testOf() { public void testBuilder() { assertEquals(LOG_NAME, STRING_ENTRY.getLogName()); assertEquals(RESOURCE, STRING_ENTRY.getResource()); - assertEquals(TIMESTAMP, (long) STRING_ENTRY.getTimestamp()); - assertEquals(RECEIVE_TIMESTAMP, (long) STRING_ENTRY.getReceiveTimestamp()); + assertEquals(TIMESTAMP.toEpochMilli(), (long) STRING_ENTRY.getTimestamp()); + assertEquals(TIMESTAMP, STRING_ENTRY.getInstantTimestamp()); + assertEquals(RECEIVE_TIMESTAMP.toEpochMilli(), (long) STRING_ENTRY.getReceiveTimestamp()); + assertEquals(RECEIVE_TIMESTAMP, STRING_ENTRY.getInstantReceiveTimestamp()); assertEquals(SEVERITY, STRING_ENTRY.getSeverity()); assertEquals(INSERT_ID, STRING_ENTRY.getInsertId()); assertEquals(HTTP_REQUEST, STRING_ENTRY.getHttpRequest()); @@ -175,8 +182,10 @@ public void testBuilder() { assertEquals(STRING_PAYLOAD, STRING_ENTRY.getPayload()); assertEquals(LOG_NAME, JSON_ENTRY.getLogName()); assertEquals(RESOURCE, JSON_ENTRY.getResource()); - assertEquals(TIMESTAMP, (long) JSON_ENTRY.getTimestamp()); - assertEquals(RECEIVE_TIMESTAMP, (long) JSON_ENTRY.getReceiveTimestamp()); + assertEquals(TIMESTAMP.toEpochMilli(), (long) JSON_ENTRY.getTimestamp()); + assertEquals(TIMESTAMP, JSON_ENTRY.getInstantTimestamp()); + assertEquals(RECEIVE_TIMESTAMP.toEpochMilli(), (long) JSON_ENTRY.getReceiveTimestamp()); + assertEquals(RECEIVE_TIMESTAMP, JSON_ENTRY.getInstantReceiveTimestamp()); assertEquals(SEVERITY, JSON_ENTRY.getSeverity()); assertEquals(INSERT_ID, JSON_ENTRY.getInsertId()); assertEquals(HTTP_REQUEST, JSON_ENTRY.getHttpRequest()); @@ -187,10 +196,13 @@ public void testBuilder() { assertEquals(TRACE_SAMPLED, JSON_ENTRY.getTraceSampled()); assertEquals(SOURCE_LOCATION, JSON_ENTRY.getSourceLocation()); assertEquals(JSON_PAYLOAD, JSON_ENTRY.getPayload()); + assertEquals(LOG_NAME, PROTO_ENTRY.getLogName()); assertEquals(RESOURCE, PROTO_ENTRY.getResource()); - assertEquals(TIMESTAMP, (long) PROTO_ENTRY.getTimestamp()); - assertEquals(RECEIVE_TIMESTAMP, (long) PROTO_ENTRY.getReceiveTimestamp()); + assertEquals(TIMESTAMP.toEpochMilli(), (long) PROTO_ENTRY.getTimestamp()); + assertEquals(TIMESTAMP, PROTO_ENTRY.getInstantTimestamp()); + assertEquals(RECEIVE_TIMESTAMP.toEpochMilli(), (long) PROTO_ENTRY.getReceiveTimestamp()); + assertEquals(RECEIVE_TIMESTAMP, PROTO_ENTRY.getInstantReceiveTimestamp()); assertEquals(SEVERITY, PROTO_ENTRY.getSeverity()); assertEquals(INSERT_ID, PROTO_ENTRY.getInsertId()); assertEquals(HTTP_REQUEST, PROTO_ENTRY.getHttpRequest()); @@ -221,8 +233,10 @@ public void testBuilder() { .build(); assertEquals(LOG_NAME, logEntry.getLogName()); assertEquals(RESOURCE, logEntry.getResource()); - assertEquals(TIMESTAMP, (long) logEntry.getTimestamp()); - assertEquals(RECEIVE_TIMESTAMP, (long) logEntry.getReceiveTimestamp()); + assertEquals(TIMESTAMP.toEpochMilli(), (long) logEntry.getTimestamp()); + assertEquals(TIMESTAMP, logEntry.getInstantTimestamp()); + assertEquals(RECEIVE_TIMESTAMP.toEpochMilli(), (long) logEntry.getReceiveTimestamp()); + assertEquals(RECEIVE_TIMESTAMP, logEntry.getInstantReceiveTimestamp()); assertEquals(SEVERITY, logEntry.getSeverity()); assertEquals(INSERT_ID, logEntry.getInsertId()); assertEquals(HTTP_REQUEST, logEntry.getHttpRequest()); @@ -314,7 +328,9 @@ private void compareLogEntry(LogEntry expected, LogEntry value) { assertEquals(expected.getLogName(), value.getLogName()); assertEquals(expected.getResource(), value.getResource()); assertEquals(expected.getTimestamp(), value.getTimestamp()); + assertEquals(expected.getInstantTimestamp(), value.getInstantTimestamp()); assertEquals(expected.getReceiveTimestamp(), value.getReceiveTimestamp()); + assertEquals(expected.getInstantReceiveTimestamp(), value.getInstantReceiveTimestamp()); assertEquals(expected.getSeverity(), value.getSeverity()); assertEquals(expected.getInsertId(), value.getInsertId()); assertEquals(expected.getHttpRequest(), value.getHttpRequest());