Skip to content
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

Use low precision Clock#now when computing timestamp for exemplars #6417

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
Comparing source compatibility of against
No changes.
*** MODIFIED INTERFACE: PUBLIC ABSTRACT io.opentelemetry.sdk.common.Clock (not serializable)
=== CLASS FILE FORMAT VERSION: 52.0 <- 52.0
+++ NEW METHOD: PUBLIC(+) long now(boolean)
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class SystemClockTest {

@EnabledOnJre(JRE.JAVA_8)
@Test
void millisPrecision() {
void now_millisPrecision() {
// If we test many times, we can be fairly sure we didn't just get lucky with having a rounded
// result on a higher than expected precision timestamp.
for (int i = 0; i < 100; i++) {
Expand All @@ -29,7 +29,7 @@ void millisPrecision() {

@DisabledOnJre(JRE.JAVA_8)
@Test
void microsPrecision() {
void now_microsPrecision() {
// If we test many times, we can be fairly sure we get at least one timestamp that isn't
// coincidentally rounded to millis precision.
int numHasMicros = 0;
Expand All @@ -41,4 +41,29 @@ void microsPrecision() {
}
assertThat(numHasMicros).isNotZero();
}

@Test
void now_lowPrecision() {
// If we test many times, we can be fairly sure we didn't just get lucky with having a rounded
// result on a higher than expected precision timestamp.
for (int i = 0; i < 100; i++) {
long now = SystemClock.getInstance().now(false);
assertThat(now % 1000000).isZero();
}
}

@DisabledOnJre(JRE.JAVA_8)
@Test
void now_highPrecision() {
// If we test many times, we can be fairly sure we get at least one timestamp that isn't
// coincidentally rounded to millis precision.
int numHasMicros = 0;
for (int i = 0; i < 100; i++) {
long now = SystemClock.getInstance().now(true);
if (now % 1000000 != 0) {
numHasMicros++;
}
}
assertThat(numHasMicros).isNotZero();
}
}
18 changes: 18 additions & 0 deletions sdk/common/src/main/java/io/opentelemetry/sdk/common/Clock.java
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,27 @@ static Clock getDefault() {
* // Spend time...
* long durationNanos = clock.now() - startNanos;
* }</pre>
*
* <p>Calling this is equivalent to calling {@link #now(boolean)} with {@code highPrecision=true}.
*/
long now();

/**
* Returns the current epoch timestamp in nanos from this clock.
*
* <p>This overload of {@link #now()} includes a {@code highPrecision} argument which specifies
* whether the implementation should attempt to resolve higher precision at the potential expense
* of performance. For example, in java 9+ its sometimes possible to resolve ns precision higher
* than the ms precision of {@link System#currentTimeMillis()}, but doing so incurs a performance
* penalty which some callers may wish to avoid. In contrast, we don't currently know if resolving
* ns precision is possible in java 8, regardless of the value of {@code highPrecision}.
*
* <p>See {@link #now()} javadoc for details on usage.
*/
default long now(boolean highPrecision) {
return now();
}

/**
* Returns a time measurement with nanosecond precision that can only be used to calculate elapsed
* time.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package io.opentelemetry.sdk.common;

import io.opentelemetry.sdk.internal.JavaVersionSpecific;
import java.util.concurrent.TimeUnit;
import javax.annotation.concurrent.ThreadSafe;

/**
Expand All @@ -26,7 +27,15 @@ static Clock getInstance() {

@Override
public long now() {
return JavaVersionSpecific.get().currentTimeNanos();
return now(true);
}

@Override
public long now(boolean highPrecision) {
jack-berg marked this conversation as resolved.
Show resolved Hide resolved
if (highPrecision) {
return JavaVersionSpecific.get().currentTimeNanos();
}
return TimeUnit.MILLISECONDS.toNanos(System.currentTimeMillis());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.sdk.metrics;

import io.opentelemetry.sdk.common.Clock;
import java.util.concurrent.TimeUnit;
import org.openjdk.jmh.annotations.Benchmark;
import org.openjdk.jmh.annotations.BenchmarkMode;
import org.openjdk.jmh.annotations.Fork;
import org.openjdk.jmh.annotations.Measurement;
import org.openjdk.jmh.annotations.Mode;
import org.openjdk.jmh.annotations.OutputTimeUnit;
import org.openjdk.jmh.annotations.Warmup;

/**
* {@code io.opentelemetry.sdk.metrics.internal.exemplar.ReservoirCell} relies on {@link Clock} to
* obtain the measurement time when storing exemplar values. This benchmark illustrates the
* performance impact of using the higher precision {@link Clock#now()} instead of {@link
* Clock#now(boolean)} with {@code highPrecision=false}.
*/
@BenchmarkMode({Mode.AverageTime})
@OutputTimeUnit(TimeUnit.NANOSECONDS)
@Warmup(iterations = 5, time = 1)
@Measurement(iterations = 10, time = 1)
@Fork(1)
public class ExemplarClockBenchmarks {

private static final Clock clock = Clock.getDefault();

@SuppressWarnings("ReturnValueIgnored")
@Benchmark
public void now_lowPrecision() {
clock.now(false);
}

@SuppressWarnings("ReturnValueIgnored")
@Benchmark
public void now_highPrecision() {
clock.now(true);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ synchronized void recordDoubleMeasurement(double value, Attributes attributes, C

private void offerMeasurement(Attributes attributes, Context context) {
this.attributes = attributes;
// Note: It may make sense in the future to attempt to pull this from an active span.
this.recordTime = clock.now();
// High precision time is not worth the additional performance expense it incurs for exemplars
Copy link
Member

Choose a reason for hiding this comment

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

👍

this.recordTime = clock.now(/* highPrecision= */ false);
Span current = Span.fromContext(context);
if (current.getSpanContext().isValid()) {
this.spanContext = current.getSpanContext();
Expand Down
Loading