Do not export zipkin exporter calls to zipkin endpoint #167
Conversation
lmolkova
requested review from
austinlparker,
discostu105 and
SergeyKanzhelev
as code owners
| if (shouldExport) | ||
| { | ||
| var zipkinSpan = this.GenerateSpan(data, this.localEndpoint); | ||
| zipkinSpans.Add(zipkinSpan); |
There was a problem hiding this comment.
@lmolkova I might be reading the code incorrectly, but there appears to be a possibility that SendSpansAsync can be called with an empty span list, resulting in a HTTP call to Zipkin, which still causes a invisible loop.
There was a problem hiding this comment.
@johnduhart sorry, this is merged already...
Would you mind sending very short PR to return from ExportAsync immediately if Span batch is empty?
It seems it should not ever happen (but seems to be reasonable anyway), see here:
And also after loop runs we should make sure zipkinSpans are not empty.
There was a problem hiding this comment.
And also after loop runs we should make sure zipkinSpans are not empty.
This was my main concern, given we're selectively including spans in this method.
Sure, I'll cut a PR to add that.
Fix #164 #134 #108
This is temp fix, we should do #166 to fix it permanently