Skip to content
This repository has been archived by the owner on Sep 13, 2022. It is now read-only.

Spans may be lost under heavy load #150

Closed
zhongfox opened this issue Aug 23, 2017 · 1 comment
Closed

Spans may be lost under heavy load #150

zhongfox opened this issue Aug 23, 2017 · 1 comment
Labels

Comments

@zhongfox
Copy link

zhongfox commented Aug 23, 2017

this code is from https://github.com/uber/jaeger-client-node/blob/master/src/reporters/udp_sender.js
when high concurrency, spans may be dropped frequently:

    append(span: any): SenderResponse {
        let spanSize: number = this._calcSpanSize(span); 
        if (spanSize > this._maxSpanBytes) {
            return { err: true, numSpans: 1 };
        }

        this._byteBufferSize += spanSize;          // comment 1
        if (this._byteBufferSize <= this._maxSpanBytes) {
            this._batch.spans.push(span);
            if (this._byteBufferSize < this._maxSpanBytes) {
                return {err: false, numSpans: 0};
            }
            return this.flush();
        }

        let flushResponse: SenderResponse = this.flush();  // comment 2
        this._batch.spans.push(span);                                  //comment 3
        this._byteBufferSize = spanSize;
        return flushResponse;
    }

here is a example:

this._maxSpanBytes: 64696
a span lenth: 639
current _byteBufferSize: 64671
now run append(span)

at comment 1: this._byteBufferSize will be 64671+639=65310, it's bigger than this._maxSpanBytes
so it will run comment 2: this.flush()

but in this.flush():

flush(): SenderResponse {
        let numSpans: number = this._batch.spans.length;
        if (numSpans == 0) {
            return {err: false, numSpans: 0}
        }

        let bufferLen = this._byteBufferSize + this._emitSpanBatchOverhead;  //comment 4
        let thriftBuffer = new Buffer(bufferLen);   

comment 4 use this._byteBufferSize, which now is 65310, which will cause "EMSGSIZE" error, if you add a send callback:

this._client.send(thriftBuffer, 0, thriftBuffer.length, this._port, this._host, function(e, sent) {
  console.log(e)
});

and the 64671 byte span is dropped!

fix :

add this._byteBufferSize -= spanSize; before comment 2.
or maybe reconstruct it

this._byteBufferSize += spanSize; and this._batch.spans.push(span); should be atomic, but the code separated them.

@yurishkuro
Copy link
Member

yes, it's a bug, thanks for raising it.

@yurishkuro yurishkuro added the bug label Aug 24, 2017
@yurishkuro yurishkuro changed the title span lose when high concurrency Spans may be lost under heavy load Aug 27, 2017
Iuriy-Budnikov pushed a commit to agile-pm/jaeger-client-node that referenced this issue Sep 25, 2021
* feat(sdk): add ReadableSpan

* fix: use toReadableSpan for toString method

* fix: rebase master

* fix: review comments

* fix: add comment
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants