Skip to content

Commit

Permalink
chore(#362): refactors flush setup based on @adriancole's feedback.
Browse files Browse the repository at this point in the history
  • Loading branch information
jcchavezs committed Jul 21, 2019
1 parent c8fdc59 commit 9475e51
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 21 deletions.
4 changes: 4 additions & 0 deletions packages/zipkin/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,10 @@ declare namespace zipkin {
class BatchRecorder implements Recorder {
constructor(args: { logger: Logger, timeout?: number });
record: (rec: Record) => void;

/**
* Calling this will flush any pending spans to the transport.
*/
flush: () => void;
}

Expand Down
19 changes: 2 additions & 17 deletions packages/zipkin/src/batch-recorder.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,6 @@ const {Span, Endpoint} = require('./model');
*/
const defaultTimeout = 60 * 1000000;

/**
* default batch interval 1 second (in miliseconds)
* @type {number}
*/
const defaultBatchInterval = 1000;

/**
* defaultTags property name
* @type {symbol}
Expand Down Expand Up @@ -69,19 +63,13 @@ class BatchRecorder {
* @param {Object} options
* @property {Logger} logger logs the data to zipkin server
* @property {number} timeout timeout for span in microseconds
* @property {number} batchInterval interval for reporting in miliseconds.
* The value -1 means that there won't be batch reporting on interval basis.
*/
constructor({logger, timeout = defaultTimeout, batchInterval = defaultBatchInterval}) {
constructor({logger, timeout = defaultTimeout}) {
this.logger = logger;
this.timeout = timeout;
this.partialSpans = new Map();
this[defaultTagsSymbol] = {};

if (batchInterval === -1) {
return;
}

// read through the partials spans regularly
// and collect any timed-out ones
const timer = setInterval(() => {
Expand All @@ -94,7 +82,7 @@ class BatchRecorder {
this._writeSpan(id, span);
}
});
}, batchInterval);
}, 1000);
if (timer.unref) { // unref might not be available in browsers
timer.unref(); // Allows Node to terminate instead of blocking on timer
}
Expand Down Expand Up @@ -149,9 +137,6 @@ class BatchRecorder {

flush() {
this.partialSpans.forEach((span, id) => {
if (span.delegate.duration === undefined) {
span.delegate.addAnnotation(now(), 'zipkin-js.flush');
}
this._writeSpan(id, span);
});
}
Expand Down
5 changes: 1 addition & 4 deletions packages/zipkin/test/batch-recorder.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -245,17 +245,14 @@ describe('Batch Recorder', () => {
trace.recordAnnotation(new Annotation.ServerRecv());
});

clock.tick('01:00'); // 1 minute is the default timeout
clock.tick(100); // 1000 is de batching interval

expect(logSpan.calledOnce).to.equal(false);

recorder.flush();

expect(logSpan.calledOnce).to.equal(true);

const loggedSpan = logSpan.getCall(0).args[0];
expect(loggedSpan.annotations[0].value).to.equal('zipkin-js.flush');

clock.uninstall();
});

Expand Down

0 comments on commit 9475e51

Please sign in to comment.