search

tryfer / node-tryfer

A Node Zipkin Tracer Library.
Apache License 2.0
44 stars 27 forks source link

fix RawZipkinTracer when traces.length > 10 #16

Closed wadey closed 10 years ago

wadey commented 10 years ago

I noticed in my testing that using node-tryfer with RawZipkinTracer directly worked, but using ZipkinTracer with the buffering casued traces to go missing. After some digging I discovered that RawZipkinTracer.sendTraces is doing:

async.forEachLimit(traces, 10, this._sendTrace.bind(this),
                  callback);

The problem is that the _sendTrace method wasn't accepting a callback, so if there are more than 10 traces they just get stuck in limbo forever. This patch fixes _sendTrace to use the callback that is sent to it.

cyli commented 10 years ago

Thank you so much for finding and fixing this bug!

In an attempt to verify and prevent regressions in the future, I added a test for this case in https://github.com/racker/node-tryfer/pull/17, and discovered another related bug in the meanwhile, which I think I've fixed. Would you mind having a look?