Skip to content

Commit

Permalink
Adds hack instructions for those in a pinch (#73)
Browse files Browse the repository at this point in the history
It is important for edge cases to stay edge cases. If someone is
invoking a builder, it is safe to assume they can manage the lifecycle
of what they are building. In the odd case they can't, it is better to
provide instructions vs absorb tech debt here.

This shows how to hack around lifecycle. It also fixes a subtle ordering
bug.
  • Loading branch information
adriancole authored Aug 10, 2019
1 parent cf0d7fe commit bbdfe45
Showing 1 changed file with 44 additions and 10 deletions.
54 changes: 44 additions & 10 deletions core/src/main/java/zipkin2/finagle/ZipkinTracer.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import java.io.IOException;
import scala.Option;
import scala.Some;
import scala.runtime.AbstractFunction1;
import scala.runtime.BoxedUnit;
import zipkin2.Span;
import zipkin2.internal.Nullable;
Expand Down Expand Up @@ -90,8 +91,28 @@ public class ZipkinTracer extends SamplingTracer implements Closable {
* tracer = ZipkinTracer.newBuilder(spanReporter).stats(zipkinStats).build();
* }</pre>
*
* <p><em>Note</em>: You must close the supplied sender externally, after this instance is
* closed.
* <h3>On closing resources</h3>
* The resulting tracer will attempt to close an underlying reporter if it implements {@link
* Closeable}. It is best to use normal tools like pre-destroy hooks to close resources in your
* application. If you somehow cannot control your resources, yet can invoke this, consider
* wrapping the input as a closeable to coordinate an ordered shutdown.
*
* <p>Ex.
* <pre>{@code
* class ReporterThatClosesSender implements Reporter<Span>, Closeable {
* final Sender sender;
* final AsyncReporter<Span> reporter;
*
* @Override public void close() throws IOException {
* reporter.close();
* sender.close();
* }
*
* @Override public void report(Span span) {
* reporter.report(span);
* }
* }
* }</pre>
*/
public static Builder newBuilder(Reporter<Span> spanReporter) {
return new Builder(spanReporter);
Expand Down Expand Up @@ -120,14 +141,26 @@ private ZipkinTracer(Reporter<Span> reporter, RawZipkinTracer underlying, Config
return close(Time.Bottom());
}

/**
* There are two queues here. The recorder has in-flight data about operations not yet complete.
* The reporter (usually) has a queue of spans for operations completed, not yet sent to Zipkin.
*
* <p>The close process tries to avoid dropping data on the floor by first flushing any in-flight
* operations in the recorder (ideally none), then any spans waiting for the next send interval to
* Zipkin.
*/
@Override public Future<BoxedUnit> close(Time deadline) {
if (reporter instanceof Closeable) {
try {
((Closeable) reporter).close();
} catch (IOException | RuntimeException ignored) {
Future<BoxedUnit> result = underlying.recorder.close(deadline);
if (!(reporter instanceof Closeable)) return result;
return result.onSuccess(new AbstractFunction1<BoxedUnit, BoxedUnit>() {
@Override public BoxedUnit apply(BoxedUnit v1) {
try {
((Closeable) reporter).close();
} catch (IOException | RuntimeException ignored) {
}
return BoxedUnit.UNIT;
}
}
return underlying.recorder.close(deadline);
});
}

@Override public Future<BoxedUnit> close(Duration after) {
Expand All @@ -136,6 +169,7 @@ private ZipkinTracer(Reporter<Span> reporter, RawZipkinTracer underlying, Config

protected interface Config {
@Nullable String localServiceName();

/** How much data to collect. Default sample rate 0.001 (0.1%). Max is 1, min 0. */
float initialSampleRate();
}
Expand Down Expand Up @@ -183,8 +217,8 @@ public static final class Builder {
}

/**
* Lower-case label of the remote node in the service graph, such as "favstar". Avoid names
* with variables or unique identifiers embedded.
* Lower-case label of the remote node in the service graph, such as "favstar". Avoid names with
* variables or unique identifiers embedded.
*
* <p>When unset, the service name is derived from {@link Annotation.ServiceName} which is
* often incorrectly set to the remote service name.
Expand Down

0 comments on commit bbdfe45

Please sign in to comment.