From f27c912f3393d03d2ab47826637b28055aaa7a55 Mon Sep 17 00:00:00 2001 From: Isaac Hier Date: Fri, 9 Feb 2018 10:40:17 -0500 Subject: [PATCH] Fix span dropping in RemoteReporter Signed-off-by: Isaac Hier --- .clang-format | 3 -- .../reporters/CompositeReporter.h | 4 +-- .../reporters/InMemoryReporter.h | 10 +++--- .../reporters/LoggingReporter.cpp | 2 +- src/jaegertracing/reporters/LoggingReporter.h | 4 +-- src/jaegertracing/reporters/NullReporter.h | 4 +-- .../reporters/RemoteReporter.cpp | 32 ++++++++++--------- src/jaegertracing/reporters/RemoteReporter.h | 10 +++--- src/jaegertracing/reporters/Reporter.h | 4 +-- 9 files changed, 36 insertions(+), 37 deletions(-) diff --git a/.clang-format b/.clang-format index 0c76f9c5..7e6effd2 100644 --- a/.clang-format +++ b/.clang-format @@ -33,7 +33,6 @@ BreakBeforeBinaryOperators: None BreakBeforeBraces: Custom BreakBeforeTernaryOperators: true BreakConstructorInitializersBeforeComma: true -BreakStringLiterals: true ColumnLimit: 80 CommentPragmas: '^ IWYU pragma:' ConstructorInitializerAllOnOneLineOrOnePerLine: false @@ -50,7 +49,6 @@ IncludeCategories: Priority: 3 - Regex: '.*' Priority: 1 -IncludeIsMainRegex: '$' IndentCaseLabels: false IndentWidth: 4 IndentWrappedFunctionNames: false @@ -72,7 +70,6 @@ PointerAlignment: Left ReflowComments: true SortIncludes: true SpaceAfterCStyleCast: false -SpaceAfterTemplateKeyword: true SpaceBeforeAssignmentOperators: true SpaceBeforeParens: ControlStatements SpaceInEmptyParentheses: false diff --git a/src/jaegertracing/reporters/CompositeReporter.h b/src/jaegertracing/reporters/CompositeReporter.h index aafc693b..02f2f77d 100644 --- a/src/jaegertracing/reporters/CompositeReporter.h +++ b/src/jaegertracing/reporters/CompositeReporter.h @@ -41,7 +41,7 @@ class CompositeReporter : public Reporter { ~CompositeReporter() { close(); } - void report(const Span& span) override + void report(const Span& span) noexcept override { std::for_each( std::begin(_reporters), @@ -49,7 +49,7 @@ class CompositeReporter : public Reporter { [&span](const ReporterPtr& reporter) { reporter->report(span); }); } - void close() override + void close() noexcept override { std::for_each(std::begin(_reporters), std::end(_reporters), diff --git a/src/jaegertracing/reporters/InMemoryReporter.h b/src/jaegertracing/reporters/InMemoryReporter.h index c199c6db..7977ed45 100644 --- a/src/jaegertracing/reporters/InMemoryReporter.h +++ b/src/jaegertracing/reporters/InMemoryReporter.h @@ -36,27 +36,27 @@ class InMemoryReporter : public Reporter { _spans.reserve(kInitialCapacity); } - void report(const Span& span) override + void report(const Span& span) noexcept override { std::lock_guard lock(_mutex); _spans.push_back(span); } - void close() override {} + void close() noexcept override {} - int spansSubmitted() const + int spansSubmitted() const noexcept { std::lock_guard lock(_mutex); return _spans.size(); } - std::vector spans() const + std::vector spans() const noexcept { std::lock_guard lock(_mutex); return _spans; } - void reset() + void reset() noexcept { std::lock_guard lock(_mutex); _spans.clear(); diff --git a/src/jaegertracing/reporters/LoggingReporter.cpp b/src/jaegertracing/reporters/LoggingReporter.cpp index 8d638b85..03a73745 100644 --- a/src/jaegertracing/reporters/LoggingReporter.cpp +++ b/src/jaegertracing/reporters/LoggingReporter.cpp @@ -22,7 +22,7 @@ namespace jaegertracing { namespace reporters { -void LoggingReporter::report(const Span& span) +void LoggingReporter::report(const Span& span) noexcept { std::ostringstream oss; oss << "Reporting span " << span; diff --git a/src/jaegertracing/reporters/LoggingReporter.h b/src/jaegertracing/reporters/LoggingReporter.h index ced7ce23..e13d56d4 100644 --- a/src/jaegertracing/reporters/LoggingReporter.h +++ b/src/jaegertracing/reporters/LoggingReporter.h @@ -30,9 +30,9 @@ class LoggingReporter : public Reporter { { } - void report(const Span& span) override; + void report(const Span& span) noexcept override; - void close() override {} + void close() noexcept override {} private: logging::Logger& _logger; diff --git a/src/jaegertracing/reporters/NullReporter.h b/src/jaegertracing/reporters/NullReporter.h index 3bd636dc..87460f65 100644 --- a/src/jaegertracing/reporters/NullReporter.h +++ b/src/jaegertracing/reporters/NullReporter.h @@ -28,9 +28,9 @@ namespace reporters { class NullReporter : public Reporter { public: - void report(const Span&) override {} + void report(const Span&) noexcept override {} - void close() override {} + void close() noexcept override {} }; } // namespace reporters diff --git a/src/jaegertracing/reporters/RemoteReporter.cpp b/src/jaegertracing/reporters/RemoteReporter.cpp index 5b115070..191c6058 100644 --- a/src/jaegertracing/reporters/RemoteReporter.cpp +++ b/src/jaegertracing/reporters/RemoteReporter.cpp @@ -45,7 +45,7 @@ RemoteReporter::RemoteReporter(const Clock::duration& bufferFlushInterval, _thread = std::thread([this]() { sweepQueue(); }); } -void RemoteReporter::report(const Span& span) +void RemoteReporter::report(const Span& span) noexcept { std::unique_lock lock(_mutex); const auto pushed = (static_cast(_queue.size()) < _fixedQueueSize); @@ -60,21 +60,25 @@ void RemoteReporter::report(const Span& span) } } -void RemoteReporter::close() +void RemoteReporter::close() noexcept { - { - std::unique_lock lock(_mutex); - if (!_running) { - return; + try { + { + std::lock_guard lock(_mutex); + if (!_running) { + return; + } + _running = false; + flush(); } - _running = false; - lock.unlock(); _cv.notify_one(); + _thread.join(); + } catch (...) { + utils::ErrorUtil::logError(_logger, "Failed in Reporter::close"); } - _thread.join(); } -void RemoteReporter::sweepQueue() +void RemoteReporter::sweepQueue() noexcept { while (true) { try { @@ -97,15 +101,13 @@ void RemoteReporter::sweepQueue() flush(); } } catch (...) { - auto logger = logging::consoleLogger(); - assert(logger); - utils::ErrorUtil::logError(*logger, + utils::ErrorUtil::logError(_logger, "Failed in Reporter::sweepQueue"); } } } -void RemoteReporter::sendSpan(const Span& span) +void RemoteReporter::sendSpan(const Span& span) noexcept { try { const auto flushed = _sender->append(span); @@ -122,7 +124,7 @@ void RemoteReporter::sendSpan(const Span& span) } } -void RemoteReporter::flush() +void RemoteReporter::flush() noexcept { try { const auto flushed = _sender->flush(); diff --git a/src/jaegertracing/reporters/RemoteReporter.h b/src/jaegertracing/reporters/RemoteReporter.h index d8151f3d..02afd54f 100644 --- a/src/jaegertracing/reporters/RemoteReporter.h +++ b/src/jaegertracing/reporters/RemoteReporter.h @@ -44,16 +44,16 @@ class RemoteReporter : public Reporter { ~RemoteReporter() { close(); } - void report(const Span& span) override; + void report(const Span& span) noexcept override; - void close() override; + void close() noexcept override; private: - void sweepQueue(); + void sweepQueue() noexcept; - void sendSpan(const Span& span); + void sendSpan(const Span& span) noexcept; - void flush(); + void flush() noexcept; bool bufferFlushIntervalExpired() const { diff --git a/src/jaegertracing/reporters/Reporter.h b/src/jaegertracing/reporters/Reporter.h index 4e6ea978..17a709f0 100644 --- a/src/jaegertracing/reporters/Reporter.h +++ b/src/jaegertracing/reporters/Reporter.h @@ -27,9 +27,9 @@ class Reporter { public: virtual ~Reporter() = default; - virtual void report(const Span& span) = 0; + virtual void report(const Span& span) noexcept = 0; - virtual void close() = 0; + virtual void close() noexcept = 0; }; } // namespace reporters