-
Notifications
You must be signed in to change notification settings - Fork 46
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Track dropped spans #11
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ | |
#include <chrono> | ||
|
||
#include "zipkin_core_constants.h" | ||
#include "zipkin_reporter_impl.h" | ||
#include <zipkin/utility.h> | ||
|
||
namespace zipkin { | ||
|
@@ -108,7 +109,26 @@ void Tracer::reportSpan(Span &&span) { | |
} | ||
} | ||
|
||
void Tracer::setReporter(ReporterPtr reporter) { | ||
reporter_ = std::move(reporter); | ||
uint64_t Tracer::droppedSpanCount() const { | ||
return reporter_->droppedSpanCount(); | ||
} | ||
|
||
uint64_t Tracer::getAndResetDroppedSpanCount() { | ||
return reporter_->getAndResetDroppedSpanCount(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What would you think of adding a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that makes sense. |
||
} | ||
|
||
SteadyClock::duration Tracer::reportPeriod() const { | ||
return reporter_->reportPeriod(); | ||
} | ||
|
||
void Tracer::setReportPeriod(SteadyClock::duration report_period) { | ||
reporter_->setReportPeriod(report_period); | ||
} | ||
|
||
size_t Tracer::bufferSpanCount() const { return reporter_->bufferSpanCount(); } | ||
|
||
void Tracer::setBufferSpanCount(size_t span_count) { | ||
reporter_->setBufferSpanCount(span_count); | ||
} | ||
|
||
} // namespace zipkin |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,7 +21,12 @@ void ReporterImpl::reportSpan(const Span &span) { | |
bool is_full; | ||
{ | ||
std::lock_guard<std::mutex> lock(write_mutex_); | ||
num_spans_reported_ += spans_.addSpan(span); | ||
|
||
if (spans_.addSpan(span)) { | ||
num_spans_reported_++; | ||
} else { | ||
dropped_span_count_++; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This might be able to have loser memory ordering because of the mutex, but that might be a premature optimization. |
||
} | ||
is_full = spans_.pendingSpans() == max_buffered_spans_; | ||
} | ||
if (is_full) | ||
|
@@ -41,6 +46,12 @@ bool ReporterImpl::flushWithTimeout( | |
}); | ||
} | ||
|
||
uint64_t ReporterImpl::droppedSpanCount() const { return dropped_span_count_; } | ||
|
||
uint64_t ReporterImpl::getAndResetDroppedSpanCount() { | ||
return dropped_span_count_.exchange(0); | ||
} | ||
|
||
void ReporterImpl::makeWriterExit() { | ||
std::lock_guard<std::mutex> lock(write_mutex_); | ||
write_exit_ = true; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -49,9 +49,46 @@ class ReporterImpl : public Reporter { | |
|
||
bool flushWithTimeout(std::chrono::system_clock::duration timeout) override; | ||
|
||
uint64_t droppedSpanCount() const override; | ||
|
||
uint64_t getAndResetDroppedSpanCount() override; | ||
|
||
/** | ||
* @brief getReportPeriod Returns the period of time to wait between auto | ||
* flushes | ||
* @return Setting for the duration between auto flushes | ||
*/ | ||
SteadyClock::duration reportPeriod() const override { | ||
return reporting_period_; | ||
} | ||
|
||
/** | ||
* @brief setReportPeriod Adjust the period of time to wait between auto | ||
* flushes | ||
* @param report_period The new size of the span buffer | ||
*/ | ||
void setReportPeriod(SteadyClock::duration reporting_period) override { | ||
reporting_period_ = reporting_period; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This has a race condition, no? Since |
||
} | ||
|
||
/** | ||
* @brief getBufferSpanCount Returns the number of spans that can be stored | ||
* @return | ||
*/ | ||
size_t bufferSpanCount() const override { return max_buffered_spans_; } | ||
|
||
/** | ||
* @brief setBufferSpanCount Adjust the number of spans that can be stored | ||
* @param span_count The new size of the span buffer | ||
*/ | ||
void setBufferSpanCount(size_t span_count) override { | ||
max_buffered_spans_ = span_count; | ||
} | ||
|
||
private: | ||
TransporterPtr transporter_; | ||
|
||
std::atomic<uint64_t> dropped_span_count_{0}; | ||
std::mutex write_mutex_; | ||
std::condition_variable write_cond_; | ||
SteadyClock::duration reporting_period_; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we rename
bufferSpanCount
tomaxBufferedSpans
orspanBufferSize
? -- the name doesn't seem quite right.