Skip to content

Commit

Permalink
Deprecates SpanCustomizer.annotate(long timestamp, String value)
Browse files Browse the repository at this point in the history
This deprecates `SpanCustomizer.annotate(long timestamp, String value)`
as it is easy to create problematic timestamps, such as those skewed
within the same trace.
  • Loading branch information
Adrian Cole authored and adriancole committed Apr 4, 2018
1 parent 2dd1145 commit 04dcb56
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 8 deletions.
4 changes: 2 additions & 2 deletions brave/src/main/java/brave/CurrentSpanCustomizer.java
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ public static CurrentSpanCustomizer create(Tracing tracing) {
return tracer.currentSpanCustomizer().annotate(value);
}

/** {@inheritDoc} */
@Override public SpanCustomizer annotate(long timestamp, String value) {
/** @deprecated use {@link #annotate(String)} as this can result in clock skew */
@Deprecated @Override public SpanCustomizer annotate(long timestamp, String value) {
return tracer.currentSpanCustomizer().annotate(timestamp, value);
}
}
3 changes: 2 additions & 1 deletion brave/src/main/java/brave/NoopSpanCustomizer.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ public enum NoopSpanCustomizer implements SpanCustomizer {
return this;
}

@Override public SpanCustomizer annotate(long timestamp, String value) {
/** @deprecated use {@link #annotate(String)} as this can result in clock skew */
@Deprecated @Override public SpanCustomizer annotate(long timestamp, String value) {
return this;
}
}
3 changes: 2 additions & 1 deletion brave/src/main/java/brave/RealSpanCustomizer.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ static RealSpanCustomizer create(TraceContext context, Recorder recorder) {
return this;
}

@Override public SpanCustomizer annotate(long timestamp, String value) {
/** @deprecated use {@link #annotate(String)} as this can result in clock skew */
@Override @Deprecated public SpanCustomizer annotate(long timestamp, String value) {
recorder().annotate(context(), timestamp, value);
return this;
}
Expand Down
17 changes: 15 additions & 2 deletions brave/src/main/java/brave/Span.java
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,12 @@ public enum Kind {
*/
public abstract Span start();

/** Like {@link #start()}, except with a given timestamp in microseconds. */
/**
* Like {@link #start()}, except with a given timestamp in microseconds.
*
* <p>Take extreme care with this feature as it is easy to have incorrect timestamps. If you must
* use this, generate the timestamp using {@link Tracing#clock(TraceContext)}.
*/
public abstract Span start(long timestamp);

/** {@inheritDoc} */
Expand All @@ -83,7 +88,12 @@ public enum Kind {
/** {@inheritDoc} */
@Override public abstract Span annotate(String value);

/** {@inheritDoc} */
/**
* Like {@link #annotate(String)}, except with a given timestamp in microseconds.
*
* <p>Take extreme care with this feature as it is easy to have incorrect timestamps. If you must
* use this, generate the timestamp using {@link Tracing#clock(TraceContext)}.
*/
@Override public abstract Span annotate(long timestamp, String value);

/** {@inheritDoc} */
Expand Down Expand Up @@ -116,6 +126,9 @@ public final Span remoteEndpoint(zipkin.Endpoint endpoint) {
*
* <p>{@link zipkin.Span#duration Zipkin's span duration} is derived by subtracting the start
* timestamp from this, and set when appropriate.
*
* <p>Take extreme care with this feature as it is easy to have incorrect timestamps. If you must
* use this, generate the timestamp using {@link Tracing#clock(TraceContext)}.
*/
// Design note: This differs from Brave 3's LocalTracer which completes with a given duration.
// This was changed for a few use cases.
Expand Down
6 changes: 4 additions & 2 deletions brave/src/main/java/brave/SpanCustomizer.java
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ public interface SpanCustomizer {
*/
SpanCustomizer annotate(String value);

/** Like {@link #annotate(String)}, except with a given timestamp in microseconds. */
SpanCustomizer annotate(long timestamp, String value);
/** @deprecated use {@link #annotate(String)} as this can result in clock skew */
// BRAVE5: remove this: backdating ops should only be available on Span, as it isn't reasonable
// for those only having a reference to SpanCustomizer to have a correct clock for the trace.
@Deprecated SpanCustomizer annotate(long timestamp, String value);
}

0 comments on commit 04dcb56

Please sign in to comment.