From ce1cd9c3bd740017e72e1c6896688e4031543113 Mon Sep 17 00:00:00 2001 From: Bogdan Drutu Date: Mon, 28 Oct 2019 19:51:39 -0700 Subject: [PATCH] Remove Timestamp and change start/end times to use epoch nanos (#646) Signed-off-by: Bogdan Drutu --- .../inmemory/InMemorySpanExporterTest.java | 5 +- .../exporters/jaeger/Adapter.java | 5 +- .../exporters/jaeger/AdapterTest.java | 27 +--- .../jaeger/JaegerGrpcSpanExporterTest.java | 8 +- .../logging/LoggingExporterTest.java | 5 +- .../exporters/otprotocol/TraceProtoUtils.java | 14 -- .../opentracingshim/testbed/TestUtils.java | 23 +--- .../opentelemetry/sdk/common/Timestamp.java | 130 ------------------ .../sdk/internal/MonotonicClock.java | 2 +- .../sdk/trace/RecordEventsReadableSpan.java | 7 +- .../io/opentelemetry/sdk/trace/SpanData.java | 31 ++--- .../opentelemetry/sdk/trace/TimedEvent.java | 2 +- .../sdk/common/TimestampTest.java | 126 ----------------- .../trace/RecordEventsReadableSpanTest.java | 25 ++-- .../opentelemetry/sdk/trace/SpanDataTest.java | 8 +- .../io/opentelemetry/sdk/trace/TestUtils.java | 6 +- .../sdk/contrib/trace/testbed/TestUtils.java | 23 +--- 17 files changed, 60 insertions(+), 387 deletions(-) delete mode 100644 sdk/src/main/java/io/opentelemetry/sdk/common/Timestamp.java delete mode 100644 sdk/src/test/java/io/opentelemetry/sdk/common/TimestampTest.java diff --git a/exporters/inmemory/src/test/java/io/opentelemetry/exporters/inmemory/InMemorySpanExporterTest.java b/exporters/inmemory/src/test/java/io/opentelemetry/exporters/inmemory/InMemorySpanExporterTest.java index 27f034f650..b234cfedc0 100644 --- a/exporters/inmemory/src/test/java/io/opentelemetry/exporters/inmemory/InMemorySpanExporterTest.java +++ b/exporters/inmemory/src/test/java/io/opentelemetry/exporters/inmemory/InMemorySpanExporterTest.java @@ -18,7 +18,6 @@ package io.opentelemetry.exporters.inmemory; import static com.google.common.truth.Truth.assertThat; -import io.opentelemetry.sdk.common.Timestamp; import io.opentelemetry.sdk.trace.SpanData; import io.opentelemetry.sdk.trace.TracerSdk; import io.opentelemetry.sdk.trace.export.SimpleSpansProcessor; @@ -104,9 +103,9 @@ public class InMemorySpanExporterTest { .setSpanId(io.opentelemetry.trace.SpanId.getInvalid()) .setName("span") .setKind(io.opentelemetry.trace.Span.Kind.SERVER) - .setStartTimestamp(Timestamp.create(100, 100)) + .setStartEpochNanos(100_000_000_100L) .setStatus(io.opentelemetry.trace.Status.OK) - .setEndTimestamp(Timestamp.create(200, 200)) + .setEndEpochNanos(200_000_000_200L) .build(); } } diff --git a/exporters/jaeger/src/main/java/io/opentelemetry/exporters/jaeger/Adapter.java b/exporters/jaeger/src/main/java/io/opentelemetry/exporters/jaeger/Adapter.java index b1fcf2893d..cfa507b477 100644 --- a/exporters/jaeger/src/main/java/io/opentelemetry/exporters/jaeger/Adapter.java +++ b/exporters/jaeger/src/main/java/io/opentelemetry/exporters/jaeger/Adapter.java @@ -69,11 +69,10 @@ final class Adapter { target.setTraceId(TraceProtoUtils.toProtoTraceId(span.getTraceId())); target.setSpanId(TraceProtoUtils.toProtoSpanId(span.getSpanId())); target.setOperationName(span.getName()); - Timestamp startTimestamp = TraceProtoUtils.toProtoTimestamp(span.getStartTimestamp()); + Timestamp startTimestamp = Timestamps.fromNanos(span.getStartEpochNanos()); target.setStartTime(startTimestamp); target.setDuration( - Timestamps.between( - startTimestamp, TraceProtoUtils.toProtoTimestamp(span.getEndTimestamp()))); + Timestamps.between(startTimestamp, Timestamps.fromNanos(span.getEndEpochNanos()))); target.addAllTags(toKeyValues(span.getAttributes())); target.addAllLogs(toJaegerLogs(span.getTimedEvents())); diff --git a/exporters/jaeger/src/test/java/io/opentelemetry/exporters/jaeger/AdapterTest.java b/exporters/jaeger/src/test/java/io/opentelemetry/exporters/jaeger/AdapterTest.java index ee94e2b390..d25f32e44c 100644 --- a/exporters/jaeger/src/test/java/io/opentelemetry/exporters/jaeger/AdapterTest.java +++ b/exporters/jaeger/src/test/java/io/opentelemetry/exporters/jaeger/AdapterTest.java @@ -22,9 +22,9 @@ import static org.junit.Assert.assertTrue; import com.google.common.collect.ImmutableMap; import com.google.protobuf.util.Durations; +import com.google.protobuf.util.Timestamps; import io.opentelemetry.exporters.jaeger.proto.api_v2.Model; import io.opentelemetry.exporters.otprotocol.TraceProtoUtils; -import io.opentelemetry.sdk.common.Timestamp; import io.opentelemetry.sdk.resources.Resource; import io.opentelemetry.sdk.trace.SpanData; import io.opentelemetry.sdk.trace.SpanData.TimedEvent; @@ -58,10 +58,8 @@ public class AdapterTest { long duration = 900; // ms long startMs = System.currentTimeMillis(); long endMs = startMs + duration; - Timestamp startTime = toTimestamp(startMs); - Timestamp endTime = toTimestamp(endMs); - SpanData span = getSpanData(startTime, endTime); + SpanData span = getSpanData(startMs, endMs); List spans = Collections.singletonList(span); Collection jaegerSpans = Adapter.toJaeger(spans); @@ -75,22 +73,15 @@ public class AdapterTest { long duration = 900; // ms long startMs = System.currentTimeMillis(); long endMs = startMs + duration; - Timestamp startTime = toTimestamp(startMs); - Timestamp endTime = toTimestamp(endMs); - SpanData span = getSpanData(startTime, endTime); + SpanData span = getSpanData(startMs, endMs); // test Model.Span jaegerSpan = Adapter.toJaeger(span); assertEquals(TraceProtoUtils.toProtoTraceId(span.getTraceId()), jaegerSpan.getTraceId()); assertEquals(TraceProtoUtils.toProtoSpanId(span.getSpanId()), jaegerSpan.getSpanId()); assertEquals("GET /api/endpoint", jaegerSpan.getOperationName()); - assertEquals( - com.google.protobuf.Timestamp.newBuilder() - .setSeconds(startTime.getSeconds()) - .setNanos(startTime.getNanos()) - .build(), - jaegerSpan.getStartTime()); + assertEquals(Timestamps.fromMillis(startMs), jaegerSpan.getStartTime()); assertEquals(duration, Durations.toMillis(jaegerSpan.getDuration())); assertEquals(4, jaegerSpan.getTagsCount()); @@ -223,7 +214,7 @@ public class AdapterTest { return TimedEvent.create(epochNanos, "the log message", attributes); } - private static SpanData getSpanData(Timestamp startTime, Timestamp endTime) { + private static SpanData getSpanData(long startMs, long endMs) { AttributeValue valueB = AttributeValue.booleanAttributeValue(true); Map attributes = ImmutableMap.of("valueB", valueB); @@ -234,8 +225,8 @@ public class AdapterTest { .setSpanId(SpanId.fromLowerBase16(SPAN_ID, 0)) .setParentSpanId(SpanId.fromLowerBase16(PARENT_SPAN_ID, 0)) .setName("GET /api/endpoint") - .setStartTimestamp(startTime) - .setEndTimestamp(endTime) + .setStartEpochNanos(TimeUnit.MILLISECONDS.toNanos(startMs)) + .setEndEpochNanos(TimeUnit.MILLISECONDS.toNanos(endMs)) .setAttributes(attributes) .setTimedEvents(Collections.singletonList(getTimedEvent())) .setLinks(Collections.singletonList(link)) @@ -253,10 +244,6 @@ public class AdapterTest { Tracestate.builder().build()); } - private static Timestamp toTimestamp(long ms) { - return Timestamp.create(ms / 1000, (int) ((ms % 1000) * 1000000)); - } - @Nullable private static Model.KeyValue getValue(List tagsList, String s) { for (Model.KeyValue kv : tagsList) { diff --git a/exporters/jaeger/src/test/java/io/opentelemetry/exporters/jaeger/JaegerGrpcSpanExporterTest.java b/exporters/jaeger/src/test/java/io/opentelemetry/exporters/jaeger/JaegerGrpcSpanExporterTest.java index e975c7488d..ac0089d916 100644 --- a/exporters/jaeger/src/test/java/io/opentelemetry/exporters/jaeger/JaegerGrpcSpanExporterTest.java +++ b/exporters/jaeger/src/test/java/io/opentelemetry/exporters/jaeger/JaegerGrpcSpanExporterTest.java @@ -33,7 +33,6 @@ import io.opentelemetry.exporters.jaeger.proto.api_v2.Collector.PostSpansRequest import io.opentelemetry.exporters.jaeger.proto.api_v2.CollectorServiceGrpc; import io.opentelemetry.exporters.jaeger.proto.api_v2.Model; import io.opentelemetry.exporters.otprotocol.TraceProtoUtils; -import io.opentelemetry.sdk.common.Timestamp; import io.opentelemetry.sdk.trace.SpanData; import io.opentelemetry.trace.Link; import io.opentelemetry.trace.Span.Kind; @@ -42,6 +41,7 @@ import io.opentelemetry.trace.Status; import io.opentelemetry.trace.TraceId; import java.net.InetAddress; import java.util.Collections; +import java.util.concurrent.TimeUnit; import org.junit.Rule; import org.junit.Test; import org.mockito.ArgumentCaptor; @@ -77,15 +77,13 @@ public class JaegerGrpcSpanExporterTest { long duration = 900; // ms long startMs = System.currentTimeMillis(); long endMs = startMs + duration; - Timestamp startTime = Timestamp.create(startMs / 1000, (int) ((startMs % 1000) * 1000000)); - Timestamp endTime = Timestamp.create(endMs / 1000, (int) ((endMs % 1000) * 1000000)); SpanData span = SpanData.newBuilder() .setTraceId(TraceId.fromLowerBase16(TRACE_ID, 0)) .setSpanId(SpanId.fromLowerBase16(SPAN_ID, 0)) .setName("GET /api/endpoint") - .setStartTimestamp(startTime) - .setEndTimestamp(endTime) + .setStartEpochNanos(TimeUnit.MILLISECONDS.toNanos(startMs)) + .setEndEpochNanos(TimeUnit.MILLISECONDS.toNanos(endMs)) .setStatus(Status.OK) .setKind(Kind.CONSUMER) .setLinks(Collections.emptyList()) diff --git a/exporters/logging/src/test/java/io/opentelemetry/exporters/logging/LoggingExporterTest.java b/exporters/logging/src/test/java/io/opentelemetry/exporters/logging/LoggingExporterTest.java index 32c146b769..8bb28a3117 100644 --- a/exporters/logging/src/test/java/io/opentelemetry/exporters/logging/LoggingExporterTest.java +++ b/exporters/logging/src/test/java/io/opentelemetry/exporters/logging/LoggingExporterTest.java @@ -20,7 +20,6 @@ import static java.util.Collections.singletonList; import static java.util.Collections.singletonMap; import static org.junit.Assert.assertEquals; -import io.opentelemetry.sdk.common.Timestamp; import io.opentelemetry.sdk.trace.SpanData; import io.opentelemetry.sdk.trace.export.SpanExporter.ResultCode; import io.opentelemetry.trace.AttributeValue; @@ -40,8 +39,8 @@ public class LoggingExporterTest { SpanData.newBuilder() .setTraceId(new TraceId(1234L, 6789L)) .setSpanId(new SpanId(9876L)) - .setStartTimestamp(Timestamp.fromNanos(epochNanos)) - .setEndTimestamp(Timestamp.fromNanos(epochNanos + 1000)) + .setStartEpochNanos(epochNanos) + .setEndEpochNanos(epochNanos + 1000) .setStatus(Status.OK) .setName("testSpan") .setKind(Kind.INTERNAL) diff --git a/exporters/otprotocol/src/main/java/io/opentelemetry/exporters/otprotocol/TraceProtoUtils.java b/exporters/otprotocol/src/main/java/io/opentelemetry/exporters/otprotocol/TraceProtoUtils.java index dd77aa846c..800ced8188 100644 --- a/exporters/otprotocol/src/main/java/io/opentelemetry/exporters/otprotocol/TraceProtoUtils.java +++ b/exporters/otprotocol/src/main/java/io/opentelemetry/exporters/otprotocol/TraceProtoUtils.java @@ -17,7 +17,6 @@ package io.opentelemetry.exporters.otprotocol; import com.google.protobuf.ByteString; -import com.google.protobuf.Timestamp; import io.opentelemetry.proto.trace.v1.ConstantSampler; import io.opentelemetry.sdk.trace.Sampler; import io.opentelemetry.sdk.trace.Samplers; @@ -53,19 +52,6 @@ public class TraceProtoUtils { return ByteString.copyFrom(traceIdBytes); } - /** - * Converts a opentelemetry Timestamp into a protobuf Timestamp. - * - * @param timestamp the opentelemetry Timestamp to convert. - * @return the protobuf Timestamp representation. - */ - public static Timestamp toProtoTimestamp(io.opentelemetry.sdk.common.Timestamp timestamp) { - return Timestamp.newBuilder() - .setNanos(timestamp.getNanos()) - .setSeconds(timestamp.getSeconds()) - .build(); - } - /** * Returns a {@code TraceConfig} from the given proto. * diff --git a/opentracing_shim/src/test/java/io/opentelemetry/opentracingshim/testbed/TestUtils.java b/opentracing_shim/src/test/java/io/opentelemetry/opentracingshim/testbed/TestUtils.java index 5dd5711c4b..ee97cf1096 100644 --- a/opentracing_shim/src/test/java/io/opentelemetry/opentracingshim/testbed/TestUtils.java +++ b/opentracing_shim/src/test/java/io/opentelemetry/opentracingshim/testbed/TestUtils.java @@ -21,7 +21,6 @@ import static org.junit.Assert.assertTrue; import io.opentelemetry.exporters.inmemory.InMemorySpanExporter; import io.opentelemetry.opentracingshim.TraceShim; -import io.opentelemetry.sdk.common.Timestamp; import io.opentelemetry.sdk.distributedcontext.DistributedContextManagerSdk; import io.opentelemetry.sdk.trace.SpanData; import io.opentelemetry.sdk.trace.TracerSdk; @@ -195,7 +194,7 @@ public final class TestUtils { } /** - * Sorts the specified {@code List} of {@code Span} by their {@code Span.Timestamp} values, + * Sorts the specified {@code List} of {@code Span} by their start epoch timestamp values, * returning it as a new {@code List}. */ public static List sortByStartTime(List spans) { @@ -205,7 +204,7 @@ public final class TestUtils { new Comparator() { @Override public int compare(SpanData o1, SpanData o2) { - return compareTimestamps(o1.getStartTimestamp(), o2.getStartTimestamp()); + return Long.compare(o1.getStartEpochNanos(), o2.getStartEpochNanos()); } }); return sortedSpans; @@ -215,25 +214,9 @@ public final class TestUtils { public static void assertSameTrace(List spans) { for (int i = 0; i < spans.size() - 1; i++) { // TODO - Include nanos in this comparison. - assertTrue( - spans.get(spans.size() - 1).getEndTimestamp().getSeconds() - >= spans.get(i).getEndTimestamp().getSeconds()); + assertTrue(spans.get(spans.size() - 1).getEndEpochNanos() >= spans.get(i).getEndEpochNanos()); assertEquals(spans.get(spans.size() - 1).getTraceId(), spans.get(i).getTraceId()); assertEquals(spans.get(spans.size() - 1).getSpanId(), spans.get(i).getParentSpanId()); } } - - // TODO: this comparator code is duplicated in another TestUtils class. find a common home for it. - private static final Comparator COMPARATOR = - new Comparator() { - @Override - public int compare(Timestamp t1, Timestamp t2) { - int secDiff = Long.compare(t1.getSeconds(), t2.getSeconds()); - return (secDiff != 0) ? secDiff : Integer.compare(t1.getNanos(), t2.getNanos()); - } - }; - - public static int compareTimestamps(Timestamp x, Timestamp y) { - return COMPARATOR.compare(x, y); - } } diff --git a/sdk/src/main/java/io/opentelemetry/sdk/common/Timestamp.java b/sdk/src/main/java/io/opentelemetry/sdk/common/Timestamp.java deleted file mode 100644 index 361791e076..0000000000 --- a/sdk/src/main/java/io/opentelemetry/sdk/common/Timestamp.java +++ /dev/null @@ -1,130 +0,0 @@ -/* - * Copyright 2019, OpenTelemetry Authors - * - * 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 io.opentelemetry.sdk.common; - -import com.google.auto.value.AutoValue; -import java.math.BigDecimal; -import java.math.RoundingMode; -import javax.annotation.concurrent.Immutable; - -/** - * A representation of an instant in time. The instant is the number of nanoseconds after the number - * of seconds since the Unix Epoch. - * - *

Defined here instead of using {@code Instant} because the API needs to be Java 1.7 compatible. - * - * @since 0.1.0 - */ -@Immutable -@AutoValue -public abstract class Timestamp { - private static final long MAX_SECONDS = 315576000000L; - private static final int MAX_NANOS = 999999999; - private static final long MILLIS_PER_SECOND = 1000L; - private static final long NANOS_PER_SECOND = 1000L * 1000L * 1000L; - private static final long NANOS_PER_MILLI = 1000L * 1000L; - - Timestamp() {} - - /** - * Creates a new timestamp from given seconds and nanoseconds. - * - * @param seconds Represents seconds of UTC time since Unix epoch 1970-01-01T00:00:00Z. Must be - * from from 0001-01-01T00:00:00Z to 9999-12-31T23:59:59Z inclusive. - * @param nanos Non-negative fractions of a second at nanosecond resolution. Negative second - * values with fractions must still have non-negative nanos values that count forward in time. - * Must be from 0 to 999,999,999 inclusive. - * @return new {@code Timestamp} with specified fields. - * @throws IllegalArgumentException if the arguments are out of range. - * @since 0.1.0 - */ - public static Timestamp create(long seconds, int nanos) { - if (seconds < -MAX_SECONDS) { - throw new IllegalArgumentException( - "'seconds' is less than minimum (" + -MAX_SECONDS + "): " + seconds); - } - if (seconds > MAX_SECONDS) { - throw new IllegalArgumentException( - "'seconds' is greater than maximum (" + MAX_SECONDS + "): " + seconds); - } - if (nanos < 0) { - throw new IllegalArgumentException("'nanos' is less than zero: " + nanos); - } - if (nanos > MAX_NANOS) { - throw new IllegalArgumentException( - "'nanos' is greater than maximum (" + MAX_NANOS + "): " + nanos); - } - return new AutoValue_Timestamp(seconds, nanos); - } - - /** - * Creates a new timestamp from the given milliseconds. - * - * @param epochMilli the timestamp represented in milliseconds since epoch. - * @return new {@code Timestamp} with specified fields. - * @throws IllegalArgumentException if the number of milliseconds is out of the range that can be - * represented by {@code Timestamp}. - * @since 0.1.0 - */ - public static Timestamp fromMillis(long epochMilli) { - long secs = floorDiv(epochMilli, MILLIS_PER_SECOND); - int mos = (int) floorMod(epochMilli, MILLIS_PER_SECOND); - return create(secs, (int) (mos * NANOS_PER_MILLI)); // Safe int * NANOS_PER_MILLI - } - - /** - * Creates a new timestamp from the given milliseconds. - * - * @param epochNanos the timestamp represented in nanoseconds since epoch. - * @return new {@code Timestamp} with specified fields. - * @throws IllegalArgumentException if the number of milliseconds is out of the range that can be - * represented by {@code Timestamp}. - * @since 0.1.0 - */ - public static Timestamp fromNanos(long epochNanos) { - long secs = floorDiv(epochNanos, NANOS_PER_SECOND); - int nanos = (int) floorMod(epochNanos, NANOS_PER_SECOND); - return create(secs, nanos); // Safe int * NANOS_PER_MILLI - } - - /** - * Returns the number of seconds since the Unix Epoch represented by this timestamp. - * - * @return the number of seconds since the Unix Epoch. - * @since 0.1.0 - */ - public abstract long getSeconds(); - - /** - * Returns the number of nanoseconds after the number of seconds since the Unix Epoch represented - * by this timestamp. - * - * @return the number of nanoseconds after the number of seconds since the Unix Epoch. - * @since 0.1.0 - */ - public abstract int getNanos(); - - // Returns the result of dividing x by y rounded using floor. - private static long floorDiv(long x, long y) { - return BigDecimal.valueOf(x).divide(BigDecimal.valueOf(y), 0, RoundingMode.FLOOR).longValue(); - } - - // Returns the floor modulus "x - (floorDiv(x, y) * y)" - private static long floorMod(long x, long y) { - return x - floorDiv(x, y) * y; - } -} diff --git a/sdk/src/main/java/io/opentelemetry/sdk/internal/MonotonicClock.java b/sdk/src/main/java/io/opentelemetry/sdk/internal/MonotonicClock.java index d8c13798b7..6a2e5df237 100644 --- a/sdk/src/main/java/io/opentelemetry/sdk/internal/MonotonicClock.java +++ b/sdk/src/main/java/io/opentelemetry/sdk/internal/MonotonicClock.java @@ -23,7 +23,7 @@ import javax.annotation.concurrent.Immutable; * This class provides a mechanism for calculating the epoch time using {@link System#nanoTime()} * and a reference epoch timestamp. * - *

This is needed because Java has millisecond granularity for Timestamp and tracing events are + *

This is needed because Java has millisecond granularity for epoch times and tracing events are * recorded more often. * *

This clock needs to be re-created periodically in order to re-sync with the kernel clock, and diff --git a/sdk/src/main/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpan.java b/sdk/src/main/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpan.java index 728b9826c1..ab9b6b3862 100644 --- a/sdk/src/main/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpan.java +++ b/sdk/src/main/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpan.java @@ -20,7 +20,6 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.collect.EvictingQueue; import io.opentelemetry.sdk.common.Clock; -import io.opentelemetry.sdk.common.Timestamp; import io.opentelemetry.sdk.resources.Resource; import io.opentelemetry.sdk.trace.config.TraceConfig; import io.opentelemetry.trace.AttributeValue; @@ -149,8 +148,6 @@ final class RecordEventsReadableSpan implements ReadableSpan, Span { @Override public SpanData toSpanData() { - Timestamp startTimestamp = Timestamp.fromNanos(startEpochNanos); - Timestamp endTimestamp = Timestamp.fromNanos(getEndEpochNanos()); SpanContext spanContext = getSpanContext(); return SpanData.newBuilder() .setName(getName()) @@ -159,8 +156,8 @@ final class RecordEventsReadableSpan implements ReadableSpan, Span { .setTraceFlags(spanContext.getTraceFlags()) .setTracestate(spanContext.getTracestate()) .setAttributes(getAttributes()) - .setStartTimestamp(startTimestamp) - .setEndTimestamp(endTimestamp) + .setStartEpochNanos(startEpochNanos) + .setEndEpochNanos(getEndEpochNanos()) .setKind(kind) .setLinks(getLinks()) .setParentSpanId(parentSpanId) diff --git a/sdk/src/main/java/io/opentelemetry/sdk/trace/SpanData.java b/sdk/src/main/java/io/opentelemetry/sdk/trace/SpanData.java index 0e0d2fea2d..5e21e0107d 100644 --- a/sdk/src/main/java/io/opentelemetry/sdk/trace/SpanData.java +++ b/sdk/src/main/java/io/opentelemetry/sdk/trace/SpanData.java @@ -17,7 +17,6 @@ package io.opentelemetry.sdk.trace; import com.google.auto.value.AutoValue; -import io.opentelemetry.sdk.common.Timestamp; import io.opentelemetry.sdk.resources.Resource; import io.opentelemetry.trace.AttributeValue; import io.opentelemetry.trace.Event; @@ -107,12 +106,12 @@ public abstract class SpanData { public abstract Kind getKind(); /** - * Returns the start {@code Timestamp} of this {@code Span}. + * Returns the start epoch timestamp in nanos of this {@code Span}. * - * @return the start {@code Timestamp} of this {@code Span}. + * @return the start epoch timestamp in nanos of this {@code Span}. * @since 0.1.0 */ - public abstract Timestamp getStartTimestamp(); + public abstract long getStartEpochNanos(); /** * Returns the attributes recorded for this {@code Span}. @@ -147,12 +146,12 @@ public abstract class SpanData { public abstract Status getStatus(); /** - * Returns the end {@code Timestamp}. + * Returns the end epoch timestamp in nanos of this {@code Span}. * - * @return the end {@code Timestamp}. + * @return the end epoch timestamp in nanos of this {@code Span}. * @since 0.1.0 */ - public abstract Timestamp getEndTimestamp(); + public abstract long getEndEpochNanos(); /** * An immutable implementation of {@link Link}. @@ -211,9 +210,9 @@ public abstract class SpanData { } /** - * Returns the {@code Timestamp} of this event. + * Returns the epoch time in nanos of this event. * - * @return the {@code Timestamp} of this event. + * @return the epoch time in nanos of this event. * @since 0.1.0 */ public abstract long getEpochNanos(); @@ -336,24 +335,22 @@ public abstract class SpanData { public abstract Builder setName(String name); /** - * Set the start timestamp of the span. Must not be null. + * Set the start timestamp of the span. * - * @param timestamp the start Timestamp + * @param epochNanos the start epoch timestamp in nanos. * @return this - * @see Timestamp * @since 0.1.0 */ - public abstract Builder setStartTimestamp(Timestamp timestamp); + public abstract Builder setStartEpochNanos(long epochNanos); /** - * Set the end timestamp of the span. Must not be null. + * Set the end timestamp of the span. * - * @param timestamp the end Timestamp + * @param epochNanos the end epoch timestamp in nanos. * @return this - * @see Timestamp * @since 0.1.0 */ - public abstract Builder setEndTimestamp(Timestamp timestamp); + public abstract Builder setEndEpochNanos(long epochNanos); /** * Set the attributes that are associated with this span, as a Map of String keys to diff --git a/sdk/src/main/java/io/opentelemetry/sdk/trace/TimedEvent.java b/sdk/src/main/java/io/opentelemetry/sdk/trace/TimedEvent.java index 4211996574..2006f9674d 100644 --- a/sdk/src/main/java/io/opentelemetry/sdk/trace/TimedEvent.java +++ b/sdk/src/main/java/io/opentelemetry/sdk/trace/TimedEvent.java @@ -23,7 +23,7 @@ import java.util.Collections; import java.util.Map; import javax.annotation.concurrent.Immutable; -/** Timed event that uses nanoTime to represent the Timestamp. */ +/** Timed event. */ @Immutable abstract class TimedEvent { diff --git a/sdk/src/test/java/io/opentelemetry/sdk/common/TimestampTest.java b/sdk/src/test/java/io/opentelemetry/sdk/common/TimestampTest.java deleted file mode 100644 index 0d59cbd49b..0000000000 --- a/sdk/src/test/java/io/opentelemetry/sdk/common/TimestampTest.java +++ /dev/null @@ -1,126 +0,0 @@ -/* - * Copyright 2019, OpenTelemetry Authors - * - * 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 io.opentelemetry.sdk.common; - -import static com.google.common.truth.Truth.assertThat; - -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.ExpectedException; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; - -/** Unit tests for {@link Timestamp}. */ -@RunWith(JUnit4.class) -public class TimestampTest { - @Rule public ExpectedException thrown = ExpectedException.none(); - - @Test - public void timestampCreate() { - assertThat(Timestamp.create(24, 42).getSeconds()).isEqualTo(24); - assertThat(Timestamp.create(24, 42).getNanos()).isEqualTo(42); - assertThat(Timestamp.create(-24, 42).getSeconds()).isEqualTo(-24); - assertThat(Timestamp.create(-24, 42).getNanos()).isEqualTo(42); - assertThat(Timestamp.create(315576000000L, 999999999).getSeconds()).isEqualTo(315576000000L); - assertThat(Timestamp.create(315576000000L, 999999999).getNanos()).isEqualTo(999999999); - assertThat(Timestamp.create(-315576000000L, 999999999).getSeconds()).isEqualTo(-315576000000L); - assertThat(Timestamp.create(-315576000000L, 999999999).getNanos()).isEqualTo(999999999); - } - - @Test - public void create_SecondsTooLow() { - thrown.expect(IllegalArgumentException.class); - thrown.expectMessage("'seconds' is less than minimum (-315576000000): -315576000001"); - Timestamp.create(-315576000001L, 0); - } - - @Test - public void create_SecondsTooHigh() { - thrown.expect(IllegalArgumentException.class); - thrown.expectMessage("'seconds' is greater than maximum (315576000000): 315576000001"); - Timestamp.create(315576000001L, 0); - } - - @Test - public void create_NanosTooLow_PositiveTime() { - thrown.expect(IllegalArgumentException.class); - thrown.expectMessage("'nanos' is less than zero: -1"); - Timestamp.create(1, -1); - } - - @Test - public void create_NanosTooHigh_PositiveTime() { - thrown.expect(IllegalArgumentException.class); - thrown.expectMessage("'nanos' is greater than maximum (999999999): 1000000000"); - Timestamp.create(1, 1000000000); - } - - @Test - public void create_NanosTooLow_NegativeTime() { - thrown.expect(IllegalArgumentException.class); - thrown.expectMessage("'nanos' is less than zero: -1"); - Timestamp.create(-1, -1); - } - - @Test - public void create_NanosTooHigh_NegativeTime() { - thrown.expect(IllegalArgumentException.class); - thrown.expectMessage("'nanos' is greater than maximum (999999999): 1000000000"); - Timestamp.create(-1, 1000000000); - } - - @Test - public void timestampFromMillis() { - assertThat(Timestamp.fromMillis(0)).isEqualTo(Timestamp.create(0, 0)); - assertThat(Timestamp.fromMillis(987)).isEqualTo(Timestamp.create(0, 987000000)); - assertThat(Timestamp.fromMillis(3456)).isEqualTo(Timestamp.create(3, 456000000)); - } - - @Test - public void timestampFromMillis_Negative() { - assertThat(Timestamp.fromMillis(-1)).isEqualTo(Timestamp.create(-1, 999000000)); - assertThat(Timestamp.fromMillis(-999)).isEqualTo(Timestamp.create(-1, 1000000)); - assertThat(Timestamp.fromMillis(-3456)).isEqualTo(Timestamp.create(-4, 544000000)); - } - - @Test - public void fromMillis_TooLow() { - thrown.expect(IllegalArgumentException.class); - thrown.expectMessage("'seconds' is less than minimum (-315576000000): -315576000001"); - Timestamp.fromMillis(-315576000001000L); - } - - @Test - public void fromMillis_TooHigh() { - thrown.expect(IllegalArgumentException.class); - thrown.expectMessage("'seconds' is greater than maximum (315576000000): 315576000001"); - Timestamp.fromMillis(315576000001000L); - } - - @Test - public void timestamp_Equal() { - // Positive tests. - assertThat(Timestamp.create(0, 0)).isEqualTo(Timestamp.create(0, 0)); - assertThat(Timestamp.create(24, 42)).isEqualTo(Timestamp.create(24, 42)); - assertThat(Timestamp.create(-24, 42)).isEqualTo(Timestamp.create(-24, 42)); - // Negative tests. - assertThat(Timestamp.create(25, 42)).isNotEqualTo(Timestamp.create(24, 42)); - assertThat(Timestamp.create(24, 43)).isNotEqualTo(Timestamp.create(24, 42)); - assertThat(Timestamp.create(-25, 42)).isNotEqualTo(Timestamp.create(-24, 42)); - assertThat(Timestamp.create(-24, 43)).isNotEqualTo(Timestamp.create(-24, 42)); - } -} diff --git a/sdk/src/test/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpanTest.java b/sdk/src/test/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpanTest.java index b6939206b3..b8bba5f6b9 100644 --- a/sdk/src/test/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpanTest.java +++ b/sdk/src/test/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpanTest.java @@ -20,7 +20,6 @@ import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; -import io.opentelemetry.sdk.common.Timestamp; import io.opentelemetry.sdk.internal.TestClock; import io.opentelemetry.sdk.resources.Resource; import io.opentelemetry.sdk.trace.config.TraceConfig; @@ -101,8 +100,8 @@ public class RecordEventsReadableSpanTest { Collections.emptyList(), Collections.singletonList(link), SPAN_NAME, - Timestamp.fromNanos(startEpochNanos), - Timestamp.fromNanos(startEpochNanos), + startEpochNanos, + startEpochNanos, Status.OK); } @@ -130,8 +129,8 @@ public class RecordEventsReadableSpanTest { Collections.singletonList(timedEvent), Collections.singletonList(link), SPAN_NEW_NAME, - Timestamp.fromNanos(startEpochNanos), - Timestamp.fromNanos(testClock.now()), + startEpochNanos, + testClock.now(), Status.OK); } finally { span.end(); @@ -159,8 +158,8 @@ public class RecordEventsReadableSpanTest { Collections.singletonList(timedEvent), Collections.singletonList(link), SPAN_NEW_NAME, - Timestamp.fromNanos(startEpochNanos), - Timestamp.fromNanos(testClock.now()), + startEpochNanos, + testClock.now(), Status.CANCELLED); } @@ -473,8 +472,8 @@ public class RecordEventsReadableSpanTest { List timedEvents, List links, String spanName, - Timestamp startTime, - Timestamp endTime, + long startEpochNanos, + long endEpochNanos, Status status) { assertThat(spanData.getTraceId()).isEqualTo(traceId); assertThat(spanData.getSpanId()).isEqualTo(spanId); @@ -485,8 +484,8 @@ public class RecordEventsReadableSpanTest { assertThat(spanData.getAttributes()).isEqualTo(attributes); assertThat(spanData.getTimedEvents()).isEqualTo(timedEvents); assertThat(spanData.getLinks()).isEqualTo(links); - assertThat(spanData.getStartTimestamp()).isEqualTo(startTime); - assertThat(spanData.getEndTimestamp()).isEqualTo(endTime); + assertThat(spanData.getStartEpochNanos()).isEqualTo(startEpochNanos); + assertThat(spanData.getEndEpochNanos()).isEqualTo(endEpochNanos); assertThat(spanData.getStatus().getCanonicalCode()).isEqualTo(status.getCanonicalCode()); } @@ -542,8 +541,8 @@ public class RecordEventsReadableSpanTest { .setName(name) .setKind(kind) .setStatus(Status.OK) - .setStartTimestamp(Timestamp.fromNanos(startEpochNanos)) - .setEndTimestamp(Timestamp.fromNanos(endEpochNanos)) + .setStartEpochNanos(startEpochNanos) + .setEndEpochNanos(endEpochNanos) .setTimedEvents( Arrays.asList( SpanData.TimedEvent.create(firstEventEpochNanos, "event1", event1Attributes), diff --git a/sdk/src/test/java/io/opentelemetry/sdk/trace/SpanDataTest.java b/sdk/src/test/java/io/opentelemetry/sdk/trace/SpanDataTest.java index db2c3ed23f..1700c979b7 100644 --- a/sdk/src/test/java/io/opentelemetry/sdk/trace/SpanDataTest.java +++ b/sdk/src/test/java/io/opentelemetry/sdk/trace/SpanDataTest.java @@ -20,7 +20,6 @@ import static java.util.Collections.emptyList; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; -import io.opentelemetry.sdk.common.Timestamp; import io.opentelemetry.sdk.trace.SpanData.TimedEvent; import io.opentelemetry.trace.AttributeValue; import io.opentelemetry.trace.Link; @@ -34,6 +33,7 @@ import io.opentelemetry.trace.Tracestate; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; +import java.util.concurrent.TimeUnit; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; @@ -43,6 +43,8 @@ import org.junit.runners.JUnit4; /** Unit tests for {@link SpanData}. */ @RunWith(JUnit4.class) public class SpanDataTest { + private static final long START_EPOCH_NANOS = TimeUnit.SECONDS.toNanos(3000) + 200; + private static final long END_EPOCH_NANOS = TimeUnit.SECONDS.toNanos(3001) + 255; @Rule public final ExpectedException thrown = ExpectedException.none(); @Test @@ -103,8 +105,8 @@ public class SpanDataTest { .setSpanId(SpanId.getInvalid()) .setTraceId(TraceId.getInvalid()) .setName("spanName") - .setStartTimestamp(Timestamp.create(3000, 200)) - .setEndTimestamp(Timestamp.create(3001, 255)) + .setStartEpochNanos(START_EPOCH_NANOS) + .setEndEpochNanos(END_EPOCH_NANOS) .setKind(Kind.SERVER) .setStatus(Status.OK); } diff --git a/sdk/src/test/java/io/opentelemetry/sdk/trace/TestUtils.java b/sdk/src/test/java/io/opentelemetry/sdk/trace/TestUtils.java index b724b6d7e7..63eb9204cf 100644 --- a/sdk/src/test/java/io/opentelemetry/sdk/trace/TestUtils.java +++ b/sdk/src/test/java/io/opentelemetry/sdk/trace/TestUtils.java @@ -16,7 +16,6 @@ package io.opentelemetry.sdk.trace; -import io.opentelemetry.sdk.common.Timestamp; import io.opentelemetry.sdk.trace.config.TraceConfig; import io.opentelemetry.trace.AttributeValue; import io.opentelemetry.trace.Span; @@ -27,6 +26,7 @@ import io.opentelemetry.trace.TraceId; import java.util.HashMap; import java.util.Map; import java.util.UUID; +import java.util.concurrent.TimeUnit; /** Common utilities for unit tests. */ public final class TestUtils { @@ -57,9 +57,9 @@ public final class TestUtils { .setSpanId(SpanId.getInvalid()) .setName("span") .setKind(Kind.SERVER) - .setStartTimestamp(Timestamp.create(100, 100)) + .setStartEpochNanos(TimeUnit.SECONDS.toNanos(100) + 100) .setStatus(Status.OK) - .setEndTimestamp(Timestamp.create(200, 200)) + .setEndEpochNanos(TimeUnit.SECONDS.toNanos(200) + 200) .build(); } diff --git a/sdk_contrib/testbed/src/test/java/io/opentelemetry/sdk/contrib/trace/testbed/TestUtils.java b/sdk_contrib/testbed/src/test/java/io/opentelemetry/sdk/contrib/trace/testbed/TestUtils.java index e933386c17..70eb43f028 100644 --- a/sdk_contrib/testbed/src/test/java/io/opentelemetry/sdk/contrib/trace/testbed/TestUtils.java +++ b/sdk_contrib/testbed/src/test/java/io/opentelemetry/sdk/contrib/trace/testbed/TestUtils.java @@ -19,7 +19,6 @@ package io.opentelemetry.sdk.contrib.trace.testbed; import static com.google.common.truth.Truth.assertThat; import io.opentelemetry.exporters.inmemory.InMemorySpanExporter; -import io.opentelemetry.sdk.common.Timestamp; import io.opentelemetry.sdk.trace.SpanData; import io.opentelemetry.sdk.trace.TracerSdk; import io.opentelemetry.sdk.trace.export.SimpleSpansProcessor; @@ -191,7 +190,7 @@ public final class TestUtils { } /** - * Sorts the specified {@code List} of {@code Span} by their {@code Span.Timestamp} values, + * Sorts the specified {@code List} of {@code Span} by their start epoch timestamp values, * returning it as a new {@code List}. */ public static List sortByStartTime(List spans) { @@ -201,7 +200,7 @@ public final class TestUtils { new Comparator() { @Override public int compare(SpanData o1, SpanData o2) { - return compareTimestamps(o1.getStartTimestamp(), o2.getStartTimestamp()); + return Long.compare(o1.getStartEpochNanos(), o2.getStartEpochNanos()); } }); return sortedSpans; @@ -211,26 +210,10 @@ public final class TestUtils { public static void assertSameTrace(List spans) { for (int i = 0; i < spans.size() - 1; i++) { // TODO - Include nanos in this comparison. - assertThat( - spans.get(spans.size() - 1).getEndTimestamp().getSeconds() - >= spans.get(i).getEndTimestamp().getSeconds()) + assertThat(spans.get(spans.size() - 1).getEndEpochNanos() >= spans.get(i).getEndEpochNanos()) .isTrue(); assertThat(spans.get(spans.size() - 1).getTraceId()).isEqualTo(spans.get(i).getTraceId()); assertThat(spans.get(spans.size() - 1).getSpanId()).isEqualTo(spans.get(i).getParentSpanId()); } } - - // TODO: this comparator code is duplicated in another TestUtils class. find a common home for it. - private static final Comparator COMPARATOR = - new Comparator() { - @Override - public int compare(Timestamp t1, Timestamp t2) { - int secDiff = Long.compare(t1.getSeconds(), t2.getSeconds()); - return (secDiff != 0) ? secDiff : Integer.compare(t1.getNanos(), t2.getNanos()); - } - }; - - public static int compareTimestamps(Timestamp x, Timestamp y) { - return COMPARATOR.compare(x, y); - } }