From 0b88e47041a1b0860fb5f4e98b02711197038d49 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Mon, 13 Jan 2020 13:35:48 -0800 Subject: [PATCH 01/66] Add buffer chain --- include/lightstep/BUILD | 8 ++++++++ include/lightstep/buffer_chain.h | 21 +++++++++++++++++++++ include/lightstep/transporter.h | 1 + 3 files changed, 30 insertions(+) create mode 100644 include/lightstep/buffer_chain.h diff --git a/include/lightstep/BUILD b/include/lightstep/BUILD index 80a98eaf..61873891 100644 --- a/include/lightstep/BUILD +++ b/include/lightstep/BUILD @@ -19,12 +19,20 @@ lightstep_cc_library( ], ) +lightstep_cc_library( + name = "buffer_chain_interface", + hdrs = [ + "buffer_chain.h", + ], +) + lightstep_cc_library( name = "transporter_interface", hdrs = [ "transporter.h", ], external_deps = [ + ":buffer_chain_interface", "@com_google_protobuf//:protobuf", "@io_opentracing_cpp//:opentracing", ], diff --git a/include/lightstep/buffer_chain.h b/include/lightstep/buffer_chain.h new file mode 100644 index 00000000..ab62f0bf --- /dev/null +++ b/include/lightstep/buffer_chain.h @@ -0,0 +1,21 @@ +#pragma once + +#include + +namespace lightstep { +class BufferChain { + public: + using FragmentCallback = bool (*)(void*, const char*, size_t); + + virtual ~BufferChain() = default; + + virtual size_t num_fragments() const noexcept = 0; + + virtual size_t num_bytes() const noexcept = 0; + + virtual bool ForEachFragment(FragmentCallback callback, + void* context) const = 0; + + void Copy(char* data, size_t length) const noexcept; +}; +} // namespace lightstep diff --git a/include/lightstep/transporter.h b/include/lightstep/transporter.h index 17221870..d645771e 100644 --- a/include/lightstep/transporter.h +++ b/include/lightstep/transporter.h @@ -1,5 +1,6 @@ #pragma once +#include #include #include #include From 375e3d4b960fe4f75e466d84db2c2dbc9cbb3f6d Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Mon, 13 Jan 2020 13:49:38 -0800 Subject: [PATCH 02/66] s/AsyncTransporter/LeacyAsyncTransporter/ --- include/lightstep/buffer_chain.h | 2 +- include/lightstep/tracer.h | 2 +- include/lightstep/transporter.h | 8 ++++---- src/recorder/manual_recorder.cpp | 5 +++-- src/recorder/manual_recorder.h | 10 +++++----- src/tracer/tracer.cpp | 8 ++++---- test/recorder/in_memory_async_transporter.cpp | 7 ++++--- test/recorder/in_memory_async_transporter.h | 6 +++--- test/recorder/manual_recorder_test.cpp | 2 +- 9 files changed, 26 insertions(+), 24 deletions(-) diff --git a/include/lightstep/buffer_chain.h b/include/lightstep/buffer_chain.h index ab62f0bf..84b68672 100644 --- a/include/lightstep/buffer_chain.h +++ b/include/lightstep/buffer_chain.h @@ -18,4 +18,4 @@ class BufferChain { void Copy(char* data, size_t length) const noexcept; }; -} // namespace lightstep +} // namespace lightstep diff --git a/include/lightstep/tracer.h b/include/lightstep/tracer.h index 686b6e84..83eca16f 100644 --- a/include/lightstep/tracer.h +++ b/include/lightstep/tracer.h @@ -139,7 +139,7 @@ struct LightStepTracerOptions { // default transporter is used. // // If `use_thread` is false, `transporter` should be derived from - // AsyncTransporter; otherwise, it must be derived from SyncTransporter. + // LegacyAsyncTransporter; otherwise, it must be derived from SyncTransporter. std::unique_ptr transporter; // `metrics_observer` can be optionally provided to track LightStep tracer diff --git a/include/lightstep/transporter.h b/include/lightstep/transporter.h index d645771e..5f837642 100644 --- a/include/lightstep/transporter.h +++ b/include/lightstep/transporter.h @@ -1,13 +1,13 @@ #pragma once -#include #include +#include #include #include namespace lightstep { // Transporter is the abstract base class for SyncTransporter and -// AsyncTransporter. +// LegacyAsyncTransporter. class Transporter { public: Transporter() noexcept = default; @@ -37,8 +37,8 @@ class SyncTransporter : public Transporter { google::protobuf::Message& response) = 0; }; -// AsyncTransporter customizes how asynchronous tracing reports are sent. -class AsyncTransporter : public Transporter { +// LegacyAsyncTransporter customizes how asynchronous tracing reports are sent. +class LegacyAsyncTransporter : public Transporter { public: // Callback interface used by Send. class Callback { diff --git a/src/recorder/manual_recorder.cpp b/src/recorder/manual_recorder.cpp index 485fe5bd..3d1afed5 100644 --- a/src/recorder/manual_recorder.cpp +++ b/src/recorder/manual_recorder.cpp @@ -5,8 +5,9 @@ namespace lightstep { //------------------------------------------------------------------------------ // Constructor //------------------------------------------------------------------------------ -ManualRecorder::ManualRecorder(Logger& logger, LightStepTracerOptions options, - std::unique_ptr&& transporter) +ManualRecorder::ManualRecorder( + Logger& logger, LightStepTracerOptions options, + std::unique_ptr&& transporter) : logger_{logger}, options_{std::move(options)}, builder_{options_.access_token, options_.tags}, diff --git a/src/recorder/manual_recorder.h b/src/recorder/manual_recorder.h index 2ea1c7b5..fe8c981b 100644 --- a/src/recorder/manual_recorder.h +++ b/src/recorder/manual_recorder.h @@ -8,13 +8,13 @@ namespace lightstep { // ManualRecorder buffers spans finished by a tracer and sends them over to -// the provided AsyncTransporter when FlushWithTimeout is called. +// the provided LegacyAsyncTransporter when FlushWithTimeout is called. class ManualRecorder final : public Recorder, - private AsyncTransporter::Callback, + private LegacyAsyncTransporter::Callback, private Noncopyable { public: ManualRecorder(Logger& logger, LightStepTracerOptions options, - std::unique_ptr&& transporter); + std::unique_ptr&& transporter); void RecordSpan(const collector::Span& span) noexcept override; @@ -44,7 +44,7 @@ class ManualRecorder final : public Recorder, size_t encoding_seqno_ = 1; size_t dropped_spans_ = 0; - // AsyncTransporter through which to send span reports. - std::unique_ptr transporter_; + // LegacyAsyncTransporter through which to send span reports. + std::unique_ptr transporter_; }; } // namespace lightstep diff --git a/src/tracer/tracer.cpp b/src/tracer/tracer.cpp index 7fd582c6..1525bb42 100644 --- a/src/tracer/tracer.cpp +++ b/src/tracer/tracer.cpp @@ -139,13 +139,13 @@ static std::shared_ptr MakeStreamTracer( //------------------------------------------------------------------------------ static std::shared_ptr MakeSingleThreadedTracer( std::shared_ptr logger, LightStepTracerOptions&& options) { - std::unique_ptr transporter; + std::unique_ptr transporter; if (options.transporter != nullptr) { - transporter = std::unique_ptr{ - dynamic_cast(options.transporter.get())}; + transporter = std::unique_ptr{ + dynamic_cast(options.transporter.get())}; if (transporter == nullptr) { logger->Error( - "`options.transporter` must be derived from AsyncTransporter"); + "`options.transporter` must be derived from LegacyAsyncTransporter"); return nullptr; } options.transporter.release(); diff --git a/test/recorder/in_memory_async_transporter.cpp b/test/recorder/in_memory_async_transporter.cpp index 32e3b492..6f75877a 100644 --- a/test/recorder/in_memory_async_transporter.cpp +++ b/test/recorder/in_memory_async_transporter.cpp @@ -4,9 +4,10 @@ namespace lightstep { //------------------------------------------------------------------------------ // Send //------------------------------------------------------------------------------ -void InMemoryAsyncTransporter::Send(const google::protobuf::Message& request, - google::protobuf::Message& response, - AsyncTransporter::Callback& callback) { +void InMemoryAsyncTransporter::Send( + const google::protobuf::Message& request, + google::protobuf::Message& response, + LegacyAsyncTransporter::Callback& callback) { active_request_ = &request; active_response_ = &response; active_callback_ = &callback; diff --git a/test/recorder/in_memory_async_transporter.h b/test/recorder/in_memory_async_transporter.h index 5c4aafb6..310df576 100644 --- a/test/recorder/in_memory_async_transporter.h +++ b/test/recorder/in_memory_async_transporter.h @@ -9,11 +9,11 @@ #include "lightstep/transporter.h" namespace lightstep { -class InMemoryAsyncTransporter : public AsyncTransporter { +class InMemoryAsyncTransporter : public LegacyAsyncTransporter { public: void Send(const google::protobuf::Message& request, google::protobuf::Message& response, - AsyncTransporter::Callback& callback) override; + LegacyAsyncTransporter::Callback& callback) override; void Write(); @@ -31,7 +31,7 @@ class InMemoryAsyncTransporter : public AsyncTransporter { bool should_disable_ = false; const google::protobuf::Message* active_request_; google::protobuf::Message* active_response_; - AsyncTransporter::Callback* active_callback_; + LegacyAsyncTransporter::Callback* active_callback_; std::vector reports_; std::vector spans_; }; diff --git a/test/recorder/manual_recorder_test.cpp b/test/recorder/manual_recorder_test.cpp index 8ce992e1..91a6678c 100644 --- a/test/recorder/manual_recorder_test.cpp +++ b/test/recorder/manual_recorder_test.cpp @@ -23,7 +23,7 @@ TEST_CASE("manual_recorder") { auto in_memory_transporter = new InMemoryAsyncTransporter{}; auto recorder = new ManualRecorder{ logger, std::move(options), - std::unique_ptr{in_memory_transporter}}; + std::unique_ptr{in_memory_transporter}}; auto tracer = std::shared_ptr{new LegacyTracerImpl{ PropagationOptions{}, std::unique_ptr{recorder}}}; CHECK(tracer); From 87a4a19201ff202705afcf49da40ec99c9419d23 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Mon, 13 Jan 2020 14:06:04 -0800 Subject: [PATCH 03/66] Add new AsyncTransporter --- include/lightstep/transporter.h | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/include/lightstep/transporter.h b/include/lightstep/transporter.h index 5f837642..0d5200e3 100644 --- a/include/lightstep/transporter.h +++ b/include/lightstep/transporter.h @@ -67,4 +67,26 @@ class LegacyAsyncTransporter : public Transporter { google::protobuf::Message& response, Callback& callback) = 0; }; + +class AsyncTransporter : public Transporter { + public: + class Callback { + public: + Callback() noexcept = default; + Callback(const Callback&) noexcept = default; + Callback(Callback&&) noexcept = default; + + virtual ~Callback() = default; + + Callback& operator=(const Callback&) noexcept = default; + Callback& operator=(Callback&&) noexcept = default; + + virtual void OnSuccess(const BufferChain& message) noexcept = 0; + + virtual void OnFailure(const BufferChain& message) noexcept = 0; + }; + + virtual void Send(std::unique_ptr&& message, + Callback& callback) = 0; +}; } // namespace lightstep From 053843c0025f78b06724d63f94598d2a474e55b9 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Mon, 13 Jan 2020 14:25:16 -0800 Subject: [PATCH 04/66] s/ManualRecorder/LegacyManualRecorder/ --- CMakeLists.txt | 2 +- src/recorder/BUILD | 6 +++--- ..._recorder.cpp => legacy_manual_recorder.cpp} | 17 +++++++++-------- ...nual_recorder.h => legacy_manual_recorder.h} | 14 +++++++------- src/tracer/BUILD | 2 +- src/tracer/tracer.cpp | 6 +++--- test/recorder/BUILD | 4 ++-- ...test.cpp => legacy_manual_recorder_test.cpp} | 6 +++--- 8 files changed, 29 insertions(+), 28 deletions(-) rename src/recorder/{manual_recorder.cpp => legacy_manual_recorder.cpp} (90%) rename src/recorder/{manual_recorder.h => legacy_manual_recorder.h} (68%) rename test/recorder/{manual_recorder_test.cpp => legacy_manual_recorder_test.cpp} (97%) diff --git a/CMakeLists.txt b/CMakeLists.txt index 046f347c..ae00e61b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -209,7 +209,7 @@ set(LIGHTSTEP_SRCS src/common/utility.cpp src/recorder/report_builder.cpp src/recorder/auto_recorder.cpp src/recorder/fork_aware_recorder.cpp - src/recorder/manual_recorder.cpp + src/recorder/legacy_manual_recorder.cpp src/recorder/transporter.cpp src/tracer/binary_carrier.cpp src/tracer/json_options.cpp diff --git a/src/recorder/BUILD b/src/recorder/BUILD index 2a738230..beac23da 100644 --- a/src/recorder/BUILD +++ b/src/recorder/BUILD @@ -73,12 +73,12 @@ lightstep_cc_library( ) lightstep_cc_library( - name = "manual_recorder_lib", + name = "legacy_manual_recorder_lib", private_hdrs = [ - "manual_recorder.h", + "legacy_manual_recorder.h", ], srcs = [ - "manual_recorder.cpp", + "legacy_manual_recorder.cpp", ], deps = [ "//src/common:logger_lib", diff --git a/src/recorder/manual_recorder.cpp b/src/recorder/legacy_manual_recorder.cpp similarity index 90% rename from src/recorder/manual_recorder.cpp rename to src/recorder/legacy_manual_recorder.cpp index 3d1afed5..be696b4f 100644 --- a/src/recorder/manual_recorder.cpp +++ b/src/recorder/legacy_manual_recorder.cpp @@ -1,11 +1,11 @@ -#include "recorder/manual_recorder.h" +#include "recorder/legacy_manual_recorder.h" #include "common/utility.h" namespace lightstep { //------------------------------------------------------------------------------ // Constructor //------------------------------------------------------------------------------ -ManualRecorder::ManualRecorder( +LegacyManualRecorder::LegacyManualRecorder( Logger& logger, LightStepTracerOptions options, std::unique_ptr&& transporter) : logger_{logger}, @@ -21,7 +21,8 @@ ManualRecorder::ManualRecorder( //------------------------------------------------------------------------------ // RecordSpan //------------------------------------------------------------------------------ -void ManualRecorder::RecordSpan(const collector::Span& span) noexcept try { +void LegacyManualRecorder::RecordSpan( + const collector::Span& span) noexcept try { if (disabled_) { dropped_spans_++; options_.metrics_observer->OnSpansDropped(1); @@ -53,14 +54,14 @@ void ManualRecorder::RecordSpan(const collector::Span& span) noexcept try { //------------------------------------------------------------------------------ // IsReportInProgress //------------------------------------------------------------------------------ -bool ManualRecorder::IsReportInProgress() const noexcept { +bool LegacyManualRecorder::IsReportInProgress() const noexcept { return encoding_seqno_ > 1 + flushed_seqno_; } //------------------------------------------------------------------------------ // FlushOne //------------------------------------------------------------------------------ -bool ManualRecorder::FlushOne() noexcept try { +bool LegacyManualRecorder::FlushOne() noexcept try { options_.metrics_observer->OnFlush(); // If a report is currently in flight, do nothing; and if there are any @@ -93,7 +94,7 @@ bool ManualRecorder::FlushOne() noexcept try { //------------------------------------------------------------------------------ // FlushWithTimeout //------------------------------------------------------------------------------ -bool ManualRecorder::FlushWithTimeout( +bool LegacyManualRecorder::FlushWithTimeout( std::chrono::system_clock::duration /*timeout*/) noexcept { if (disabled_) { return false; @@ -104,7 +105,7 @@ bool ManualRecorder::FlushWithTimeout( //------------------------------------------------------------------------------ // OnSuccess //------------------------------------------------------------------------------ -void ManualRecorder::OnSuccess() noexcept { +void LegacyManualRecorder::OnSuccess() noexcept { ++flushed_seqno_; active_request_.Clear(); LogReportResponse(logger_, options_.verbose, active_response_); @@ -119,7 +120,7 @@ void ManualRecorder::OnSuccess() noexcept { //------------------------------------------------------------------------------ // OnFailure //------------------------------------------------------------------------------ -void ManualRecorder::OnFailure(std::error_code error) noexcept { +void LegacyManualRecorder::OnFailure(std::error_code error) noexcept { ++flushed_seqno_; active_request_.Clear(); options_.metrics_observer->OnSpansDropped( diff --git a/src/recorder/manual_recorder.h b/src/recorder/legacy_manual_recorder.h similarity index 68% rename from src/recorder/manual_recorder.h rename to src/recorder/legacy_manual_recorder.h index fe8c981b..be691d2e 100644 --- a/src/recorder/manual_recorder.h +++ b/src/recorder/legacy_manual_recorder.h @@ -7,14 +7,14 @@ #include "recorder/report_builder.h" namespace lightstep { -// ManualRecorder buffers spans finished by a tracer and sends them over to -// the provided LegacyAsyncTransporter when FlushWithTimeout is called. -class ManualRecorder final : public Recorder, - private LegacyAsyncTransporter::Callback, - private Noncopyable { +// LegacyManualRecorder buffers spans finished by a tracer and sends them over +// to the provided LegacyAsyncTransporter when FlushWithTimeout is called. +class LegacyManualRecorder final : public Recorder, + private LegacyAsyncTransporter::Callback, + private Noncopyable { public: - ManualRecorder(Logger& logger, LightStepTracerOptions options, - std::unique_ptr&& transporter); + LegacyManualRecorder(Logger& logger, LightStepTracerOptions options, + std::unique_ptr&& transporter); void RecordSpan(const collector::Span& span) noexcept override; diff --git a/src/tracer/BUILD b/src/tracer/BUILD index e4d023ad..3c8cf8f4 100644 --- a/src/tracer/BUILD +++ b/src/tracer/BUILD @@ -152,7 +152,7 @@ lightstep_cc_library( "//src/common/platform:utility_lib", "//src/recorder:auto_recorder_lib", "//src/recorder:grpc_transporter_interface", - "//src/recorder:manual_recorder_lib", + "//src/recorder:legacy_manual_recorder_lib", "//src/recorder:stream_recorder_interface", "//src/tracer/legacy:legacy_tracer_impl_lib", "//lightstep-tracer-common:collector_proto_cc", diff --git a/src/tracer/tracer.cpp b/src/tracer/tracer.cpp index 1525bb42..0bb14b98 100644 --- a/src/tracer/tracer.cpp +++ b/src/tracer/tracer.cpp @@ -14,7 +14,7 @@ #include "lightstep/version.h" #include "recorder/auto_recorder.h" #include "recorder/grpc_transporter.h" -#include "recorder/manual_recorder.h" +#include "recorder/legacy_manual_recorder.h" #include "recorder/stream_recorder.h" #include "tracer/immutable_span_context.h" #include "tracer/legacy/legacy_tracer_impl.h" @@ -155,8 +155,8 @@ static std::shared_ptr MakeSingleThreadedTracer( return nullptr; } auto propagation_options = MakePropagationOptions(options); - auto recorder = std::unique_ptr{ - new ManualRecorder{*logger, std::move(options), std::move(transporter)}}; + auto recorder = std::unique_ptr{new LegacyManualRecorder{ + *logger, std::move(options), std::move(transporter)}}; return std::shared_ptr{new LegacyTracerImpl{ std::move(logger), std::move(propagation_options), std::move(recorder)}}; } diff --git a/test/recorder/BUILD b/test/recorder/BUILD index 350f3581..a608b029 100644 --- a/test/recorder/BUILD +++ b/test/recorder/BUILD @@ -48,9 +48,9 @@ lightstep_cc_library( ) lightstep_catch_test( - name = "manual_recorder_test", + name = "legacy_manual_recorder_test", srcs = [ - "manual_recorder_test.cpp", + "legacy_manual_recorder_test.cpp", ], deps = [ "//:manual_tracer_lib", diff --git a/test/recorder/manual_recorder_test.cpp b/test/recorder/legacy_manual_recorder_test.cpp similarity index 97% rename from test/recorder/manual_recorder_test.cpp rename to test/recorder/legacy_manual_recorder_test.cpp index 91a6678c..d097daf2 100644 --- a/test/recorder/manual_recorder_test.cpp +++ b/test/recorder/legacy_manual_recorder_test.cpp @@ -2,7 +2,7 @@ #include "3rd_party/catch2/catch.hpp" #include "lightstep/tracer.h" -#include "recorder/manual_recorder.h" +#include "recorder/legacy_manual_recorder.h" #include "test/recorder/in_memory_async_transporter.h" #include "test/testing_condition_variable_wrapper.h" #include "test/utility.h" @@ -12,7 +12,7 @@ using namespace lightstep; using namespace opentracing; -TEST_CASE("manual_recorder") { +TEST_CASE("legacy_manual_recorder") { Logger logger{}; auto metrics_observer = new CountingMetricsObserver{}; LightStepTracerOptions options; @@ -21,7 +21,7 @@ TEST_CASE("manual_recorder") { std::function{[&] { return max_buffered_spans; }}; options.metrics_observer.reset(metrics_observer); auto in_memory_transporter = new InMemoryAsyncTransporter{}; - auto recorder = new ManualRecorder{ + auto recorder = new LegacyManualRecorder{ logger, std::move(options), std::unique_ptr{in_memory_transporter}}; auto tracer = std::shared_ptr{new LegacyTracerImpl{ From 10c054c4c26b931e915a810b3865d31351b58618 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Mon, 13 Jan 2020 21:12:27 -0800 Subject: [PATCH 05/66] Add new manual recorder --- src/recorder/BUILD | 17 ++++++++++++ src/recorder/manual_recorder.cpp | 45 ++++++++++++++++++++++++++++++++ src/recorder/manual_recorder.h | 29 ++++++++++++++++++++ src/tracer/BUILD | 1 + src/tracer/tracer.cpp | 33 ++++++++++++++++++++--- 5 files changed, 122 insertions(+), 3 deletions(-) create mode 100644 src/recorder/manual_recorder.cpp create mode 100644 src/recorder/manual_recorder.h diff --git a/src/recorder/BUILD b/src/recorder/BUILD index beac23da..bc51ec87 100644 --- a/src/recorder/BUILD +++ b/src/recorder/BUILD @@ -90,6 +90,23 @@ lightstep_cc_library( ], ) +lightstep_cc_library( + name = "manual_recorder_lib", + private_hdrs = [ + "manual_recorder.h", + ], + srcs = [ + "manual_recorder.cpp", + ], + deps = [ + "//src/common:logger_lib", + "//src/common:noncopyable_lib", + "//src/common:utility_lib", + ":fork_aware_recorder_lib", + ":transporter_lib", + ], +) + lightstep_cc_library( name = "auto_recorder_lib", private_hdrs = [ diff --git a/src/recorder/manual_recorder.cpp b/src/recorder/manual_recorder.cpp new file mode 100644 index 00000000..f8ab5f3d --- /dev/null +++ b/src/recorder/manual_recorder.cpp @@ -0,0 +1,45 @@ +#include "recorder/manual_recorder.h" + +namespace lightstep { +//-------------------------------------------------------------------------------------------------- +// constructor +//-------------------------------------------------------------------------------------------------- +ManualRecorder::ManualRecorder( + Logger& logger, LightStepTracerOptions options, + std::unique_ptr&& transporter) { + (void)logger; + (void)options; + (void)transporter; +} + +//-------------------------------------------------------------------------------------------------- +// RecordSpan +//-------------------------------------------------------------------------------------------------- +void ManualRecorder::RecordSpan( + std::unique_ptr&& span) noexcept { + (void)span; +} + +//-------------------------------------------------------------------------------------------------- +// FlushWithTimeout +//-------------------------------------------------------------------------------------------------- +bool ManualRecorder::FlushWithTimeout( + std::chrono::system_clock::duration /*timeout*/) noexcept { + return true; +} + +//-------------------------------------------------------------------------------------------------- +// PrepareForFork +//-------------------------------------------------------------------------------------------------- +void ManualRecorder::PrepareForFork() noexcept {} + +//-------------------------------------------------------------------------------------------------- +// OnForkedParent +//-------------------------------------------------------------------------------------------------- +void ManualRecorder::OnForkedParent() noexcept {} + +//-------------------------------------------------------------------------------------------------- +// OnForkedChild +//-------------------------------------------------------------------------------------------------- +void ManualRecorder::OnForkedChild() noexcept {} +} // namespace lightstep diff --git a/src/recorder/manual_recorder.h b/src/recorder/manual_recorder.h new file mode 100644 index 00000000..8ddb1248 --- /dev/null +++ b/src/recorder/manual_recorder.h @@ -0,0 +1,29 @@ +#pragma once + +#include "common/logger.h" +#include "common/noncopyable.h" +#include "lightstep/transporter.h" +#include "recorder/fork_aware_recorder.h" + +namespace lightstep { +class ManualRecorder : public ForkAwareRecorder, private Noncopyable { + public: + ManualRecorder(Logger& logger, LightStepTracerOptions options, + std::unique_ptr&& transporter); + + // Recorder + void RecordSpan(std::unique_ptr&& span) noexcept override; + + bool FlushWithTimeout( + std::chrono::system_clock::duration timeout) noexcept override; + + // ForkAwareRecorder + void PrepareForFork() noexcept override; + + void OnForkedParent() noexcept override; + + void OnForkedChild() noexcept override; + + private: +}; +} // namespace lightstep diff --git a/src/tracer/BUILD b/src/tracer/BUILD index 3c8cf8f4..7a8ac12e 100644 --- a/src/tracer/BUILD +++ b/src/tracer/BUILD @@ -153,6 +153,7 @@ lightstep_cc_library( "//src/recorder:auto_recorder_lib", "//src/recorder:grpc_transporter_interface", "//src/recorder:legacy_manual_recorder_lib", + "//src/recorder:manual_recorder_lib", "//src/recorder:stream_recorder_interface", "//src/tracer/legacy:legacy_tracer_impl_lib", "//lightstep-tracer-common:collector_proto_cc", diff --git a/src/tracer/tracer.cpp b/src/tracer/tracer.cpp index 0bb14b98..b064f68a 100644 --- a/src/tracer/tracer.cpp +++ b/src/tracer/tracer.cpp @@ -15,6 +15,7 @@ #include "recorder/auto_recorder.h" #include "recorder/grpc_transporter.h" #include "recorder/legacy_manual_recorder.h" +#include "recorder/manual_recorder.h" #include "recorder/stream_recorder.h" #include "tracer/immutable_span_context.h" #include "tracer/legacy/legacy_tracer_impl.h" @@ -135,9 +136,9 @@ static std::shared_ptr MakeStreamTracer( } //------------------------------------------------------------------------------ -// MakeSingleThreadedTracer +// MakeLegacySingleThreadedTracer //------------------------------------------------------------------------------ -static std::shared_ptr MakeSingleThreadedTracer( +static std::shared_ptr MakeLegacySingleThreadedTracer( std::shared_ptr logger, LightStepTracerOptions&& options) { std::unique_ptr transporter; if (options.transporter != nullptr) { @@ -145,7 +146,8 @@ static std::shared_ptr MakeSingleThreadedTracer( dynamic_cast(options.transporter.get())}; if (transporter == nullptr) { logger->Error( - "`options.transporter` must be derived from LegacyAsyncTransporter"); + "`options.transporter` must be derived from AsyncTransporter " + "LegacyAsyncTransporter"); return nullptr; } options.transporter.release(); @@ -161,6 +163,31 @@ static std::shared_ptr MakeSingleThreadedTracer( std::move(logger), std::move(propagation_options), std::move(recorder)}}; } +//------------------------------------------------------------------------------ +// MakeSingleThreadedTracer +//------------------------------------------------------------------------------ +static std::shared_ptr MakeSingleThreadedTracer( + std::shared_ptr logger, LightStepTracerOptions&& options) { + std::unique_ptr transporter; + if (options.transporter != nullptr) { + transporter = std::unique_ptr{ + dynamic_cast(options.transporter.get())}; + if (transporter == nullptr) { + return MakeLegacySingleThreadedTracer(logger, std::move(options)); + } + options.transporter.release(); + } else { + logger->Error( + "`options.transporter` must be set if `options.use_thread` is false"); + return nullptr; + } + auto propagation_options = MakePropagationOptions(options); + auto recorder = std::unique_ptr{ + new ManualRecorder{*logger, std::move(options), std::move(transporter)}}; + return std::shared_ptr{new TracerImpl{ + std::move(logger), std::move(propagation_options), std::move(recorder)}}; +} + //------------------------------------------------------------------------------ // MakeLightStepTracer //------------------------------------------------------------------------------ From 541e5ba9e213d566425d407509373f4f18d905b2 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Wed, 15 Jan 2020 00:05:13 -0800 Subject: [PATCH 06/66] Set up manual recorder --- src/recorder/BUILD | 1 + src/recorder/manual_recorder.cpp | 22 +++++++++++++++------- src/recorder/manual_recorder.h | 6 ++++++ 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/src/recorder/BUILD b/src/recorder/BUILD index bc51ec87..cb6bff05 100644 --- a/src/recorder/BUILD +++ b/src/recorder/BUILD @@ -102,6 +102,7 @@ lightstep_cc_library( "//src/common:logger_lib", "//src/common:noncopyable_lib", "//src/common:utility_lib", + "//src/common:circular_buffer_lib", ":fork_aware_recorder_lib", ":transporter_lib", ], diff --git a/src/recorder/manual_recorder.cpp b/src/recorder/manual_recorder.cpp index f8ab5f3d..bbeb08f7 100644 --- a/src/recorder/manual_recorder.cpp +++ b/src/recorder/manual_recorder.cpp @@ -4,12 +4,16 @@ namespace lightstep { //-------------------------------------------------------------------------------------------------- // constructor //-------------------------------------------------------------------------------------------------- -ManualRecorder::ManualRecorder( - Logger& logger, LightStepTracerOptions options, - std::unique_ptr&& transporter) { - (void)logger; - (void)options; - (void)transporter; +ManualRecorder::ManualRecorder(Logger& logger, LightStepTracerOptions options, + std::unique_ptr&& transporter) + : logger_{logger}, + tracer_options_{std::move(options)}, + transporter_{std::move(transporter)}, + span_buffer_{tracer_options_.max_buffered_spans.value()} { + // If no MetricsObserver was provided, use a default one that does nothing. + if (options_.metrics_observer == nullptr) { + options_.metrics_observer.reset(new MetricsObserver{}); + } } //-------------------------------------------------------------------------------------------------- @@ -17,7 +21,11 @@ ManualRecorder::ManualRecorder( //-------------------------------------------------------------------------------------------------- void ManualRecorder::RecordSpan( std::unique_ptr&& span) noexcept { - (void)span; + span->AddFraming(); + if (!span_buffer_.Add(span)) { + tracer_options_.metrics_observer->OnSpansDropped(1); + span.reset(); + } } //-------------------------------------------------------------------------------------------------- diff --git a/src/recorder/manual_recorder.h b/src/recorder/manual_recorder.h index 8ddb1248..cb6a8042 100644 --- a/src/recorder/manual_recorder.h +++ b/src/recorder/manual_recorder.h @@ -1,5 +1,6 @@ #pragma once +#include "common/circular_buffer.h" #include "common/logger.h" #include "common/noncopyable.h" #include "lightstep/transporter.h" @@ -25,5 +26,10 @@ class ManualRecorder : public ForkAwareRecorder, private Noncopyable { void OnForkedChild() noexcept override; private: + Logger& logger_; + LightStepTracerOptions tracer_options_; + std::unique_ptr transporter_; + + CircularBuffer span_buffer_; }; } // namespace lightstep From 1f8d443691dab1a875eea90e9d072e095a55efb4 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Wed, 15 Jan 2020 14:28:11 -0800 Subject: [PATCH 07/66] Add ReportRequest --- src/recorder/manual_recorder.cpp | 4 ++-- src/recorder/serialization/BUILD | 20 +++++++++++++++++++ src/recorder/serialization/report_request.cpp | 4 ++++ src/recorder/serialization/report_request.h | 8 ++++++++ 4 files changed, 34 insertions(+), 2 deletions(-) create mode 100644 src/recorder/serialization/BUILD create mode 100644 src/recorder/serialization/report_request.cpp create mode 100644 src/recorder/serialization/report_request.h diff --git a/src/recorder/manual_recorder.cpp b/src/recorder/manual_recorder.cpp index bbeb08f7..c54502d7 100644 --- a/src/recorder/manual_recorder.cpp +++ b/src/recorder/manual_recorder.cpp @@ -11,8 +11,8 @@ ManualRecorder::ManualRecorder(Logger& logger, LightStepTracerOptions options, transporter_{std::move(transporter)}, span_buffer_{tracer_options_.max_buffered_spans.value()} { // If no MetricsObserver was provided, use a default one that does nothing. - if (options_.metrics_observer == nullptr) { - options_.metrics_observer.reset(new MetricsObserver{}); + if (tracer_options_.metrics_observer == nullptr) { + tracer_options_.metrics_observer.reset(new MetricsObserver{}); } } diff --git a/src/recorder/serialization/BUILD b/src/recorder/serialization/BUILD new file mode 100644 index 00000000..5458f19d --- /dev/null +++ b/src/recorder/serialization/BUILD @@ -0,0 +1,20 @@ +load( + "//bazel:lightstep_build_system.bzl", + "lightstep_cc_library", + "lightstep_package", +) + +lightstep_package() + +lightstep_cc_library( + name = "report_request_lib", + private_hdrs = [ + "report_request.h", + ], + srcs = [ + "report_request.cpp", + ], + deps = [ + "//include/lightstep:buffer_chain_interface", + ], +) diff --git a/src/recorder/serialization/report_request.cpp b/src/recorder/serialization/report_request.cpp new file mode 100644 index 00000000..4ab77775 --- /dev/null +++ b/src/recorder/serialization/report_request.cpp @@ -0,0 +1,4 @@ +#include "recorder/serialization/report_request.h" + +namespace lightstep { +} // namespace lightstep diff --git a/src/recorder/serialization/report_request.h b/src/recorder/serialization/report_request.h new file mode 100644 index 00000000..e80d171a --- /dev/null +++ b/src/recorder/serialization/report_request.h @@ -0,0 +1,8 @@ +#pragma once + +#include "lightstep/buffer_chain.h" + +namespace lightstep { +class ReportRequest final : public BufferChain { +}; +} // namespace lightstep From 7f1982b314e6cd106bafdf19e010d01889dada18 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Wed, 15 Jan 2020 15:01:41 -0800 Subject: [PATCH 08/66] Factor out report request header serialization code --- src/recorder/serialization/BUILD | 16 ++++++++ .../serialization/report_request_header.cpp | 37 +++++++++++++++++++ .../serialization/report_request_header.h | 17 +++++++++ test/recorder/serialization/BUILD | 19 ++++++++++ .../report_request_header_test.cpp | 20 ++++++++++ 5 files changed, 109 insertions(+) create mode 100644 src/recorder/serialization/report_request_header.cpp create mode 100644 src/recorder/serialization/report_request_header.h create mode 100644 test/recorder/serialization/BUILD create mode 100644 test/recorder/serialization/report_request_header_test.cpp diff --git a/src/recorder/serialization/BUILD b/src/recorder/serialization/BUILD index 5458f19d..cf0c9e72 100644 --- a/src/recorder/serialization/BUILD +++ b/src/recorder/serialization/BUILD @@ -18,3 +18,19 @@ lightstep_cc_library( "//include/lightstep:buffer_chain_interface", ], ) + +lightstep_cc_library( + name = "report_request_header_lib", + private_hdrs = [ + "report_request_header.h", + ], + srcs = [ + "report_request_header.cpp", + ], + deps = [ + "//include/lightstep:tracer_interface", + "//lightstep-tracer-common:collector_proto_cc", + "//src/common:protobuf_lib", + "//src/common:utility_lib", + ], +) diff --git a/src/recorder/serialization/report_request_header.cpp b/src/recorder/serialization/report_request_header.cpp new file mode 100644 index 00000000..6c87a285 --- /dev/null +++ b/src/recorder/serialization/report_request_header.cpp @@ -0,0 +1,37 @@ +#include "recorder/serialization/report_request_header.h" + +#include "common/protobuf.h" +#include "common/utility.h" +#include "lightstep-tracer-common/collector.pb.h" + +#include +#include + +namespace lightstep { +std::string WriteReportRequestHeader( + const LightStepTracerOptions& tracer_options, uint64_t reporter_id) { + collector::Reporter reporter; + reporter.set_reporter_id(reporter_id); + reporter.mutable_tags()->Reserve( + static_cast(tracer_options.tags.size())); + for (const auto& tag : tracer_options.tags) { + *reporter.mutable_tags()->Add() = ToKeyValue(tag.first, tag.second); + } + + collector::Auth auth; + auth.set_access_token(tracer_options.access_token); + + std::ostringstream oss; + { + google::protobuf::io::OstreamOutputStream zero_copy_stream{&oss}; + google::protobuf::io::CodedOutputStream coded_stream{&zero_copy_stream}; + + WriteEmbeddedMessage( + coded_stream, collector::ReportRequest::kReporterFieldNumber, reporter); + WriteEmbeddedMessage(coded_stream, + collector::ReportRequest::kAuthFieldNumber, auth); + } + + return oss.str(); +} +} // namespace lightstep diff --git a/src/recorder/serialization/report_request_header.h b/src/recorder/serialization/report_request_header.h new file mode 100644 index 00000000..a55238e7 --- /dev/null +++ b/src/recorder/serialization/report_request_header.h @@ -0,0 +1,17 @@ +#pragma once + +#include +#include + +#include "lightstep/tracer.h" + +namespace lightstep { +/** + * Serializes the common parts of a ReportRequest. + * @param tracer_options the options used to construct the tracer. + * @param reporter_id a unique ID for the reporter. + * @return a string with the common parts of the ReportRequest serialization. + */ +std::string WriteReportRequestHeader( + const LightStepTracerOptions& tracer_options, uint64_t reporter_id); +} // namespace lightstep diff --git a/test/recorder/serialization/BUILD b/test/recorder/serialization/BUILD new file mode 100644 index 00000000..1c1c863d --- /dev/null +++ b/test/recorder/serialization/BUILD @@ -0,0 +1,19 @@ +load( + "//bazel:lightstep_build_system.bzl", + "lightstep_catch_test", + "lightstep_cc_library", + "lightstep_package", +) + +lightstep_package() + +lightstep_catch_test( + name = "report_request_header_test", + srcs = [ + "report_request_header_test.cpp", + ], + deps = [ + "//src/recorder/serialization:report_request_header_lib", + ], +) + diff --git a/test/recorder/serialization/report_request_header_test.cpp b/test/recorder/serialization/report_request_header_test.cpp new file mode 100644 index 00000000..3ade4066 --- /dev/null +++ b/test/recorder/serialization/report_request_header_test.cpp @@ -0,0 +1,20 @@ +#include "recorder/serialization/report_request_header.h" + +#include "3rd_party/catch2/catch.hpp" +#include "lightstep-tracer-common/collector.pb.h" +using namespace lightstep; + +TEST_CASE("WriteReportRequestHeader") { + LightStepTracerOptions tracer_options; + tracer_options.access_token = "abc123"; + tracer_options.tags = {{"xyz", 456}}; + auto serialization = WriteReportRequestHeader(tracer_options, 789); + collector::ReportRequest report; + REQUIRE(report.ParseFromString(serialization)); + REQUIRE(report.auth().access_token() == "abc123"); + REQUIRE(report.reporter().reporter_id() == 789); + auto& tags = report.reporter().tags(); + REQUIRE(tags.size() == 1); + REQUIRE(tags[0].key() == "xyz"); + REQUIRE(tags[0].int_value() == 456); +} From 40c11ce38586344d7946750da5261e634e1494f1 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Wed, 15 Jan 2020 15:46:22 -0800 Subject: [PATCH 09/66] Migrate code to use WriteReportRequestHeader --- src/recorder/stream_recorder/BUILD | 2 +- src/recorder/stream_recorder/satellite_streamer.cpp | 4 ++-- test/recorder/stream_recorder/BUILD | 2 +- test/recorder/stream_recorder/connection_stream_test.cpp | 6 +++--- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/recorder/stream_recorder/BUILD b/src/recorder/stream_recorder/BUILD index 0fad9c35..11932b24 100644 --- a/src/recorder/stream_recorder/BUILD +++ b/src/recorder/stream_recorder/BUILD @@ -147,9 +147,9 @@ lightstep_cc_library( "//src/network:event_lib", "//src/network:timer_event_lib", "//src/network:vector_write_lib", + "//src/recorder/serialization:report_request_header_lib", ":stream_recorder_metrics_lib", ":embedded_metrics_message_lib", - ":utility_lib", ":host_header_lib", ":span_stream_lib", ":connection_stream_lib", diff --git a/src/recorder/stream_recorder/satellite_streamer.cpp b/src/recorder/stream_recorder/satellite_streamer.cpp index b7cc7b57..b47c24b8 100644 --- a/src/recorder/stream_recorder/satellite_streamer.cpp +++ b/src/recorder/stream_recorder/satellite_streamer.cpp @@ -4,7 +4,7 @@ #include #include "common/random.h" -#include "recorder/stream_recorder/utility.h" +#include "recorder/serialization/report_request_header.h" namespace lightstep { //-------------------------------------------------------------------------------------------------- @@ -21,7 +21,7 @@ SatelliteStreamer::SatelliteStreamer( tracer_options_{tracer_options}, recorder_options_{recorder_options}, header_common_fragment_{ - WriteStreamHeaderCommonFragment(tracer_options, GenerateId())}, + WriteReportRequestHeader(tracer_options, GenerateId())}, endpoint_manager_{logger, event_base, tracer_options, recorder_options, [this] { this->OnEndpointManagerReady(); }}, span_buffer_{span_buffer}, diff --git a/test/recorder/stream_recorder/BUILD b/test/recorder/stream_recorder/BUILD index 0058c137..72221332 100644 --- a/test/recorder/stream_recorder/BUILD +++ b/test/recorder/stream_recorder/BUILD @@ -103,7 +103,7 @@ lightstep_catch_test( ], deps = [ "//src/recorder/stream_recorder:connection_stream_lib", - "//src/recorder/stream_recorder:utility_lib", + "//src/recorder/serialization:report_request_header_lib", "//test:number_simulation_lib", "//test:utility_lib", ], diff --git a/test/recorder/stream_recorder/connection_stream_test.cpp b/test/recorder/stream_recorder/connection_stream_test.cpp index 6442eb5e..a9340cec 100644 --- a/test/recorder/stream_recorder/connection_stream_test.cpp +++ b/test/recorder/stream_recorder/connection_stream_test.cpp @@ -2,7 +2,7 @@ #include "3rd_party/catch2/catch.hpp" #include "recorder/stream_recorder/span_stream.h" -#include "recorder/stream_recorder/utility.h" +#include "recorder/serialization/report_request_header.h" #include "test/number_simulation.h" #include "test/utility.h" using namespace lightstep; @@ -38,7 +38,7 @@ TEST_CASE("ConnectionStream") { StreamRecorderMetrics metrics{metrics_observer}; SpanStream span_stream{span_buffer, metrics}; std::string header_common_fragment = - WriteStreamHeaderCommonFragment(tracer_options, 123); + WriteReportRequestHeader(tracer_options, 123); auto host_header_fragment = MakeFragment("Host:abc\r\n"); ConnectionStream connection_stream{ host_header_fragment, @@ -244,7 +244,7 @@ TEST_CASE( "Verify through simulation that ConnectionStream behaves correctly.") { LightStepTracerOptions tracer_options; std::string header_common_fragment = - WriteStreamHeaderCommonFragment(tracer_options, 123); + WriteReportRequestHeader(tracer_options, 123); MetricsObserver metrics_observer; StreamRecorderMetrics metrics{metrics_observer}; auto host_header_fragment = MakeFragment("Host:abc\r\n"); From b1cc70188ca6bf6f18acfab07a91c30de2d22c8d Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Wed, 15 Jan 2020 15:52:14 -0800 Subject: [PATCH 10/66] Remove unused function --- src/recorder/stream_recorder/BUILD | 5 --- src/recorder/stream_recorder/utility.cpp | 36 ------------------- src/recorder/stream_recorder/utility.h | 12 ------- .../recorder/stream_recorder/utility_test.cpp | 16 --------- 4 files changed, 69 deletions(-) diff --git a/src/recorder/stream_recorder/BUILD b/src/recorder/stream_recorder/BUILD index 11932b24..48f011e2 100644 --- a/src/recorder/stream_recorder/BUILD +++ b/src/recorder/stream_recorder/BUILD @@ -166,12 +166,7 @@ lightstep_cc_library( "utility.cpp", ], deps = [ - "//include/lightstep:tracer_interface", - "//lightstep-tracer-common:collector_proto_cc", - "//src/common:protobuf_lib", "//src/common/platform:string_lib", - "//src/common:utility_lib", - "//src/common:fragment_array_input_stream_lib", ], ) diff --git a/src/recorder/stream_recorder/utility.cpp b/src/recorder/stream_recorder/utility.cpp index bd3e0e21..41445346 100644 --- a/src/recorder/stream_recorder/utility.cpp +++ b/src/recorder/stream_recorder/utility.cpp @@ -6,12 +6,6 @@ #include #include "common/platform/string.h" -#include "common/protobuf.h" -#include "common/utility.h" -#include "lightstep-tracer-common/collector.pb.h" - -#include -#include namespace lightstep { //-------------------------------------------------------------------------------------------------- @@ -45,34 +39,4 @@ SeparateEndpoints( } return {std::move(hosts), std::move(indexed_endpoints)}; } - -//-------------------------------------------------------------------------------------------------- -// WriteStreamHeaderCommonFragment -//-------------------------------------------------------------------------------------------------- -std::string WriteStreamHeaderCommonFragment( - const LightStepTracerOptions& tracer_options, uint64_t reporter_id) { - collector::Reporter reporter; - reporter.set_reporter_id(reporter_id); - reporter.mutable_tags()->Reserve( - static_cast(tracer_options.tags.size())); - for (const auto& tag : tracer_options.tags) { - *reporter.mutable_tags()->Add() = ToKeyValue(tag.first, tag.second); - } - - collector::Auth auth; - auth.set_access_token(tracer_options.access_token); - - std::ostringstream oss; - { - google::protobuf::io::OstreamOutputStream zero_copy_stream{&oss}; - google::protobuf::io::CodedOutputStream coded_stream{&zero_copy_stream}; - - WriteEmbeddedMessage( - coded_stream, collector::ReportRequest::kReporterFieldNumber, reporter); - WriteEmbeddedMessage(coded_stream, - collector::ReportRequest::kAuthFieldNumber, auth); - } - - return oss.str(); -} } // namespace lightstep diff --git a/src/recorder/stream_recorder/utility.h b/src/recorder/stream_recorder/utility.h index 4af47a31..c9d6348d 100644 --- a/src/recorder/stream_recorder/utility.h +++ b/src/recorder/stream_recorder/utility.h @@ -5,9 +5,6 @@ #include #include -#include "common/fragment_array_input_stream.h" -#include "lightstep/tracer.h" - namespace lightstep { /** * Separates a vector of host-port pairs into a bector of unique hosts and @@ -18,13 +15,4 @@ namespace lightstep { std::pair, std::vector>> SeparateEndpoints( const std::vector>& endpoints); - -/** - * Serializes the common parts of a ReportRequest. - * @param tracer_options the options used to construct the tracer. - * @param reporter_id a unique ID for the reporter. - * @return a string with the common parts of the ReportRequest serialization. - */ -std::string WriteStreamHeaderCommonFragment( - const LightStepTracerOptions& tracer_options, uint64_t reporter_id); } // namespace lightstep diff --git a/test/recorder/stream_recorder/utility_test.cpp b/test/recorder/stream_recorder/utility_test.cpp index 2c65caa2..abd26ec2 100644 --- a/test/recorder/stream_recorder/utility_test.cpp +++ b/test/recorder/stream_recorder/utility_test.cpp @@ -1,7 +1,6 @@ #include "recorder/stream_recorder/utility.h" #include "3rd_party/catch2/catch.hpp" -#include "lightstep-tracer-common/collector.pb.h" using namespace lightstep; TEST_CASE("SeparateEndpoints") { @@ -27,18 +26,3 @@ TEST_CASE("SeparateEndpoints") { REQUIRE(hosts.size() == 1); } } - -TEST_CASE("WriteStreamHeaderCommonFragment") { - LightStepTracerOptions tracer_options; - tracer_options.access_token = "abc123"; - tracer_options.tags = {{"xyz", 456}}; - auto serialization = WriteStreamHeaderCommonFragment(tracer_options, 789); - collector::ReportRequest report; - REQUIRE(report.ParseFromString(serialization)); - REQUIRE(report.auth().access_token() == "abc123"); - REQUIRE(report.reporter().reporter_id() == 789); - auto& tags = report.reporter().tags(); - REQUIRE(tags.size() == 1); - REQUIRE(tags[0].key() == "xyz"); - REQUIRE(tags[0].int_value() == 456); -} From 9932689337c266e86e14e395b351862a20a29fe2 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Wed, 15 Jan 2020 22:56:44 -0800 Subject: [PATCH 11/66] Move EmbeddedMetricsMessage --- src/recorder/serialization/BUILD | 14 ++++++++++++++ .../embedded_metrics_message.cpp | 2 +- .../embedded_metrics_message.h | 0 src/recorder/serialization/report_request.h | 2 ++ src/recorder/stream_recorder/BUILD | 17 +---------------- .../stream_recorder/connection_stream.h | 2 +- test/recorder/serialization/BUILD | 9 +++++++++ .../embedded_metrics_message_test.cpp | 2 +- test/recorder/stream_recorder/BUILD | 10 ---------- 9 files changed, 29 insertions(+), 29 deletions(-) rename src/recorder/{stream_recorder => serialization}/embedded_metrics_message.cpp (97%) rename src/recorder/{stream_recorder => serialization}/embedded_metrics_message.h (100%) rename test/recorder/{stream_recorder => serialization}/embedded_metrics_message_test.cpp (92%) diff --git a/src/recorder/serialization/BUILD b/src/recorder/serialization/BUILD index cf0c9e72..4d84b07b 100644 --- a/src/recorder/serialization/BUILD +++ b/src/recorder/serialization/BUILD @@ -34,3 +34,17 @@ lightstep_cc_library( "//src/common:utility_lib", ], ) + +lightstep_cc_library( + name = "embedded_metrics_message_lib", + private_hdrs = [ + "embedded_metrics_message.h", + ], + srcs = [ + "embedded_metrics_message.cpp", + ], + deps = [ + "//lightstep-tracer-common:collector_proto_cc", + "//src/common:protobuf_lib", + ], +) diff --git a/src/recorder/stream_recorder/embedded_metrics_message.cpp b/src/recorder/serialization/embedded_metrics_message.cpp similarity index 97% rename from src/recorder/stream_recorder/embedded_metrics_message.cpp rename to src/recorder/serialization/embedded_metrics_message.cpp index bd7d97a8..7f79dd90 100644 --- a/src/recorder/stream_recorder/embedded_metrics_message.cpp +++ b/src/recorder/serialization/embedded_metrics_message.cpp @@ -1,4 +1,4 @@ -#include "recorder/stream_recorder/embedded_metrics_message.h" +#include "recorder/serialization/embedded_metrics_message.h" #include "common/protobuf.h" diff --git a/src/recorder/stream_recorder/embedded_metrics_message.h b/src/recorder/serialization/embedded_metrics_message.h similarity index 100% rename from src/recorder/stream_recorder/embedded_metrics_message.h rename to src/recorder/serialization/embedded_metrics_message.h diff --git a/src/recorder/serialization/report_request.h b/src/recorder/serialization/report_request.h index e80d171a..b11688d9 100644 --- a/src/recorder/serialization/report_request.h +++ b/src/recorder/serialization/report_request.h @@ -4,5 +4,7 @@ namespace lightstep { class ReportRequest final : public BufferChain { + public: + private: }; } // namespace lightstep diff --git a/src/recorder/stream_recorder/BUILD b/src/recorder/stream_recorder/BUILD index 48f011e2..d633a533 100644 --- a/src/recorder/stream_recorder/BUILD +++ b/src/recorder/stream_recorder/BUILD @@ -58,7 +58,7 @@ lightstep_cc_library( "//src/common:utility_lib", "//src/common:fragment_input_stream_lib", "//src/common:fragment_array_input_stream_lib", - ":embedded_metrics_message_lib", + "//src/recorder/serialization:embedded_metrics_message_lib", ":span_stream_lib", ":stream_recorder_metrics_lib", ], @@ -149,7 +149,6 @@ lightstep_cc_library( "//src/network:vector_write_lib", "//src/recorder/serialization:report_request_header_lib", ":stream_recorder_metrics_lib", - ":embedded_metrics_message_lib", ":host_header_lib", ":span_stream_lib", ":connection_stream_lib", @@ -170,20 +169,6 @@ lightstep_cc_library( ], ) -lightstep_cc_library( - name = "embedded_metrics_message_lib", - private_hdrs = [ - "embedded_metrics_message.h", - ], - srcs = [ - "embedded_metrics_message.cpp", - ], - deps = [ - "//lightstep-tracer-common:collector_proto_cc", - "//src/common:protobuf_lib", - ], -) - lightstep_cc_library( name = "host_header_lib", private_hdrs = [ diff --git a/src/recorder/stream_recorder/connection_stream.h b/src/recorder/stream_recorder/connection_stream.h index abc8e17c..61477a92 100644 --- a/src/recorder/stream_recorder/connection_stream.h +++ b/src/recorder/stream_recorder/connection_stream.h @@ -8,7 +8,7 @@ #include "common/fragment_input_stream.h" #include "common/function_ref.h" #include "common/utility.h" -#include "recorder/stream_recorder/embedded_metrics_message.h" +#include "recorder/serialization/embedded_metrics_message.h" #include "recorder/stream_recorder/span_stream.h" #include "recorder/stream_recorder/stream_recorder_metrics.h" diff --git a/test/recorder/serialization/BUILD b/test/recorder/serialization/BUILD index 1c1c863d..32007c2a 100644 --- a/test/recorder/serialization/BUILD +++ b/test/recorder/serialization/BUILD @@ -17,3 +17,12 @@ lightstep_catch_test( ], ) +lightstep_catch_test( + name = "embedded_metrics_message_test", + srcs = [ + "embedded_metrics_message_test.cpp", + ], + deps = [ + "//src/recorder/serialization:embedded_metrics_message_lib", + ], +) diff --git a/test/recorder/stream_recorder/embedded_metrics_message_test.cpp b/test/recorder/serialization/embedded_metrics_message_test.cpp similarity index 92% rename from test/recorder/stream_recorder/embedded_metrics_message_test.cpp rename to test/recorder/serialization/embedded_metrics_message_test.cpp index 8a974197..31458114 100644 --- a/test/recorder/stream_recorder/embedded_metrics_message_test.cpp +++ b/test/recorder/serialization/embedded_metrics_message_test.cpp @@ -1,4 +1,4 @@ -#include "recorder/stream_recorder/embedded_metrics_message.h" +#include "recorder/serialization/embedded_metrics_message.h" #include "3rd_party/catch2/catch.hpp" using namespace lightstep; diff --git a/test/recorder/stream_recorder/BUILD b/test/recorder/stream_recorder/BUILD index 72221332..e0d66f05 100644 --- a/test/recorder/stream_recorder/BUILD +++ b/test/recorder/stream_recorder/BUILD @@ -75,16 +75,6 @@ lightstep_catch_test( ], ) -lightstep_catch_test( - name = "embedded_metrics_message_test", - srcs = [ - "embedded_metrics_message_test.cpp", - ], - deps = [ - "//src/recorder/stream_recorder:embedded_metrics_message_lib", - ], -) - lightstep_catch_test( name = "span_stream_test", srcs = [ From 47377ae57239759c323475892eb6c99cb93607f6 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Thu, 16 Jan 2020 00:06:50 -0800 Subject: [PATCH 12/66] Set up ReportRequest --- include/lightstep/buffer_chain.h | 4 +-- src/recorder/serialization/BUILD | 3 ++ src/recorder/serialization/report_request.cpp | 30 ++++++++++++++++++- src/recorder/serialization/report_request.h | 21 +++++++++++++ 4 files changed, 55 insertions(+), 3 deletions(-) diff --git a/include/lightstep/buffer_chain.h b/include/lightstep/buffer_chain.h index 84b68672..ebc280bb 100644 --- a/include/lightstep/buffer_chain.h +++ b/include/lightstep/buffer_chain.h @@ -5,7 +5,7 @@ namespace lightstep { class BufferChain { public: - using FragmentCallback = bool (*)(void*, const char*, size_t); + using FragmentCallback = bool (*)(void*, const void*, size_t); virtual ~BufferChain() = default; @@ -16,6 +16,6 @@ class BufferChain { virtual bool ForEachFragment(FragmentCallback callback, void* context) const = 0; - void Copy(char* data, size_t length) const noexcept; + void CopyOut(char* data, size_t length) const noexcept; }; } // namespace lightstep diff --git a/src/recorder/serialization/BUILD b/src/recorder/serialization/BUILD index 4d84b07b..f18b3ad5 100644 --- a/src/recorder/serialization/BUILD +++ b/src/recorder/serialization/BUILD @@ -16,6 +16,8 @@ lightstep_cc_library( ], deps = [ "//include/lightstep:buffer_chain_interface", + "//src/common:serialization_chain_lib", + ":embedded_metrics_message_lib", ], ) @@ -32,6 +34,7 @@ lightstep_cc_library( "//lightstep-tracer-common:collector_proto_cc", "//src/common:protobuf_lib", "//src/common:utility_lib", + ":embedded_metrics_message_lib", ], ) diff --git a/src/recorder/serialization/report_request.cpp b/src/recorder/serialization/report_request.cpp index 4ab77775..38b99834 100644 --- a/src/recorder/serialization/report_request.cpp +++ b/src/recorder/serialization/report_request.cpp @@ -1,4 +1,32 @@ #include "recorder/serialization/report_request.h" namespace lightstep { -} // namespace lightstep +//-------------------------------------------------------------------------------------------------- +// constructor +//-------------------------------------------------------------------------------------------------- +ReportRequest::ReportRequest(const std::shared_ptr& header, + std::unique_ptr&& metrics) + : header_{header} { + (void)metrics; +} + +//-------------------------------------------------------------------------------------------------- +// num_fragments +//-------------------------------------------------------------------------------------------------- +size_t ReportRequest::num_fragments() const noexcept { return 0; } + +//-------------------------------------------------------------------------------------------------- +// num_bytes +//-------------------------------------------------------------------------------------------------- +size_t ReportRequest::num_bytes() const noexcept { return 0; } + +//-------------------------------------------------------------------------------------------------- +// ForEachFragment +//-------------------------------------------------------------------------------------------------- +bool ReportRequest::ForEachFragment(FragmentCallback callback, + void* context) const { + (void)callback; + (void)context; + return true; +} +} // namespace lightstep diff --git a/src/recorder/serialization/report_request.h b/src/recorder/serialization/report_request.h index b11688d9..b6bfc3f2 100644 --- a/src/recorder/serialization/report_request.h +++ b/src/recorder/serialization/report_request.h @@ -1,10 +1,31 @@ #pragma once +#include + #include "lightstep/buffer_chain.h" +#include "recorder/serialization/embedded_metrics_message.h" +#include "common/serialization_chain.h" namespace lightstep { class ReportRequest final : public BufferChain { public: + ReportRequest(const std::shared_ptr& header, + std::unique_ptr&& metrics); + + // BufferChain + size_t num_fragments() const noexcept override; + + size_t num_bytes() const noexcept override; + + bool ForEachFragment(FragmentCallback callback, void* context) const override; + private: + std::shared_ptr header_; + + std::unique_ptr metrics_; + std::pair metrics_fragment_; + + int num_spans_{0}; + std::unique_ptr spans_; }; } // namespace lightstep From 28b8683599ace23654e44a1ed48fd5ff401543e8 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Thu, 16 Jan 2020 00:33:29 -0800 Subject: [PATCH 13/66] Fill out BufferChain --- src/common/BUILD | 10 ++++++++++ src/common/buffer_chain.cpp | 23 +++++++++++++++++++++++ src/recorder/serialization/BUILD | 2 +- 3 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 src/common/buffer_chain.cpp diff --git a/src/common/BUILD b/src/common/BUILD index d32bd70a..e003a89b 100644 --- a/src/common/BUILD +++ b/src/common/BUILD @@ -13,6 +13,16 @@ lightstep_cc_library( ], ) +lightstep_cc_library( + name = "buffer_chain_lib", + srcs = [ + "buffer_chain.cpp", + ], + deps = [ + "//include/lightstep:buffer_chain_interface", + ], +) + lightstep_cc_library( name = "spin_lock_mutex_lib", private_hdrs = [ diff --git a/src/common/buffer_chain.cpp b/src/common/buffer_chain.cpp new file mode 100644 index 00000000..3683cf52 --- /dev/null +++ b/src/common/buffer_chain.cpp @@ -0,0 +1,23 @@ +#include "lightstep/buffer_chain.h" + +#include +#include + +namespace lightstep { +//-------------------------------------------------------------------------------------------------- +// CopyOut +//-------------------------------------------------------------------------------------------------- +void BufferChain::CopyOut(char* data, size_t length) const noexcept { + if (length < this->num_bytes()) { + std::terminate(); + } + auto callback = [](void* context, const void* fragment_data, + size_t fragment_size) noexcept { + auto out = static_cast(context); + *out = std::copy_n(static_cast(fragment_data), fragment_size, + *out); + return true; + }; + this->ForEachFragment(callback, static_cast(&data)); +} +} // namespace lightstep diff --git a/src/recorder/serialization/BUILD b/src/recorder/serialization/BUILD index f18b3ad5..7037df30 100644 --- a/src/recorder/serialization/BUILD +++ b/src/recorder/serialization/BUILD @@ -15,7 +15,7 @@ lightstep_cc_library( "report_request.cpp", ], deps = [ - "//include/lightstep:buffer_chain_interface", + "//src/common:buffer_chain_lib", "//src/common:serialization_chain_lib", ":embedded_metrics_message_lib", ], From 36bcad279e411d91203fe75004573544676e303b Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Thu, 16 Jan 2020 01:17:18 -0800 Subject: [PATCH 14/66] Support joining SerializationChains --- src/common/serialization_chain.cpp | 24 ++++++++++++++++++++++++ src/common/serialization_chain.h | 17 +++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/src/common/serialization_chain.cpp b/src/common/serialization_chain.cpp index ecc1898a..12a231c0 100644 --- a/src/common/serialization_chain.cpp +++ b/src/common/serialization_chain.cpp @@ -28,6 +28,30 @@ static int WriteChunkHeader(char* data, size_t data_length, //-------------------------------------------------------------------------------------------------- SerializationChain::SerializationChain() noexcept : current_block_{&head_} {} +//-------------------------------------------------------------------------------------------------- +// Chain +//-------------------------------------------------------------------------------------------------- +void SerializationChain::Chain( + std::unique_ptr&& other) noexcept { + assert(other->next_ == nullptr); + other->next_ = std::move(next_); + next_ = std::move(other); +} + +//-------------------------------------------------------------------------------------------------- +// ForEachSerializationChain +//-------------------------------------------------------------------------------------------------- +bool SerializationChain::ForEachSerializationChain( + FunctionRef callback) { + if (!callback(*this)) { + return true; + } + if (next_ != nullptr) { + return next_->ForEachSerializationChain(callback); + } + return true; +} + //-------------------------------------------------------------------------------------------------- // AddFraming //-------------------------------------------------------------------------------------------------- diff --git a/src/common/serialization_chain.h b/src/common/serialization_chain.h index bf1cbff0..2987e8ba 100644 --- a/src/common/serialization_chain.h +++ b/src/common/serialization_chain.h @@ -3,7 +3,9 @@ #include #include #include +#include +#include "common/function_ref.h" #include "common/fragment_input_stream.h" #include "common/hex_conversion.h" #include "common/noncopyable.h" @@ -25,6 +27,19 @@ class SerializationChain final SerializationChain() noexcept; + /** + * Move another SerializationChain into this one. + * @param other the SerializationChain to add + */ + void Chain(std::unique_ptr&& other) noexcept; + + /** + * Iterate over all the owned SerializationChains + * @param callback the callback to call with each SerializationChain + */ + bool ForEachSerializationChain( + FunctionRef callback); + /** * Adds http/1.1 chunk framing and a message header so that the data can be * parsed as part of a protobuf ReportRequest. @@ -50,6 +65,8 @@ class SerializationChain final void Seek(int fragment_index, int position) noexcept override; private: + std::unique_ptr next_; + struct Block { std::unique_ptr next; int size; From 80383f32f3e2202749891eead1a6087e770754a4 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Thu, 16 Jan 2020 13:50:04 -0800 Subject: [PATCH 15/66] Add SerializationChain test coverage --- include/lightstep/transporter.h | 4 +- src/common/serialization_chain.h | 4 ++ test/common/serialization_chain_test.cpp | 51 ++++++++++++++++++++++++ 3 files changed, 57 insertions(+), 2 deletions(-) diff --git a/include/lightstep/transporter.h b/include/lightstep/transporter.h index 0d5200e3..d34068ec 100644 --- a/include/lightstep/transporter.h +++ b/include/lightstep/transporter.h @@ -81,9 +81,9 @@ class AsyncTransporter : public Transporter { Callback& operator=(const Callback&) noexcept = default; Callback& operator=(Callback&&) noexcept = default; - virtual void OnSuccess(const BufferChain& message) noexcept = 0; + virtual void OnSuccess(BufferChain& message) noexcept = 0; - virtual void OnFailure(const BufferChain& message) noexcept = 0; + virtual void OnFailure(BufferChain& message) noexcept = 0; }; virtual void Send(std::unique_ptr&& message, diff --git a/src/common/serialization_chain.h b/src/common/serialization_chain.h index 2987e8ba..5bb7bde2 100644 --- a/src/common/serialization_chain.h +++ b/src/common/serialization_chain.h @@ -29,6 +29,10 @@ class SerializationChain final /** * Move another SerializationChain into this one. + * + * Note: This should be used only as an intrusive container when the order of + * the SerializaionChains is not important. + * * @param other the SerializationChain to add */ void Chain(std::unique_ptr&& other) noexcept; diff --git a/test/common/serialization_chain_test.cpp b/test/common/serialization_chain_test.cpp index 914d9838..d7979ee2 100644 --- a/test/common/serialization_chain_test.cpp +++ b/test/common/serialization_chain_test.cpp @@ -74,4 +74,55 @@ TEST_CASE("SerializationChain") { } } } + + SECTION( + "ForEachSerializationChain behaves correctly when there are no other " + "owned chains") { + int i = 0; + auto was_successful = chain.ForEachSerializationChain([&]( + const SerializationChain& c) noexcept { + REQUIRE(&c == &chain); + ++i; + return true; + }); + REQUIRE(was_successful); + REQUIRE(i == 1); + was_successful = chain.ForEachSerializationChain([&]( + const SerializationChain& /*c*/) noexcept { return false; }); + } + + SECTION("We can chain SerializationChains together") { + auto chain2 = new SerializationChain{}; + chain.Chain(std::unique_ptr{chain2}); + int i = 0; + auto was_successful = chain.ForEachSerializationChain([&]( + const SerializationChain& c) noexcept { + if (i == 0) { + REQUIRE(&c == &chain); + } else { + REQUIRE(&c == chain2); + } + ++i; + return true; + }); + REQUIRE(i == 2); + REQUIRE(was_successful); + auto chain3 = new SerializationChain{}; + chain.Chain(std::unique_ptr{chain3}); + i = 0; + was_successful = chain.ForEachSerializationChain([&]( + const SerializationChain& c) noexcept { + if (i == 0) { + REQUIRE(&c == &chain); + } else if (i == 1) { + REQUIRE(&c == chain3); + } else { + REQUIRE(&c == chain2); + } + ++i; + return true; + }); + REQUIRE(i == 3); + REQUIRE(was_successful); + } } From 7f918fe2eff9eadb287104b72b29acf7885e388c Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Thu, 16 Jan 2020 15:57:15 -0800 Subject: [PATCH 16/66] Fill out ReportRequest --- src/common/serialization_chain.cpp | 14 +++++++++---- src/common/serialization_chain.h | 6 ++++++ src/recorder/serialization/report_request.cpp | 21 ++++++++++++------- src/recorder/serialization/report_request.h | 12 +++++++++-- test/common/serialization_chain_test.cpp | 9 ++++++-- test/recorder/serialization/BUILD | 10 +++++++++ .../serialization/report_request_test.cpp | 8 +++++++ 7 files changed, 64 insertions(+), 16 deletions(-) create mode 100644 test/recorder/serialization/report_request_test.cpp diff --git a/src/common/serialization_chain.cpp b/src/common/serialization_chain.cpp index 12a231c0..9e18964c 100644 --- a/src/common/serialization_chain.cpp +++ b/src/common/serialization_chain.cpp @@ -7,6 +7,7 @@ namespace lightstep { static const char* LineTerminator = "\r\n"; +const int LineTerminatorSize = 2; //-------------------------------------------------------------------------------------------------- // WriteChunkHeader @@ -59,7 +60,6 @@ void SerializationChain::AddFraming() noexcept { auto protobuf_header_size = ComputeLengthDelimitedHeaderSerializationSize( num_bytes_written_); - (void)WriteChunkHeader; header_size_ = WriteChunkHeader( header_.data(), header_.size(), static_cast(num_bytes_written_) + protobuf_header_size); @@ -72,6 +72,11 @@ void SerializationChain::AddFraming() noexcept { stream, static_cast(num_bytes_written_)); header_size_ += protobuf_header_size; + if (num_bytes_written_ > 0) { + num_bytes_after_framing_ = + header_size_ + num_bytes_written_ + LineTerminatorSize; + } + // Prepare the data structure to act as a FragmentInputStream. current_block_ = &head_; } @@ -161,12 +166,13 @@ bool SerializationChain::ForEachFragment(Callback callback) const noexcept { // chunk trailer if (fragment_index_ == num_blocks_ + 1) { - assert(fragment_position_ < 2); + assert(fragment_position_ < LineTerminatorSize); return callback(static_cast(const_cast(LineTerminator) + fragment_position_), - 2 - fragment_position_); + LineTerminatorSize - fragment_position_); } - return callback(static_cast(const_cast(LineTerminator)), 2); + return callback(static_cast(const_cast(LineTerminator)), + LineTerminatorSize); } //-------------------------------------------------------------------------------------------------- diff --git a/src/common/serialization_chain.h b/src/common/serialization_chain.h index 5bb7bde2..8ed09cf2 100644 --- a/src/common/serialization_chain.h +++ b/src/common/serialization_chain.h @@ -50,6 +50,11 @@ class SerializationChain final */ void AddFraming() noexcept; + /** + * @return The number of bytes after the SerializationChain has been framed + */ + int num_bytes_after_framing() const noexcept { return num_bytes_after_framing_; } + // ZeroCopyOutputStream bool Next(void** data, int* size) override; @@ -79,6 +84,7 @@ class SerializationChain final int num_blocks_{1}; int num_bytes_written_{0}; + int num_bytes_after_framing_{0}; int current_block_position_{0}; int header_size_{0}; Block* current_block_; diff --git a/src/recorder/serialization/report_request.cpp b/src/recorder/serialization/report_request.cpp index 38b99834..831f3b53 100644 --- a/src/recorder/serialization/report_request.cpp +++ b/src/recorder/serialization/report_request.cpp @@ -11,20 +11,25 @@ ReportRequest::ReportRequest(const std::shared_ptr& header, } //-------------------------------------------------------------------------------------------------- -// num_fragments -//-------------------------------------------------------------------------------------------------- -size_t ReportRequest::num_fragments() const noexcept { return 0; } - -//-------------------------------------------------------------------------------------------------- -// num_bytes -//-------------------------------------------------------------------------------------------------- -size_t ReportRequest::num_bytes() const noexcept { return 0; } +// AddSpan +//-------------------------------------------------------------------------------------------------- +void ReportRequest::AddSpan(std::unique_ptr&& span) noexcept { + ++num_spans_; + num_fragments_ += span->num_fragments(); + num_bytes_ += span->num_bytes_after_framing(); + if (spans_ == nullptr) { + spans_ = std::move(span); + return; + } + spans_->Chain(std::move(span)); +} //-------------------------------------------------------------------------------------------------- // ForEachFragment //-------------------------------------------------------------------------------------------------- bool ReportRequest::ForEachFragment(FragmentCallback callback, void* context) const { + (void)callback; (void)context; return true; diff --git a/src/recorder/serialization/report_request.h b/src/recorder/serialization/report_request.h index b6bfc3f2..733a8aed 100644 --- a/src/recorder/serialization/report_request.h +++ b/src/recorder/serialization/report_request.h @@ -12,10 +12,16 @@ class ReportRequest final : public BufferChain { ReportRequest(const std::shared_ptr& header, std::unique_ptr&& metrics); + void AddSpan(std::unique_ptr&& span) noexcept; + // BufferChain - size_t num_fragments() const noexcept override; + size_t num_fragments() const noexcept override { + return static_cast(num_fragments_); + } - size_t num_bytes() const noexcept override; + size_t num_bytes() const noexcept override { + return static_cast(num_bytes_); + } bool ForEachFragment(FragmentCallback callback, void* context) const override; @@ -25,7 +31,9 @@ class ReportRequest final : public BufferChain { std::unique_ptr metrics_; std::pair metrics_fragment_; + int num_bytes_{0}; int num_spans_{0}; + int num_fragments_{0}; std::unique_ptr spans_; }; } // namespace lightstep diff --git a/test/common/serialization_chain_test.cpp b/test/common/serialization_chain_test.cpp index d7979ee2..da16bda9 100644 --- a/test/common/serialization_chain_test.cpp +++ b/test/common/serialization_chain_test.cpp @@ -19,6 +19,7 @@ TEST_CASE("SerializationChain") { SECTION("An empty chain has no fragments.") { stream.reset(); chain.AddFraming(); + REQUIRE(chain.num_bytes_after_framing() == 0); REQUIRE(chain.num_fragments() == 0); } @@ -27,7 +28,9 @@ TEST_CASE("SerializationChain") { stream.reset(); chain.AddFraming(); REQUIRE(chain.num_fragments() == 3); - REQUIRE(ToString(chain) == AddSpanChunkFraming("abc")); + auto s = ToString(chain); + REQUIRE(s.size() == chain.num_bytes_after_framing()); + REQUIRE(s == AddSpanChunkFraming("abc")); } SECTION("We can write strings larger than a single block.") { @@ -36,7 +39,9 @@ TEST_CASE("SerializationChain") { stream.reset(); chain.AddFraming(); REQUIRE(chain.num_fragments() == 4); - REQUIRE(ToString(chain) == AddSpanChunkFraming(s)); + auto serialization = ToString(chain); + REQUIRE(serialization.size() == chain.num_bytes_after_framing()); + REQUIRE(serialization == AddSpanChunkFraming(s)); } SECTION("We can seek to any byte in the fragment stream.") { diff --git a/test/recorder/serialization/BUILD b/test/recorder/serialization/BUILD index 32007c2a..411efd98 100644 --- a/test/recorder/serialization/BUILD +++ b/test/recorder/serialization/BUILD @@ -26,3 +26,13 @@ lightstep_catch_test( "//src/recorder/serialization:embedded_metrics_message_lib", ], ) + +lightstep_catch_test( + name = "report_request_test", + srcs = [ + "report_request_test.cpp", + ], + deps = [ + "//src/recorder/serialization:report_request_lib", + ], +) diff --git a/test/recorder/serialization/report_request_test.cpp b/test/recorder/serialization/report_request_test.cpp new file mode 100644 index 00000000..4aa0bed2 --- /dev/null +++ b/test/recorder/serialization/report_request_test.cpp @@ -0,0 +1,8 @@ +#include "recorder/serialization/report_request.h" + +#include "3rd_party/catch2/catch.hpp" +#include "lightstep-tracer-common/collector.pb.h" +using namespace lightstep; + +TEST_CASE("ReportRequest") { +} From 300ed9452f6de067d4851c4b14363a96432c741f Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Thu, 16 Jan 2020 16:58:42 -0800 Subject: [PATCH 17/66] Fill out ReportRequest --- src/recorder/serialization/report_request.cpp | 28 +++++++++++++++---- src/recorder/serialization/report_request.h | 2 +- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/src/recorder/serialization/report_request.cpp b/src/recorder/serialization/report_request.cpp index 831f3b53..9e9f47d4 100644 --- a/src/recorder/serialization/report_request.cpp +++ b/src/recorder/serialization/report_request.cpp @@ -5,9 +5,10 @@ namespace lightstep { // constructor //-------------------------------------------------------------------------------------------------- ReportRequest::ReportRequest(const std::shared_ptr& header, - std::unique_ptr&& metrics) + int num_dropped_spans) : header_{header} { - (void)metrics; + (void)num_dropped_spans; + num_bytes_ = static_cast(header_->size()); } //-------------------------------------------------------------------------------------------------- @@ -29,9 +30,24 @@ void ReportRequest::AddSpan(std::unique_ptr&& span) noexcept //-------------------------------------------------------------------------------------------------- bool ReportRequest::ForEachFragment(FragmentCallback callback, void* context) const { - - (void)callback; - (void)context; - return true; + if (!callback(context, static_cast(header_->data()), + header_->size())) { + return false; + } + if (metrics_ != nullptr) { + if (!callback(context, metrics_fragment_.first, + static_cast(metrics_fragment_.second))) { + return false; + } + } + if (spans_ == nullptr) { + return true; + } + return spans_->ForEachSerializationChain( + [&](const SerializationChain& chain) { + return chain.ForEachFragment([&](void* data, int length) { + return callback(context, data, static_cast(length)); + }); + }); } } // namespace lightstep diff --git a/src/recorder/serialization/report_request.h b/src/recorder/serialization/report_request.h index 733a8aed..b387389f 100644 --- a/src/recorder/serialization/report_request.h +++ b/src/recorder/serialization/report_request.h @@ -10,7 +10,7 @@ namespace lightstep { class ReportRequest final : public BufferChain { public: ReportRequest(const std::shared_ptr& header, - std::unique_ptr&& metrics); + int num_dropped_spans); void AddSpan(std::unique_ptr&& span) noexcept; From 68cf920993750b571a6d3a9843c89e449d5244eb Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Thu, 16 Jan 2020 23:47:17 -0800 Subject: [PATCH 18/66] Add ChainedStream --- src/common/BUILD | 17 ++++++ src/common/chained_stream.cpp | 108 ++++++++++++++++++++++++++++++++++ src/common/chained_stream.h | 63 ++++++++++++++++++++ 3 files changed, 188 insertions(+) create mode 100644 src/common/chained_stream.cpp create mode 100644 src/common/chained_stream.h diff --git a/src/common/BUILD b/src/common/BUILD index e003a89b..660f8abf 100644 --- a/src/common/BUILD +++ b/src/common/BUILD @@ -59,6 +59,23 @@ lightstep_cc_library( ], ) +lightstep_cc_library( + name = "chained_stream_lib", + private_hdrs = [ + "chained_stream.h", + ], + srcs = [ + "chained_stream.cpp", + ], + deps = [ + ":fragment_input_stream_lib", + ":noncopyable_lib", + ], + external_deps = [ + "@com_google_protobuf//:protobuf", + ], +) + lightstep_cc_library( name = "circular_buffer_lib", private_hdrs = [ diff --git a/src/common/chained_stream.cpp b/src/common/chained_stream.cpp new file mode 100644 index 00000000..2ab35e14 --- /dev/null +++ b/src/common/chained_stream.cpp @@ -0,0 +1,108 @@ +#include "common/chained_stream.h" + +namespace lightstep { +//-------------------------------------------------------------------------------------------------- +// constructor +//-------------------------------------------------------------------------------------------------- +// +ChainedStream::ChainedStream() noexcept : current_block_{&head_} {} +//-------------------------------------------------------------------------------------------------- +// Next +//-------------------------------------------------------------------------------------------------- +bool ChainedStream::Next(void** data, int* size) { + if (current_block_position_ < BlockSize) { + *size = BlockSize - current_block_position_; + *data = static_cast(current_block_->data.data() + + current_block_position_); + num_bytes_written_ += *size; + current_block_position_ = BlockSize; + current_block_->size = BlockSize; + return true; + } + current_block_->next.reset(new Block{}); + current_block_ = current_block_->next.get(); + current_block_->size = BlockSize; + current_block_position_ = BlockSize; + *size = BlockSize; + num_bytes_written_ += *size; + *data = static_cast(current_block_->data.data()); + ++num_blocks_; + return true; +} + +//-------------------------------------------------------------------------------------------------- +// BackUp +//-------------------------------------------------------------------------------------------------- +void ChainedStream::BackUp(int count) { + num_bytes_written_ -= count; + current_block_position_ -= count; + current_block_->size -= count; +} + +//-------------------------------------------------------------------------------------------------- +// num_fragments +//-------------------------------------------------------------------------------------------------- +int ChainedStream::num_fragments() const noexcept { + if (num_bytes_written_ == 0) { + return 0; + } + return num_blocks_ - fragment_index_; +} + +//-------------------------------------------------------------------------------------------------- +// ForEachFragment +//-------------------------------------------------------------------------------------------------- +bool ChainedStream::ForEachFragment(Callback callback) const noexcept { + assert(fragment_index_ >= 0 && fragment_index_ <= num_blocks_); + + if (num_blocks_ == 0) { + return true; + } + + auto block = current_block_; + if (fragment_index_ > 0) { + auto block_size = block->size; + assert(block_size >= fragment_position_); + if (!callback(static_cast(const_cast(block->data.data()) + + fragment_position_), + block_size - fragment_position_)) { + return false; + } + block = block->next.get(); + } + while (block != nullptr) { + if (!callback(static_cast(const_cast(block->data.data())), + block->size)) { + return false; + } + block = block->next.get(); + } + return true; +} + +//-------------------------------------------------------------------------------------------------- +// Clear +//-------------------------------------------------------------------------------------------------- +void ChainedStream::Clear() noexcept { + num_blocks_ = 0; + num_bytes_written_ = 0; + fragment_index_ = 0; + fragment_position_ = 0; + current_block_ = nullptr; +} + +//-------------------------------------------------------------------------------------------------- +// Seek +//-------------------------------------------------------------------------------------------------- +void ChainedStream::Seek(int relative_fragment_index, int position) noexcept { + if (relative_fragment_index == 0) { + fragment_position_ += position; + return; + } + for (int i=0; inext.get(); + } + fragment_index_ += relative_fragment_index; + fragment_position_ = position; +} +} // namespace lightstep diff --git a/src/common/chained_stream.h b/src/common/chained_stream.h new file mode 100644 index 00000000..c08202d6 --- /dev/null +++ b/src/common/chained_stream.h @@ -0,0 +1,63 @@ +#pragma once + +#include +#include +#include +#include + +#include "common/function_ref.h" +#include "common/fragment_input_stream.h" +#include "common/noncopyable.h" + +#include + +namespace lightstep { +/** + * Maintains a linked chain of blocks as they aree written + */ +class ChainedStream final + : public google::protobuf::io::ZeroCopyOutputStream, + public FragmentInputStream, + private Noncopyable { + public: + static const int BlockSize = 256; + + ChainedStream() noexcept; + + // ZeroCopyOutputStream + bool Next(void** data, int* size) override; + + void BackUp(int count) override; + + google::protobuf::int64 ByteCount() const override { + return static_cast(num_bytes_written_); + } + + // FragmentInputStream + int num_fragments() const noexcept override; + + bool ForEachFragment(Callback callback) const noexcept override; + + void Clear() noexcept override; + + void Seek(int relative_fragment_index, int position) noexcept override; + + private: + struct Block { + std::unique_ptr next; + int size; + std::array data; + }; + + int num_blocks_{1}; + int num_bytes_written_{0}; + int num_bytes_after_framing_{0}; + int current_block_position_{0}; + Block* current_block_; + + int fragment_index_{0}; + int fragment_position_{0}; + + Block head_; +}; +} // namespace lightstep From 2dcd8959ca68b5301e33831e28dcb88e092e2f01 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Fri, 17 Jan 2020 00:15:45 -0800 Subject: [PATCH 19/66] Add test for ChainedStream --- test/common/BUILD | 11 +++++++++++ test/common/chained_stream_test.cpp | 14 ++++++++++++++ 2 files changed, 25 insertions(+) create mode 100644 test/common/chained_stream_test.cpp diff --git a/test/common/BUILD b/test/common/BUILD index a34070a4..ec928420 100644 --- a/test/common/BUILD +++ b/test/common/BUILD @@ -183,6 +183,17 @@ lightstep_catch_test( ], ) +lightstep_catch_test( + name = "chained_stream_test", + srcs = [ + "chained_stream_test.cpp", + ], + deps = [ + "//src/common:chained_stream_lib", + "//test:utility_lib", + ], +) + proto_library( name = "test_proto", srcs = ["test.proto"], diff --git a/test/common/chained_stream_test.cpp b/test/common/chained_stream_test.cpp new file mode 100644 index 00000000..17d38ba2 --- /dev/null +++ b/test/common/chained_stream_test.cpp @@ -0,0 +1,14 @@ +#include "common/chained_stream.h" + +#include +#include + +#include "test/utility.h" + +#include + +#include "3rd_party/catch2/catch.hpp" +using namespace lightstep; + +TEST_CASE("ChainedStream") { +} From d204bc0f1c12174e1a947a3cdf18bf560db028b0 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Fri, 17 Jan 2020 12:35:13 -0800 Subject: [PATCH 20/66] Fix ChainedStream --- src/common/chained_stream.cpp | 22 ++++++++++++++++++++++ src/common/chained_stream.h | 4 ++++ test/common/chained_stream_test.cpp | 27 +++++++++++++++++++++++++++ 3 files changed, 53 insertions(+) diff --git a/src/common/chained_stream.cpp b/src/common/chained_stream.cpp index 2ab35e14..d1e5372c 100644 --- a/src/common/chained_stream.cpp +++ b/src/common/chained_stream.cpp @@ -1,15 +1,28 @@ #include "common/chained_stream.h" +#include + namespace lightstep { //-------------------------------------------------------------------------------------------------- // constructor //-------------------------------------------------------------------------------------------------- // ChainedStream::ChainedStream() noexcept : current_block_{&head_} {} + +//-------------------------------------------------------------------------------------------------- +// CloseOutput +//-------------------------------------------------------------------------------------------------- +void ChainedStream::CloseOutput() noexcept { + output_closed_ = true; + current_block_ = &head_; +} + //-------------------------------------------------------------------------------------------------- // Next //-------------------------------------------------------------------------------------------------- bool ChainedStream::Next(void** data, int* size) { + assert(!output_closed_); + if (current_block_position_ < BlockSize) { *size = BlockSize - current_block_position_; *data = static_cast(current_block_->data.data() + @@ -34,6 +47,8 @@ bool ChainedStream::Next(void** data, int* size) { // BackUp //-------------------------------------------------------------------------------------------------- void ChainedStream::BackUp(int count) { + assert(!output_closed_); + num_bytes_written_ -= count; current_block_position_ -= count; current_block_->size -= count; @@ -43,6 +58,8 @@ void ChainedStream::BackUp(int count) { // num_fragments //-------------------------------------------------------------------------------------------------- int ChainedStream::num_fragments() const noexcept { + assert(output_closed_); + if (num_bytes_written_ == 0) { return 0; } @@ -53,6 +70,7 @@ int ChainedStream::num_fragments() const noexcept { // ForEachFragment //-------------------------------------------------------------------------------------------------- bool ChainedStream::ForEachFragment(Callback callback) const noexcept { + assert(output_closed_); assert(fragment_index_ >= 0 && fragment_index_ <= num_blocks_); if (num_blocks_ == 0) { @@ -84,6 +102,8 @@ bool ChainedStream::ForEachFragment(Callback callback) const noexcept { // Clear //-------------------------------------------------------------------------------------------------- void ChainedStream::Clear() noexcept { + assert(output_closed_); + num_blocks_ = 0; num_bytes_written_ = 0; fragment_index_ = 0; @@ -95,6 +115,8 @@ void ChainedStream::Clear() noexcept { // Seek //-------------------------------------------------------------------------------------------------- void ChainedStream::Seek(int relative_fragment_index, int position) noexcept { + assert(output_closed_); + if (relative_fragment_index == 0) { fragment_position_ += position; return; diff --git a/src/common/chained_stream.h b/src/common/chained_stream.h index c08202d6..81d2b79f 100644 --- a/src/common/chained_stream.h +++ b/src/common/chained_stream.h @@ -24,6 +24,8 @@ class ChainedStream final ChainedStream() noexcept; + void CloseOutput() noexcept; + // ZeroCopyOutputStream bool Next(void** data, int* size) override; @@ -49,6 +51,8 @@ class ChainedStream final std::array data; }; + bool output_closed_{false}; + int num_blocks_{1}; int num_bytes_written_{0}; int num_bytes_after_framing_{0}; diff --git a/test/common/chained_stream_test.cpp b/test/common/chained_stream_test.cpp index 17d38ba2..cac4829a 100644 --- a/test/common/chained_stream_test.cpp +++ b/test/common/chained_stream_test.cpp @@ -11,4 +11,31 @@ using namespace lightstep; TEST_CASE("ChainedStream") { + ChainedStream chain; + + std::unique_ptr stream{ + new google::protobuf::io::CodedOutputStream{&chain}}; + + SECTION("An empty chain has no fragments.") { + stream.reset(); + chain.CloseOutput(); + REQUIRE(chain.num_fragments() == 0); + } + + SECTION("We can write a string smaller than the block size.") { + stream->WriteString("abc"); + stream.reset(); + chain.CloseOutput(); + REQUIRE(chain.num_fragments() == 1); + REQUIRE(ToString(chain) == "abc"); + } + + SECTION("We can write strings larger than a single block.") { + std::string s(ChainedStream::BlockSize + 1, 'X'); + stream->WriteString(s); + stream.reset(); + chain.CloseOutput(); + REQUIRE(chain.num_fragments() == 2); + REQUIRE(ToString(chain) == s); + } } From 154c6c4d8b324884ff6a57b7875b428e34f17d08 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Fri, 17 Jan 2020 12:55:57 -0800 Subject: [PATCH 21/66] Add test coverage for ChainedStream --- src/common/chained_stream.cpp | 19 ++++++++-------- test/common/chained_stream_test.cpp | 34 +++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 10 deletions(-) diff --git a/src/common/chained_stream.cpp b/src/common/chained_stream.cpp index d1e5372c..18625ce8 100644 --- a/src/common/chained_stream.cpp +++ b/src/common/chained_stream.cpp @@ -6,7 +6,6 @@ namespace lightstep { //-------------------------------------------------------------------------------------------------- // constructor //-------------------------------------------------------------------------------------------------- -// ChainedStream::ChainedStream() noexcept : current_block_{&head_} {} //-------------------------------------------------------------------------------------------------- @@ -78,16 +77,16 @@ bool ChainedStream::ForEachFragment(Callback callback) const noexcept { } auto block = current_block_; - if (fragment_index_ > 0) { - auto block_size = block->size; - assert(block_size >= fragment_position_); - if (!callback(static_cast(const_cast(block->data.data()) + - fragment_position_), - block_size - fragment_position_)) { - return false; - } - block = block->next.get(); + + auto block_size = block->size; + assert(block_size >= fragment_position_); + if (!callback(static_cast(const_cast(block->data.data()) + + fragment_position_), + block_size - fragment_position_)) { + return false; } + + block = block->next.get(); while (block != nullptr) { if (!callback(static_cast(const_cast(block->data.data())), block->size)) { diff --git a/test/common/chained_stream_test.cpp b/test/common/chained_stream_test.cpp index cac4829a..1c108343 100644 --- a/test/common/chained_stream_test.cpp +++ b/test/common/chained_stream_test.cpp @@ -38,4 +38,38 @@ TEST_CASE("ChainedStream") { REQUIRE(chain.num_fragments() == 2); REQUIRE(ToString(chain) == s); } + + SECTION("We can seek to any byte in the fragment stream.") { + std::string s(SerializationChain::BlockSize + 2, 'X'); + stream->WriteString(s); + stream.reset(); + chain.CloseOutput(); + for (size_t i = 1; i <= s.size(); ++i) { + SECTION("cosumption instance " + std::to_string(i)) { + Consume({&chain}, i); + REQUIRE(ToString(chain) == s.substr(i)); + } + } + } + + SECTION("We can advance to any byte in the fragment stream randomly.") { + std::string s(3 * ChainedStream::BlockSize + 10, 'X'); + stream->WriteString(s); + stream.reset(); + chain.CloseOutput(); + std::mt19937 random_number_generator{0}; + for (int i = 0; i < 100; ++i) { + size_t num_bytes_consumed = 0; + SECTION("Random advance " + std::to_string(i)) { + while (num_bytes_consumed < s.size()) { + std::uniform_int_distribution distribution{ + 1, static_cast(s.size()) - num_bytes_consumed}; + auto n = distribution(random_number_generator); + Consume({&chain}, n); + num_bytes_consumed += n; + REQUIRE(ToString(chain) == s.substr(num_bytes_consumed)); + } + } + } + } } From da6e7cfccf54cd4e12f64a7055bc66d6462edd91 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Fri, 17 Jan 2020 14:53:38 -0800 Subject: [PATCH 22/66] Add ComposableFragmentInputStream --- src/common/BUILD | 13 ++++ src/common/chained_stream.cpp | 2 +- .../composable_fragment_input_stream.cpp | 77 +++++++++++++++++++ src/common/composable_fragment_input_stream.h | 33 ++++++++ test/common/BUILD | 11 +++ .../composable_fragment_input_stream_test.cpp | 9 +++ 6 files changed, 144 insertions(+), 1 deletion(-) create mode 100644 src/common/composable_fragment_input_stream.cpp create mode 100644 src/common/composable_fragment_input_stream.h create mode 100644 test/common/composable_fragment_input_stream_test.cpp diff --git a/src/common/BUILD b/src/common/BUILD index 660f8abf..a43409dc 100644 --- a/src/common/BUILD +++ b/src/common/BUILD @@ -243,6 +243,19 @@ lightstep_cc_library( ], ) +lightstep_cc_library( + name = "composable_fragment_input_stream_lib", + private_hdrs = [ + "composable_fragment_input_stream.h", + ], + srcs = [ + "composable_fragment_input_stream.cpp", + ], + deps = [ + ":fragment_input_stream_lib", + ], +) + lightstep_cc_library( name = "fragment_array_input_stream_lib", private_hdrs = [ diff --git a/src/common/chained_stream.cpp b/src/common/chained_stream.cpp index 18625ce8..6dee117c 100644 --- a/src/common/chained_stream.cpp +++ b/src/common/chained_stream.cpp @@ -115,7 +115,7 @@ void ChainedStream::Clear() noexcept { //-------------------------------------------------------------------------------------------------- void ChainedStream::Seek(int relative_fragment_index, int position) noexcept { assert(output_closed_); - + assert(fragment_index_ + relative_fragment_index <= num_blocks_); if (relative_fragment_index == 0) { fragment_position_ += position; return; diff --git a/src/common/composable_fragment_input_stream.cpp b/src/common/composable_fragment_input_stream.cpp new file mode 100644 index 00000000..d0619d93 --- /dev/null +++ b/src/common/composable_fragment_input_stream.cpp @@ -0,0 +1,77 @@ +#include "common/composable_fragment_input_stream.h" + +#include + +namespace lightstep { +//-------------------------------------------------------------------------------------------------- +// Append +//-------------------------------------------------------------------------------------------------- +void ComposableFragmentInputStream::Append( + std::unique_ptr&& stream) noexcept { + if (next_ == nullptr) { + next_ = std::move(stream); + last_ = next_.get(); + } + assert(last_ != nullptr); + auto prev_last = last_; + last_ = stream.get(); + prev_last->Append(std::move(stream)); +} + +//-------------------------------------------------------------------------------------------------- +// num_fragments +//-------------------------------------------------------------------------------------------------- +int ComposableFragmentInputStream::num_fragments() const noexcept { + auto stream = this; + int result = 0; + while (stream != nullptr) { + result += stream->segment_num_fragments(); + stream = stream->next_.get(); + } + return result; +} + +//-------------------------------------------------------------------------------------------------- +// ForEachFragment +//-------------------------------------------------------------------------------------------------- +bool ComposableFragmentInputStream::ForEachFragment(Callback callback) const + noexcept { + auto stream = this; + while (stream != nullptr) { + if (!stream->SegmentForEachFragment(callback)) { + return false; + } + stream = stream->next_.get(); + } + return true; +} + +//-------------------------------------------------------------------------------------------------- +// Clear +//-------------------------------------------------------------------------------------------------- +void ComposableFragmentInputStream::Clear() noexcept { + auto stream = this; + while (stream != nullptr) { + stream->Clear(); + stream = stream->next_.get(); + } +} + +//-------------------------------------------------------------------------------------------------- +// Seek +//-------------------------------------------------------------------------------------------------- +void ComposableFragmentInputStream::Seek(int fragment_index, + int position) noexcept { + auto stream = this; + while (stream != nullptr) { + auto segment_num_fragments = stream->segment_num_fragments(); + if (fragment_index < segment_num_fragments) { + return stream->SegmentSeek(fragment_index, position); + } + fragment_index -= segment_num_fragments; + stream->Clear(); + stream = stream->next_.get(); + } + assert(fragment_index == 0 && position == 0); +} +} // namespace lightstep diff --git a/src/common/composable_fragment_input_stream.h b/src/common/composable_fragment_input_stream.h new file mode 100644 index 00000000..686612da --- /dev/null +++ b/src/common/composable_fragment_input_stream.h @@ -0,0 +1,33 @@ +#pragma once + +#include + +#include "common/fragment_input_stream.h" + +namespace lightstep { +class ComposableFragmentInputStream : public FragmentInputStream { + public: + void Append(std::unique_ptr&& stream) noexcept; + + virtual int segment_num_fragments() const noexcept = 0; + + virtual bool SegmentForEachFragment(Callback callback) const noexcept = 0; + + virtual void SegmentClear() noexcept = 0; + + virtual void SegmentSeek(int fragment_index, int position) noexcept = 0; + + // FragmentInputStream + int num_fragments() const noexcept final; + + bool ForEachFragment(Callback callback) const noexcept final; + + void Clear() noexcept final; + + void Seek(int fragment_index, int position) noexcept final; + + private: + std::unique_ptr next_; + ComposableFragmentInputStream* last_{nullptr}; +}; +} // namespace lightstep diff --git a/test/common/BUILD b/test/common/BUILD index ec928420..3409d037 100644 --- a/test/common/BUILD +++ b/test/common/BUILD @@ -161,6 +161,17 @@ lightstep_catch_test( ], ) +lightstep_catch_test( + name = "composable_fragment_input_stream_test", + srcs = [ + "composable_fragment_input_stream_test.cpp", + ], + deps = [ + "//src/common:composable_fragment_input_stream_lib", + "//test:utility_lib", + ], +) + lightstep_catch_test( name = "fragment_array_input_stream_test", srcs = [ diff --git a/test/common/composable_fragment_input_stream_test.cpp b/test/common/composable_fragment_input_stream_test.cpp new file mode 100644 index 00000000..bb7dee9b --- /dev/null +++ b/test/common/composable_fragment_input_stream_test.cpp @@ -0,0 +1,9 @@ +#include + +#include "3rd_party/catch2/catch.hpp" +#include "common/composable_fragment_input_stream.h" +#include "test/utility.h" +using namespace lightstep; + +TEST_CASE("ComposableFragmentInputStream") { +} From 673cd2678e16e3b2e4879d4c19e0035f82ee9e2e Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Tue, 21 Jan 2020 21:40:41 -0800 Subject: [PATCH 23/66] Fix ComposableFragmentInputStream --- .../composable_fragment_input_stream.cpp | 5 +-- test/BUILD | 10 ++++++ test/common/BUILD | 2 ++ .../composable_fragment_input_stream_test.cpp | 28 +++++++++++++++ ...composable_fragment_input_stream_wrapper.h | 36 +++++++++++++++++++ 5 files changed, 79 insertions(+), 2 deletions(-) create mode 100644 test/composable_fragment_input_stream_wrapper.h diff --git a/src/common/composable_fragment_input_stream.cpp b/src/common/composable_fragment_input_stream.cpp index d0619d93..56aba936 100644 --- a/src/common/composable_fragment_input_stream.cpp +++ b/src/common/composable_fragment_input_stream.cpp @@ -11,6 +11,7 @@ void ComposableFragmentInputStream::Append( if (next_ == nullptr) { next_ = std::move(stream); last_ = next_.get(); + return; } assert(last_ != nullptr); auto prev_last = last_; @@ -52,7 +53,7 @@ bool ComposableFragmentInputStream::ForEachFragment(Callback callback) const void ComposableFragmentInputStream::Clear() noexcept { auto stream = this; while (stream != nullptr) { - stream->Clear(); + stream->SegmentClear(); stream = stream->next_.get(); } } @@ -69,7 +70,7 @@ void ComposableFragmentInputStream::Seek(int fragment_index, return stream->SegmentSeek(fragment_index, position); } fragment_index -= segment_num_fragments; - stream->Clear(); + stream->SegmentClear(); stream = stream->next_.get(); } assert(fragment_index == 0 && position == 0); diff --git a/test/BUILD b/test/BUILD index 9cf89b12..9da60555 100644 --- a/test/BUILD +++ b/test/BUILD @@ -141,3 +141,13 @@ lightstep_cc_library( "//include/lightstep:tracer_interface", ], ) + +lightstep_cc_library( + name = "composable_fragment_input_stream_wrapper_lib", + private_hdrs = [ + "composable_fragment_input_stream_wrapper.h", + ], + deps = [ + "//src/common:composable_fragment_input_stream_lib", + ], +) diff --git a/test/common/BUILD b/test/common/BUILD index 3409d037..bfa9633b 100644 --- a/test/common/BUILD +++ b/test/common/BUILD @@ -168,7 +168,9 @@ lightstep_catch_test( ], deps = [ "//src/common:composable_fragment_input_stream_lib", + "//src/common:fragment_array_input_stream_lib", "//test:utility_lib", + "//test:composable_fragment_input_stream_wrapper_lib", ], ) diff --git a/test/common/composable_fragment_input_stream_test.cpp b/test/common/composable_fragment_input_stream_test.cpp index bb7dee9b..7aa44800 100644 --- a/test/common/composable_fragment_input_stream_test.cpp +++ b/test/common/composable_fragment_input_stream_test.cpp @@ -2,8 +2,36 @@ #include "3rd_party/catch2/catch.hpp" #include "common/composable_fragment_input_stream.h" +#include "common/fragment_array_input_stream.h" #include "test/utility.h" +#include "test/composable_fragment_input_stream_wrapper.h" using namespace lightstep; TEST_CASE("ComposableFragmentInputStream") { + ComposableFragmentInputStreamWrapper stream{ + std::unique_ptr{new FragmentArrayInputStream{ + MakeFragment("abc"), MakeFragment("123")}}}; + stream.Append(std::unique_ptr{ + new ComposableFragmentInputStreamWrapper{ + std::unique_ptr{new FragmentArrayInputStream{ + MakeFragment("xyz"), MakeFragment("456")}}}}); + REQUIRE(ToString(stream) == "abc123xyz456"); + REQUIRE(stream.num_fragments() == 4); + + stream.Append(std::unique_ptr{ + new ComposableFragmentInputStreamWrapper{ + std::unique_ptr{ + new FragmentArrayInputStream{MakeFragment("qrz")}}}}); + REQUIRE(ToString(stream) == "abc123xyz456qrz"); + REQUIRE(stream.num_fragments() == 5); + + SECTION("We can consume from ComposableFragmentInputStream") { + auto contents = ToString(stream); + for (size_t i=0; i + +#include "common/composable_fragment_input_stream.h" + +namespace lightstep { +/** + * Wraps a FragmentInputStream to make it composable for testing. + */ +class ComposableFragmentInputStreamWrapper final + : public ComposableFragmentInputStream { + public: + explicit ComposableFragmentInputStreamWrapper( + std::unique_ptr&& stream) noexcept + : stream_{std::move(stream)} {} + + // ComposableFragmentInputStream + int segment_num_fragments() const noexcept override { + return stream_->num_fragments(); + } + + bool SegmentForEachFragment(Callback callback) const noexcept override { + return stream_->ForEachFragment(callback); + } + + void SegmentClear() noexcept override { return stream_->Clear(); } + + void SegmentSeek(int fragment_index, int position) noexcept override { + return stream_->Seek(fragment_index, position); + } + + private: + std::unique_ptr stream_; +}; +} // namespace lightstep From 14502832c5d0da9a98c5dad9cf403e6c6240f4cb Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Tue, 21 Jan 2020 22:13:44 -0800 Subject: [PATCH 24/66] Make ChainedStream composable --- src/common/BUILD | 1 + src/common/buffer_chain.cpp | 2 +- src/common/chained_stream.cpp | 21 ++++++++++--------- src/common/chained_stream.h | 21 +++++++++---------- .../composable_fragment_input_stream.cpp | 2 +- src/common/composable_fragment_input_stream.h | 18 ++++++++-------- src/common/serialization_chain.h | 8 ++++--- src/recorder/serialization/report_request.cpp | 3 ++- src/recorder/serialization/report_request.h | 4 ++-- .../composable_fragment_input_stream_test.cpp | 4 ++-- ...composable_fragment_input_stream_wrapper.h | 2 +- .../serialization/report_request_test.cpp | 3 +-- .../connection_stream_test.cpp | 2 +- 13 files changed, 47 insertions(+), 44 deletions(-) diff --git a/src/common/BUILD b/src/common/BUILD index a43409dc..d9b90c77 100644 --- a/src/common/BUILD +++ b/src/common/BUILD @@ -70,6 +70,7 @@ lightstep_cc_library( deps = [ ":fragment_input_stream_lib", ":noncopyable_lib", + ":composable_fragment_input_stream_lib", ], external_deps = [ "@com_google_protobuf//:protobuf", diff --git a/src/common/buffer_chain.cpp b/src/common/buffer_chain.cpp index 3683cf52..f2d5514c 100644 --- a/src/common/buffer_chain.cpp +++ b/src/common/buffer_chain.cpp @@ -20,4 +20,4 @@ void BufferChain::CopyOut(char* data, size_t length) const noexcept { }; this->ForEachFragment(callback, static_cast(&data)); } -} // namespace lightstep +} // namespace lightstep diff --git a/src/common/chained_stream.cpp b/src/common/chained_stream.cpp index 6dee117c..2a02fb9e 100644 --- a/src/common/chained_stream.cpp +++ b/src/common/chained_stream.cpp @@ -54,9 +54,9 @@ void ChainedStream::BackUp(int count) { } //-------------------------------------------------------------------------------------------------- -// num_fragments +// segment_num_fragments //-------------------------------------------------------------------------------------------------- -int ChainedStream::num_fragments() const noexcept { +int ChainedStream::segment_num_fragments() const noexcept { assert(output_closed_); if (num_bytes_written_ == 0) { @@ -66,9 +66,9 @@ int ChainedStream::num_fragments() const noexcept { } //-------------------------------------------------------------------------------------------------- -// ForEachFragment +// SegmentForEachFragment //-------------------------------------------------------------------------------------------------- -bool ChainedStream::ForEachFragment(Callback callback) const noexcept { +bool ChainedStream::SegmentForEachFragment(Callback callback) const noexcept { assert(output_closed_); assert(fragment_index_ >= 0 && fragment_index_ <= num_blocks_); @@ -98,9 +98,9 @@ bool ChainedStream::ForEachFragment(Callback callback) const noexcept { } //-------------------------------------------------------------------------------------------------- -// Clear +// SegmentClear //-------------------------------------------------------------------------------------------------- -void ChainedStream::Clear() noexcept { +void ChainedStream::SegmentClear() noexcept { assert(output_closed_); num_blocks_ = 0; @@ -111,19 +111,20 @@ void ChainedStream::Clear() noexcept { } //-------------------------------------------------------------------------------------------------- -// Seek +// SegmentSeek //-------------------------------------------------------------------------------------------------- -void ChainedStream::Seek(int relative_fragment_index, int position) noexcept { +void ChainedStream::SegmentSeek(int relative_fragment_index, + int position) noexcept { assert(output_closed_); assert(fragment_index_ + relative_fragment_index <= num_blocks_); if (relative_fragment_index == 0) { fragment_position_ += position; return; } - for (int i=0; inext.get(); } fragment_index_ += relative_fragment_index; fragment_position_ = position; } -} // namespace lightstep +} // namespace lightstep diff --git a/src/common/chained_stream.h b/src/common/chained_stream.h index 81d2b79f..5c073e7e 100644 --- a/src/common/chained_stream.h +++ b/src/common/chained_stream.h @@ -1,12 +1,12 @@ #pragma once #include +#include #include #include -#include +#include "common/composable_fragment_input_stream.h" #include "common/function_ref.h" -#include "common/fragment_input_stream.h" #include "common/noncopyable.h" #include @@ -15,10 +15,9 @@ namespace lightstep { /** * Maintains a linked chain of blocks as they aree written */ -class ChainedStream final - : public google::protobuf::io::ZeroCopyOutputStream, - public FragmentInputStream, - private Noncopyable { +class ChainedStream final : public google::protobuf::io::ZeroCopyOutputStream, + public ComposableFragmentInputStream, + private Noncopyable { public: static const int BlockSize = 256; @@ -36,13 +35,13 @@ class ChainedStream final } // FragmentInputStream - int num_fragments() const noexcept override; + int segment_num_fragments() const noexcept override; - bool ForEachFragment(Callback callback) const noexcept override; + bool SegmentForEachFragment(Callback callback) const noexcept override; - void Clear() noexcept override; + void SegmentClear() noexcept override; - void Seek(int relative_fragment_index, int position) noexcept override; + void SegmentSeek(int relative_fragment_index, int position) noexcept override; private: struct Block { @@ -64,4 +63,4 @@ class ChainedStream final Block head_; }; -} // namespace lightstep +} // namespace lightstep diff --git a/src/common/composable_fragment_input_stream.cpp b/src/common/composable_fragment_input_stream.cpp index 56aba936..91c11cca 100644 --- a/src/common/composable_fragment_input_stream.cpp +++ b/src/common/composable_fragment_input_stream.cpp @@ -75,4 +75,4 @@ void ComposableFragmentInputStream::Seek(int fragment_index, } assert(fragment_index == 0 && position == 0); } -} // namespace lightstep +} // namespace lightstep diff --git a/src/common/composable_fragment_input_stream.h b/src/common/composable_fragment_input_stream.h index 686612da..8c74b50f 100644 --- a/src/common/composable_fragment_input_stream.h +++ b/src/common/composable_fragment_input_stream.h @@ -7,17 +7,17 @@ namespace lightstep { class ComposableFragmentInputStream : public FragmentInputStream { public: - void Append(std::unique_ptr&& stream) noexcept; + void Append(std::unique_ptr&& stream) noexcept; - virtual int segment_num_fragments() const noexcept = 0; + virtual int segment_num_fragments() const noexcept = 0; - virtual bool SegmentForEachFragment(Callback callback) const noexcept = 0; + virtual bool SegmentForEachFragment(Callback callback) const noexcept = 0; - virtual void SegmentClear() noexcept = 0; + virtual void SegmentClear() noexcept = 0; - virtual void SegmentSeek(int fragment_index, int position) noexcept = 0; + virtual void SegmentSeek(int fragment_index, int position) noexcept = 0; - // FragmentInputStream + // FragmentInputStream int num_fragments() const noexcept final; bool ForEachFragment(Callback callback) const noexcept final; @@ -27,7 +27,7 @@ class ComposableFragmentInputStream : public FragmentInputStream { void Seek(int fragment_index, int position) noexcept final; private: - std::unique_ptr next_; - ComposableFragmentInputStream* last_{nullptr}; + std::unique_ptr next_; + ComposableFragmentInputStream* last_{nullptr}; }; -} // namespace lightstep +} // namespace lightstep diff --git a/src/common/serialization_chain.h b/src/common/serialization_chain.h index 8ed09cf2..29a7298e 100644 --- a/src/common/serialization_chain.h +++ b/src/common/serialization_chain.h @@ -1,12 +1,12 @@ #pragma once #include +#include #include #include -#include -#include "common/function_ref.h" #include "common/fragment_input_stream.h" +#include "common/function_ref.h" #include "common/hex_conversion.h" #include "common/noncopyable.h" #include "common/serialization.h" @@ -53,7 +53,9 @@ class SerializationChain final /** * @return The number of bytes after the SerializationChain has been framed */ - int num_bytes_after_framing() const noexcept { return num_bytes_after_framing_; } + int num_bytes_after_framing() const noexcept { + return num_bytes_after_framing_; + } // ZeroCopyOutputStream bool Next(void** data, int* size) override; diff --git a/src/recorder/serialization/report_request.cpp b/src/recorder/serialization/report_request.cpp index 9e9f47d4..415f4f14 100644 --- a/src/recorder/serialization/report_request.cpp +++ b/src/recorder/serialization/report_request.cpp @@ -14,7 +14,8 @@ ReportRequest::ReportRequest(const std::shared_ptr& header, //-------------------------------------------------------------------------------------------------- // AddSpan //-------------------------------------------------------------------------------------------------- -void ReportRequest::AddSpan(std::unique_ptr&& span) noexcept { +void ReportRequest::AddSpan( + std::unique_ptr&& span) noexcept { ++num_spans_; num_fragments_ += span->num_fragments(); num_bytes_ += span->num_bytes_after_framing(); diff --git a/src/recorder/serialization/report_request.h b/src/recorder/serialization/report_request.h index b387389f..282773c9 100644 --- a/src/recorder/serialization/report_request.h +++ b/src/recorder/serialization/report_request.h @@ -2,9 +2,9 @@ #include +#include "common/serialization_chain.h" #include "lightstep/buffer_chain.h" #include "recorder/serialization/embedded_metrics_message.h" -#include "common/serialization_chain.h" namespace lightstep { class ReportRequest final : public BufferChain { @@ -36,4 +36,4 @@ class ReportRequest final : public BufferChain { int num_fragments_{0}; std::unique_ptr spans_; }; -} // namespace lightstep +} // namespace lightstep diff --git a/test/common/composable_fragment_input_stream_test.cpp b/test/common/composable_fragment_input_stream_test.cpp index 7aa44800..19a4aef5 100644 --- a/test/common/composable_fragment_input_stream_test.cpp +++ b/test/common/composable_fragment_input_stream_test.cpp @@ -3,8 +3,8 @@ #include "3rd_party/catch2/catch.hpp" #include "common/composable_fragment_input_stream.h" #include "common/fragment_array_input_stream.h" -#include "test/utility.h" #include "test/composable_fragment_input_stream_wrapper.h" +#include "test/utility.h" using namespace lightstep; TEST_CASE("ComposableFragmentInputStream") { @@ -27,7 +27,7 @@ TEST_CASE("ComposableFragmentInputStream") { SECTION("We can consume from ComposableFragmentInputStream") { auto contents = ToString(stream); - for (size_t i=0; i stream_; }; -} // namespace lightstep +} // namespace lightstep diff --git a/test/recorder/serialization/report_request_test.cpp b/test/recorder/serialization/report_request_test.cpp index 4aa0bed2..eced07d2 100644 --- a/test/recorder/serialization/report_request_test.cpp +++ b/test/recorder/serialization/report_request_test.cpp @@ -4,5 +4,4 @@ #include "lightstep-tracer-common/collector.pb.h" using namespace lightstep; -TEST_CASE("ReportRequest") { -} +TEST_CASE("ReportRequest") {} diff --git a/test/recorder/stream_recorder/connection_stream_test.cpp b/test/recorder/stream_recorder/connection_stream_test.cpp index a9340cec..46e85c11 100644 --- a/test/recorder/stream_recorder/connection_stream_test.cpp +++ b/test/recorder/stream_recorder/connection_stream_test.cpp @@ -1,8 +1,8 @@ #include "recorder/stream_recorder/connection_stream.h" #include "3rd_party/catch2/catch.hpp" -#include "recorder/stream_recorder/span_stream.h" #include "recorder/serialization/report_request_header.h" +#include "recorder/stream_recorder/span_stream.h" #include "test/number_simulation.h" #include "test/utility.h" using namespace lightstep; From 776f28bf8dcd6c24baece38470322859cc2e5751 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Tue, 21 Jan 2020 23:08:39 -0800 Subject: [PATCH 25/66] Add ChainedStream to Recorder interface --- src/recorder/BUILD | 1 + src/recorder/recorder.h | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/src/recorder/BUILD b/src/recorder/BUILD index cb6bff05..0aae5cc2 100644 --- a/src/recorder/BUILD +++ b/src/recorder/BUILD @@ -36,6 +36,7 @@ lightstep_cc_library( ], deps = [ "//src/common:serialization_chain_lib", + "//src/common:chained_stream_lib", "//src/common:timestamp_lib", "//lightstep-tracer-common:collector_proto_cc", "//include/lightstep:tracer_interface", diff --git a/src/recorder/recorder.h b/src/recorder/recorder.h index a702b69a..435513a1 100644 --- a/src/recorder/recorder.h +++ b/src/recorder/recorder.h @@ -4,6 +4,7 @@ #include #include "common/serialization_chain.h" +#include "common/chained_stream.h" #include "common/timestamp.h" #include @@ -23,6 +24,11 @@ class Recorder { Recorder& operator=(Recorder&&) = delete; Recorder& operator=(const Recorder&) = delete; + virtual Fragment ReserveHeaderSpans(ChainedStream& /*stream*/) { return {}; } + + virtual void RecordSpan(Fragment /*header_fragment*/, + std::unique_ptr&& /*span*/) noexcept {} + /** * Record a Span * @span the protobuf span From 30795675fc6f9d800ca7369f59da6e46dfe84e97 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Tue, 21 Jan 2020 23:27:30 -0800 Subject: [PATCH 26/66] Add new ChainedStream into Span --- src/recorder/recorder.h | 2 +- src/tracer/span.cpp | 5 +++++ src/tracer/span.h | 5 +++++ 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/recorder/recorder.h b/src/recorder/recorder.h index 435513a1..c501cdae 100644 --- a/src/recorder/recorder.h +++ b/src/recorder/recorder.h @@ -24,7 +24,7 @@ class Recorder { Recorder& operator=(Recorder&&) = delete; Recorder& operator=(const Recorder&) = delete; - virtual Fragment ReserveHeaderSpans(ChainedStream& /*stream*/) { return {}; } + virtual Fragment ReserveHeaderSpace(ChainedStream& /*stream*/) { return {}; } virtual void RecordSpan(Fragment /*header_fragment*/, std::unique_ptr&& /*span*/) noexcept {} diff --git a/src/tracer/span.cpp b/src/tracer/span.cpp index a3010ef2..50432b52 100644 --- a/src/tracer/span.cpp +++ b/src/tracer/span.cpp @@ -24,6 +24,11 @@ Span::Span(std::shared_ptr&& tracer, const opentracing::StartSpanOptions& options) : serialization_chain_{new SerializationChain{}}, stream_{serialization_chain_.get()}, + chained_stream_{new ChainedStream{}}, + header_fragment_{ + tracer->recorder().ReserveHeaderSpace(*chained_stream_) + }, + coded_stream_{chained_stream_.get()}, tracer_{std::move(tracer)} { WriteOperationName(stream_, operation_name); diff --git a/src/tracer/span.h b/src/tracer/span.h index 4419607d..d7ff797d 100644 --- a/src/tracer/span.h +++ b/src/tracer/span.h @@ -6,6 +6,7 @@ #include #include "common/serialization_chain.h" +#include "common/chained_stream.h" #include "common/spin_lock_mutex.h" #include "tracer/baggage_flat_map.h" #include "tracer/lightstep_span_context.h" @@ -97,6 +98,10 @@ class Span final : public opentracing::Span, public LightStepSpanContext { std::unique_ptr serialization_chain_; google::protobuf::io::CodedOutputStream stream_; + std::unique_ptr chained_stream_; + Fragment header_fragment_; + google::protobuf::io::CodedOutputStream coded_stream_; + std::chrono::steady_clock::time_point start_steady_; std::atomic is_finished_{false}; From 3009a7b21bcf50bc4ed8ccc7b5db0df074d58364 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Wed, 22 Jan 2020 14:02:39 -0800 Subject: [PATCH 27/66] Fill out the ChainedStream --- src/tracer/span.cpp | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/tracer/span.cpp b/src/tracer/span.cpp index 50432b52..69fe9551 100644 --- a/src/tracer/span.cpp +++ b/src/tracer/span.cpp @@ -31,6 +31,7 @@ Span::Span(std::shared_ptr&& tracer, coded_stream_{chained_stream_.get()}, tracer_{std::move(tracer)} { WriteOperationName(stream_, operation_name); + WriteOperationName(coded_stream_, operation_name); // Set the start timestamps. std::chrono::system_clock::time_point start_timestamp; @@ -38,6 +39,7 @@ Span::Span(std::shared_ptr&& tracer, tracer_->recorder(), options.start_system_timestamp, options.start_steady_timestamp); WriteStartTimestamp(stream_, start_timestamp); + WriteStartTimestamp(coded_stream_, start_timestamp); // Set any span references. sampled_ = false; @@ -68,6 +70,7 @@ Span::Span(std::shared_ptr&& tracer, // Set tags. for (auto& tag : options.tags) { WriteTag(stream_, tag.first, tag.second); + WriteTag(coded_stream_, tag.first, tag.second); // If sampling_priority is set, it overrides whatever sampling decision was // derived from the referenced spans. @@ -105,6 +108,7 @@ void Span::SetOperationName(opentracing::string_view name) noexcept try { return; } WriteOperationName(stream_, name); + WriteOperationName(coded_stream_, name); } catch (const std::exception& e) { tracer_->logger().Error("SetOperationName failed: ", e.what()); } @@ -119,6 +123,7 @@ void Span::SetTag(opentracing::string_view key, return; } WriteTag(stream_, key, value); + WriteTag(coded_stream_, key, value); if (key == SamplingPriorityKey) { sampled_ = is_sampled(value); } @@ -171,6 +176,7 @@ void Span::Log(std::initializer_list< return; } WriteLog(stream_, timestamp, fields.begin(), fields.end()); + WriteLog(coded_stream_, timestamp, fields.begin(), fields.end()); } catch (const std::exception& e) { tracer_->logger().Error("Log failed: ", e.what()); } @@ -218,6 +224,8 @@ bool Span::SetSpanReference( trace_id = referenced_context->trace_id_low(); WriteSpanReference(stream_, reference.first, trace_id, referenced_context->span_id()); + WriteSpanReference(coded_stream_, reference.first, trace_id, + referenced_context->span_id()); sampled_ = sampled_ || referenced_context->sampled(); referenced_context->ForeachBaggageItem( [this](const std::string& key, const std::string& value) { @@ -249,22 +257,28 @@ void Span::FinishImpl( // Set timing information. auto duration = finish_timestamp - start_steady_; WriteDuration(stream_, duration); + WriteDuration(coded_stream_, duration); // Set logs for (auto& log_record : options.log_records) { try { WriteLog(stream_, log_record.timestamp, log_record.fields.data(), log_record.fields.data() + log_record.fields.size()); + WriteLog(coded_stream_, log_record.timestamp, log_record.fields.data(), + log_record.fields.data() + log_record.fields.size()); } catch (const std::exception& e) { tracer_->logger().Error("Dropping log record: ", e.what()); } } WriteSpanContext(stream_, trace_id_, span_id_, baggage_.as_vector()); + WriteSpanContext(coded_stream_, trace_id_, span_id_, baggage_.as_vector()); // Record the span stream_.Trim(); + coded_stream_.Trim(); tracer_->recorder().RecordSpan(std::move(serialization_chain_)); + tracer_->recorder().RecordSpan(header_fragment_, std::move(chained_stream_)); } catch (const std::exception& e) { tracer_->logger().Error("FinishWithOptions failed: ", e.what()); } From 35ba5b1ae508e8d28bcd84eb0cecf25481b32dd1 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Wed, 22 Jan 2020 22:18:05 -0800 Subject: [PATCH 28/66] Add function for serializing ReportRequest headers --- src/common/BUILD | 13 +++++++++ src/common/report_request_framing.cpp | 20 +++++++++++++ src/common/report_request_framing.h | 17 +++++++++++ test/common/BUILD | 11 ++++++++ test/common/report_request_framing_test.cpp | 31 +++++++++++++++++++++ 5 files changed, 92 insertions(+) create mode 100644 src/common/report_request_framing.cpp create mode 100644 src/common/report_request_framing.h create mode 100644 test/common/report_request_framing_test.cpp diff --git a/src/common/BUILD b/src/common/BUILD index d9b90c77..8880efdf 100644 --- a/src/common/BUILD +++ b/src/common/BUILD @@ -288,6 +288,19 @@ lightstep_cc_library( ], ) +lightstep_cc_library( + name = "report_request_framing_lib", + private_hdrs = [ + "report_request_framing.h", + ], + srcs = [ + "report_request_framing.cpp", + ], + deps = [ + ":serialization_lib", + ], +) + lightstep_cc_library( name = "timestamp_lib", private_hdrs = [ diff --git a/src/common/report_request_framing.cpp b/src/common/report_request_framing.cpp new file mode 100644 index 00000000..0c6407c6 --- /dev/null +++ b/src/common/report_request_framing.cpp @@ -0,0 +1,20 @@ +#include "common/report_request_framing.h" + +#include + +namespace lightstep { +//-------------------------------------------------------------------------------------------------- +// WriteReportRequestSpansHeader +//-------------------------------------------------------------------------------------------------- +size_t WriteReportRequestSpansHeader(char* data, size_t size, uint32_t body_size) noexcept { + assert(size >= ReportRequestSpansMaxHeaderSize); + auto header_size = + ComputeLengthDelimitedHeaderSerializationSize( + body_size); + auto serialization_start = data + (size - header_size); + DirectCodedOutputStream stream{ + reinterpret_cast(serialization_start)}; + WriteKeyLength(stream, body_size); + return header_size; +} +} // namespace lightstep diff --git a/src/common/report_request_framing.h b/src/common/report_request_framing.h new file mode 100644 index 00000000..9653c872 --- /dev/null +++ b/src/common/report_request_framing.h @@ -0,0 +1,17 @@ +#pragma once + +#include + +#include "common/serialization.h" + +namespace lightstep { +static const size_t ReportRequestSpansField = 3; + +const size_t ReportRequestSpansMaxHeaderSize = + StaticKeySerializationSize::value + + google::protobuf::io::CodedOutputStream::StaticVarintSize32< + std::numeric_limits::max()>::value; + +size_t WriteReportRequestSpansHeader(char* data, size_t size, uint32_t body_size) noexcept; +} // namespace lightstep diff --git a/test/common/BUILD b/test/common/BUILD index bfa9633b..89899f55 100644 --- a/test/common/BUILD +++ b/test/common/BUILD @@ -252,3 +252,14 @@ lightstep_catch_test( "//src/common:timestamp_lib", ], ) + +lightstep_catch_test( + name = "report_request_framing_test", + srcs = [ + "report_request_framing_test.cpp", + ], + deps = [ + "//lightstep-tracer-common:collector_proto_cc", + "//src/common:report_request_framing_lib", + ], +) diff --git a/test/common/report_request_framing_test.cpp b/test/common/report_request_framing_test.cpp new file mode 100644 index 00000000..eba61057 --- /dev/null +++ b/test/common/report_request_framing_test.cpp @@ -0,0 +1,31 @@ +#include "common/report_request_framing.h" + +#include "lightstep-tracer-common/collector.pb.h" + +#include "3rd_party/catch2/catch.hpp" +using namespace lightstep; + +TEST_CASE("ReportRequestFraming") { + std::string header_serialization(ReportRequestSpansMaxHeaderSize, ' '); + + SECTION("We can successfully parse out serialized spans") { + collector::ReportRequest report; + collector::Span span; + span.mutable_span_context()->set_trace_id(123); + auto s = span.SerializeAsString(); + auto header_size = WriteReportRequestSpansHeader( + &header_serialization[0], header_serialization.size(), s.size()); + s = header_serialization.substr(header_serialization.size() - header_size) + + s; + REQUIRE(report.ParseFromString(s)); + REQUIRE(report.spans().size() == 1); + REQUIRE(report.spans()[0].span_context().trace_id() == 123); + } + + SECTION("We can serialize the largest header") { + REQUIRE(WriteReportRequestSpansHeader( + &header_serialization[0], header_serialization.size(), + std::numeric_limits::max()) == + header_serialization.size()); + } +} From 48783215c26a78f670daac3985d63d5474a88aad Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Wed, 22 Jan 2020 23:36:23 -0800 Subject: [PATCH 29/66] Add code for framing chunked http --- src/common/BUILD | 14 ++++++++++++++ src/common/chunked_http_framing.cpp | 19 +++++++++++++++++++ src/common/chunked_http_framing.h | 10 ++++++++++ 3 files changed, 43 insertions(+) create mode 100644 src/common/chunked_http_framing.cpp create mode 100644 src/common/chunked_http_framing.h diff --git a/src/common/BUILD b/src/common/BUILD index 8880efdf..60fe5dca 100644 --- a/src/common/BUILD +++ b/src/common/BUILD @@ -301,6 +301,20 @@ lightstep_cc_library( ], ) +lightstep_cc_library( + name = "chunked_http_framing_lib", + private_hdrs = [ + "chunked_http_framing.h", + ], + srcs = [ + "chunked_http_framing.cpp", + ], + deps = [ + ":serialization_lib", + ":hex_conversion_lib", + ], +) + lightstep_cc_library( name = "timestamp_lib", private_hdrs = [ diff --git a/src/common/chunked_http_framing.cpp b/src/common/chunked_http_framing.cpp new file mode 100644 index 00000000..c480ee96 --- /dev/null +++ b/src/common/chunked_http_framing.cpp @@ -0,0 +1,19 @@ +#include "common/chunked_http_framing.h" + +namespace lightstep { +//-------------------------------------------------------------------------------------------------- +// WriteHttpChunkHeader +//-------------------------------------------------------------------------------------------------- +size_t WriteHttpChunkHeader(char* data, size_t size, + uint32_t chunk_size) noexcept { + assert(size >= ChunkedHttpMaxHeaderSize); + auto serialization_start = data + (size - ChunkedHttpMaxHeaderSize); + auto chunk_size_str = + Uint32ToHex(static_cast(chunk_size), serialization_start); + assert(chunk_size_str.size() == Num32BitHexDigits); + auto iter = serialization_start + chunk_size_str.size(); + *iter++ = '\r'; + *iter++ = '\n'; + return ChunkedHttpMaxHeaderSize; +} +} // namespace lightstep diff --git a/src/common/chunked_http_framing.h b/src/common/chunked_http_framing.h new file mode 100644 index 00000000..42903e53 --- /dev/null +++ b/src/common/chunked_http_framing.h @@ -0,0 +1,10 @@ +#pragma once + +#include "common/hex_conversion.h" + +namespace lightstep { +const size_t ChunkedHttpMaxHeaderSize = Num32BitHexDigits + 2; + +size_t WriteHttpChunkHeader(char* data, size_t size, + uint32_t chunk_size) noexcept; +} // namespace lightstep From ec3e1739073ba03e0a589bd46a6d5eabcf6d620c Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Thu, 23 Jan 2020 13:57:31 -0800 Subject: [PATCH 30/66] Add test for chunked http framing --- test/common/BUILD | 10 ++++++++++ test/common/chunked_http_framing_test.cpp | 10 ++++++++++ 2 files changed, 20 insertions(+) create mode 100644 test/common/chunked_http_framing_test.cpp diff --git a/test/common/BUILD b/test/common/BUILD index 89899f55..9d65dae9 100644 --- a/test/common/BUILD +++ b/test/common/BUILD @@ -263,3 +263,13 @@ lightstep_catch_test( "//src/common:report_request_framing_lib", ], ) + +lightstep_catch_test( + name = "chunked_http_framing_test", + srcs = [ + "chunked_http_framing_test.cpp", + ], + deps = [ + "//src/common:chunked_http_framing_lib", + ], +) diff --git a/test/common/chunked_http_framing_test.cpp b/test/common/chunked_http_framing_test.cpp new file mode 100644 index 00000000..1fbdae30 --- /dev/null +++ b/test/common/chunked_http_framing_test.cpp @@ -0,0 +1,10 @@ +#include "common/chunked_http_framing.h" + +#include "3rd_party/catch2/catch.hpp" +using namespace lightstep; + +TEST_CASE("ChunkedHttpFraming") { + std::string header_serialization(ChunkedHttpMaxHeaderSize + 1, ' '); + WriteHttpChunkHeader(&header_serialization[0], header_serialization.size(), 10); + REQUIRE(header_serialization == " 0000000a\r\n"); +} From 6fc1174e5d5622909fef545e7d0d3c87102724c4 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Thu, 23 Jan 2020 14:18:14 -0800 Subject: [PATCH 31/66] Add ability to write a footer --- src/recorder/BUILD | 3 +++ src/recorder/recorder.h | 4 ++++ src/tracer/span.cpp | 1 + 3 files changed, 8 insertions(+) diff --git a/src/recorder/BUILD b/src/recorder/BUILD index 0aae5cc2..aedc90ab 100644 --- a/src/recorder/BUILD +++ b/src/recorder/BUILD @@ -41,6 +41,9 @@ lightstep_cc_library( "//lightstep-tracer-common:collector_proto_cc", "//include/lightstep:tracer_interface", ], + external_deps = [ + "@com_google_protobuf//:protobuf", + ], ) lightstep_cc_library( diff --git a/src/recorder/recorder.h b/src/recorder/recorder.h index c501cdae..f75eaf64 100644 --- a/src/recorder/recorder.h +++ b/src/recorder/recorder.h @@ -7,6 +7,7 @@ #include "common/chained_stream.h" #include "common/timestamp.h" +#include #include #include "lightstep-tracer-common/collector.pb.h" @@ -26,6 +27,9 @@ class Recorder { virtual Fragment ReserveHeaderSpace(ChainedStream& /*stream*/) { return {}; } + virtual void WriteFooter( + google::protobuf::io::CodedOutputStream& /*coded_stream*/) {} + virtual void RecordSpan(Fragment /*header_fragment*/, std::unique_ptr&& /*span*/) noexcept {} diff --git a/src/tracer/span.cpp b/src/tracer/span.cpp index 69fe9551..82c7a62c 100644 --- a/src/tracer/span.cpp +++ b/src/tracer/span.cpp @@ -276,6 +276,7 @@ void Span::FinishImpl( // Record the span stream_.Trim(); + tracer_->recorder().WriteFooter(coded_stream_); coded_stream_.Trim(); tracer_->recorder().RecordSpan(std::move(serialization_chain_)); tracer_->recorder().RecordSpan(header_fragment_, std::move(chained_stream_)); From 33a0c5ab6543ddbc39e927cc0db3172f0763b4fc Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Thu, 23 Jan 2020 15:59:24 -0800 Subject: [PATCH 32/66] Work on replace SerializationChain --- src/common/BUILD | 3 ++ src/common/chunked_http_framing.h | 4 ++ src/recorder/stream_recorder/BUILD | 2 + .../stream_recorder/stream_recorder.cpp | 46 +++++++++++++++++++ .../stream_recorder/stream_recorder.h | 8 ++++ 5 files changed, 63 insertions(+) diff --git a/src/common/BUILD b/src/common/BUILD index 60fe5dca..558bcc59 100644 --- a/src/common/BUILD +++ b/src/common/BUILD @@ -313,6 +313,9 @@ lightstep_cc_library( ":serialization_lib", ":hex_conversion_lib", ], + external_deps = [ + "@io_opentracing_cpp//:opentracing", + ], ) lightstep_cc_library( diff --git a/src/common/chunked_http_framing.h b/src/common/chunked_http_framing.h index 42903e53..cb3aec67 100644 --- a/src/common/chunked_http_framing.h +++ b/src/common/chunked_http_framing.h @@ -2,9 +2,13 @@ #include "common/hex_conversion.h" +#include + namespace lightstep { const size_t ChunkedHttpMaxHeaderSize = Num32BitHexDigits + 2; +const opentracing::string_view ChunkedHttpFooter = "\r\n"; + size_t WriteHttpChunkHeader(char* data, size_t size, uint32_t chunk_size) noexcept; } // namespace lightstep diff --git a/src/recorder/stream_recorder/BUILD b/src/recorder/stream_recorder/BUILD index d633a533..d9124a5a 100644 --- a/src/recorder/stream_recorder/BUILD +++ b/src/recorder/stream_recorder/BUILD @@ -77,6 +77,8 @@ lightstep_cc_library( deps = [ "//src/common:circular_buffer_lib", "//src/common:logger_lib", + "//src/common:report_request_framing_lib", + "//src/common:chunked_http_framing_lib", "//src/common:noncopyable_lib", "//src/common:protobuf_lib", "//src/common/platform:network_environment_lib", diff --git a/src/recorder/stream_recorder/stream_recorder.cpp b/src/recorder/stream_recorder/stream_recorder.cpp index c4e39c88..df1b56cf 100644 --- a/src/recorder/stream_recorder/stream_recorder.cpp +++ b/src/recorder/stream_recorder/stream_recorder.cpp @@ -1,7 +1,10 @@ #include "recorder/stream_recorder/stream_recorder.h" #include +#include +#include "common/chunked_http_framing.h" +#include "common/report_request_framing.h" #include "common/protobuf.h" namespace lightstep { @@ -43,9 +46,52 @@ StreamRecorder::~StreamRecorder() noexcept { shutdown_condition_variable_.notify_all(); } +//-------------------------------------------------------------------------------------------------- +// ReserveHedaerSpace +//-------------------------------------------------------------------------------------------------- +Fragment StreamRecorder::ReserveHeaderSpace(ChainedStream& stream) { + const size_t max_header_size = + ReportRequestSpansMaxHeaderSize + ChunkedHttpMaxHeaderSize; + static_assert(ChainedStream::BlockSize >= max_header_size, + "BockSize too small"); + void* data; + int size; + if(!stream.Next(&data, &size)) { + throw std::bad_alloc{}; + } + stream.BackUp(size - static_cast(max_header_size)); + return {data, size}; +} + +//-------------------------------------------------------------------------------------------------- +// WriteFooter +//-------------------------------------------------------------------------------------------------- +void StreamRecorder::WriteFooter( + google::protobuf::io::CodedOutputStream& coded_stream) { + coded_stream.WriteRaw(ChunkedHttpFooter.data(), ChunkedHttpFooter.size()); +} + //-------------------------------------------------------------------------------------------------- // RecordSpan //-------------------------------------------------------------------------------------------------- +void StreamRecorder::RecordSpan( + Fragment header_fragment, std::unique_ptr&& span) noexcept { + // Frame the Span + char* header_data = static_cast(header_fragment.first); + auto reserved_header_size = static_cast(header_fragment.second); + auto protobuf_body_size = span->ByteCount() - ChunkedHttpFooter.size(); + auto protobuf_header_size = + WriteReportRequestSpansHeader(header_data, reserved_header_size, protobuf_body_size); + auto chunk_body_size = protobuf_body_size + protobuf_header_size; + auto chunk_header_size = WriteHttpChunkHeader( + header_data, reserved_header_size - protobuf_header_size, chunk_body_size); + span->CloseOutput(); + + // Advance past reserved header space we didn't use. + span->Seek(0, static_cast(reserved_header_size - protobuf_header_size - chunk_header_size)); + +} + void StreamRecorder::RecordSpan( std::unique_ptr&& span) noexcept { span->AddFraming(); diff --git a/src/recorder/stream_recorder/stream_recorder.h b/src/recorder/stream_recorder/stream_recorder.h index 19cfd6ea..1ebdf140 100644 --- a/src/recorder/stream_recorder/stream_recorder.h +++ b/src/recorder/stream_recorder/stream_recorder.h @@ -89,8 +89,16 @@ class StreamRecorder : public ForkAwareRecorder, private Noncopyable { } // Recorder + Fragment ReserveHeaderSpace(ChainedStream& stream) override; + + void WriteFooter( + google::protobuf::io::CodedOutputStream& coded_stream) override; + void RecordSpan(std::unique_ptr&& span) noexcept override; + void RecordSpan(Fragment header_fragment, + std::unique_ptr&& span) noexcept override; + bool FlushWithTimeout( std::chrono::system_clock::duration timeout) noexcept override; From 584e80618d35b6c6e51f1a5b3ee75f7a0efc22d9 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Fri, 24 Jan 2020 15:15:35 -0800 Subject: [PATCH 33/66] Renaming --- src/common/chunked_http_framing.cpp | 2 +- src/common/report_request_framing.cpp | 5 ++-- src/common/report_request_framing.h | 5 ++-- src/recorder/recorder.h | 2 +- .../stream_recorder/stream_recorder.cpp | 23 ++++++++++--------- src/tracer/span.cpp | 4 +--- src/tracer/span.h | 2 +- test/common/chunked_http_framing_test.cpp | 3 ++- test/number_simulation.cpp | 2 +- .../connection_stream_test.cpp | 14 +++++------ .../stream_recorder/span_stream_test.cpp | 12 +++++----- test/utility.cpp | 6 ++--- test/utility.h | 4 ++-- 13 files changed, 43 insertions(+), 41 deletions(-) diff --git a/src/common/chunked_http_framing.cpp b/src/common/chunked_http_framing.cpp index c480ee96..dfc89407 100644 --- a/src/common/chunked_http_framing.cpp +++ b/src/common/chunked_http_framing.cpp @@ -16,4 +16,4 @@ size_t WriteHttpChunkHeader(char* data, size_t size, *iter++ = '\n'; return ChunkedHttpMaxHeaderSize; } -} // namespace lightstep +} // namespace lightstep diff --git a/src/common/report_request_framing.cpp b/src/common/report_request_framing.cpp index 0c6407c6..05dacb1c 100644 --- a/src/common/report_request_framing.cpp +++ b/src/common/report_request_framing.cpp @@ -6,7 +6,8 @@ namespace lightstep { //-------------------------------------------------------------------------------------------------- // WriteReportRequestSpansHeader //-------------------------------------------------------------------------------------------------- -size_t WriteReportRequestSpansHeader(char* data, size_t size, uint32_t body_size) noexcept { +size_t WriteReportRequestSpansHeader(char* data, size_t size, + uint32_t body_size) noexcept { assert(size >= ReportRequestSpansMaxHeaderSize); auto header_size = ComputeLengthDelimitedHeaderSerializationSize( @@ -17,4 +18,4 @@ size_t WriteReportRequestSpansHeader(char* data, size_t size, uint32_t body_size WriteKeyLength(stream, body_size); return header_size; } -} // namespace lightstep +} // namespace lightstep diff --git a/src/common/report_request_framing.h b/src/common/report_request_framing.h index 9653c872..3a0d6e1f 100644 --- a/src/common/report_request_framing.h +++ b/src/common/report_request_framing.h @@ -13,5 +13,6 @@ const size_t ReportRequestSpansMaxHeaderSize = google::protobuf::io::CodedOutputStream::StaticVarintSize32< std::numeric_limits::max()>::value; -size_t WriteReportRequestSpansHeader(char* data, size_t size, uint32_t body_size) noexcept; -} // namespace lightstep +size_t WriteReportRequestSpansHeader(char* data, size_t size, + uint32_t body_size) noexcept; +} // namespace lightstep diff --git a/src/recorder/recorder.h b/src/recorder/recorder.h index f75eaf64..94fa1f5b 100644 --- a/src/recorder/recorder.h +++ b/src/recorder/recorder.h @@ -3,8 +3,8 @@ #include #include -#include "common/serialization_chain.h" #include "common/chained_stream.h" +#include "common/serialization_chain.h" #include "common/timestamp.h" #include diff --git a/src/recorder/stream_recorder/stream_recorder.cpp b/src/recorder/stream_recorder/stream_recorder.cpp index df1b56cf..4e9ee608 100644 --- a/src/recorder/stream_recorder/stream_recorder.cpp +++ b/src/recorder/stream_recorder/stream_recorder.cpp @@ -1,11 +1,11 @@ #include "recorder/stream_recorder/stream_recorder.h" -#include #include +#include #include "common/chunked_http_framing.h" -#include "common/report_request_framing.h" #include "common/protobuf.h" +#include "common/report_request_framing.h" namespace lightstep { //-------------------------------------------------------------------------------------------------- @@ -50,13 +50,13 @@ StreamRecorder::~StreamRecorder() noexcept { // ReserveHedaerSpace //-------------------------------------------------------------------------------------------------- Fragment StreamRecorder::ReserveHeaderSpace(ChainedStream& stream) { - const size_t max_header_size = - ReportRequestSpansMaxHeaderSize + ChunkedHttpMaxHeaderSize; + const size_t max_header_size = + ReportRequestSpansMaxHeaderSize + ChunkedHttpMaxHeaderSize; static_assert(ChainedStream::BlockSize >= max_header_size, "BockSize too small"); void* data; int size; - if(!stream.Next(&data, &size)) { + if (!stream.Next(&data, &size)) { throw std::bad_alloc{}; } stream.BackUp(size - static_cast(max_header_size)); @@ -67,7 +67,7 @@ Fragment StreamRecorder::ReserveHeaderSpace(ChainedStream& stream) { // WriteFooter //-------------------------------------------------------------------------------------------------- void StreamRecorder::WriteFooter( - google::protobuf::io::CodedOutputStream& coded_stream) { + google::protobuf::io::CodedOutputStream& coded_stream) { coded_stream.WriteRaw(ChunkedHttpFooter.data(), ChunkedHttpFooter.size()); } @@ -80,16 +80,17 @@ void StreamRecorder::RecordSpan( char* header_data = static_cast(header_fragment.first); auto reserved_header_size = static_cast(header_fragment.second); auto protobuf_body_size = span->ByteCount() - ChunkedHttpFooter.size(); - auto protobuf_header_size = - WriteReportRequestSpansHeader(header_data, reserved_header_size, protobuf_body_size); + auto protobuf_header_size = WriteReportRequestSpansHeader( + header_data, reserved_header_size, protobuf_body_size); auto chunk_body_size = protobuf_body_size + protobuf_header_size; auto chunk_header_size = WriteHttpChunkHeader( - header_data, reserved_header_size - protobuf_header_size, chunk_body_size); + header_data, reserved_header_size - protobuf_header_size, + chunk_body_size); span->CloseOutput(); // Advance past reserved header space we didn't use. - span->Seek(0, static_cast(reserved_header_size - protobuf_header_size - chunk_header_size)); - + span->Seek(0, static_cast(reserved_header_size - protobuf_header_size - + chunk_header_size)); } void StreamRecorder::RecordSpan( diff --git a/src/tracer/span.cpp b/src/tracer/span.cpp index 82c7a62c..2e8a0a9e 100644 --- a/src/tracer/span.cpp +++ b/src/tracer/span.cpp @@ -25,9 +25,7 @@ Span::Span(std::shared_ptr&& tracer, : serialization_chain_{new SerializationChain{}}, stream_{serialization_chain_.get()}, chained_stream_{new ChainedStream{}}, - header_fragment_{ - tracer->recorder().ReserveHeaderSpace(*chained_stream_) - }, + header_fragment_{tracer->recorder().ReserveHeaderSpace(*chained_stream_)}, coded_stream_{chained_stream_.get()}, tracer_{std::move(tracer)} { WriteOperationName(stream_, operation_name); diff --git a/src/tracer/span.h b/src/tracer/span.h index d7ff797d..a599825c 100644 --- a/src/tracer/span.h +++ b/src/tracer/span.h @@ -5,8 +5,8 @@ #include #include -#include "common/serialization_chain.h" #include "common/chained_stream.h" +#include "common/serialization_chain.h" #include "common/spin_lock_mutex.h" #include "tracer/baggage_flat_map.h" #include "tracer/lightstep_span_context.h" diff --git a/test/common/chunked_http_framing_test.cpp b/test/common/chunked_http_framing_test.cpp index 1fbdae30..91c1aecb 100644 --- a/test/common/chunked_http_framing_test.cpp +++ b/test/common/chunked_http_framing_test.cpp @@ -5,6 +5,7 @@ using namespace lightstep; TEST_CASE("ChunkedHttpFraming") { std::string header_serialization(ChunkedHttpMaxHeaderSize + 1, ' '); - WriteHttpChunkHeader(&header_serialization[0], header_serialization.size(), 10); + WriteHttpChunkHeader(&header_serialization[0], header_serialization.size(), + 10); REQUIRE(header_serialization == " 0000000a\r\n"); } diff --git a/test/number_simulation.cpp b/test/number_simulation.cpp index c135da8c..0bfcaeae 100644 --- a/test/number_simulation.cpp +++ b/test/number_simulation.cpp @@ -53,7 +53,7 @@ static void GenerateRandomBinaryNumbers( uint32_t x; opentracing::string_view s; std::tie(x, s) = GenerateRandomBinaryNumber(32); - if (AddString(buffer, s)) { + if (AddSpanChunkFramedString(buffer, s)) { numbers.push_back(x); } } diff --git a/test/recorder/stream_recorder/connection_stream_test.cpp b/test/recorder/stream_recorder/connection_stream_test.cpp index 46e85c11..d6985b1c 100644 --- a/test/recorder/stream_recorder/connection_stream_test.cpp +++ b/test/recorder/stream_recorder/connection_stream_test.cpp @@ -144,7 +144,7 @@ TEST_CASE("ConnectionStream") { SECTION( "After writing the header, ConnectionStream writes the contents of the " "span buffer.") { - AddString(span_buffer, "abc"); + AddSpanChunkFramedString(span_buffer, "abc"); connection_stream.Flush( [&contents]( std::initializer_list fragment_streams) { @@ -162,14 +162,14 @@ TEST_CASE("ConnectionStream") { contents = ToString(fragment_streams); return Consume(fragment_streams, static_cast(contents.size())); }); - AddString(span_buffer, "abc"); + AddSpanChunkFramedString(span_buffer, "abc"); connection_stream.Flush( [&contents]( std::initializer_list fragment_streams) { contents = ToString(fragment_streams); return Consume(fragment_streams, 4); }); - AddString(span_buffer, "123"); + AddSpanChunkFramedString(span_buffer, "123"); connection_stream.Flush( [&contents]( std::initializer_list fragment_streams) { @@ -190,7 +190,7 @@ TEST_CASE("ConnectionStream") { contents = ToString(fragment_streams); return Consume(fragment_streams, static_cast(contents.size())); }); - AddString(span_buffer, "abc"); + AddSpanChunkFramedString(span_buffer, "abc"); connection_stream.Flush( [&contents]( std::initializer_list fragment_streams) { @@ -198,7 +198,7 @@ TEST_CASE("ConnectionStream") { return Consume(fragment_streams, 4); }); connection_stream.Shutdown(); - AddString(span_buffer, "123"); + AddSpanChunkFramedString(span_buffer, "123"); connection_stream.Flush( [&contents]( std::initializer_list fragment_streams) { @@ -217,7 +217,7 @@ TEST_CASE("ConnectionStream") { contents = ToString(fragment_streams); return Consume(fragment_streams, static_cast(contents.size())); }); - AddString(span_buffer, "abc"); + AddSpanChunkFramedString(span_buffer, "abc"); connection_stream.Flush( [&contents]( std::initializer_list fragment_streams) { @@ -225,7 +225,7 @@ TEST_CASE("ConnectionStream") { return Consume(fragment_streams, 4); }); connection_stream.Reset(); - AddString(span_buffer, "123"); + AddSpanChunkFramedString(span_buffer, "123"); connection_stream.Flush( [&contents]( std::initializer_list fragment_streams) { diff --git a/test/recorder/stream_recorder/span_stream_test.cpp b/test/recorder/stream_recorder/span_stream_test.cpp index 7766f262..87ccb4ba 100644 --- a/test/recorder/stream_recorder/span_stream_test.cpp +++ b/test/recorder/stream_recorder/span_stream_test.cpp @@ -21,20 +21,20 @@ TEST_CASE("SpanStream") { } SECTION("SpanStream mirrors the contents of its attached buffer") { - REQUIRE(AddString(buffer, "abc123")); + REQUIRE(AddSpanChunkFramedString(buffer, "abc123")); span_stream.Allot(); REQUIRE(ToString(span_stream) == AddSpanChunkFraming("abc123")); } SECTION("SpanStream is empty after it's been cleared") { - REQUIRE(AddString(buffer, "abc123")); + REQUIRE(AddSpanChunkFramedString(buffer, "abc123")); span_stream.Allot(); span_stream.Clear(); REQUIRE(span_stream.empty()); } SECTION("SpanStream leaves a remnant if a span is partially consumed") { - REQUIRE(AddString(buffer, "abc123")); + REQUIRE(AddSpanChunkFramedString(buffer, "abc123")); span_stream.Allot(); auto contents = ToString(span_stream); REQUIRE(!Consume({&span_stream}, 3)); @@ -45,13 +45,13 @@ TEST_CASE("SpanStream") { } SECTION("SpanStream leaves no remnant when spans are completely consumed") { - REQUIRE(AddString(buffer, "abc123")); + REQUIRE(AddSpanChunkFramedString(buffer, "abc123")); span_stream.Allot(); span_stream.Clear(); REQUIRE(span_stream.ConsumeRemnant() == nullptr); - REQUIRE(AddString(buffer, "abc")); - REQUIRE(AddString(buffer, "123")); + REQUIRE(AddSpanChunkFramedString(buffer, "abc")); + REQUIRE(AddSpanChunkFramedString(buffer, "123")); span_stream.Allot(); auto span1 = AddSpanChunkFraming("abc"); REQUIRE(!Consume({&span_stream}, static_cast(span1.size()))); diff --git a/test/utility.cpp b/test/utility.cpp index 4c085f46..75070705 100644 --- a/test/utility.cpp +++ b/test/utility.cpp @@ -103,10 +103,10 @@ std::string ToString(const FragmentInputStream& fragment_input_stream) { } //-------------------------------------------------------------------------------------------------- -// AddString +// AddSpanChunkFramedString //-------------------------------------------------------------------------------------------------- -bool AddString(CircularBuffer& buffer, - const std::string& s) { +bool AddSpanChunkFramedString(CircularBuffer& buffer, + const std::string& s) { std::unique_ptr chain{new SerializationChain{}}; { google::protobuf::io::CodedOutputStream stream{chain.get()}; diff --git a/test/utility.h b/test/utility.h index 29b5b1fb..b9a9bdca 100644 --- a/test/utility.h +++ b/test/utility.h @@ -66,8 +66,8 @@ std::string ToString(const FragmentInputStream& fragment_input_stream); * @param s the string to add. * @return true if the string was succesfully added. */ -bool AddString(CircularBuffer& buffer, - const std::string& s); +bool AddSpanChunkFramedString(CircularBuffer& buffer, + const std::string& s); /** * Adds http/1.1 chunk framing and ReportRequest embedded span framing to a From cfd079bc32b380d606d4f6e9b1e6aac8020fdef5 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Fri, 24 Jan 2020 15:29:19 -0800 Subject: [PATCH 34/66] Add version of AddSpanChunkFramedString for ChainedStream --- test/BUILD | 1 + test/utility.cpp | 12 ++++++++++++ test/utility.h | 4 ++++ 3 files changed, 17 insertions(+) diff --git a/test/BUILD b/test/BUILD index 9da60555..b77223cd 100644 --- a/test/BUILD +++ b/test/BUILD @@ -21,6 +21,7 @@ lightstep_cc_library( "//src/common:fragment_input_stream_lib", "//src/common:serialization_lib", "//src/common:serialization_chain_lib", + "//src/common:chained_stream_lib", "//src/common:circular_buffer_lib", ], external_deps = [ diff --git a/test/utility.cpp b/test/utility.cpp index 75070705..75a8f52a 100644 --- a/test/utility.cpp +++ b/test/utility.cpp @@ -116,6 +116,18 @@ bool AddSpanChunkFramedString(CircularBuffer& buffer, return buffer.Add(chain); } +bool AddSpanChunkFramedString(CircularBuffer& buffer, + const std::string& s) { + auto framed_s = AddSpanChunkFraming(s); + std::unique_ptr chain{new ChainedStream{}}; + { + google::protobuf::io::CodedOutputStream stream{chain.get()}; + stream.WriteString(s); + } + chain->CloseOutput(); + return buffer.Add(chain); +} + //-------------------------------------------------------------------------------------------------- // AddSpanChunkFraming //-------------------------------------------------------------------------------------------------- diff --git a/test/utility.h b/test/utility.h index b9a9bdca..2b26ab86 100644 --- a/test/utility.h +++ b/test/utility.h @@ -6,6 +6,7 @@ #include "common/circular_buffer.h" #include "common/fragment_input_stream.h" #include "common/serialization_chain.h" +#include "common/chained_stream.h" #include #include "lightstep-tracer-common/collector.pb.h" @@ -69,6 +70,9 @@ std::string ToString(const FragmentInputStream& fragment_input_stream); bool AddSpanChunkFramedString(CircularBuffer& buffer, const std::string& s); +bool AddSpanChunkFramedString(CircularBuffer& buffer, + const std::string& s); + /** * Adds http/1.1 chunk framing and ReportRequest embedded span framing to a * string. From 926d4930d6d556981cb24dac6a81efd240a1cca4 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Fri, 24 Jan 2020 16:53:20 -0800 Subject: [PATCH 35/66] Switch StreamRecorder to use ChainedStream --- src/common/chained_stream.cpp | 2 ++ src/recorder/stream_recorder/BUILD | 4 ++-- .../stream_recorder/connection_stream.h | 3 ++- .../stream_recorder/satellite_streamer.cpp | 2 +- .../stream_recorder/satellite_streamer.h | 4 ++-- src/recorder/stream_recorder/span_stream.cpp | 20 +++++++++---------- src/recorder/stream_recorder/span_stream.h | 12 +++++------ .../stream_recorder/stream_recorder.cpp | 9 +++------ .../stream_recorder/stream_recorder.h | 8 +++----- src/tracer/span.cpp | 2 +- test/number_simulation.cpp | 4 ++-- test/number_simulation.h | 4 ++-- .../connection_stream_test.cpp | 4 ++-- .../stream_recorder/span_stream_test.cpp | 2 +- test/utility.cpp | 2 +- 15 files changed, 40 insertions(+), 42 deletions(-) diff --git a/src/common/chained_stream.cpp b/src/common/chained_stream.cpp index 2a02fb9e..4a7fb928 100644 --- a/src/common/chained_stream.cpp +++ b/src/common/chained_stream.cpp @@ -119,6 +119,7 @@ void ChainedStream::SegmentSeek(int relative_fragment_index, assert(fragment_index_ + relative_fragment_index <= num_blocks_); if (relative_fragment_index == 0) { fragment_position_ += position; + assert(fragment_position_ <= current_block_->size); return; } for (int i = 0; i < relative_fragment_index; ++i) { @@ -126,5 +127,6 @@ void ChainedStream::SegmentSeek(int relative_fragment_index, } fragment_index_ += relative_fragment_index; fragment_position_ = position; + assert(fragment_position_ <= current_block_->size); } } // namespace lightstep diff --git a/src/recorder/stream_recorder/BUILD b/src/recorder/stream_recorder/BUILD index d9124a5a..890dc40e 100644 --- a/src/recorder/stream_recorder/BUILD +++ b/src/recorder/stream_recorder/BUILD @@ -39,7 +39,7 @@ lightstep_cc_library( ], deps = [ "//src/common:circular_buffer_lib", - "//src/common:serialization_chain_lib", + "//src/common:chained_stream_lib", "//src/common:fragment_input_stream_lib", ":stream_recorder_metrics_lib", ], @@ -82,7 +82,7 @@ lightstep_cc_library( "//src/common:noncopyable_lib", "//src/common:protobuf_lib", "//src/common/platform:network_environment_lib", - "//src/common:serialization_chain_lib", + "//src/common:chained_stream_lib", "//src/network:event_lib", "//src/network:timer_event_lib", "//src/recorder:fork_aware_recorder_lib", diff --git a/src/recorder/stream_recorder/connection_stream.h b/src/recorder/stream_recorder/connection_stream.h index 61477a92..530e192e 100644 --- a/src/recorder/stream_recorder/connection_stream.h +++ b/src/recorder/stream_recorder/connection_stream.h @@ -4,6 +4,7 @@ #include #include +#include "common/hex_conversion.h" #include "common/fragment_array_input_stream.h" #include "common/fragment_input_stream.h" #include "common/function_ref.h" @@ -72,7 +73,7 @@ class ConnectionStream { FragmentArrayInputStream header_stream_; FragmentArrayInputStream terminal_stream_; - std::unique_ptr span_remnant_; + std::unique_ptr span_remnant_; bool shutting_down_; diff --git a/src/recorder/stream_recorder/satellite_streamer.cpp b/src/recorder/stream_recorder/satellite_streamer.cpp index b47c24b8..59f601f5 100644 --- a/src/recorder/stream_recorder/satellite_streamer.cpp +++ b/src/recorder/stream_recorder/satellite_streamer.cpp @@ -15,7 +15,7 @@ SatelliteStreamer::SatelliteStreamer( const LightStepTracerOptions& tracer_options, const StreamRecorderOptions& recorder_options, StreamRecorderMetrics& metrics, - CircularBuffer& span_buffer) + CircularBuffer& span_buffer) : logger_{logger}, event_base_{event_base}, tracer_options_{tracer_options}, diff --git a/src/recorder/stream_recorder/satellite_streamer.h b/src/recorder/stream_recorder/satellite_streamer.h index 3acff999..f1994842 100644 --- a/src/recorder/stream_recorder/satellite_streamer.h +++ b/src/recorder/stream_recorder/satellite_streamer.h @@ -20,7 +20,7 @@ class SatelliteStreamer : private Noncopyable { const LightStepTracerOptions& tracer_options, const StreamRecorderOptions& recorder_options, StreamRecorderMetrics& metrics, - CircularBuffer& span_buffer); + CircularBuffer& span_buffer); /** * @return the associated Logger. @@ -90,7 +90,7 @@ class SatelliteStreamer : private Noncopyable { const StreamRecorderOptions& recorder_options_; std::string header_common_fragment_; SatelliteEndpointManager endpoint_manager_; - CircularBuffer& span_buffer_; + CircularBuffer& span_buffer_; SpanStream span_stream_; std::vector> connections_; RandomTraverser connection_traverser_; diff --git a/src/recorder/stream_recorder/span_stream.cpp b/src/recorder/stream_recorder/span_stream.cpp index 573bd0cb..180dc638 100644 --- a/src/recorder/stream_recorder/span_stream.cpp +++ b/src/recorder/stream_recorder/span_stream.cpp @@ -4,7 +4,7 @@ namespace lightstep { //-------------------------------------------------------------------------------------------------- // constructor //-------------------------------------------------------------------------------------------------- -SpanStream::SpanStream(CircularBuffer& span_buffer, +SpanStream::SpanStream(CircularBuffer& span_buffer, StreamRecorderMetrics& metrics) noexcept : span_buffer_{span_buffer}, metrics_{metrics} {} @@ -16,8 +16,8 @@ void SpanStream::Allot() noexcept { allotment_ = span_buffer_.Peek(); } //-------------------------------------------------------------------------------------------------- // ConsumeRemnant //-------------------------------------------------------------------------------------------------- -std::unique_ptr SpanStream::ConsumeRemnant() noexcept { - return std::unique_ptr{remnant_.release()}; +std::unique_ptr SpanStream::ConsumeRemnant() noexcept { + return std::unique_ptr{remnant_.release()}; } //-------------------------------------------------------------------------------------------------- @@ -26,7 +26,7 @@ std::unique_ptr SpanStream::ConsumeRemnant() noexcept { int SpanStream::num_fragments() const noexcept { int result = 0; allotment_.ForEach([&result]( - const AtomicUniquePtr& span) noexcept { + const AtomicUniquePtr& span) noexcept { result += span->num_fragments(); return true; }); @@ -38,7 +38,7 @@ int SpanStream::num_fragments() const noexcept { //-------------------------------------------------------------------------------------------------- bool SpanStream::ForEachFragment(Callback callback) const noexcept { return allotment_.ForEach( - [callback](const AtomicUniquePtr& span) { + [callback](const AtomicUniquePtr& span) { return span->ForEachFragment(callback); }); } @@ -50,7 +50,7 @@ void SpanStream::Clear() noexcept { remnant_.reset(); metrics_.OnSpansSent(allotment_.size()); span_buffer_.Consume(allotment_.size()); - allotment_ = CircularBufferRange>{}; + allotment_ = CircularBufferRange>{}; } //-------------------------------------------------------------------------------------------------- @@ -61,7 +61,7 @@ void SpanStream::Seek(int fragment_index, int position) noexcept { int full_span_count = 0; int span_count = 0; allotment_.ForEach([&, fragment_index ]( - const AtomicUniquePtr& span) mutable noexcept { + const AtomicUniquePtr& span) mutable noexcept { auto num_fragments = span->num_fragments(); if (num_fragments <= fragment_index) { fragment_index -= num_fragments; @@ -79,9 +79,9 @@ void SpanStream::Seek(int fragment_index, int position) noexcept { }); span_buffer_.Consume( span_count, [ this, fragment_index, position ]( - CircularBufferRange> + CircularBufferRange> range) mutable noexcept { - range.ForEach([&](AtomicUniquePtr & span) noexcept { + range.ForEach([&](AtomicUniquePtr & span) noexcept { auto num_fragments = span->num_fragments(); if (num_fragments <= fragment_index) { fragment_index -= num_fragments; @@ -94,6 +94,6 @@ void SpanStream::Seek(int fragment_index, int position) noexcept { }); }); metrics_.OnSpansSent(full_span_count); - allotment_ = CircularBufferRange>{}; + allotment_ = CircularBufferRange>{}; } } // namespace lightstep diff --git a/src/recorder/stream_recorder/span_stream.h b/src/recorder/stream_recorder/span_stream.h index 966d8c87..39f277d7 100644 --- a/src/recorder/stream_recorder/span_stream.h +++ b/src/recorder/stream_recorder/span_stream.h @@ -1,7 +1,7 @@ #pragma once #include "common/circular_buffer.h" -#include "common/serialization_chain.h" +#include "common/chained_stream.h" #include "recorder/stream_recorder/stream_recorder_metrics.h" namespace lightstep { @@ -11,7 +11,7 @@ namespace lightstep { */ class SpanStream final : public FragmentInputStream { public: - SpanStream(CircularBuffer& span_buffer, + SpanStream(CircularBuffer& span_buffer, StreamRecorderMetrics& metrics) noexcept; /** @@ -23,7 +23,7 @@ class SpanStream final : public FragmentInputStream { * Returns and removes the last partially written span. * @return the last partially written span */ - std::unique_ptr ConsumeRemnant() noexcept; + std::unique_ptr ConsumeRemnant() noexcept; /** * @return the associagted StreamRecorderMetrics @@ -40,9 +40,9 @@ class SpanStream final : public FragmentInputStream { void Seek(int fragment_index, int position) noexcept override; private: - CircularBuffer& span_buffer_; + CircularBuffer& span_buffer_; StreamRecorderMetrics& metrics_; - CircularBufferRange> allotment_; - std::unique_ptr remnant_; + CircularBufferRange> allotment_; + std::unique_ptr remnant_; }; } // namespace lightstep diff --git a/src/recorder/stream_recorder/stream_recorder.cpp b/src/recorder/stream_recorder/stream_recorder.cpp index 4e9ee608..65e96646 100644 --- a/src/recorder/stream_recorder/stream_recorder.cpp +++ b/src/recorder/stream_recorder/stream_recorder.cpp @@ -60,7 +60,7 @@ Fragment StreamRecorder::ReserveHeaderSpace(ChainedStream& stream) { throw std::bad_alloc{}; } stream.BackUp(size - static_cast(max_header_size)); - return {data, size}; + return {data, static_cast(max_header_size)}; } //-------------------------------------------------------------------------------------------------- @@ -79,7 +79,8 @@ void StreamRecorder::RecordSpan( // Frame the Span char* header_data = static_cast(header_fragment.first); auto reserved_header_size = static_cast(header_fragment.second); - auto protobuf_body_size = span->ByteCount() - ChunkedHttpFooter.size(); + auto protobuf_body_size = + span->ByteCount() - ChunkedHttpFooter.size() - header_fragment.second; auto protobuf_header_size = WriteReportRequestSpansHeader( header_data, reserved_header_size, protobuf_body_size); auto chunk_body_size = protobuf_body_size + protobuf_header_size; @@ -91,11 +92,7 @@ void StreamRecorder::RecordSpan( // Advance past reserved header space we didn't use. span->Seek(0, static_cast(reserved_header_size - protobuf_header_size - chunk_header_size)); -} -void StreamRecorder::RecordSpan( - std::unique_ptr&& span) noexcept { - span->AddFraming(); if (!span_buffer_.Add(span)) { // Note: the compiler doesn't want to inline this logger call and it shows // up in profiling with high span droppage even if the logging isn't turned diff --git a/src/recorder/stream_recorder/stream_recorder.h b/src/recorder/stream_recorder/stream_recorder.h index 1ebdf140..a1edd5b3 100644 --- a/src/recorder/stream_recorder/stream_recorder.h +++ b/src/recorder/stream_recorder/stream_recorder.h @@ -12,7 +12,7 @@ #include "common/logger.h" #include "common/noncopyable.h" #include "common/platform/network_environment.h" -#include "common/serialization_chain.h" +#include "common/chained_stream.h" #include "lightstep/tracer.h" #include "network/event_base.h" #include "network/timer_event.h" @@ -84,7 +84,7 @@ class StreamRecorder : public ForkAwareRecorder, private Noncopyable { /** * @return the associated span buffer. */ - CircularBuffer& span_buffer() noexcept { + CircularBuffer& span_buffer() noexcept { return span_buffer_; } @@ -94,8 +94,6 @@ class StreamRecorder : public ForkAwareRecorder, private Noncopyable { void WriteFooter( google::protobuf::io::CodedOutputStream& coded_stream) override; - void RecordSpan(std::unique_ptr&& span) noexcept override; - void RecordSpan(Fragment header_fragment, std::unique_ptr&& span) noexcept override; @@ -134,7 +132,7 @@ class StreamRecorder : public ForkAwareRecorder, private Noncopyable { LightStepTracerOptions tracer_options_; StreamRecorderOptions recorder_options_; StreamRecorderMetrics metrics_; - CircularBuffer span_buffer_; + CircularBuffer span_buffer_; std::atomic exit_{false}; diff --git a/src/tracer/span.cpp b/src/tracer/span.cpp index 2e8a0a9e..c7348448 100644 --- a/src/tracer/span.cpp +++ b/src/tracer/span.cpp @@ -273,8 +273,8 @@ void Span::FinishImpl( WriteSpanContext(coded_stream_, trace_id_, span_id_, baggage_.as_vector()); // Record the span - stream_.Trim(); tracer_->recorder().WriteFooter(coded_stream_); + stream_.Trim(); coded_stream_.Trim(); tracer_->recorder().RecordSpan(std::move(serialization_chain_)); tracer_->recorder().RecordSpan(header_fragment_, std::move(chained_stream_)); diff --git a/test/number_simulation.cpp b/test/number_simulation.cpp index 0bfcaeae..5cae0fb8 100644 --- a/test/number_simulation.cpp +++ b/test/number_simulation.cpp @@ -47,7 +47,7 @@ GenerateRandomBinaryNumber(size_t max_digits) { // GenerateRandomBinaryNumbers //-------------------------------------------------------------------------------------------------- static void GenerateRandomBinaryNumbers( - CircularBuffer& buffer, std::vector& numbers, + CircularBuffer& buffer, std::vector& numbers, size_t n) { while (n-- != 0) { uint32_t x; @@ -163,7 +163,7 @@ static bool HasPendingData(ConnectionStream& connection_stream) { //-------------------------------------------------------------------------------------------------- // RunBinaryNumberProducer //-------------------------------------------------------------------------------------------------- -void RunBinaryNumberProducer(CircularBuffer& buffer, +void RunBinaryNumberProducer(CircularBuffer& buffer, std::vector& numbers, size_t num_threads, size_t n) { std::vector> thread_numbers(num_threads); diff --git a/test/number_simulation.h b/test/number_simulation.h index 22176868..b6b9f7d5 100644 --- a/test/number_simulation.h +++ b/test/number_simulation.h @@ -7,7 +7,7 @@ #include #include "common/circular_buffer.h" -#include "common/serialization_chain.h" +#include "common/chained_stream.h" #include "recorder/stream_recorder/connection_stream.h" #include @@ -20,7 +20,7 @@ namespace lightstep { * @param num_threads the number of threads to write numbers on. * @param n the number of numbers to write. */ -void RunBinaryNumberProducer(CircularBuffer& buffer, +void RunBinaryNumberProducer(CircularBuffer& buffer, std::vector& numbers, size_t num_threads, size_t n); diff --git a/test/recorder/stream_recorder/connection_stream_test.cpp b/test/recorder/stream_recorder/connection_stream_test.cpp index d6985b1c..c4d3a544 100644 --- a/test/recorder/stream_recorder/connection_stream_test.cpp +++ b/test/recorder/stream_recorder/connection_stream_test.cpp @@ -33,7 +33,7 @@ static collector::ReportRequest ParseStreamHeader(std::string& s) { TEST_CASE("ConnectionStream") { LightStepTracerOptions tracer_options; - CircularBuffer span_buffer{1000}; + CircularBuffer span_buffer{1000}; MetricsObserver metrics_observer; StreamRecorderMetrics metrics{metrics_observer}; SpanStream span_stream{span_buffer, metrics}; @@ -252,7 +252,7 @@ TEST_CASE( const size_t num_connections = 10; const size_t n = 25000; for (size_t max_size : {1, 2, 10, 100, 1000}) { - CircularBuffer buffer{max_size}; + CircularBuffer buffer{max_size}; SpanStream span_stream{buffer, metrics}; std::vector connection_streams; connection_streams.reserve(num_connections); diff --git a/test/recorder/stream_recorder/span_stream_test.cpp b/test/recorder/stream_recorder/span_stream_test.cpp index 87ccb4ba..9b5a481b 100644 --- a/test/recorder/stream_recorder/span_stream_test.cpp +++ b/test/recorder/stream_recorder/span_stream_test.cpp @@ -9,7 +9,7 @@ using namespace lightstep; TEST_CASE("SpanStream") { const size_t max_spans = 10; - CircularBuffer buffer{max_spans}; + CircularBuffer buffer{max_spans}; MetricsObserver metrics_observer; StreamRecorderMetrics metrics{metrics_observer}; SpanStream span_stream{buffer, metrics}; diff --git a/test/utility.cpp b/test/utility.cpp index 75a8f52a..5df30121 100644 --- a/test/utility.cpp +++ b/test/utility.cpp @@ -122,7 +122,7 @@ bool AddSpanChunkFramedString(CircularBuffer& buffer, std::unique_ptr chain{new ChainedStream{}}; { google::protobuf::io::CodedOutputStream stream{chain.get()}; - stream.WriteString(s); + stream.WriteString(framed_s); } chain->CloseOutput(); return buffer.Add(chain); From 84a1cbecd10776896414aa2ca28c74b06c35b4b5 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Fri, 24 Jan 2020 17:30:00 -0800 Subject: [PATCH 36/66] Update InMemoryRecorder --- test/recorder/in_memory_recorder.cpp | 20 ++++++-------------- test/recorder/in_memory_recorder.h | 3 ++- 2 files changed, 8 insertions(+), 15 deletions(-) diff --git a/test/recorder/in_memory_recorder.cpp b/test/recorder/in_memory_recorder.cpp index d8ad3ec3..c0a696c8 100644 --- a/test/recorder/in_memory_recorder.cpp +++ b/test/recorder/in_memory_recorder.cpp @@ -43,24 +43,16 @@ void InMemoryRecorder::RecordSpan(const collector::Span& span) noexcept { } void InMemoryRecorder::RecordSpan( - std::unique_ptr&& span) noexcept { - span->AddFraming(); + Fragment /*header_fragment*/, + std::unique_ptr&& span) noexcept { + span->CloseOutput(); auto serialization = ToString(*span); - auto header_last = - std::find(serialization.begin(), serialization.end(), '\n') + 1; - auto message_size = serialization.size() - - std::distance(serialization.begin(), header_last) - 2; - collector::ReportRequest request; - if (!request.ParseFromArray(static_cast(&*header_last), - message_size)) { + collector::Span protobuf_span; + if (!protobuf_span.ParseFromString(serialization)) { std::cerr << "Failed to parse span\n"; std::terminate(); } - if (request.spans().size() != 1) { - std::cerr << "No span found\n"; - std::terminate(); - } std::lock_guard lock_guard{mutex_}; - spans_.emplace_back(std::move(request.spans()[0])); + spans_.emplace_back(std::move(protobuf_span)); } } // namespace lightstep diff --git a/test/recorder/in_memory_recorder.h b/test/recorder/in_memory_recorder.h index e58e7bb6..1fedc852 100644 --- a/test/recorder/in_memory_recorder.h +++ b/test/recorder/in_memory_recorder.h @@ -18,7 +18,8 @@ class InMemoryRecorder final : public Recorder { // Recorder void RecordSpan(const collector::Span& span) noexcept override; - void RecordSpan(std::unique_ptr&& span) noexcept override; + void RecordSpan(Fragment header_fragment, + std::unique_ptr&& span) noexcept override; private: mutable std::mutex mutex_; From 78d63a00f70a2926ab78aa96dcaff32117a36661 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Fri, 24 Jan 2020 17:34:50 -0800 Subject: [PATCH 37/66] Remove SerializationChain from Span --- src/tracer/span.cpp | 18 +----------------- src/tracer/span.h | 4 ---- 2 files changed, 1 insertion(+), 21 deletions(-) diff --git a/src/tracer/span.cpp b/src/tracer/span.cpp index c7348448..266ab6c8 100644 --- a/src/tracer/span.cpp +++ b/src/tracer/span.cpp @@ -22,13 +22,10 @@ namespace lightstep { Span::Span(std::shared_ptr&& tracer, opentracing::string_view operation_name, const opentracing::StartSpanOptions& options) - : serialization_chain_{new SerializationChain{}}, - stream_{serialization_chain_.get()}, - chained_stream_{new ChainedStream{}}, + : chained_stream_{new ChainedStream{}}, header_fragment_{tracer->recorder().ReserveHeaderSpace(*chained_stream_)}, coded_stream_{chained_stream_.get()}, tracer_{std::move(tracer)} { - WriteOperationName(stream_, operation_name); WriteOperationName(coded_stream_, operation_name); // Set the start timestamps. @@ -36,7 +33,6 @@ Span::Span(std::shared_ptr&& tracer, std::tie(start_timestamp, start_steady_) = ComputeStartTimestamps( tracer_->recorder(), options.start_system_timestamp, options.start_steady_timestamp); - WriteStartTimestamp(stream_, start_timestamp); WriteStartTimestamp(coded_stream_, start_timestamp); // Set any span references. @@ -67,7 +63,6 @@ Span::Span(std::shared_ptr&& tracer, // Set tags. for (auto& tag : options.tags) { - WriteTag(stream_, tag.first, tag.second); WriteTag(coded_stream_, tag.first, tag.second); // If sampling_priority is set, it overrides whatever sampling decision was @@ -105,7 +100,6 @@ void Span::SetOperationName(opentracing::string_view name) noexcept try { if (is_finished_) { return; } - WriteOperationName(stream_, name); WriteOperationName(coded_stream_, name); } catch (const std::exception& e) { tracer_->logger().Error("SetOperationName failed: ", e.what()); @@ -120,7 +114,6 @@ void Span::SetTag(opentracing::string_view key, if (is_finished_) { return; } - WriteTag(stream_, key, value); WriteTag(coded_stream_, key, value); if (key == SamplingPriorityKey) { sampled_ = is_sampled(value); @@ -173,7 +166,6 @@ void Span::Log(std::initializer_list< if (is_finished_) { return; } - WriteLog(stream_, timestamp, fields.begin(), fields.end()); WriteLog(coded_stream_, timestamp, fields.begin(), fields.end()); } catch (const std::exception& e) { tracer_->logger().Error("Log failed: ", e.what()); @@ -220,8 +212,6 @@ bool Span::SetSpanReference( } trace_id_high = referenced_context->trace_id_high(); trace_id = referenced_context->trace_id_low(); - WriteSpanReference(stream_, reference.first, trace_id, - referenced_context->span_id()); WriteSpanReference(coded_stream_, reference.first, trace_id, referenced_context->span_id()); sampled_ = sampled_ || referenced_context->sampled(); @@ -254,14 +244,11 @@ void Span::FinishImpl( // Set timing information. auto duration = finish_timestamp - start_steady_; - WriteDuration(stream_, duration); WriteDuration(coded_stream_, duration); // Set logs for (auto& log_record : options.log_records) { try { - WriteLog(stream_, log_record.timestamp, log_record.fields.data(), - log_record.fields.data() + log_record.fields.size()); WriteLog(coded_stream_, log_record.timestamp, log_record.fields.data(), log_record.fields.data() + log_record.fields.size()); } catch (const std::exception& e) { @@ -269,14 +256,11 @@ void Span::FinishImpl( } } - WriteSpanContext(stream_, trace_id_, span_id_, baggage_.as_vector()); WriteSpanContext(coded_stream_, trace_id_, span_id_, baggage_.as_vector()); // Record the span tracer_->recorder().WriteFooter(coded_stream_); - stream_.Trim(); coded_stream_.Trim(); - tracer_->recorder().RecordSpan(std::move(serialization_chain_)); tracer_->recorder().RecordSpan(header_fragment_, std::move(chained_stream_)); } catch (const std::exception& e) { tracer_->logger().Error("FinishWithOptions failed: ", e.what()); diff --git a/src/tracer/span.h b/src/tracer/span.h index a599825c..0580cd9d 100644 --- a/src/tracer/span.h +++ b/src/tracer/span.h @@ -6,7 +6,6 @@ #include #include "common/chained_stream.h" -#include "common/serialization_chain.h" #include "common/spin_lock_mutex.h" #include "tracer/baggage_flat_map.h" #include "tracer/lightstep_span_context.h" @@ -95,9 +94,6 @@ class Span final : public opentracing::Span, public LightStepSpanContext { // more sense to use a spin lock for this use case. mutable SpinLockMutex mutex_; - std::unique_ptr serialization_chain_; - google::protobuf::io::CodedOutputStream stream_; - std::unique_ptr chained_stream_; Fragment header_fragment_; google::protobuf::io::CodedOutputStream coded_stream_; From 842c25b9e2c56fc272ae540601c5c4c29ff45451 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Fri, 24 Jan 2020 17:41:14 -0800 Subject: [PATCH 38/66] Remove SerializationChain from Recorder interface --- src/recorder/manual_recorder.cpp | 12 ------------ src/recorder/manual_recorder.h | 2 -- src/recorder/recorder.h | 12 +++++------- 3 files changed, 5 insertions(+), 21 deletions(-) diff --git a/src/recorder/manual_recorder.cpp b/src/recorder/manual_recorder.cpp index c54502d7..64535746 100644 --- a/src/recorder/manual_recorder.cpp +++ b/src/recorder/manual_recorder.cpp @@ -16,18 +16,6 @@ ManualRecorder::ManualRecorder(Logger& logger, LightStepTracerOptions options, } } -//-------------------------------------------------------------------------------------------------- -// RecordSpan -//-------------------------------------------------------------------------------------------------- -void ManualRecorder::RecordSpan( - std::unique_ptr&& span) noexcept { - span->AddFraming(); - if (!span_buffer_.Add(span)) { - tracer_options_.metrics_observer->OnSpansDropped(1); - span.reset(); - } -} - //-------------------------------------------------------------------------------------------------- // FlushWithTimeout //-------------------------------------------------------------------------------------------------- diff --git a/src/recorder/manual_recorder.h b/src/recorder/manual_recorder.h index cb6a8042..048f1ef0 100644 --- a/src/recorder/manual_recorder.h +++ b/src/recorder/manual_recorder.h @@ -13,8 +13,6 @@ class ManualRecorder : public ForkAwareRecorder, private Noncopyable { std::unique_ptr&& transporter); // Recorder - void RecordSpan(std::unique_ptr&& span) noexcept override; - bool FlushWithTimeout( std::chrono::system_clock::duration timeout) noexcept override; diff --git a/src/recorder/recorder.h b/src/recorder/recorder.h index 94fa1f5b..7195c2d6 100644 --- a/src/recorder/recorder.h +++ b/src/recorder/recorder.h @@ -30,6 +30,11 @@ class Recorder { virtual void WriteFooter( google::protobuf::io::CodedOutputStream& /*coded_stream*/) {} + /** + * Record a Span + * @header_fragment the fragment reserved to hold header framing for the span + * @span the serialization of a protobuf span and framing + */ virtual void RecordSpan(Fragment /*header_fragment*/, std::unique_ptr&& /*span*/) noexcept {} @@ -39,13 +44,6 @@ class Recorder { */ virtual void RecordSpan(const collector::Span& /*span*/) noexcept {} - /** - * Record a Span - * @span the serialization of a protobuf span and framing - */ - virtual void RecordSpan( - std::unique_ptr&& /*span*/) noexcept {} - /** * Block until the recorder is flushed or a time limit is exceeded. * @param timeout the maximum amount of time to block. From 583b2f7f7a8c8d5b83cdd9b143cf51a02c18bb15 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Fri, 24 Jan 2020 17:44:05 -0800 Subject: [PATCH 39/66] Remove SerializationChain tests --- test/common/BUILD | 11 -- test/common/chained_stream_test.cpp | 2 +- test/common/serialization_chain_test.cpp | 133 ----------------------- 3 files changed, 1 insertion(+), 145 deletions(-) delete mode 100644 test/common/serialization_chain_test.cpp diff --git a/test/common/BUILD b/test/common/BUILD index 9d65dae9..e67bb956 100644 --- a/test/common/BUILD +++ b/test/common/BUILD @@ -185,17 +185,6 @@ lightstep_catch_test( ], ) -lightstep_catch_test( - name = "serialization_chain_test", - srcs = [ - "serialization_chain_test.cpp", - ], - deps = [ - "//src/common:serialization_chain_lib", - "//test:utility_lib", - ], -) - lightstep_catch_test( name = "chained_stream_test", srcs = [ diff --git a/test/common/chained_stream_test.cpp b/test/common/chained_stream_test.cpp index 1c108343..2884d259 100644 --- a/test/common/chained_stream_test.cpp +++ b/test/common/chained_stream_test.cpp @@ -40,7 +40,7 @@ TEST_CASE("ChainedStream") { } SECTION("We can seek to any byte in the fragment stream.") { - std::string s(SerializationChain::BlockSize + 2, 'X'); + std::string s(ChainedStream::BlockSize + 2, 'X'); stream->WriteString(s); stream.reset(); chain.CloseOutput(); diff --git a/test/common/serialization_chain_test.cpp b/test/common/serialization_chain_test.cpp deleted file mode 100644 index da16bda9..00000000 --- a/test/common/serialization_chain_test.cpp +++ /dev/null @@ -1,133 +0,0 @@ -#include "common/serialization_chain.h" - -#include -#include - -#include "test/utility.h" - -#include - -#include "3rd_party/catch2/catch.hpp" -using namespace lightstep; - -TEST_CASE("SerializationChain") { - SerializationChain chain; - - std::unique_ptr stream{ - new google::protobuf::io::CodedOutputStream{&chain}}; - - SECTION("An empty chain has no fragments.") { - stream.reset(); - chain.AddFraming(); - REQUIRE(chain.num_bytes_after_framing() == 0); - REQUIRE(chain.num_fragments() == 0); - } - - SECTION("We can write a string smaller than the block size.") { - stream->WriteString("abc"); - stream.reset(); - chain.AddFraming(); - REQUIRE(chain.num_fragments() == 3); - auto s = ToString(chain); - REQUIRE(s.size() == chain.num_bytes_after_framing()); - REQUIRE(s == AddSpanChunkFraming("abc")); - } - - SECTION("We can write strings larger than a single block.") { - std::string s(SerializationChain::BlockSize + 1, 'X'); - stream->WriteString(s); - stream.reset(); - chain.AddFraming(); - REQUIRE(chain.num_fragments() == 4); - auto serialization = ToString(chain); - REQUIRE(serialization.size() == chain.num_bytes_after_framing()); - REQUIRE(serialization == AddSpanChunkFraming(s)); - } - - SECTION("We can seek to any byte in the fragment stream.") { - std::string s(SerializationChain::BlockSize + 2, 'X'); - stream->WriteString(s); - stream.reset(); - chain.AddFraming(); - std::string serialization = AddSpanChunkFraming(s); - for (size_t i = 1; i <= serialization.size(); ++i) { - SECTION("cosumption instance " + std::to_string(i)) { - Consume({&chain}, i); - REQUIRE(ToString(chain) == serialization.substr(i)); - } - } - } - - SECTION("We can advance to any byte in the fragment stream randomly.") { - std::string s(3 * SerializationChain::BlockSize + 10, 'X'); - stream->WriteString(s); - stream.reset(); - chain.AddFraming(); - std::string serialization = AddSpanChunkFraming(s); - std::mt19937 random_number_generator{0}; - for (int i = 0; i < 100; ++i) { - size_t num_bytes_consumed = 0; - SECTION("Random advance " + std::to_string(i)) { - while (num_bytes_consumed < serialization.size()) { - std::uniform_int_distribution distribution{ - 1, static_cast(serialization.size()) - num_bytes_consumed}; - auto n = distribution(random_number_generator); - Consume({&chain}, n); - num_bytes_consumed += n; - REQUIRE(ToString(chain) == serialization.substr(num_bytes_consumed)); - } - } - } - } - - SECTION( - "ForEachSerializationChain behaves correctly when there are no other " - "owned chains") { - int i = 0; - auto was_successful = chain.ForEachSerializationChain([&]( - const SerializationChain& c) noexcept { - REQUIRE(&c == &chain); - ++i; - return true; - }); - REQUIRE(was_successful); - REQUIRE(i == 1); - was_successful = chain.ForEachSerializationChain([&]( - const SerializationChain& /*c*/) noexcept { return false; }); - } - - SECTION("We can chain SerializationChains together") { - auto chain2 = new SerializationChain{}; - chain.Chain(std::unique_ptr{chain2}); - int i = 0; - auto was_successful = chain.ForEachSerializationChain([&]( - const SerializationChain& c) noexcept { - if (i == 0) { - REQUIRE(&c == &chain); - } else { - REQUIRE(&c == chain2); - } - ++i; - return true; - }); - REQUIRE(i == 2); - REQUIRE(was_successful); - auto chain3 = new SerializationChain{}; - chain.Chain(std::unique_ptr{chain3}); - i = 0; - was_successful = chain.ForEachSerializationChain([&]( - const SerializationChain& c) noexcept { - if (i == 0) { - REQUIRE(&c == &chain); - } else if (i == 1) { - REQUIRE(&c == chain3); - } else { - REQUIRE(&c == chain2); - } - ++i; - return true; - }); - REQUIRE(i == 3); - REQUIRE(was_successful); - } -} From 2b1c876921959c10481f1ef6e436bf1607d6ce41 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Fri, 24 Jan 2020 17:46:32 -0800 Subject: [PATCH 40/66] Remove SerializationChain from test/utility --- test/utility.cpp | 11 ----------- test/utility.h | 3 --- 2 files changed, 14 deletions(-) diff --git a/test/utility.cpp b/test/utility.cpp index 5df30121..38797636 100644 --- a/test/utility.cpp +++ b/test/utility.cpp @@ -105,17 +105,6 @@ std::string ToString(const FragmentInputStream& fragment_input_stream) { //-------------------------------------------------------------------------------------------------- // AddSpanChunkFramedString //-------------------------------------------------------------------------------------------------- -bool AddSpanChunkFramedString(CircularBuffer& buffer, - const std::string& s) { - std::unique_ptr chain{new SerializationChain{}}; - { - google::protobuf::io::CodedOutputStream stream{chain.get()}; - stream.WriteString(s); - } - chain->AddFraming(); - return buffer.Add(chain); -} - bool AddSpanChunkFramedString(CircularBuffer& buffer, const std::string& s) { auto framed_s = AddSpanChunkFraming(s); diff --git a/test/utility.h b/test/utility.h index 2b26ab86..46867b75 100644 --- a/test/utility.h +++ b/test/utility.h @@ -67,9 +67,6 @@ std::string ToString(const FragmentInputStream& fragment_input_stream); * @param s the string to add. * @return true if the string was succesfully added. */ -bool AddSpanChunkFramedString(CircularBuffer& buffer, - const std::string& s); - bool AddSpanChunkFramedString(CircularBuffer& buffer, const std::string& s); From 4240d083449a8260dddda82ce1933ab284c03711 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Fri, 24 Jan 2020 17:59:38 -0800 Subject: [PATCH 41/66] Remove SerializationChain references --- test/BUILD | 2 +- test/utility.cpp | 5 ++--- test/utility.h | 1 - 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/test/BUILD b/test/BUILD index b77223cd..8375e6d4 100644 --- a/test/BUILD +++ b/test/BUILD @@ -19,8 +19,8 @@ lightstep_cc_library( "//src/common:utility_lib", "//src/network:socket_lib", "//src/common:fragment_input_stream_lib", + "//src/common:report_request_framing_lib", "//src/common:serialization_lib", - "//src/common:serialization_chain_lib", "//src/common:chained_stream_lib", "//src/common:circular_buffer_lib", ], diff --git a/test/utility.cpp b/test/utility.cpp index 38797636..3e8ff11e 100644 --- a/test/utility.cpp +++ b/test/utility.cpp @@ -7,8 +7,8 @@ #include #include +#include "common/report_request_framing.h" #include "common/serialization.h" -#include "common/serialization_chain.h" #include "common/utility.h" #include "network/socket.h" @@ -126,8 +126,7 @@ std::string AddSpanChunkFraming(opentracing::string_view s) { { google::protobuf::io::OstreamOutputStream zero_copy_stream{&oss}; google::protobuf::io::CodedOutputStream stream{&zero_copy_stream}; - WriteKeyLength(stream, - s.size()); + WriteKeyLength(stream, s.size()); } return oss.str(); }(); diff --git a/test/utility.h b/test/utility.h index 46867b75..dfd72909 100644 --- a/test/utility.h +++ b/test/utility.h @@ -5,7 +5,6 @@ #include "common/circular_buffer.h" #include "common/fragment_input_stream.h" -#include "common/serialization_chain.h" #include "common/chained_stream.h" #include From 8f171d4e4713ab34fed32204bf05ea2334dc7fe2 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Fri, 24 Jan 2020 18:04:07 -0800 Subject: [PATCH 42/66] Remove SerializationChain references --- src/recorder/BUILD | 1 - src/recorder/manual_recorder.h | 2 +- src/recorder/recorder.h | 1 - 3 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/recorder/BUILD b/src/recorder/BUILD index aedc90ab..9252f7a7 100644 --- a/src/recorder/BUILD +++ b/src/recorder/BUILD @@ -35,7 +35,6 @@ lightstep_cc_library( "recorder.h", ], deps = [ - "//src/common:serialization_chain_lib", "//src/common:chained_stream_lib", "//src/common:timestamp_lib", "//lightstep-tracer-common:collector_proto_cc", diff --git a/src/recorder/manual_recorder.h b/src/recorder/manual_recorder.h index 048f1ef0..c6e3d928 100644 --- a/src/recorder/manual_recorder.h +++ b/src/recorder/manual_recorder.h @@ -28,6 +28,6 @@ class ManualRecorder : public ForkAwareRecorder, private Noncopyable { LightStepTracerOptions tracer_options_; std::unique_ptr transporter_; - CircularBuffer span_buffer_; + CircularBuffer span_buffer_; }; } // namespace lightstep diff --git a/src/recorder/recorder.h b/src/recorder/recorder.h index 7195c2d6..d9738860 100644 --- a/src/recorder/recorder.h +++ b/src/recorder/recorder.h @@ -4,7 +4,6 @@ #include #include "common/chained_stream.h" -#include "common/serialization_chain.h" #include "common/timestamp.h" #include From 1759804754ecd041f790009ed4e693aacf4e5246 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Fri, 24 Jan 2020 18:12:59 -0800 Subject: [PATCH 43/66] Update ReportRequest --- src/recorder/serialization/BUILD | 2 +- src/recorder/serialization/report_request.cpp | 18 +++++++++--------- src/recorder/serialization/report_request.h | 6 +++--- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/recorder/serialization/BUILD b/src/recorder/serialization/BUILD index 7037df30..8ea1096f 100644 --- a/src/recorder/serialization/BUILD +++ b/src/recorder/serialization/BUILD @@ -16,7 +16,7 @@ lightstep_cc_library( ], deps = [ "//src/common:buffer_chain_lib", - "//src/common:serialization_chain_lib", + "//src/common:chained_stream_lib", ":embedded_metrics_message_lib", ], ) diff --git a/src/recorder/serialization/report_request.cpp b/src/recorder/serialization/report_request.cpp index 415f4f14..f832a3a3 100644 --- a/src/recorder/serialization/report_request.cpp +++ b/src/recorder/serialization/report_request.cpp @@ -15,15 +15,18 @@ ReportRequest::ReportRequest(const std::shared_ptr& header, // AddSpan //-------------------------------------------------------------------------------------------------- void ReportRequest::AddSpan( - std::unique_ptr&& span) noexcept { + std::unique_ptr&& span) noexcept { ++num_spans_; num_fragments_ += span->num_fragments(); - num_bytes_ += span->num_bytes_after_framing(); + span->ForEachFragment([&](void* /*data*/, int length) { + num_bytes_ += static_cast(length); + return true; + }); if (spans_ == nullptr) { spans_ = std::move(span); return; } - spans_->Chain(std::move(span)); + spans_->Append(std::move(span)); } //-------------------------------------------------------------------------------------------------- @@ -44,11 +47,8 @@ bool ReportRequest::ForEachFragment(FragmentCallback callback, if (spans_ == nullptr) { return true; } - return spans_->ForEachSerializationChain( - [&](const SerializationChain& chain) { - return chain.ForEachFragment([&](void* data, int length) { - return callback(context, data, static_cast(length)); - }); - }); + return spans_->ForEachFragment([&](void* data, int length) { + return callback(context, data, static_cast(length)); + }); } } // namespace lightstep diff --git a/src/recorder/serialization/report_request.h b/src/recorder/serialization/report_request.h index 282773c9..d9817668 100644 --- a/src/recorder/serialization/report_request.h +++ b/src/recorder/serialization/report_request.h @@ -2,7 +2,7 @@ #include -#include "common/serialization_chain.h" +#include "common/chained_stream.h" #include "lightstep/buffer_chain.h" #include "recorder/serialization/embedded_metrics_message.h" @@ -12,7 +12,7 @@ class ReportRequest final : public BufferChain { ReportRequest(const std::shared_ptr& header, int num_dropped_spans); - void AddSpan(std::unique_ptr&& span) noexcept; + void AddSpan(std::unique_ptr&& span) noexcept; // BufferChain size_t num_fragments() const noexcept override { @@ -34,6 +34,6 @@ class ReportRequest final : public BufferChain { int num_bytes_{0}; int num_spans_{0}; int num_fragments_{0}; - std::unique_ptr spans_; + std::unique_ptr spans_; }; } // namespace lightstep From e96fe0b3d26a2a9ae8721a784ed4e77525833c87 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Fri, 24 Jan 2020 23:23:09 -0800 Subject: [PATCH 44/66] Removee SerializationChain --- src/common/BUILD | 19 --- src/common/serialization_chain.cpp | 213 ----------------------------- src/common/serialization_chain.h | 106 -------------- src/tracer/BUILD | 1 - 4 files changed, 339 deletions(-) delete mode 100644 src/common/serialization_chain.cpp delete mode 100644 src/common/serialization_chain.h diff --git a/src/common/BUILD b/src/common/BUILD index 558bcc59..3ea5df24 100644 --- a/src/common/BUILD +++ b/src/common/BUILD @@ -40,25 +40,6 @@ lightstep_cc_library( ], ) -lightstep_cc_library( - name = "serialization_chain_lib", - private_hdrs = [ - "serialization_chain.h", - ], - srcs = [ - "serialization_chain.cpp", - ], - deps = [ - ":fragment_input_stream_lib", - ":noncopyable_lib", - ":hex_conversion_lib", - ":serialization_lib", - ], - external_deps = [ - "@com_google_protobuf//:protobuf", - ], -) - lightstep_cc_library( name = "chained_stream_lib", private_hdrs = [ diff --git a/src/common/serialization_chain.cpp b/src/common/serialization_chain.cpp deleted file mode 100644 index 9e18964c..00000000 --- a/src/common/serialization_chain.cpp +++ /dev/null @@ -1,213 +0,0 @@ -#include "common/serialization_chain.h" - -#include -#include - -#include - -namespace lightstep { -static const char* LineTerminator = "\r\n"; -const int LineTerminatorSize = 2; - -//-------------------------------------------------------------------------------------------------- -// WriteChunkHeader -//-------------------------------------------------------------------------------------------------- -static int WriteChunkHeader(char* data, size_t data_length, - size_t chunk_size) noexcept { - (void)data_length; - auto chunk_size_str = Uint32ToHex(static_cast(chunk_size), data); - assert(chunk_size_str.size() + 2 <= data_length); - assert(!chunk_size_str.empty()); - auto iter = data + chunk_size_str.size(); - *iter++ = '\r'; - *iter++ = '\n'; - return static_cast(std::distance(data, iter)); -} - -//-------------------------------------------------------------------------------------------------- -// constructor -//-------------------------------------------------------------------------------------------------- -SerializationChain::SerializationChain() noexcept : current_block_{&head_} {} - -//-------------------------------------------------------------------------------------------------- -// Chain -//-------------------------------------------------------------------------------------------------- -void SerializationChain::Chain( - std::unique_ptr&& other) noexcept { - assert(other->next_ == nullptr); - other->next_ = std::move(next_); - next_ = std::move(other); -} - -//-------------------------------------------------------------------------------------------------- -// ForEachSerializationChain -//-------------------------------------------------------------------------------------------------- -bool SerializationChain::ForEachSerializationChain( - FunctionRef callback) { - if (!callback(*this)) { - return true; - } - if (next_ != nullptr) { - return next_->ForEachSerializationChain(callback); - } - return true; -} - -//-------------------------------------------------------------------------------------------------- -// AddFraming -//-------------------------------------------------------------------------------------------------- -void SerializationChain::AddFraming() noexcept { - auto protobuf_header_size = - ComputeLengthDelimitedHeaderSerializationSize( - num_bytes_written_); - header_size_ = WriteChunkHeader( - header_.data(), header_.size(), - static_cast(num_bytes_written_) + protobuf_header_size); - assert(header_size_ > 0); - - // Serialize the spans key field and length - DirectCodedOutputStream stream{reinterpret_cast( - header_.data() + header_size_)}; - WriteKeyLength( - stream, static_cast(num_bytes_written_)); - header_size_ += protobuf_header_size; - - if (num_bytes_written_ > 0) { - num_bytes_after_framing_ = - header_size_ + num_bytes_written_ + LineTerminatorSize; - } - - // Prepare the data structure to act as a FragmentInputStream. - current_block_ = &head_; -} - -//-------------------------------------------------------------------------------------------------- -// Next -//-------------------------------------------------------------------------------------------------- -bool SerializationChain::Next(void** data, int* size) { - if (current_block_position_ < BlockSize) { - *size = BlockSize - current_block_position_; - *data = static_cast(current_block_->data.data() + - current_block_position_); - num_bytes_written_ += *size; - current_block_position_ = BlockSize; - current_block_->size = BlockSize; - return true; - } - current_block_->next.reset(new Block{}); - current_block_ = current_block_->next.get(); - current_block_->size = BlockSize; - current_block_position_ = BlockSize; - *size = BlockSize; - num_bytes_written_ += *size; - *data = static_cast(current_block_->data.data()); - ++num_blocks_; - return true; -} - -//-------------------------------------------------------------------------------------------------- -// BackUp -//-------------------------------------------------------------------------------------------------- -void SerializationChain::BackUp(int count) { - num_bytes_written_ -= count; - current_block_position_ -= count; - current_block_->size -= count; -} - -//-------------------------------------------------------------------------------------------------- -// num_fragments -//-------------------------------------------------------------------------------------------------- -int SerializationChain::num_fragments() const noexcept { - if (num_bytes_written_ == 0) { - return 0; - } - return num_blocks_ + 2 - fragment_index_; -} - -//-------------------------------------------------------------------------------------------------- -// ForEachFragment -//-------------------------------------------------------------------------------------------------- -bool SerializationChain::ForEachFragment(Callback callback) const noexcept { - assert(fragment_index_ >= 0 && fragment_index_ <= num_blocks_ + 1); - - if (num_blocks_ == 0) { - return true; - } - - // header - if (fragment_index_ == 0) { - assert(fragment_position_ < header_size_); - if (!callback(static_cast(const_cast(header_.data()) + - fragment_position_), - header_size_ - fragment_position_)) { - return false; - } - } - - // data - auto block = current_block_; - if (fragment_index_ > 0 && fragment_index_ <= num_blocks_) { - auto block_size = block->size; - assert(block_size >= fragment_position_); - if (!callback(static_cast(const_cast(block->data.data()) + - fragment_position_), - block_size - fragment_position_)) { - return false; - } - block = block->next.get(); - } - while (block != nullptr) { - if (!callback(static_cast(const_cast(block->data.data())), - block->size)) { - return false; - } - block = block->next.get(); - } - - // chunk trailer - if (fragment_index_ == num_blocks_ + 1) { - assert(fragment_position_ < LineTerminatorSize); - return callback(static_cast(const_cast(LineTerminator) + - fragment_position_), - LineTerminatorSize - fragment_position_); - } - return callback(static_cast(const_cast(LineTerminator)), - LineTerminatorSize); -} - -//-------------------------------------------------------------------------------------------------- -// Clear -//-------------------------------------------------------------------------------------------------- -void SerializationChain::Clear() noexcept { - num_blocks_ = 0; - num_bytes_written_ = 0; - fragment_index_ = 0; - fragment_position_ = 0; - current_block_ = nullptr; -} - -//-------------------------------------------------------------------------------------------------- -// Seek -//-------------------------------------------------------------------------------------------------- -void SerializationChain::Seek(int fragment_index, int position) noexcept { - if (fragment_index == 0) { - fragment_position_ += position; - return; - } - auto prev_fragment_index = fragment_index_; - fragment_index_ += fragment_index; - if (fragment_index_ == num_blocks_ + 1) { - assert(position < 2); - fragment_position_ = position; - current_block_ = nullptr; - return; - } - - int prev_block_index = std::max(prev_fragment_index - 1, 0); - int block_index = fragment_index_ - 1; - for (int i = prev_block_index; i < block_index; ++i) { - current_block_ = current_block_->next.get(); - } - fragment_position_ = position; -} -} // namespace lightstep diff --git a/src/common/serialization_chain.h b/src/common/serialization_chain.h deleted file mode 100644 index 29a7298e..00000000 --- a/src/common/serialization_chain.h +++ /dev/null @@ -1,106 +0,0 @@ -#pragma once - -#include -#include -#include -#include - -#include "common/fragment_input_stream.h" -#include "common/function_ref.h" -#include "common/hex_conversion.h" -#include "common/noncopyable.h" -#include "common/serialization.h" - -#include - -namespace lightstep { -/** - * Maintains a linked chain of blocks for a serialization. - */ -class SerializationChain final - : public google::protobuf::io::ZeroCopyOutputStream, - public FragmentInputStream, - private Noncopyable { - public: - static const int BlockSize = 256; - static const size_t ReportRequestSpansField = 3; - - SerializationChain() noexcept; - - /** - * Move another SerializationChain into this one. - * - * Note: This should be used only as an intrusive container when the order of - * the SerializaionChains is not important. - * - * @param other the SerializationChain to add - */ - void Chain(std::unique_ptr&& other) noexcept; - - /** - * Iterate over all the owned SerializationChains - * @param callback the callback to call with each SerializationChain - */ - bool ForEachSerializationChain( - FunctionRef callback); - - /** - * Adds http/1.1 chunk framing and a message header so that the data can be - * parsed as part of a protobuf ReportRequest. - */ - void AddFraming() noexcept; - - /** - * @return The number of bytes after the SerializationChain has been framed - */ - int num_bytes_after_framing() const noexcept { - return num_bytes_after_framing_; - } - - // ZeroCopyOutputStream - bool Next(void** data, int* size) override; - - void BackUp(int count) override; - - google::protobuf::int64 ByteCount() const override { - return static_cast(num_bytes_written_); - } - - // FragmentInputStream - int num_fragments() const noexcept override; - - bool ForEachFragment(Callback callback) const noexcept override; - - void Clear() noexcept override; - - void Seek(int fragment_index, int position) noexcept override; - - private: - std::unique_ptr next_; - - struct Block { - std::unique_ptr next; - int size; - std::array data; - }; - - int num_blocks_{1}; - int num_bytes_written_{0}; - int num_bytes_after_framing_{0}; - int current_block_position_{0}; - int header_size_{0}; - Block* current_block_; - - int fragment_index_{0}; - int fragment_position_{0}; - - Block head_; - static const size_t MaxHeaderSize = - Num64BitHexDigits + 2 + - StaticKeySerializationSize::value + - google::protobuf::io::CodedOutputStream::StaticVarintSize32< - std::numeric_limits::max()>::value; - std::array header_; -}; -} // namespace lightstep diff --git a/src/tracer/BUILD b/src/tracer/BUILD index 7a8ac12e..5f534d82 100644 --- a/src/tracer/BUILD +++ b/src/tracer/BUILD @@ -95,7 +95,6 @@ lightstep_cc_library( "//src/common:logger_lib", "//src/common:utility_lib", "//src/common:random_lib", - "//src/common:serialization_chain_lib", "//src/common:spin_lock_mutex_lib", "//src/recorder:recorder_interface", ":immutable_span_context_lib", From 3ede7dcc15757e0c7a74498831c97b4ae3da1ce4 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Fri, 24 Jan 2020 23:38:38 -0800 Subject: [PATCH 45/66] Fill out ManualRecorder --- src/recorder/BUILD | 1 + src/recorder/manual_recorder.cpp | 49 ++++++++++++++++++++++++++++++++ src/recorder/manual_recorder.h | 5 ++++ 3 files changed, 55 insertions(+) diff --git a/src/recorder/BUILD b/src/recorder/BUILD index 9252f7a7..a2f25f57 100644 --- a/src/recorder/BUILD +++ b/src/recorder/BUILD @@ -104,6 +104,7 @@ lightstep_cc_library( deps = [ "//src/common:logger_lib", "//src/common:noncopyable_lib", + "//src/common:report_request_framing_lib", "//src/common:utility_lib", "//src/common:circular_buffer_lib", ":fork_aware_recorder_lib", diff --git a/src/recorder/manual_recorder.cpp b/src/recorder/manual_recorder.cpp index 64535746..a031644a 100644 --- a/src/recorder/manual_recorder.cpp +++ b/src/recorder/manual_recorder.cpp @@ -1,5 +1,7 @@ #include "recorder/manual_recorder.h" +#include "common/report_request_framing.h" + namespace lightstep { //-------------------------------------------------------------------------------------------------- // constructor @@ -16,6 +18,53 @@ ManualRecorder::ManualRecorder(Logger& logger, LightStepTracerOptions options, } } +//-------------------------------------------------------------------------------------------------- +// ReserveHeaderSpace +//-------------------------------------------------------------------------------------------------- +Fragment ManualRecorder::ReserveHeaderSpace(ChainedStream& stream) { + const size_t max_header_size = ReportRequestSpansMaxHeaderSize; + static_assert(ChainedStream::BlockSize >= max_header_size, + "BockSize too small"); + void* data; + int size; + if (!stream.Next(&data, &size)) { + throw std::bad_alloc{}; + } + stream.BackUp(size - static_cast(max_header_size)); + return {data, static_cast(max_header_size)}; +} + +//-------------------------------------------------------------------------------------------------- +// RecordSpan +//-------------------------------------------------------------------------------------------------- +void ManualRecorder::RecordSpan( + Fragment header_fragment, std::unique_ptr&& span) noexcept { + // Frame the Span + char* header_data = static_cast(header_fragment.first); + auto reserved_header_size = static_cast(header_fragment.second); + auto protobuf_body_size = span->ByteCount() - header_fragment.second; + auto protobuf_header_size = WriteReportRequestSpansHeader( + header_data, reserved_header_size, protobuf_body_size); + span->CloseOutput(); + + // Advance past reserved header space we didn't use. + span->Seek(0, static_cast(reserved_header_size - protobuf_header_size)); + + if (!span_buffer_.Add(span)) { + // Note: the compiler doesn't want to inline this logger call and it shows + // up in profiling with high span droppage even if the logging isn't turned + // on. + // + // Hence, the additional checking to avoid a function call. + if (static_cast(logger_.level()) <= + static_cast(LogLevel::debug)) { + logger_.Debug("Dropping span"); + } + /* metrics_.OnSpansDropped(1); */ + span.reset(); + } +} + //-------------------------------------------------------------------------------------------------- // FlushWithTimeout //-------------------------------------------------------------------------------------------------- diff --git a/src/recorder/manual_recorder.h b/src/recorder/manual_recorder.h index c6e3d928..e82a7abf 100644 --- a/src/recorder/manual_recorder.h +++ b/src/recorder/manual_recorder.h @@ -13,6 +13,11 @@ class ManualRecorder : public ForkAwareRecorder, private Noncopyable { std::unique_ptr&& transporter); // Recorder + Fragment ReserveHeaderSpace(ChainedStream& stream) override; + + void RecordSpan(Fragment header_fragment, + std::unique_ptr&& span) noexcept override; + bool FlushWithTimeout( std::chrono::system_clock::duration timeout) noexcept override; From f52b204a5092ab4ee6703de8cca5bc1f03b175f4 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Sun, 26 Jan 2020 23:39:06 -0800 Subject: [PATCH 46/66] Factor out MetricsTracker --- src/recorder/BUILD | 13 ++++++++++++ ...corder_metrics.cpp => metrics_tracker.cpp} | 13 ++++++------ ...m_recorder_metrics.h => metrics_tracker.h} | 4 ++-- src/recorder/serialization/report_request.cpp | 3 +-- src/recorder/stream_recorder/BUILD | 21 ++++--------------- .../stream_recorder/connection_stream.h | 3 +-- .../stream_recorder/satellite_streamer.cpp | 3 +-- .../stream_recorder/satellite_streamer.h | 4 ++-- src/recorder/stream_recorder/span_stream.cpp | 2 +- src/recorder/stream_recorder/span_stream.h | 12 +++++------ .../stream_recorder/stream_recorder.h | 14 ++++++------- test/number_simulation.cpp | 6 +++--- test/number_simulation.h | 2 +- .../connection_stream_test.cpp | 4 ++-- .../stream_recorder/span_stream_test.cpp | 2 +- test/utility.h | 2 +- 16 files changed, 51 insertions(+), 57 deletions(-) rename src/recorder/{stream_recorder/stream_recorder_metrics.cpp => metrics_tracker.cpp} (75%) rename src/recorder/{stream_recorder/stream_recorder_metrics.h => metrics_tracker.h} (91%) diff --git a/src/recorder/BUILD b/src/recorder/BUILD index a2f25f57..738ea5c4 100644 --- a/src/recorder/BUILD +++ b/src/recorder/BUILD @@ -163,3 +163,16 @@ lightstep_cc_library( ":stream_recorder_interface", ], ) + +lightstep_cc_library( + name = "metrics_tracker_lib", + private_hdrs = [ + "metrics_tracker.h", + ], + srcs = [ + "metrics_tracker.cpp", + ], + deps = [ + "//include/lightstep:tracer_interface", + ], +) diff --git a/src/recorder/stream_recorder/stream_recorder_metrics.cpp b/src/recorder/metrics_tracker.cpp similarity index 75% rename from src/recorder/stream_recorder/stream_recorder_metrics.cpp rename to src/recorder/metrics_tracker.cpp index 09232ee1..7f34655a 100644 --- a/src/recorder/stream_recorder/stream_recorder_metrics.cpp +++ b/src/recorder/metrics_tracker.cpp @@ -1,36 +1,35 @@ -#include "recorder/stream_recorder/stream_recorder_metrics.h" +#include "recorder/metrics_tracker.h" namespace lightstep { //-------------------------------------------------------------------------------------------------- // constructor //-------------------------------------------------------------------------------------------------- -StreamRecorderMetrics::StreamRecorderMetrics( - MetricsObserver& metrics_observer) noexcept +MetricsTracker::MetricsTracker(MetricsObserver& metrics_observer) noexcept : metrics_observer_{metrics_observer} {} //-------------------------------------------------------------------------------------------------- // OnSpansSent //-------------------------------------------------------------------------------------------------- -void StreamRecorderMetrics::OnSpansSent(int num_spans) noexcept { +void MetricsTracker::OnSpansSent(int num_spans) noexcept { metrics_observer_.OnSpansSent(num_spans); } //-------------------------------------------------------------------------------------------------- // OnFlush //-------------------------------------------------------------------------------------------------- -void StreamRecorderMetrics::OnFlush() noexcept { metrics_observer_.OnFlush(); } +void MetricsTracker::OnFlush() noexcept { metrics_observer_.OnFlush(); } //-------------------------------------------------------------------------------------------------- // ConsumeDroppedSpans //-------------------------------------------------------------------------------------------------- -int StreamRecorderMetrics::ConsumeDroppedSpans() noexcept { +int MetricsTracker::ConsumeDroppedSpans() noexcept { return num_dropped_spans_.exchange(0); } //-------------------------------------------------------------------------------------------------- // UnconsumeDroppedSpans //-------------------------------------------------------------------------------------------------- -void StreamRecorderMetrics::UnconsumeDroppedSpans(int num_spans) noexcept { +void MetricsTracker::UnconsumeDroppedSpans(int num_spans) noexcept { num_dropped_spans_ += num_spans; } } // namespace lightstep diff --git a/src/recorder/stream_recorder/stream_recorder_metrics.h b/src/recorder/metrics_tracker.h similarity index 91% rename from src/recorder/stream_recorder/stream_recorder_metrics.h rename to src/recorder/metrics_tracker.h index c7d15294..9109087e 100644 --- a/src/recorder/stream_recorder/stream_recorder_metrics.h +++ b/src/recorder/metrics_tracker.h @@ -8,9 +8,9 @@ namespace lightstep { /** * Manages the metrics associated with a StreamRecorder. */ -class StreamRecorderMetrics { +class MetricsTracker { public: - explicit StreamRecorderMetrics(MetricsObserver& metrics_observer) noexcept; + explicit MetricsTracker(MetricsObserver& metrics_observer) noexcept; /** * Record dropped spans. diff --git a/src/recorder/serialization/report_request.cpp b/src/recorder/serialization/report_request.cpp index f832a3a3..488f080b 100644 --- a/src/recorder/serialization/report_request.cpp +++ b/src/recorder/serialization/report_request.cpp @@ -14,8 +14,7 @@ ReportRequest::ReportRequest(const std::shared_ptr& header, //-------------------------------------------------------------------------------------------------- // AddSpan //-------------------------------------------------------------------------------------------------- -void ReportRequest::AddSpan( - std::unique_ptr&& span) noexcept { +void ReportRequest::AddSpan(std::unique_ptr&& span) noexcept { ++num_spans_; num_fragments_ += span->num_fragments(); span->ForEachFragment([&](void* /*data*/, int length) { diff --git a/src/recorder/stream_recorder/BUILD b/src/recorder/stream_recorder/BUILD index 890dc40e..662aedba 100644 --- a/src/recorder/stream_recorder/BUILD +++ b/src/recorder/stream_recorder/BUILD @@ -16,19 +16,6 @@ lightstep_cc_library( ], ) -lightstep_cc_library( - name = "stream_recorder_metrics_lib", - private_hdrs = [ - "stream_recorder_metrics.h", - ], - srcs = [ - "stream_recorder_metrics.cpp", - ], - deps = [ - "//include/lightstep:metrics_observer_interface", - ], -) - lightstep_cc_library( name = "span_stream_lib", private_hdrs = [ @@ -41,7 +28,7 @@ lightstep_cc_library( "//src/common:circular_buffer_lib", "//src/common:chained_stream_lib", "//src/common:fragment_input_stream_lib", - ":stream_recorder_metrics_lib", + "//src/recorder:metrics_tracker_lib", ], ) @@ -60,7 +47,7 @@ lightstep_cc_library( "//src/common:fragment_array_input_stream_lib", "//src/recorder/serialization:embedded_metrics_message_lib", ":span_stream_lib", - ":stream_recorder_metrics_lib", + "//src/recorder:metrics_tracker_lib", ], ) @@ -88,7 +75,7 @@ lightstep_cc_library( "//src/recorder:fork_aware_recorder_lib", "//src/recorder:stream_recorder_interface", ":stream_recorder_options_lib", - ":stream_recorder_metrics_lib", + "//src/recorder:metrics_tracker_lib", ":satellite_streamer_lib", ], ) @@ -150,7 +137,7 @@ lightstep_cc_library( "//src/network:timer_event_lib", "//src/network:vector_write_lib", "//src/recorder/serialization:report_request_header_lib", - ":stream_recorder_metrics_lib", + "//src/recorder:metrics_tracker_lib", ":host_header_lib", ":span_stream_lib", ":connection_stream_lib", diff --git a/src/recorder/stream_recorder/connection_stream.h b/src/recorder/stream_recorder/connection_stream.h index 530e192e..4e80b4c1 100644 --- a/src/recorder/stream_recorder/connection_stream.h +++ b/src/recorder/stream_recorder/connection_stream.h @@ -4,14 +4,13 @@ #include #include -#include "common/hex_conversion.h" #include "common/fragment_array_input_stream.h" #include "common/fragment_input_stream.h" #include "common/function_ref.h" +#include "common/hex_conversion.h" #include "common/utility.h" #include "recorder/serialization/embedded_metrics_message.h" #include "recorder/stream_recorder/span_stream.h" -#include "recorder/stream_recorder/stream_recorder_metrics.h" namespace lightstep { /** diff --git a/src/recorder/stream_recorder/satellite_streamer.cpp b/src/recorder/stream_recorder/satellite_streamer.cpp index 59f601f5..818bd9dd 100644 --- a/src/recorder/stream_recorder/satellite_streamer.cpp +++ b/src/recorder/stream_recorder/satellite_streamer.cpp @@ -13,8 +13,7 @@ namespace lightstep { SatelliteStreamer::SatelliteStreamer( Logger& logger, EventBase& event_base, const LightStepTracerOptions& tracer_options, - const StreamRecorderOptions& recorder_options, - StreamRecorderMetrics& metrics, + const StreamRecorderOptions& recorder_options, MetricsTracker& metrics, CircularBuffer& span_buffer) : logger_{logger}, event_base_{event_base}, diff --git a/src/recorder/stream_recorder/satellite_streamer.h b/src/recorder/stream_recorder/satellite_streamer.h index f1994842..4cf26be2 100644 --- a/src/recorder/stream_recorder/satellite_streamer.h +++ b/src/recorder/stream_recorder/satellite_streamer.h @@ -5,10 +5,10 @@ #include "common/noncopyable.h" #include "common/random_traverser.h" +#include "recorder/metrics_tracker.h" #include "recorder/stream_recorder/satellite_connection.h" #include "recorder/stream_recorder/satellite_endpoint_manager.h" #include "recorder/stream_recorder/span_stream.h" -#include "recorder/stream_recorder/stream_recorder_metrics.h" namespace lightstep { /** @@ -19,7 +19,7 @@ class SatelliteStreamer : private Noncopyable { SatelliteStreamer(Logger& logger, EventBase& event_base, const LightStepTracerOptions& tracer_options, const StreamRecorderOptions& recorder_options, - StreamRecorderMetrics& metrics, + MetricsTracker& metrics, CircularBuffer& span_buffer); /** diff --git a/src/recorder/stream_recorder/span_stream.cpp b/src/recorder/stream_recorder/span_stream.cpp index 180dc638..d5ef9a20 100644 --- a/src/recorder/stream_recorder/span_stream.cpp +++ b/src/recorder/stream_recorder/span_stream.cpp @@ -5,7 +5,7 @@ namespace lightstep { // constructor //-------------------------------------------------------------------------------------------------- SpanStream::SpanStream(CircularBuffer& span_buffer, - StreamRecorderMetrics& metrics) noexcept + MetricsTracker& metrics) noexcept : span_buffer_{span_buffer}, metrics_{metrics} {} //-------------------------------------------------------------------------------------------------- diff --git a/src/recorder/stream_recorder/span_stream.h b/src/recorder/stream_recorder/span_stream.h index 39f277d7..90672e41 100644 --- a/src/recorder/stream_recorder/span_stream.h +++ b/src/recorder/stream_recorder/span_stream.h @@ -1,8 +1,8 @@ #pragma once -#include "common/circular_buffer.h" #include "common/chained_stream.h" -#include "recorder/stream_recorder/stream_recorder_metrics.h" +#include "common/circular_buffer.h" +#include "recorder/metrics_tracker.h" namespace lightstep { /** @@ -12,7 +12,7 @@ namespace lightstep { class SpanStream final : public FragmentInputStream { public: SpanStream(CircularBuffer& span_buffer, - StreamRecorderMetrics& metrics) noexcept; + MetricsTracker& metrics) noexcept; /** * Allots spans from the associated circular buffer to stream to satellites. @@ -26,9 +26,9 @@ class SpanStream final : public FragmentInputStream { std::unique_ptr ConsumeRemnant() noexcept; /** - * @return the associagted StreamRecorderMetrics + * @return the associagted MetricsTracker */ - StreamRecorderMetrics& metrics() const noexcept { return metrics_; } + MetricsTracker& metrics() const noexcept { return metrics_; } // FragmentInputStream int num_fragments() const noexcept override; @@ -41,7 +41,7 @@ class SpanStream final : public FragmentInputStream { private: CircularBuffer& span_buffer_; - StreamRecorderMetrics& metrics_; + MetricsTracker& metrics_; CircularBufferRange> allotment_; std::unique_ptr remnant_; }; diff --git a/src/recorder/stream_recorder/stream_recorder.h b/src/recorder/stream_recorder/stream_recorder.h index a1edd5b3..44f4c3ce 100644 --- a/src/recorder/stream_recorder/stream_recorder.h +++ b/src/recorder/stream_recorder/stream_recorder.h @@ -8,17 +8,17 @@ #include "stream_recorder_impl.h" +#include "common/chained_stream.h" #include "common/circular_buffer.h" #include "common/logger.h" #include "common/noncopyable.h" #include "common/platform/network_environment.h" -#include "common/chained_stream.h" #include "lightstep/tracer.h" #include "network/event_base.h" #include "network/timer_event.h" #include "recorder/fork_aware_recorder.h" +#include "recorder/metrics_tracker.h" #include "recorder/stream_recorder.h" -#include "recorder/stream_recorder/stream_recorder_metrics.h" #include "recorder/stream_recorder/stream_recorder_options.h" namespace lightstep { @@ -77,16 +77,14 @@ class StreamRecorder : public ForkAwareRecorder, private Noncopyable { } /** - * @return the associated StreamRecorderMetrics. + * @return the associated MetricsTracker. */ - StreamRecorderMetrics& metrics() noexcept { return metrics_; } + MetricsTracker& metrics() noexcept { return metrics_; } /** * @return the associated span buffer. */ - CircularBuffer& span_buffer() noexcept { - return span_buffer_; - } + CircularBuffer& span_buffer() noexcept { return span_buffer_; } // Recorder Fragment ReserveHeaderSpace(ChainedStream& stream) override; @@ -131,7 +129,7 @@ class StreamRecorder : public ForkAwareRecorder, private Noncopyable { LightStepTracerOptions tracer_options_; StreamRecorderOptions recorder_options_; - StreamRecorderMetrics metrics_; + MetricsTracker metrics_; CircularBuffer span_buffer_; std::atomic exit_{false}; diff --git a/test/number_simulation.cpp b/test/number_simulation.cpp index 5cae0fb8..06e57259 100644 --- a/test/number_simulation.cpp +++ b/test/number_simulation.cpp @@ -46,9 +46,9 @@ GenerateRandomBinaryNumber(size_t max_digits) { //-------------------------------------------------------------------------------------------------- // GenerateRandomBinaryNumbers //-------------------------------------------------------------------------------------------------- -static void GenerateRandomBinaryNumbers( - CircularBuffer& buffer, std::vector& numbers, - size_t n) { +static void GenerateRandomBinaryNumbers(CircularBuffer& buffer, + std::vector& numbers, + size_t n) { while (n-- != 0) { uint32_t x; opentracing::string_view s; diff --git a/test/number_simulation.h b/test/number_simulation.h index b6b9f7d5..fe8a46ca 100644 --- a/test/number_simulation.h +++ b/test/number_simulation.h @@ -6,8 +6,8 @@ #include #include -#include "common/circular_buffer.h" #include "common/chained_stream.h" +#include "common/circular_buffer.h" #include "recorder/stream_recorder/connection_stream.h" #include diff --git a/test/recorder/stream_recorder/connection_stream_test.cpp b/test/recorder/stream_recorder/connection_stream_test.cpp index c4d3a544..e64c308e 100644 --- a/test/recorder/stream_recorder/connection_stream_test.cpp +++ b/test/recorder/stream_recorder/connection_stream_test.cpp @@ -35,7 +35,7 @@ TEST_CASE("ConnectionStream") { LightStepTracerOptions tracer_options; CircularBuffer span_buffer{1000}; MetricsObserver metrics_observer; - StreamRecorderMetrics metrics{metrics_observer}; + MetricsTracker metrics{metrics_observer}; SpanStream span_stream{span_buffer, metrics}; std::string header_common_fragment = WriteReportRequestHeader(tracer_options, 123); @@ -246,7 +246,7 @@ TEST_CASE( std::string header_common_fragment = WriteReportRequestHeader(tracer_options, 123); MetricsObserver metrics_observer; - StreamRecorderMetrics metrics{metrics_observer}; + MetricsTracker metrics{metrics_observer}; auto host_header_fragment = MakeFragment("Host:abc\r\n"); const size_t num_producer_threads = 4; const size_t num_connections = 10; diff --git a/test/recorder/stream_recorder/span_stream_test.cpp b/test/recorder/stream_recorder/span_stream_test.cpp index 9b5a481b..1bbc86ae 100644 --- a/test/recorder/stream_recorder/span_stream_test.cpp +++ b/test/recorder/stream_recorder/span_stream_test.cpp @@ -11,7 +11,7 @@ TEST_CASE("SpanStream") { const size_t max_spans = 10; CircularBuffer buffer{max_spans}; MetricsObserver metrics_observer; - StreamRecorderMetrics metrics{metrics_observer}; + MetricsTracker metrics{metrics_observer}; SpanStream span_stream{buffer, metrics}; SECTION("When the attached buffer is empty, SpanStream has no fragments") { diff --git a/test/utility.h b/test/utility.h index dfd72909..3e6f8fa9 100644 --- a/test/utility.h +++ b/test/utility.h @@ -3,9 +3,9 @@ #include #include +#include "common/chained_stream.h" #include "common/circular_buffer.h" #include "common/fragment_input_stream.h" -#include "common/chained_stream.h" #include #include "lightstep-tracer-common/collector.pb.h" From 6ebc38ea541c634caa160dd881f466cf0ba8c5f1 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Sun, 26 Jan 2020 23:50:17 -0800 Subject: [PATCH 47/66] Add MetricsTracker to ManualRecorder --- src/recorder/BUILD | 1 + src/recorder/manual_recorder.cpp | 18 +++++++++++++----- src/recorder/manual_recorder.h | 2 ++ 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/recorder/BUILD b/src/recorder/BUILD index 738ea5c4..2e14e7d4 100644 --- a/src/recorder/BUILD +++ b/src/recorder/BUILD @@ -107,6 +107,7 @@ lightstep_cc_library( "//src/common:report_request_framing_lib", "//src/common:utility_lib", "//src/common:circular_buffer_lib", + "//src/recorder:metrics_tracker_lib", ":fork_aware_recorder_lib", ":transporter_lib", ], diff --git a/src/recorder/manual_recorder.cpp b/src/recorder/manual_recorder.cpp index a031644a..24682f3a 100644 --- a/src/recorder/manual_recorder.cpp +++ b/src/recorder/manual_recorder.cpp @@ -3,6 +3,17 @@ #include "common/report_request_framing.h" namespace lightstep { +//-------------------------------------------------------------------------------------------------- +// GetMetricsObserver +//-------------------------------------------------------------------------------------------------- +static MetricsObserver& GetMetricsObserver( + LightStepTracerOptions& tracer_options) { + if (tracer_options.metrics_observer == nullptr) { + tracer_options.metrics_observer.reset(new MetricsObserver{}); + } + return *tracer_options.metrics_observer; +} + //-------------------------------------------------------------------------------------------------- // constructor //-------------------------------------------------------------------------------------------------- @@ -11,11 +22,8 @@ ManualRecorder::ManualRecorder(Logger& logger, LightStepTracerOptions options, : logger_{logger}, tracer_options_{std::move(options)}, transporter_{std::move(transporter)}, + metrics_{GetMetricsObserver(options)}, span_buffer_{tracer_options_.max_buffered_spans.value()} { - // If no MetricsObserver was provided, use a default one that does nothing. - if (tracer_options_.metrics_observer == nullptr) { - tracer_options_.metrics_observer.reset(new MetricsObserver{}); - } } //-------------------------------------------------------------------------------------------------- @@ -60,7 +68,7 @@ void ManualRecorder::RecordSpan( static_cast(LogLevel::debug)) { logger_.Debug("Dropping span"); } - /* metrics_.OnSpansDropped(1); */ + metrics_.OnSpansDropped(1); span.reset(); } } diff --git a/src/recorder/manual_recorder.h b/src/recorder/manual_recorder.h index e82a7abf..8f833b2a 100644 --- a/src/recorder/manual_recorder.h +++ b/src/recorder/manual_recorder.h @@ -5,6 +5,7 @@ #include "common/noncopyable.h" #include "lightstep/transporter.h" #include "recorder/fork_aware_recorder.h" +#include "recorder/metrics_tracker.h" namespace lightstep { class ManualRecorder : public ForkAwareRecorder, private Noncopyable { @@ -33,6 +34,7 @@ class ManualRecorder : public ForkAwareRecorder, private Noncopyable { LightStepTracerOptions tracer_options_; std::unique_ptr transporter_; + MetricsTracker metrics_; CircularBuffer span_buffer_; }; } // namespace lightstep From 3eb6603bfc494a946370e16ddd9ae19b3dcbf4e7 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Mon, 27 Jan 2020 12:58:10 -0800 Subject: [PATCH 48/66] Fill out ReportRequest --- src/recorder/serialization/report_request.cpp | 19 +++++++++- src/recorder/serialization/report_request.h | 2 + test/recorder/serialization/BUILD | 1 + .../serialization/report_request_test.cpp | 37 ++++++++++++++++++- 4 files changed, 57 insertions(+), 2 deletions(-) diff --git a/src/recorder/serialization/report_request.cpp b/src/recorder/serialization/report_request.cpp index 488f080b..a727b410 100644 --- a/src/recorder/serialization/report_request.cpp +++ b/src/recorder/serialization/report_request.cpp @@ -7,8 +7,15 @@ namespace lightstep { ReportRequest::ReportRequest(const std::shared_ptr& header, int num_dropped_spans) : header_{header} { - (void)num_dropped_spans; num_bytes_ = static_cast(header_->size()); + if (num_dropped_spans == 0) { + return; + } + auto metrics = new EmbeddedMetricsMessage{}; + metrics_.reset(metrics); + metrics->set_num_dropped_spans(num_dropped_spans); + metrics_fragment_ = metrics->MakeFragment(); + num_bytes_ += metrics_fragment_.second; } //-------------------------------------------------------------------------------------------------- @@ -28,6 +35,16 @@ void ReportRequest::AddSpan(std::unique_ptr&& span) noexcept { spans_->Append(std::move(span)); } +//-------------------------------------------------------------------------------------------------- +// num_dropped_spans +//-------------------------------------------------------------------------------------------------- +int ReportRequest::num_dropped_spans() const noexcept { + if (metrics_ == nullptr) { + return 0; + } + return metrics_->num_dropped_spans(); +} + //-------------------------------------------------------------------------------------------------- // ForEachFragment //-------------------------------------------------------------------------------------------------- diff --git a/src/recorder/serialization/report_request.h b/src/recorder/serialization/report_request.h index d9817668..b28aaa9b 100644 --- a/src/recorder/serialization/report_request.h +++ b/src/recorder/serialization/report_request.h @@ -14,6 +14,8 @@ class ReportRequest final : public BufferChain { void AddSpan(std::unique_ptr&& span) noexcept; + int num_dropped_spans() const noexcept; + // BufferChain size_t num_fragments() const noexcept override { return static_cast(num_fragments_); diff --git a/test/recorder/serialization/BUILD b/test/recorder/serialization/BUILD index 411efd98..72b6cc4a 100644 --- a/test/recorder/serialization/BUILD +++ b/test/recorder/serialization/BUILD @@ -34,5 +34,6 @@ lightstep_catch_test( ], deps = [ "//src/recorder/serialization:report_request_lib", + "//src/recorder/serialization:report_request_header_lib", ], ) diff --git a/test/recorder/serialization/report_request_test.cpp b/test/recorder/serialization/report_request_test.cpp index eced07d2..638d15a9 100644 --- a/test/recorder/serialization/report_request_test.cpp +++ b/test/recorder/serialization/report_request_test.cpp @@ -1,7 +1,42 @@ #include "recorder/serialization/report_request.h" +#include "recorder/serialization/report_request_header.h" #include "3rd_party/catch2/catch.hpp" #include "lightstep-tracer-common/collector.pb.h" using namespace lightstep; -TEST_CASE("ReportRequest") {} +static collector::ReportRequest ToProtobufReportRequest( + const ReportRequest& report_request) { + std::string s(report_request.num_bytes(), ' '); + report_request.CopyOut(&s[0], s.size()); + collector::ReportRequest result; + if (!result.ParseFromString(s)) { + std::cerr << "Failed to parse report request\n"; + std::terminate(); + } + return result; +} + +TEST_CASE("ReportRequest") { + LightStepTracerOptions options; + uint64_t reporter_id = 123; + auto header = std::make_shared( + WriteReportRequestHeader(options, reporter_id)); + ReportRequest report_request{header, 0}; + REQUIRE(report_request.num_dropped_spans() == 0); + + SECTION("We can serialize a ReportRequest with no spans") { + auto protobuf_report_request = ToProtobufReportRequest(report_request); + REQUIRE(protobuf_report_request.reporter().reporter_id() == 123); + } + + SECTION("We can add metrics to a ReportRequest") { + report_request = ReportRequest{header, 42}; + REQUIRE(report_request.num_dropped_spans() == 42); + auto protobuf_report_request = ToProtobufReportRequest(report_request); + auto& dropped_span_count = + protobuf_report_request.internal_metrics().counts()[0]; + REQUIRE(dropped_span_count.name() == "spans.dropped"); + REQUIRE(dropped_span_count.int_value() == 42); + } +} From 3554a50d5f87e72e9e0bb193220de56b9e7e4113 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Mon, 27 Jan 2020 13:40:17 -0800 Subject: [PATCH 49/66] Fill out ReportRequest tests --- test/recorder/serialization/BUILD | 1 + .../serialization/report_request_test.cpp | 48 +++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/test/recorder/serialization/BUILD b/test/recorder/serialization/BUILD index 72b6cc4a..1ced455e 100644 --- a/test/recorder/serialization/BUILD +++ b/test/recorder/serialization/BUILD @@ -33,6 +33,7 @@ lightstep_catch_test( "report_request_test.cpp", ], deps = [ + "//src/common:report_request_framing_lib", "//src/recorder/serialization:report_request_lib", "//src/recorder/serialization:report_request_header_lib", ], diff --git a/test/recorder/serialization/report_request_test.cpp b/test/recorder/serialization/report_request_test.cpp index 638d15a9..866c34c4 100644 --- a/test/recorder/serialization/report_request_test.cpp +++ b/test/recorder/serialization/report_request_test.cpp @@ -1,8 +1,13 @@ #include "recorder/serialization/report_request.h" +#include + +#include "common/report_request_framing.h" #include "recorder/serialization/report_request_header.h" #include "3rd_party/catch2/catch.hpp" #include "lightstep-tracer-common/collector.pb.h" + +#include using namespace lightstep; static collector::ReportRequest ToProtobufReportRequest( @@ -17,6 +22,25 @@ static collector::ReportRequest ToProtobufReportRequest( return result; } +static std::unique_ptr ToSerialization( + const collector::Span& span) { + auto s = span.SerializeAsString(); + std::array buffer; + auto header_size = WriteReportRequestSpansHeader( + buffer.data(), buffer.size(), static_cast(s.size())); + s.insert(0, buffer.data() + (buffer.size() - header_size), header_size); + + std::unique_ptr result{new ChainedStream{}}; + std::unique_ptr stream{ + new google::protobuf::io::CodedOutputStream{result.get()}}; + stream->WriteRaw(static_cast(s.data()), + static_cast(s.size())); + stream.reset(); + result->CloseOutput(); + + return result; +} + TEST_CASE("ReportRequest") { LightStepTracerOptions options; uint64_t reporter_id = 123; @@ -39,4 +63,28 @@ TEST_CASE("ReportRequest") { REQUIRE(dropped_span_count.name() == "spans.dropped"); REQUIRE(dropped_span_count.int_value() == 42); } + + SECTION("We can add a span to the ReportRequest") { + collector::Span span; + span.mutable_span_context()->set_trace_id(1); + report_request.AddSpan(ToSerialization(span)); + auto protobuf_report_request = ToProtobufReportRequest(report_request); + REQUIRE(protobuf_report_request.spans().size() == 1); + REQUIRE(protobuf_report_request.spans()[0].span_context().trace_id() == 1); + } + + SECTION("We can add mulltiple spans to the ReportRequest") { + collector::Span span1; + span1.mutable_span_context()->set_trace_id(1); + report_request.AddSpan(ToSerialization(span1)); + + collector::Span span2; + span2.mutable_span_context()->set_trace_id(2); + report_request.AddSpan(ToSerialization(span2)); + + auto protobuf_report_request = ToProtobufReportRequest(report_request); + REQUIRE(protobuf_report_request.spans().size() == 2); + REQUIRE(protobuf_report_request.spans()[0].span_context().trace_id() == 1); + REQUIRE(protobuf_report_request.spans()[1].span_context().trace_id() == 2); + } } From d71b2dfaaf2760d9da779d9d5597d7b6f36afdfb Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Mon, 27 Jan 2020 14:20:56 -0800 Subject: [PATCH 50/66] Implement AsyncTransporter::Callback interface in ManualRecorder --- src/recorder/BUILD | 1 + src/recorder/manual_recorder.cpp | 39 +++++++++++++++++-- src/recorder/manual_recorder.h | 9 ++++- src/recorder/serialization/report_request.h | 2 + .../serialization/report_request_test.cpp | 10 ++++- 5 files changed, 55 insertions(+), 6 deletions(-) diff --git a/src/recorder/BUILD b/src/recorder/BUILD index 2e14e7d4..62142c06 100644 --- a/src/recorder/BUILD +++ b/src/recorder/BUILD @@ -108,6 +108,7 @@ lightstep_cc_library( "//src/common:utility_lib", "//src/common:circular_buffer_lib", "//src/recorder:metrics_tracker_lib", + "//src/recorder/serialization:report_request_lib", ":fork_aware_recorder_lib", ":transporter_lib", ], diff --git a/src/recorder/manual_recorder.cpp b/src/recorder/manual_recorder.cpp index 24682f3a..65b2bd4c 100644 --- a/src/recorder/manual_recorder.cpp +++ b/src/recorder/manual_recorder.cpp @@ -1,6 +1,10 @@ #include "recorder/manual_recorder.h" +#include +#include + #include "common/report_request_framing.h" +#include "recorder/serialization/report_request.h" namespace lightstep { //-------------------------------------------------------------------------------------------------- @@ -23,8 +27,7 @@ ManualRecorder::ManualRecorder(Logger& logger, LightStepTracerOptions options, tracer_options_{std::move(options)}, transporter_{std::move(transporter)}, metrics_{GetMetricsObserver(options)}, - span_buffer_{tracer_options_.max_buffered_spans.value()} { -} + span_buffer_{tracer_options_.max_buffered_spans.value()} {} //-------------------------------------------------------------------------------------------------- // ReserveHeaderSpace @@ -77,8 +80,11 @@ void ManualRecorder::RecordSpan( // FlushWithTimeout //-------------------------------------------------------------------------------------------------- bool ManualRecorder::FlushWithTimeout( - std::chrono::system_clock::duration /*timeout*/) noexcept { + std::chrono::system_clock::duration /*timeout*/) noexcept try { return true; +} catch (const std::exception& e) { + logger_.Error("Failed to flush report: ", e.what()); + return false; } //-------------------------------------------------------------------------------------------------- @@ -95,4 +101,31 @@ void ManualRecorder::OnForkedParent() noexcept {} // OnForkedChild //-------------------------------------------------------------------------------------------------- void ManualRecorder::OnForkedChild() noexcept {} + +//-------------------------------------------------------------------------------------------------- +// OnSuccess +//-------------------------------------------------------------------------------------------------- +void ManualRecorder::OnSuccess(BufferChain& message) noexcept { + auto report_request = dynamic_cast(&message); + assert(report_request != nullptr); + if (report_request != nullptr) { + // This should never happen + return; + } + metrics_.OnSpansSent(report_request->num_spans()); +} + +//-------------------------------------------------------------------------------------------------- +// OnFailure +//-------------------------------------------------------------------------------------------------- +void ManualRecorder::OnFailure(BufferChain& message) noexcept { + auto report_request = dynamic_cast(&message); + assert(report_request != nullptr); + if (report_request != nullptr) { + // This should never happen + return; + } + metrics_.OnSpansDropped(report_request->num_spans()); + metrics_.UnconsumeDroppedSpans(report_request->num_dropped_spans()); +} } // namespace lightstep diff --git a/src/recorder/manual_recorder.h b/src/recorder/manual_recorder.h index 8f833b2a..c5ca39d6 100644 --- a/src/recorder/manual_recorder.h +++ b/src/recorder/manual_recorder.h @@ -8,7 +8,9 @@ #include "recorder/metrics_tracker.h" namespace lightstep { -class ManualRecorder : public ForkAwareRecorder, private Noncopyable { +class ManualRecorder : public ForkAwareRecorder, + public AsyncTransporter::Callback, + private Noncopyable { public: ManualRecorder(Logger& logger, LightStepTracerOptions options, std::unique_ptr&& transporter); @@ -29,6 +31,11 @@ class ManualRecorder : public ForkAwareRecorder, private Noncopyable { void OnForkedChild() noexcept override; + // AsyncTransporter::Callback + void OnSuccess(BufferChain& message) noexcept override; + + void OnFailure(BufferChain& message) noexcept override; + private: Logger& logger_; LightStepTracerOptions tracer_options_; diff --git a/src/recorder/serialization/report_request.h b/src/recorder/serialization/report_request.h index b28aaa9b..ef3c1670 100644 --- a/src/recorder/serialization/report_request.h +++ b/src/recorder/serialization/report_request.h @@ -16,6 +16,8 @@ class ReportRequest final : public BufferChain { int num_dropped_spans() const noexcept; + int num_spans() const noexcept { return num_spans_; } + // BufferChain size_t num_fragments() const noexcept override { return static_cast(num_fragments_); diff --git a/test/recorder/serialization/report_request_test.cpp b/test/recorder/serialization/report_request_test.cpp index 866c34c4..f9a40d1c 100644 --- a/test/recorder/serialization/report_request_test.cpp +++ b/test/recorder/serialization/report_request_test.cpp @@ -2,10 +2,10 @@ #include -#include "common/report_request_framing.h" -#include "recorder/serialization/report_request_header.h" #include "3rd_party/catch2/catch.hpp" +#include "common/report_request_framing.h" #include "lightstep-tracer-common/collector.pb.h" +#include "recorder/serialization/report_request_header.h" #include using namespace lightstep; @@ -48,6 +48,7 @@ TEST_CASE("ReportRequest") { WriteReportRequestHeader(options, reporter_id)); ReportRequest report_request{header, 0}; REQUIRE(report_request.num_dropped_spans() == 0); + REQUIRE(report_request.num_spans() == 0); SECTION("We can serialize a ReportRequest with no spans") { auto protobuf_report_request = ToProtobufReportRequest(report_request); @@ -68,6 +69,9 @@ TEST_CASE("ReportRequest") { collector::Span span; span.mutable_span_context()->set_trace_id(1); report_request.AddSpan(ToSerialization(span)); + + REQUIRE(report_request.num_spans() == 1); + auto protobuf_report_request = ToProtobufReportRequest(report_request); REQUIRE(protobuf_report_request.spans().size() == 1); REQUIRE(protobuf_report_request.spans()[0].span_context().trace_id() == 1); @@ -82,6 +86,8 @@ TEST_CASE("ReportRequest") { span2.mutable_span_context()->set_trace_id(2); report_request.AddSpan(ToSerialization(span2)); + REQUIRE(report_request.num_spans() == 2); + auto protobuf_report_request = ToProtobufReportRequest(report_request); REQUIRE(protobuf_report_request.spans().size() == 2); REQUIRE(protobuf_report_request.spans()[0].span_context().trace_id() == 1); From dfc99063e1d016524b758457e3b3aa24485a9bce Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Mon, 27 Jan 2020 20:20:12 -0800 Subject: [PATCH 51/66] Fill out manual recorder --- src/recorder/BUILD | 2 ++ src/recorder/manual_recorder.cpp | 27 +++++++++++++++++++++++++++ src/recorder/manual_recorder.h | 4 ++++ 3 files changed, 33 insertions(+) diff --git a/src/recorder/BUILD b/src/recorder/BUILD index 62142c06..8e9a4531 100644 --- a/src/recorder/BUILD +++ b/src/recorder/BUILD @@ -104,11 +104,13 @@ lightstep_cc_library( deps = [ "//src/common:logger_lib", "//src/common:noncopyable_lib", + "//src/common:random_lib", "//src/common:report_request_framing_lib", "//src/common:utility_lib", "//src/common:circular_buffer_lib", "//src/recorder:metrics_tracker_lib", "//src/recorder/serialization:report_request_lib", + "//src/recorder/serialization:report_request_header_lib", ":fork_aware_recorder_lib", ":transporter_lib", ], diff --git a/src/recorder/manual_recorder.cpp b/src/recorder/manual_recorder.cpp index 65b2bd4c..c0dd7ba9 100644 --- a/src/recorder/manual_recorder.cpp +++ b/src/recorder/manual_recorder.cpp @@ -3,8 +3,10 @@ #include #include +#include "common/random.h" #include "common/report_request_framing.h" #include "recorder/serialization/report_request.h" +#include "recorder/serialization/report_request_header.h" namespace lightstep { //-------------------------------------------------------------------------------------------------- @@ -26,6 +28,8 @@ ManualRecorder::ManualRecorder(Logger& logger, LightStepTracerOptions options, : logger_{logger}, tracer_options_{std::move(options)}, transporter_{std::move(transporter)}, + report_request_header_{new std::string{ + WriteReportRequestHeader(tracer_options_, GenerateId())}}, metrics_{GetMetricsObserver(options)}, span_buffer_{tracer_options_.max_buffered_spans.value()} {} @@ -81,6 +85,29 @@ void ManualRecorder::RecordSpan( //-------------------------------------------------------------------------------------------------- bool ManualRecorder::FlushWithTimeout( std::chrono::system_clock::duration /*timeout*/) noexcept try { + std::unique_ptr report_request{new ReportRequest{ + report_request_header_, metrics_.ConsumeDroppedSpans()}}; + { + std::lock_guard lock_guard{flush_mutex_}; + auto num_spans = span_buffer_.size(); + if (num_spans == 0 && report_request->num_dropped_spans() == 0) { + // Nothing to do + return true; + } + span_buffer_.Consume( + num_spans, [&](CircularBufferRange> & + spans) noexcept { + spans.ForEach([&](AtomicUniquePtr & span) noexcept { + std::unique_ptr span_out; + span.Swap(span_out); + report_request->AddSpan(std::move(span_out)); + return true; + }); + return true; + }); + } + transporter_->Send(std::unique_ptr{report_request.release()}, + *this); return true; } catch (const std::exception& e) { logger_.Error("Failed to flush report: ", e.what()); diff --git a/src/recorder/manual_recorder.h b/src/recorder/manual_recorder.h index c5ca39d6..03561659 100644 --- a/src/recorder/manual_recorder.h +++ b/src/recorder/manual_recorder.h @@ -1,5 +1,7 @@ #pragma once +#include + #include "common/circular_buffer.h" #include "common/logger.h" #include "common/noncopyable.h" @@ -40,8 +42,10 @@ class ManualRecorder : public ForkAwareRecorder, Logger& logger_; LightStepTracerOptions tracer_options_; std::unique_ptr transporter_; + std::shared_ptr report_request_header_; MetricsTracker metrics_; + std::mutex flush_mutex_; CircularBuffer span_buffer_; }; } // namespace lightstep From e8c8b9faafefc0edd62a4cf8319179dd576a7f57 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Mon, 27 Jan 2020 20:22:38 -0800 Subject: [PATCH 52/66] Make ManualRecorder fork safe --- src/recorder/manual_recorder.cpp | 15 ++++----------- src/recorder/manual_recorder.h | 4 ---- 2 files changed, 4 insertions(+), 15 deletions(-) diff --git a/src/recorder/manual_recorder.cpp b/src/recorder/manual_recorder.cpp index c0dd7ba9..f18ca356 100644 --- a/src/recorder/manual_recorder.cpp +++ b/src/recorder/manual_recorder.cpp @@ -114,20 +114,13 @@ bool ManualRecorder::FlushWithTimeout( return false; } -//-------------------------------------------------------------------------------------------------- -// PrepareForFork -//-------------------------------------------------------------------------------------------------- -void ManualRecorder::PrepareForFork() noexcept {} - -//-------------------------------------------------------------------------------------------------- -// OnForkedParent -//-------------------------------------------------------------------------------------------------- -void ManualRecorder::OnForkedParent() noexcept {} - //-------------------------------------------------------------------------------------------------- // OnForkedChild //-------------------------------------------------------------------------------------------------- -void ManualRecorder::OnForkedChild() noexcept {} +void ManualRecorder::OnForkedChild() noexcept { + metrics_.ConsumeDroppedSpans(); + span_buffer_.Clear(); +} //-------------------------------------------------------------------------------------------------- // OnSuccess diff --git a/src/recorder/manual_recorder.h b/src/recorder/manual_recorder.h index 03561659..6dcee11f 100644 --- a/src/recorder/manual_recorder.h +++ b/src/recorder/manual_recorder.h @@ -27,10 +27,6 @@ class ManualRecorder : public ForkAwareRecorder, std::chrono::system_clock::duration timeout) noexcept override; // ForkAwareRecorder - void PrepareForFork() noexcept override; - - void OnForkedParent() noexcept override; - void OnForkedChild() noexcept override; // AsyncTransporter::Callback From da9e394c2ea983aa5410efa9ae71a453c9f1fc50 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Mon, 27 Jan 2020 21:14:06 -0800 Subject: [PATCH 53/66] Add ManualRcorder test --- test/recorder/BUILD | 14 ++++++++++++++ test/recorder/legacy_manual_recorder_test.cpp | 5 +++-- test/recorder/manual_recorder_test.cpp | 7 +++++++ 3 files changed, 24 insertions(+), 2 deletions(-) create mode 100644 test/recorder/manual_recorder_test.cpp diff --git a/test/recorder/BUILD b/test/recorder/BUILD index a608b029..cf5732db 100644 --- a/test/recorder/BUILD +++ b/test/recorder/BUILD @@ -61,6 +61,20 @@ lightstep_catch_test( ], ) +lightstep_catch_test( + name = "manual_recorder_test", + srcs = [ + "manual_recorder_test.cpp", + ], + deps = [ + "//:manual_tracer_lib", + "//src/tracer:counting_metrics_observer_lib", + "//test:utility_lib", + "//test:testing_condition_variable_wrapper_lib", + ":in_memory_async_transporter_lib", + ], +) + lightstep_catch_test( name = "auto_recorder_test", srcs = [ diff --git a/test/recorder/legacy_manual_recorder_test.cpp b/test/recorder/legacy_manual_recorder_test.cpp index d097daf2..ffbcff6e 100644 --- a/test/recorder/legacy_manual_recorder_test.cpp +++ b/test/recorder/legacy_manual_recorder_test.cpp @@ -1,8 +1,9 @@ +#include "recorder/legacy_manual_recorder.h" + #include #include "3rd_party/catch2/catch.hpp" #include "lightstep/tracer.h" -#include "recorder/legacy_manual_recorder.h" #include "test/recorder/in_memory_async_transporter.h" #include "test/testing_condition_variable_wrapper.h" #include "test/utility.h" @@ -12,7 +13,7 @@ using namespace lightstep; using namespace opentracing; -TEST_CASE("legacy_manual_recorder") { +TEST_CASE("LegacyManualRecorder") { Logger logger{}; auto metrics_observer = new CountingMetricsObserver{}; LightStepTracerOptions options; diff --git a/test/recorder/manual_recorder_test.cpp b/test/recorder/manual_recorder_test.cpp new file mode 100644 index 00000000..e544ee55 --- /dev/null +++ b/test/recorder/manual_recorder_test.cpp @@ -0,0 +1,7 @@ +#include "recorder/manual_recorder.h" + +#include "3rd_party/catch2/catch.hpp" +using namespace lightstep; + +TEST_CASE("ManualRecorder") { +} From 338ed37dc0fe65d752dfc9988c7243a4c40b1993 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Mon, 27 Jan 2020 21:28:11 -0800 Subject: [PATCH 54/66] s/InMemoryAsyncTransporter/LegacyInMemoryAsyncTransporter/ --- test/recorder/BUILD | 10 +++++----- ...rter.cpp => legacy_in_memory_async_transporter.cpp} | 8 ++++---- ...nsporter.h => legacy_in_memory_async_transporter.h} | 2 +- test/recorder/legacy_manual_recorder_test.cpp | 4 ++-- 4 files changed, 12 insertions(+), 12 deletions(-) rename test/recorder/{in_memory_async_transporter.cpp => legacy_in_memory_async_transporter.cpp} (89%) rename test/recorder/{in_memory_async_transporter.h => legacy_in_memory_async_transporter.h} (93%) diff --git a/test/recorder/BUILD b/test/recorder/BUILD index cf5732db..93c82063 100644 --- a/test/recorder/BUILD +++ b/test/recorder/BUILD @@ -22,12 +22,12 @@ lightstep_cc_library( ) lightstep_cc_library( - name = "in_memory_async_transporter_lib", + name = "legacy_in_memory_async_transporter_lib", private_hdrs = [ - "in_memory_async_transporter.h", + "legacy_in_memory_async_transporter.h", ], srcs = [ - "in_memory_async_transporter.cpp", + "legacy_in_memory_async_transporter.cpp", ], deps = [ "//src/recorder:transporter_lib", @@ -57,7 +57,7 @@ lightstep_catch_test( "//src/tracer:counting_metrics_observer_lib", "//test:utility_lib", "//test:testing_condition_variable_wrapper_lib", - ":in_memory_async_transporter_lib", + ":legacy_in_memory_async_transporter_lib", ], ) @@ -71,7 +71,7 @@ lightstep_catch_test( "//src/tracer:counting_metrics_observer_lib", "//test:utility_lib", "//test:testing_condition_variable_wrapper_lib", - ":in_memory_async_transporter_lib", + # ":in_memory_async_transporter_lib", ], ) diff --git a/test/recorder/in_memory_async_transporter.cpp b/test/recorder/legacy_in_memory_async_transporter.cpp similarity index 89% rename from test/recorder/in_memory_async_transporter.cpp rename to test/recorder/legacy_in_memory_async_transporter.cpp index 6f75877a..3443b443 100644 --- a/test/recorder/in_memory_async_transporter.cpp +++ b/test/recorder/legacy_in_memory_async_transporter.cpp @@ -1,10 +1,10 @@ -#include "in_memory_async_transporter.h" +#include "legacy_in_memory_async_transporter.h" namespace lightstep { //------------------------------------------------------------------------------ // Send //------------------------------------------------------------------------------ -void InMemoryAsyncTransporter::Send( +void LegacyInMemoryAsyncTransporter::Send( const google::protobuf::Message& request, google::protobuf::Message& response, LegacyAsyncTransporter::Callback& callback) { @@ -16,7 +16,7 @@ void InMemoryAsyncTransporter::Send( //------------------------------------------------------------------------------ // Write //------------------------------------------------------------------------------ -void InMemoryAsyncTransporter::Write() { +void LegacyInMemoryAsyncTransporter::Write() { if (active_request_ == nullptr || active_response_ == nullptr || active_callback_ == nullptr) { std::cerr << "No context, success callback, or request\n"; @@ -45,7 +45,7 @@ void InMemoryAsyncTransporter::Write() { //------------------------------------------------------------------------------ // Fail //------------------------------------------------------------------------------ -void InMemoryAsyncTransporter::Fail(std::error_code error) { +void LegacyInMemoryAsyncTransporter::Fail(std::error_code error) { if (active_callback_ == nullptr) { std::cerr << "No context or failure callback\n"; std::terminate(); diff --git a/test/recorder/in_memory_async_transporter.h b/test/recorder/legacy_in_memory_async_transporter.h similarity index 93% rename from test/recorder/in_memory_async_transporter.h rename to test/recorder/legacy_in_memory_async_transporter.h index 310df576..c49e5dd8 100644 --- a/test/recorder/in_memory_async_transporter.h +++ b/test/recorder/legacy_in_memory_async_transporter.h @@ -9,7 +9,7 @@ #include "lightstep/transporter.h" namespace lightstep { -class InMemoryAsyncTransporter : public LegacyAsyncTransporter { +class LegacyInMemoryAsyncTransporter : public LegacyAsyncTransporter { public: void Send(const google::protobuf::Message& request, google::protobuf::Message& response, diff --git a/test/recorder/legacy_manual_recorder_test.cpp b/test/recorder/legacy_manual_recorder_test.cpp index ffbcff6e..6684257e 100644 --- a/test/recorder/legacy_manual_recorder_test.cpp +++ b/test/recorder/legacy_manual_recorder_test.cpp @@ -4,7 +4,7 @@ #include "3rd_party/catch2/catch.hpp" #include "lightstep/tracer.h" -#include "test/recorder/in_memory_async_transporter.h" +#include "test/recorder/legacy_in_memory_async_transporter.h" #include "test/testing_condition_variable_wrapper.h" #include "test/utility.h" #include "tracer/counting_metrics_observer.h" @@ -21,7 +21,7 @@ TEST_CASE("LegacyManualRecorder") { options.max_buffered_spans = std::function{[&] { return max_buffered_spans; }}; options.metrics_observer.reset(metrics_observer); - auto in_memory_transporter = new InMemoryAsyncTransporter{}; + auto in_memory_transporter = new LegacyInMemoryAsyncTransporter{}; auto recorder = new LegacyManualRecorder{ logger, std::move(options), std::unique_ptr{in_memory_transporter}}; From 97cd1b6ef8f494a1bed5663deaa6d973bfd6b754 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Mon, 27 Jan 2020 22:16:05 -0800 Subject: [PATCH 55/66] Add InMemoryAsyncTransporter --- include/lightstep/transporter.h | 2 +- test/recorder/BUILD | 16 +++++- test/recorder/in_memory_async_transporter.cpp | 50 +++++++++++++++++++ test/recorder/in_memory_async_transporter.h | 31 ++++++++++++ .../legacy_in_memory_async_transporter.h | 11 ++-- test/recorder/manual_recorder_test.cpp | 2 + 6 files changed, 105 insertions(+), 7 deletions(-) create mode 100644 test/recorder/in_memory_async_transporter.cpp create mode 100644 test/recorder/in_memory_async_transporter.h diff --git a/include/lightstep/transporter.h b/include/lightstep/transporter.h index d34068ec..67abc209 100644 --- a/include/lightstep/transporter.h +++ b/include/lightstep/transporter.h @@ -87,6 +87,6 @@ class AsyncTransporter : public Transporter { }; virtual void Send(std::unique_ptr&& message, - Callback& callback) = 0; + Callback& callback) noexcept = 0; }; } // namespace lightstep diff --git a/test/recorder/BUILD b/test/recorder/BUILD index 93c82063..e47557a0 100644 --- a/test/recorder/BUILD +++ b/test/recorder/BUILD @@ -18,6 +18,7 @@ lightstep_cc_library( deps = [ "//src/recorder:recorder_interface", "//test:utility_lib", + "//lightstep-tracer-common:collector_proto_cc", ], ) @@ -32,6 +33,19 @@ lightstep_cc_library( deps = [ "//src/recorder:transporter_lib", ], + +) +lightstep_cc_library( + name = "in_memory_async_transporter_lib", + private_hdrs = [ + "in_memory_async_transporter.h", + ], + srcs = [ + "in_memory_async_transporter.cpp", + ], + deps = [ + "//src/recorder:transporter_lib", + ], ) lightstep_cc_library( @@ -71,7 +85,7 @@ lightstep_catch_test( "//src/tracer:counting_metrics_observer_lib", "//test:utility_lib", "//test:testing_condition_variable_wrapper_lib", - # ":in_memory_async_transporter_lib", + ":in_memory_async_transporter_lib", ], ) diff --git a/test/recorder/in_memory_async_transporter.cpp b/test/recorder/in_memory_async_transporter.cpp new file mode 100644 index 00000000..3975e300 --- /dev/null +++ b/test/recorder/in_memory_async_transporter.cpp @@ -0,0 +1,50 @@ +#include "test/recorder/in_memory_async_transporter.h" + +#include +#include +#include + +namespace lightstep { +//-------------------------------------------------------------------------------------------------- +// Succeed +//-------------------------------------------------------------------------------------------------- +void InMemoryAsyncTransporter::Succeed() noexcept { + if (active_callback_ == nullptr || active_message_ == nullptr) { + std::terminate(); + } + std::string s(active_message_->num_bytes(), ' '); + active_message_->CopyOut(&s[0], s.size()); + collector::ReportRequest report; + if(!report.ParseFromString(s)) { + std::terminate(); + } + reports_.emplace_back(report); + std::copy(report.spans().begin(), report.spans().end(), + std::back_inserter(spans_)); + + active_callback_->OnSuccess(*active_message_); + active_callback_ = nullptr; + active_message_.reset(); +} + +//-------------------------------------------------------------------------------------------------- +// Fail +//-------------------------------------------------------------------------------------------------- +void InMemoryAsyncTransporter::Fail() noexcept { + if (active_callback_ == nullptr || active_message_ == nullptr) { + std::terminate(); + } + active_callback_->OnFailure(*active_message_); + active_callback_ = nullptr; + active_message_.reset(); +} + +//-------------------------------------------------------------------------------------------------- +// Send +//-------------------------------------------------------------------------------------------------- +void InMemoryAsyncTransporter::Send(std::unique_ptr&& message, + Callback& callback) noexcept { + active_callback_ = &callback; + active_message_ = std::move(message); +} +} // namespace lightstep diff --git a/test/recorder/in_memory_async_transporter.h b/test/recorder/in_memory_async_transporter.h new file mode 100644 index 00000000..34fb48d8 --- /dev/null +++ b/test/recorder/in_memory_async_transporter.h @@ -0,0 +1,31 @@ +#pragma once + +#include + +#include "lightstep-tracer-common/collector.pb.h" +#include "lightstep/transporter.h" + +namespace lightstep { +class InMemoryAsyncTransporter final : public AsyncTransporter { + public: + const std::vector& reports() const noexcept { + return reports_; + } + + const std::vector& spans() const noexcept { return spans_; } + + void Succeed() noexcept; + + void Fail() noexcept; + + // AsyncTranspoter + void Send(std::unique_ptr&& message, + Callback& callback) noexcept override; + private: + std::vector reports_; + std::vector spans_; + + Callback* active_callback_{nullptr}; + std::unique_ptr active_message_; +}; +} // namespace lightstep diff --git a/test/recorder/legacy_in_memory_async_transporter.h b/test/recorder/legacy_in_memory_async_transporter.h index c49e5dd8..f1d6f26b 100644 --- a/test/recorder/legacy_in_memory_async_transporter.h +++ b/test/recorder/legacy_in_memory_async_transporter.h @@ -9,12 +9,8 @@ #include "lightstep/transporter.h" namespace lightstep { -class LegacyInMemoryAsyncTransporter : public LegacyAsyncTransporter { +class LegacyInMemoryAsyncTransporter final : public LegacyAsyncTransporter { public: - void Send(const google::protobuf::Message& request, - google::protobuf::Message& response, - LegacyAsyncTransporter::Callback& callback) override; - void Write(); void Fail(std::error_code error); @@ -27,6 +23,11 @@ class LegacyInMemoryAsyncTransporter : public LegacyAsyncTransporter { void set_should_disable(bool value) { should_disable_ = value; } + // LegacyAsyncTransporter + void Send(const google::protobuf::Message& request, + google::protobuf::Message& response, + LegacyAsyncTransporter::Callback& callback) override; + private: bool should_disable_ = false; const google::protobuf::Message* active_request_; diff --git a/test/recorder/manual_recorder_test.cpp b/test/recorder/manual_recorder_test.cpp index e544ee55..7df509fa 100644 --- a/test/recorder/manual_recorder_test.cpp +++ b/test/recorder/manual_recorder_test.cpp @@ -1,5 +1,7 @@ #include "recorder/manual_recorder.h" +#include "test/recorder/in_memory_async_transporter.h" + #include "3rd_party/catch2/catch.hpp" using namespace lightstep; From 63ee0ebfda08fef3806a2c70db59ed7c2238ffc7 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Mon, 27 Jan 2020 23:50:36 -0800 Subject: [PATCH 56/66] Fill out manual recorder test --- test/recorder/BUILD | 2 ++ test/recorder/legacy_manual_recorder_test.cpp | 1 - test/recorder/manual_recorder_test.cpp | 18 +++++++++++++++++- 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/test/recorder/BUILD b/test/recorder/BUILD index e47557a0..229e5409 100644 --- a/test/recorder/BUILD +++ b/test/recorder/BUILD @@ -16,6 +16,7 @@ lightstep_cc_library( "in_memory_recorder.cpp", ], deps = [ + "//src/common:buffer_chain_lib", "//src/recorder:recorder_interface", "//test:utility_lib", "//lightstep-tracer-common:collector_proto_cc", @@ -44,6 +45,7 @@ lightstep_cc_library( "in_memory_async_transporter.cpp", ], deps = [ + "//src/common:buffer_chain_lib", "//src/recorder:transporter_lib", ], ) diff --git a/test/recorder/legacy_manual_recorder_test.cpp b/test/recorder/legacy_manual_recorder_test.cpp index 6684257e..9595ec94 100644 --- a/test/recorder/legacy_manual_recorder_test.cpp +++ b/test/recorder/legacy_manual_recorder_test.cpp @@ -5,7 +5,6 @@ #include "3rd_party/catch2/catch.hpp" #include "lightstep/tracer.h" #include "test/recorder/legacy_in_memory_async_transporter.h" -#include "test/testing_condition_variable_wrapper.h" #include "test/utility.h" #include "tracer/counting_metrics_observer.h" #include "tracer/legacy/legacy_tracer_impl.h" diff --git a/test/recorder/manual_recorder_test.cpp b/test/recorder/manual_recorder_test.cpp index 7df509fa..6a4068a9 100644 --- a/test/recorder/manual_recorder_test.cpp +++ b/test/recorder/manual_recorder_test.cpp @@ -1,9 +1,25 @@ #include "recorder/manual_recorder.h" #include "test/recorder/in_memory_async_transporter.h" - +#include "tracer/counting_metrics_observer.h" +#include "tracer/tracer_impl.h" +#include "lightstep/tracer.h" #include "3rd_party/catch2/catch.hpp" using namespace lightstep; TEST_CASE("ManualRecorder") { + Logger logger{}; + auto metrics_observer = new CountingMetricsObserver{}; + LightStepTracerOptions options; + size_t max_buffered_spans{5}; + options.max_buffered_spans = + std::function{[&] { return max_buffered_spans; }}; + options.metrics_observer.reset(metrics_observer); + auto in_memory_transporter = new InMemoryAsyncTransporter{}; + auto recorder = new ManualRecorder{ + logger, std::move(options), + std::unique_ptr{in_memory_transporter}}; + auto tracer = std::shared_ptr{new TracerImpl{ + PropagationOptions{}, std::unique_ptr{recorder}}}; + REQUIRE(tracer); } From 1cd0be1fbc0bc0856d196d7dc48c409c9c1de0f3 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Tue, 28 Jan 2020 11:52:37 -0800 Subject: [PATCH 57/66] Add tests for ManualRecorder --- src/recorder/manual_recorder.cpp | 6 ++-- test/recorder/manual_recorder_test.cpp | 41 ++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 3 deletions(-) diff --git a/src/recorder/manual_recorder.cpp b/src/recorder/manual_recorder.cpp index f18ca356..046bc421 100644 --- a/src/recorder/manual_recorder.cpp +++ b/src/recorder/manual_recorder.cpp @@ -30,7 +30,7 @@ ManualRecorder::ManualRecorder(Logger& logger, LightStepTracerOptions options, transporter_{std::move(transporter)}, report_request_header_{new std::string{ WriteReportRequestHeader(tracer_options_, GenerateId())}}, - metrics_{GetMetricsObserver(options)}, + metrics_{GetMetricsObserver(tracer_options_)}, span_buffer_{tracer_options_.max_buffered_spans.value()} {} //-------------------------------------------------------------------------------------------------- @@ -128,7 +128,7 @@ void ManualRecorder::OnForkedChild() noexcept { void ManualRecorder::OnSuccess(BufferChain& message) noexcept { auto report_request = dynamic_cast(&message); assert(report_request != nullptr); - if (report_request != nullptr) { + if (report_request == nullptr) { // This should never happen return; } @@ -141,7 +141,7 @@ void ManualRecorder::OnSuccess(BufferChain& message) noexcept { void ManualRecorder::OnFailure(BufferChain& message) noexcept { auto report_request = dynamic_cast(&message); assert(report_request != nullptr); - if (report_request != nullptr) { + if (report_request == nullptr) { // This should never happen return; } diff --git a/test/recorder/manual_recorder_test.cpp b/test/recorder/manual_recorder_test.cpp index 6a4068a9..59b5f58b 100644 --- a/test/recorder/manual_recorder_test.cpp +++ b/test/recorder/manual_recorder_test.cpp @@ -3,6 +3,7 @@ #include "test/recorder/in_memory_async_transporter.h" #include "tracer/counting_metrics_observer.h" #include "tracer/tracer_impl.h" +#include "test/utility.h" #include "lightstep/tracer.h" #include "3rd_party/catch2/catch.hpp" using namespace lightstep; @@ -22,4 +23,44 @@ TEST_CASE("ManualRecorder") { auto tracer = std::shared_ptr{new TracerImpl{ PropagationOptions{}, std::unique_ptr{recorder}}}; REQUIRE(tracer); + + SECTION("Buffered spans get transported after Flush is manually called.") { + auto span = tracer->StartSpan("abc"); + CHECK(span); + span->Finish(); + CHECK(in_memory_transporter->reports().empty()); + CHECK(tracer->Flush()); + in_memory_transporter->Succeed(); + CHECK(in_memory_transporter->reports().size() == 1); + } + + SECTION( + "If the tranporter fails, it's spans are reported as dropped in the " + "following report.") { + logger.set_level(LogLevel::off); + auto span1 = tracer->StartSpan("abc"); + CHECK(span1); + span1->Finish(); + CHECK(tracer->Flush()); + in_memory_transporter->Fail(); + + auto span2 = tracer->StartSpan("xyz"); + CHECK(span2); + span2->Finish(); + CHECK(tracer->Flush()); + in_memory_transporter->Succeed(); + CHECK(LookupSpansDropped(in_memory_transporter->reports().at(0)) == 1); + } + +#if 0 + SECTION("Flush is called when the tracer's buffer is filled.") { + for (size_t i = 0; i < max_buffered_spans; ++i) { + auto span = tracer->StartSpan("abc"); + CHECK(span); + span->Finish(); + } + in_memory_transporter->Write(); + CHECK(in_memory_transporter->reports().size() == 1); + } +#endif } From f57feea92cb848c34dc13f03057f9588de6d9a34 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Tue, 28 Jan 2020 17:30:21 -0800 Subject: [PATCH 58/66] Fill out more of ManualRecorder --- include/lightstep/transporter.h | 2 + src/recorder/manual_recorder.cpp | 18 +++--- test/recorder/in_memory_async_transporter.cpp | 16 +++++ test/recorder/in_memory_async_transporter.h | 8 +++ test/recorder/manual_recorder_test.cpp | 62 +++++++++++++++++-- 5 files changed, 90 insertions(+), 16 deletions(-) diff --git a/include/lightstep/transporter.h b/include/lightstep/transporter.h index 67abc209..e4db887e 100644 --- a/include/lightstep/transporter.h +++ b/include/lightstep/transporter.h @@ -86,6 +86,8 @@ class AsyncTransporter : public Transporter { virtual void OnFailure(BufferChain& message) noexcept = 0; }; + virtual void OnSpanBufferFull() noexcept {} + virtual void Send(std::unique_ptr&& message, Callback& callback) noexcept = 0; }; diff --git a/src/recorder/manual_recorder.cpp b/src/recorder/manual_recorder.cpp index 046bc421..e3671a3c 100644 --- a/src/recorder/manual_recorder.cpp +++ b/src/recorder/manual_recorder.cpp @@ -65,18 +65,15 @@ void ManualRecorder::RecordSpan( // Advance past reserved header space we didn't use. span->Seek(0, static_cast(reserved_header_size - protobuf_header_size)); + auto was_added = span_buffer_.Add(span); + if (was_added) { + return; + } + transporter_->OnSpanBufferFull(); + + // Attempt to add the span again in case the transporter decided to flush if (!span_buffer_.Add(span)) { - // Note: the compiler doesn't want to inline this logger call and it shows - // up in profiling with high span droppage even if the logging isn't turned - // on. - // - // Hence, the additional checking to avoid a function call. - if (static_cast(logger_.level()) <= - static_cast(LogLevel::debug)) { - logger_.Debug("Dropping span"); - } metrics_.OnSpansDropped(1); - span.reset(); } } @@ -108,6 +105,7 @@ bool ManualRecorder::FlushWithTimeout( } transporter_->Send(std::unique_ptr{report_request.release()}, *this); + metrics_.OnFlush(); return true; } catch (const std::exception& e) { logger_.Error("Failed to flush report: ", e.what()); diff --git a/test/recorder/in_memory_async_transporter.cpp b/test/recorder/in_memory_async_transporter.cpp index 3975e300..94e48314 100644 --- a/test/recorder/in_memory_async_transporter.cpp +++ b/test/recorder/in_memory_async_transporter.cpp @@ -5,6 +5,13 @@ #include namespace lightstep { +//-------------------------------------------------------------------------------------------------- +// constructor +//-------------------------------------------------------------------------------------------------- +InMemoryAsyncTransporter::InMemoryAsyncTransporter( + const std::function& on_span_buffer_full) + : on_span_buffer_full_{on_span_buffer_full} {} + //-------------------------------------------------------------------------------------------------- // Succeed //-------------------------------------------------------------------------------------------------- @@ -39,6 +46,15 @@ void InMemoryAsyncTransporter::Fail() noexcept { active_message_.reset(); } +//-------------------------------------------------------------------------------------------------- +// OnSpanBufferFull +//-------------------------------------------------------------------------------------------------- +void InMemoryAsyncTransporter::OnSpanBufferFull() noexcept { + if (on_span_buffer_full_) { + on_span_buffer_full_(); + } +} + //-------------------------------------------------------------------------------------------------- // Send //-------------------------------------------------------------------------------------------------- diff --git a/test/recorder/in_memory_async_transporter.h b/test/recorder/in_memory_async_transporter.h index 34fb48d8..7941551b 100644 --- a/test/recorder/in_memory_async_transporter.h +++ b/test/recorder/in_memory_async_transporter.h @@ -1,6 +1,7 @@ #pragma once #include +#include #include "lightstep-tracer-common/collector.pb.h" #include "lightstep/transporter.h" @@ -8,6 +9,9 @@ namespace lightstep { class InMemoryAsyncTransporter final : public AsyncTransporter { public: + explicit InMemoryAsyncTransporter( + const std::function& on_span_buffer_full = {}); + const std::vector& reports() const noexcept { return reports_; } @@ -19,9 +23,13 @@ class InMemoryAsyncTransporter final : public AsyncTransporter { void Fail() noexcept; // AsyncTranspoter + void OnSpanBufferFull() noexcept override; + void Send(std::unique_ptr&& message, Callback& callback) noexcept override; private: + std::function on_span_buffer_full_; + std::vector reports_; std::vector spans_; diff --git a/test/recorder/manual_recorder_test.cpp b/test/recorder/manual_recorder_test.cpp index 59b5f58b..a4fdc358 100644 --- a/test/recorder/manual_recorder_test.cpp +++ b/test/recorder/manual_recorder_test.cpp @@ -16,11 +16,18 @@ TEST_CASE("ManualRecorder") { options.max_buffered_spans = std::function{[&] { return max_buffered_spans; }}; options.metrics_observer.reset(metrics_observer); - auto in_memory_transporter = new InMemoryAsyncTransporter{}; + std::shared_ptr tracer; + bool flush_on_full = true; + auto on_span_buffer_full = [&] { + if (flush_on_full) { + tracer->Flush(); + } + }; + auto in_memory_transporter = new InMemoryAsyncTransporter{on_span_buffer_full}; auto recorder = new ManualRecorder{ logger, std::move(options), std::unique_ptr{in_memory_transporter}}; - auto tracer = std::shared_ptr{new TracerImpl{ + tracer = std::shared_ptr{new TracerImpl{ PropagationOptions{}, std::unique_ptr{recorder}}}; REQUIRE(tracer); @@ -52,15 +59,58 @@ TEST_CASE("ManualRecorder") { CHECK(LookupSpansDropped(in_memory_transporter->reports().at(0)) == 1); } -#if 0 SECTION("Flush is called when the tracer's buffer is filled.") { - for (size_t i = 0; i < max_buffered_spans; ++i) { + for (size_t i = 0; i < max_buffered_spans + 1; ++i) { auto span = tracer->StartSpan("abc"); CHECK(span); span->Finish(); } - in_memory_transporter->Write(); + in_memory_transporter->Succeed(); CHECK(in_memory_transporter->reports().size() == 1); } -#endif + + SECTION("If we don't flush when the span buffer fills, we'll drop the span") { + flush_on_full = false; + for (size_t i = 0; i < max_buffered_spans + 1; ++i) { + auto span = tracer->StartSpan("abc"); + CHECK(span); + span->Finish(); + } + REQUIRE(metrics_observer->num_spans_dropped == 1); + } + + SECTION( + "MetricsObserver::OnFlush gets called whenever the recorder is " + "successfully flushed.") { + auto span = tracer->StartSpan("abc"); + span->Finish(); + tracer->Flush(); + in_memory_transporter->Succeed(); + CHECK(metrics_observer->num_flushes == 1); + } + + SECTION( + "MetricsObserver::OnSpansSent gets called with the number of spans " + "transported") { + auto span1 = tracer->StartSpan("abc"); + span1->Finish(); + auto span2 = tracer->StartSpan("abc"); + span2->Finish(); + tracer->Flush(); + in_memory_transporter->Succeed(); + CHECK(metrics_observer->num_spans_sent == 2); + } + + SECTION( + "MetricsObserver::OnSpansDropped gets called when spans are dropped.") { + logger.set_level(LogLevel::off); + auto span1 = tracer->StartSpan("abc"); + span1->Finish(); + auto span2 = tracer->StartSpan("abc"); + span2->Finish(); + tracer->Flush(); + in_memory_transporter->Fail(); + CHECK(metrics_observer->num_spans_sent == 0); + CHECK(metrics_observer->num_spans_dropped == 2); + } } From a755aa6c2933e4e93fbc1f154bc6157fb92de1ea Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Tue, 28 Jan 2020 17:44:25 -0800 Subject: [PATCH 59/66] Fix build error --- src/common/chained_stream.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/common/chained_stream.h b/src/common/chained_stream.h index 5c073e7e..3bde65d9 100644 --- a/src/common/chained_stream.h +++ b/src/common/chained_stream.h @@ -54,7 +54,6 @@ class ChainedStream final : public google::protobuf::io::ZeroCopyOutputStream, int num_blocks_{1}; int num_bytes_written_{0}; - int num_bytes_after_framing_{0}; int current_block_position_{0}; Block* current_block_; From bfa63d8c50d7ba2ea980e6505a8371137893a29a Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Tue, 28 Jan 2020 20:00:30 -0800 Subject: [PATCH 60/66] Add manual tracer benchmarking --- benchmark/BUILD | 9 ++ benchmark/manual_tracer_benchmark.cpp | 135 ++++++++++++++++++++++++++ 2 files changed, 144 insertions(+) create mode 100644 benchmark/manual_tracer_benchmark.cpp diff --git a/benchmark/BUILD b/benchmark/BUILD index 73ddd02a..fd16dae1 100644 --- a/benchmark/BUILD +++ b/benchmark/BUILD @@ -14,3 +14,12 @@ lightstep_google_benchmark( ], ) +lightstep_google_benchmark( + name = "manual_tracer_benchmark", + srcs = [ + "manual_tracer_benchmark.cpp", + ], + deps = [ + "//:tracer_lib", + ], +) diff --git a/benchmark/manual_tracer_benchmark.cpp b/benchmark/manual_tracer_benchmark.cpp new file mode 100644 index 00000000..51ef3e4a --- /dev/null +++ b/benchmark/manual_tracer_benchmark.cpp @@ -0,0 +1,135 @@ +#include +#include + +#include "lightstep/tracer.h" + +#include "benchmark/benchmark.h" + +int MaxBufferedSpans = 500; + +//-------------------------------------------------------------------------------------------------- +// LegacyNullTransport +//-------------------------------------------------------------------------------------------------- +namespace { +class LegacyNullTransporter final : public lightstep::LegacyAsyncTransporter { + public: + // LegacyAsyncTransporter + void Send(const google::protobuf::Message& request, + google::protobuf::Message& /*response*/, + Callback& callback) override { + using google::protobuf::uint8; + auto size = request.ByteSizeLong(); + std::unique_ptr buffer{new uint8[size]}; + request.SerializeWithCachedSizesToArray(buffer.get()); + callback.OnSuccess(); + } +}; +} // namespace + +//-------------------------------------------------------------------------------------------------- +// NullTransporter +//-------------------------------------------------------------------------------------------------- +namespace { +class NullTransporter final : public lightstep::AsyncTransporter { + public: + // AsyncTransporter + void Send(std::unique_ptr&& message, + Callback& callback) noexcept override { + auto size = message->num_bytes(); + std::unique_ptr buffer{new char[size]}; + message->CopyOut(buffer.get(), size); + callback.OnSuccess(*message); + } +}; +} // namespace + +//-------------------------------------------------------------------------------------------------- +// MakeLegacyManualTracer +//-------------------------------------------------------------------------------------------------- +static std::shared_ptr MakeLegacyManualTracer() { + lightstep::LightStepTracerOptions tracer_options; + tracer_options.transporter.reset(new LegacyNullTransporter{}); + tracer_options.use_thread = false; + tracer_options.component_name = "abc"; + tracer_options.access_token = "123"; + tracer_options.max_buffered_spans = static_cast(MaxBufferedSpans); + + return lightstep::MakeLightStepTracer(std::move(tracer_options)); +} + +//-------------------------------------------------------------------------------------------------- +// MakeManualTracer +//-------------------------------------------------------------------------------------------------- +static std::shared_ptr MakeManualTracer() { + lightstep::LightStepTracerOptions tracer_options; + tracer_options.transporter.reset(new NullTransporter{}); + tracer_options.use_thread = false; + tracer_options.component_name = "abc"; + tracer_options.access_token = "123"; + tracer_options.max_buffered_spans = static_cast(MaxBufferedSpans); + + return lightstep::MakeLightStepTracer(std::move(tracer_options)); +} + +//-------------------------------------------------------------------------------------------------- +// MakeTracer +//-------------------------------------------------------------------------------------------------- +static std::shared_ptr MakeTracer( + opentracing::string_view tracer_type = "legacy_manual") { + if (tracer_type == "legacy_manual") { + return MakeLegacyManualTracer(); + } + if (tracer_type == "manual") { + return MakeManualTracer(); + } + std::cerr << "Unknown tracer type: " << tracer_type << "\n"; + std::terminate(); +} + + +//-------------------------------------------------------------------------------------------------- +// BM_SmallSpanReport +//-------------------------------------------------------------------------------------------------- +static void BM_SmallSpanReport(benchmark::State& state, const char* tracer_type) { + auto tracer = MakeTracer(tracer_type); + assert(tracer != nullptr); + for (auto _ : state) { + for (int i=0; iStartSpan("abc123"); + } + tracer->Flush(); + } +} +BENCHMARK_CAPTURE(BM_SmallSpanReport, legacy_manual, "legacy_manual"); +BENCHMARK_CAPTURE(BM_SmallSpanReport, manual, "manual"); + +//-------------------------------------------------------------------------------------------------- +// BM_TaggedSpanReport +//-------------------------------------------------------------------------------------------------- +static void BM_TaggedSpanReport(benchmark::State& state, const char* tracer_type) { + auto tracer = MakeTracer(tracer_type); + assert(tracer != nullptr); + for (auto _ : state) { + for (int i = 0; i < MaxBufferedSpans; ++i) { + auto span = tracer->StartSpan("abc123"); + char key[5]; + key[0] = 'a'; + key[1] = 'b'; + key[2] = 'c'; + key[3] = '0'; + key[4] = '\0'; + for (int i = 0; i < 10; ++i) { + span->SetTag(opentracing::string_view{key, 4}, "123"); + ++key[3]; + } + } + tracer->Flush(); + } +} +BENCHMARK_CAPTURE(BM_TaggedSpanReport, legacy_manual, "legacy_manual"); +BENCHMARK_CAPTURE(BM_TaggedSpanReport, manual, "manual"); + +//-------------------------------------------------------------------------------------------------- +// BENCHMARK_MAIN +//-------------------------------------------------------------------------------------------------- +BENCHMARK_MAIN(); From 15ba487507cd9029919755e078052d27c311d2df Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Tue, 28 Jan 2020 20:18:55 -0800 Subject: [PATCH 61/66] Add logging benchmark --- benchmark/manual_tracer_benchmark.cpp | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/benchmark/manual_tracer_benchmark.cpp b/benchmark/manual_tracer_benchmark.cpp index 51ef3e4a..8322a2df 100644 --- a/benchmark/manual_tracer_benchmark.cpp +++ b/benchmark/manual_tracer_benchmark.cpp @@ -129,6 +129,26 @@ static void BM_TaggedSpanReport(benchmark::State& state, const char* tracer_type BENCHMARK_CAPTURE(BM_TaggedSpanReport, legacy_manual, "legacy_manual"); BENCHMARK_CAPTURE(BM_TaggedSpanReport, manual, "manual"); +//-------------------------------------------------------------------------------------------------- +// BM_LoggedSpanReport +//-------------------------------------------------------------------------------------------------- +static void BM_LoggedSpanReport(benchmark::State& state, + const char* tracer_type) { + auto tracer = MakeTracer(tracer_type); + assert(tracer != nullptr); + for (auto _ : state) { + for (int i = 0; i < MaxBufferedSpans; ++i) { + auto span = tracer->StartSpan("abc123"); + for (int i = 0; i < 10; ++i) { + span->Log({{"abc", 123}}); + } + } + tracer->Flush(); + } +} +BENCHMARK_CAPTURE(BM_LoggedSpanReport, legacy_manual, "legacy_manual"); +BENCHMARK_CAPTURE(BM_LoggedSpanReport, manual, "manual"); + //-------------------------------------------------------------------------------------------------- // BENCHMARK_MAIN //-------------------------------------------------------------------------------------------------- From 63eee75d0ab17112ae93e134bb83e70268dc3a5a Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Tue, 28 Jan 2020 22:33:02 -0800 Subject: [PATCH 62/66] Fix cmake build --- CMakeLists.txt | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index ae00e61b..7ff8dce9 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -195,6 +195,7 @@ include(LightStepTracerCommon) include(LightStepTracerConfiguration) set(LIGHTSTEP_SRCS src/common/utility.cpp + src/common/buffer_chain.cpp src/common/fragment_input_stream.cpp src/common/fragment_array_input_stream.cpp src/common/protobuf.cpp @@ -204,13 +205,19 @@ set(LIGHTSTEP_SRCS src/common/utility.cpp src/common/random.cpp src/common/random_traverser.cpp src/common/serialization.cpp - src/common/serialization_chain.cpp + src/common/chunked_http_framing.cpp + src/common/report_request_framing.cpp + src/common/composable_fragment_input_stream.cpp + src/common/chained_stream.cpp src/common/timestamp.cpp src/recorder/report_builder.cpp src/recorder/auto_recorder.cpp src/recorder/fork_aware_recorder.cpp src/recorder/legacy_manual_recorder.cpp + src/recorder/manual_recorder.cpp src/recorder/transporter.cpp + src/recorder/serialization/report_request.cpp + src/recorder/serialization/report_request_header.cpp src/tracer/binary_carrier.cpp src/tracer/json_options.cpp src/tracer/propagation/b3_propagator.cpp @@ -263,7 +270,7 @@ if (WITH_LIBEVENT) src/recorder/stream_recorder/satellite_connection.cpp src/recorder/stream_recorder/satellite_streamer.cpp src/recorder/stream_recorder/span_stream.cpp - src/recorder/stream_recorder/stream_recorder_metrics.cpp + src/recorder/metrics_tracker.cpp src/recorder/stream_recorder/connection_stream.cpp src/recorder/stream_recorder/host_header.cpp src/recorder/stream_recorder/embedded_metrics_message.cpp From bf213475179b48fe3a80b053a1c9436234263680 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Tue, 28 Jan 2020 22:55:21 -0800 Subject: [PATCH 63/66] Fix cmake build --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 7ff8dce9..962c0456 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -218,6 +218,7 @@ set(LIGHTSTEP_SRCS src/common/utility.cpp src/recorder/transporter.cpp src/recorder/serialization/report_request.cpp src/recorder/serialization/report_request_header.cpp + src/recorder/serialization/embedded_metrics_message.cpp src/tracer/binary_carrier.cpp src/tracer/json_options.cpp src/tracer/propagation/b3_propagator.cpp @@ -273,7 +274,6 @@ if (WITH_LIBEVENT) src/recorder/metrics_tracker.cpp src/recorder/stream_recorder/connection_stream.cpp src/recorder/stream_recorder/host_header.cpp - src/recorder/stream_recorder/embedded_metrics_message.cpp src/recorder/stream_recorder/status_line_parser.cpp src/recorder/stream_recorder/utility.cpp src/network/event.cpp From ebdb40f395033dcbcfaf9ec4b037313997d94180 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Wed, 29 Jan 2020 00:27:15 -0800 Subject: [PATCH 64/66] Fix clang-tidy errors --- src/common/report_request_framing.h | 4 ++-- src/common/serialization.h | 4 ++-- src/recorder/manual_recorder.cpp | 2 +- src/recorder/serialization/report_request.cpp | 4 ++-- src/recorder/serialization/report_request.h | 2 +- src/recorder/stream_recorder/stream_recorder.cpp | 2 +- test/recorder/in_memory_async_transporter.cpp | 4 ++-- test/recorder/in_memory_async_transporter.h | 2 +- 8 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/common/report_request_framing.h b/src/common/report_request_framing.h index 3a0d6e1f..65314ab3 100644 --- a/src/common/report_request_framing.h +++ b/src/common/report_request_framing.h @@ -5,9 +5,9 @@ #include "common/serialization.h" namespace lightstep { -static const size_t ReportRequestSpansField = 3; +constexpr size_t ReportRequestSpansField = 3; -const size_t ReportRequestSpansMaxHeaderSize = +constexpr size_t ReportRequestSpansMaxHeaderSize = StaticKeySerializationSize::value + google::protobuf::io::CodedOutputStream::StaticVarintSize32< diff --git a/src/common/serialization.h b/src/common/serialization.h index 818649ae..f2d068b7 100644 --- a/src/common/serialization.h +++ b/src/common/serialization.h @@ -20,7 +20,7 @@ template struct StaticSerializationKey { // See https://developers.google.com/protocol-buffers/docs/encoding#structure // for documentation on encoding. - static const uint32_t value = static_cast( + static constexpr uint32_t value = static_cast( (FieldNumber << 3) | static_cast(WireTypeValue)); }; @@ -29,7 +29,7 @@ struct StaticSerializationKey { */ template struct StaticKeySerializationSize { - static const size_t value = + static constexpr size_t value = google::protobuf::io::CodedOutputStream::StaticVarintSize32< StaticSerializationKey::value>::value; }; diff --git a/src/recorder/manual_recorder.cpp b/src/recorder/manual_recorder.cpp index e3671a3c..0c92cf1f 100644 --- a/src/recorder/manual_recorder.cpp +++ b/src/recorder/manual_recorder.cpp @@ -55,7 +55,7 @@ Fragment ManualRecorder::ReserveHeaderSpace(ChainedStream& stream) { void ManualRecorder::RecordSpan( Fragment header_fragment, std::unique_ptr&& span) noexcept { // Frame the Span - char* header_data = static_cast(header_fragment.first); + auto header_data = static_cast(header_fragment.first); auto reserved_header_size = static_cast(header_fragment.second); auto protobuf_body_size = span->ByteCount() - header_fragment.second; auto protobuf_header_size = WriteReportRequestSpansHeader( diff --git a/src/recorder/serialization/report_request.cpp b/src/recorder/serialization/report_request.cpp index a727b410..bfb9e70d 100644 --- a/src/recorder/serialization/report_request.cpp +++ b/src/recorder/serialization/report_request.cpp @@ -4,9 +4,9 @@ namespace lightstep { //-------------------------------------------------------------------------------------------------- // constructor //-------------------------------------------------------------------------------------------------- -ReportRequest::ReportRequest(const std::shared_ptr& header, +ReportRequest::ReportRequest(std::shared_ptr header, int num_dropped_spans) - : header_{header} { + : header_{std::move(header)} { num_bytes_ = static_cast(header_->size()); if (num_dropped_spans == 0) { return; diff --git a/src/recorder/serialization/report_request.h b/src/recorder/serialization/report_request.h index ef3c1670..31c22489 100644 --- a/src/recorder/serialization/report_request.h +++ b/src/recorder/serialization/report_request.h @@ -9,7 +9,7 @@ namespace lightstep { class ReportRequest final : public BufferChain { public: - ReportRequest(const std::shared_ptr& header, + ReportRequest(std::shared_ptr header, int num_dropped_spans); void AddSpan(std::unique_ptr&& span) noexcept; diff --git a/src/recorder/stream_recorder/stream_recorder.cpp b/src/recorder/stream_recorder/stream_recorder.cpp index 65e96646..0bdaee6a 100644 --- a/src/recorder/stream_recorder/stream_recorder.cpp +++ b/src/recorder/stream_recorder/stream_recorder.cpp @@ -77,7 +77,7 @@ void StreamRecorder::WriteFooter( void StreamRecorder::RecordSpan( Fragment header_fragment, std::unique_ptr&& span) noexcept { // Frame the Span - char* header_data = static_cast(header_fragment.first); + auto header_data = static_cast(header_fragment.first); auto reserved_header_size = static_cast(header_fragment.second); auto protobuf_body_size = span->ByteCount() - ChunkedHttpFooter.size() - header_fragment.second; diff --git a/test/recorder/in_memory_async_transporter.cpp b/test/recorder/in_memory_async_transporter.cpp index 94e48314..f44933af 100644 --- a/test/recorder/in_memory_async_transporter.cpp +++ b/test/recorder/in_memory_async_transporter.cpp @@ -9,8 +9,8 @@ namespace lightstep { // constructor //-------------------------------------------------------------------------------------------------- InMemoryAsyncTransporter::InMemoryAsyncTransporter( - const std::function& on_span_buffer_full) - : on_span_buffer_full_{on_span_buffer_full} {} + std::function on_span_buffer_full) + : on_span_buffer_full_{std::move(on_span_buffer_full)} {} //-------------------------------------------------------------------------------------------------- // Succeed diff --git a/test/recorder/in_memory_async_transporter.h b/test/recorder/in_memory_async_transporter.h index 7941551b..7b1283de 100644 --- a/test/recorder/in_memory_async_transporter.h +++ b/test/recorder/in_memory_async_transporter.h @@ -10,7 +10,7 @@ namespace lightstep { class InMemoryAsyncTransporter final : public AsyncTransporter { public: explicit InMemoryAsyncTransporter( - const std::function& on_span_buffer_full = {}); + std::function on_span_buffer_full = {}); const std::vector& reports() const noexcept { return reports_; From fd8c58afd16cff152082e6d0e7a2e1cb75c2aa4a Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Wed, 29 Jan 2020 20:39:48 -0800 Subject: [PATCH 65/66] Update documentation --- benchmark/manual_tracer_benchmark.cpp | 15 ++++++----- include/lightstep/buffer_chain.h | 21 +++++++++++++++ include/lightstep/transporter.h | 26 +++++++++++++++++++ src/common/chained_stream.h | 4 +++ src/common/chunked_http_framing.h | 8 ++++++ src/common/composable_fragment_input_stream.h | 26 +++++++++++++++++++ src/common/report_request_framing.h | 8 ++++++ src/recorder/manual_recorder.h | 3 +++ src/recorder/serialization/report_request.h | 13 ++++++++++ test/recorder/in_memory_async_transporter.cpp | 6 ++--- test/recorder/in_memory_async_transporter.h | 24 ++++++++++++++--- test/recorder/manual_recorder_test.cpp | 9 ++++--- 12 files changed, 145 insertions(+), 18 deletions(-) diff --git a/benchmark/manual_tracer_benchmark.cpp b/benchmark/manual_tracer_benchmark.cpp index 8322a2df..c3614000 100644 --- a/benchmark/manual_tracer_benchmark.cpp +++ b/benchmark/manual_tracer_benchmark.cpp @@ -15,9 +15,9 @@ class LegacyNullTransporter final : public lightstep::LegacyAsyncTransporter { public: // LegacyAsyncTransporter void Send(const google::protobuf::Message& request, - google::protobuf::Message& /*response*/, - Callback& callback) override { - using google::protobuf::uint8; + google::protobuf::Message& /*response*/, + Callback& callback) override { + using google::protobuf::uint8; auto size = request.ByteSizeLong(); std::unique_ptr buffer{new uint8[size]}; request.SerializeWithCachedSizesToArray(buffer.get()); @@ -86,15 +86,15 @@ static std::shared_ptr MakeTracer( std::terminate(); } - //-------------------------------------------------------------------------------------------------- // BM_SmallSpanReport //-------------------------------------------------------------------------------------------------- -static void BM_SmallSpanReport(benchmark::State& state, const char* tracer_type) { +static void BM_SmallSpanReport(benchmark::State& state, + const char* tracer_type) { auto tracer = MakeTracer(tracer_type); assert(tracer != nullptr); for (auto _ : state) { - for (int i=0; iStartSpan("abc123"); } tracer->Flush(); @@ -106,7 +106,8 @@ BENCHMARK_CAPTURE(BM_SmallSpanReport, manual, "manual"); //-------------------------------------------------------------------------------------------------- // BM_TaggedSpanReport //-------------------------------------------------------------------------------------------------- -static void BM_TaggedSpanReport(benchmark::State& state, const char* tracer_type) { +static void BM_TaggedSpanReport(benchmark::State& state, + const char* tracer_type) { auto tracer = MakeTracer(tracer_type); assert(tracer != nullptr); for (auto _ : state) { diff --git a/include/lightstep/buffer_chain.h b/include/lightstep/buffer_chain.h index ebc280bb..cff393be 100644 --- a/include/lightstep/buffer_chain.h +++ b/include/lightstep/buffer_chain.h @@ -3,19 +3,40 @@ #include namespace lightstep { +/** + * BufferChain provides an interface to access a chained sequestion of + * contiguous memory buffers. + */ class BufferChain { public: using FragmentCallback = bool (*)(void*, const void*, size_t); virtual ~BufferChain() = default; + /** + * @return the number of fragments in chain + */ virtual size_t num_fragments() const noexcept = 0; + /** + * @return the total number of bytes in all fragments + */ virtual size_t num_bytes() const noexcept = 0; + /** + * Iterate over each fragment in the buffer chain + * @param callback the callback to call for each fragment + * @param context a pointer to pass to the callback. + */ virtual bool ForEachFragment(FragmentCallback callback, void* context) const = 0; + /** + * Copy out all the fragments into an area of contiguous memory + * @param data the location to start copying + * @param length the size of the buffer destination. Length must be at least + * num_bytes. + */ void CopyOut(char* data, size_t length) const noexcept; }; } // namespace lightstep diff --git a/include/lightstep/transporter.h b/include/lightstep/transporter.h index e4db887e..97a859e8 100644 --- a/include/lightstep/transporter.h +++ b/include/lightstep/transporter.h @@ -38,6 +38,8 @@ class SyncTransporter : public Transporter { }; // LegacyAsyncTransporter customizes how asynchronous tracing reports are sent. +// +// Deprecated: Use AsyncTransporter class LegacyAsyncTransporter : public Transporter { public: // Callback interface used by Send. @@ -68,8 +70,14 @@ class LegacyAsyncTransporter : public Transporter { Callback& callback) = 0; }; +/** + * Provides a hook to customize how ReportRequests are transported. + */ class AsyncTransporter : public Transporter { public: + /** + * A Callback to be invoked after transporting a ReportRequest + */ class Callback { public: Callback() noexcept = default; @@ -81,13 +89,31 @@ class AsyncTransporter : public Transporter { Callback& operator=(const Callback&) noexcept = default; Callback& operator=(Callback&&) noexcept = default; + /** + * Call when a ReportRequest was successfully transported. + * @param message the ReportRequest + */ virtual void OnSuccess(BufferChain& message) noexcept = 0; + /** + * Call when a ReportRequest not successfully transported. + * @param message the ReportRequest + */ virtual void OnFailure(BufferChain& message) noexcept = 0; }; + /** + * Called when the tracer's span buffer is full. Implementors can either flush + * the pending spans early (so that we don't drop anything) or do nothing and + * let subsequent finished spans drop. + */ virtual void OnSpanBufferFull() noexcept {} + /** + * Send a ReportRequest + * @param message a BufferChain representing the ReportRequest's serialization + * @param callback a Callback to be called after message is transported + */ virtual void Send(std::unique_ptr&& message, Callback& callback) noexcept = 0; }; diff --git a/src/common/chained_stream.h b/src/common/chained_stream.h index 3bde65d9..d965ef1a 100644 --- a/src/common/chained_stream.h +++ b/src/common/chained_stream.h @@ -23,6 +23,10 @@ class ChainedStream final : public google::protobuf::io::ZeroCopyOutputStream, ChainedStream() noexcept; + /** + * Close the stream for output. After calling this, we can no longer write to + * the stream, but we can interact with it as a FragmentInputStream. + */ void CloseOutput() noexcept; // ZeroCopyOutputStream diff --git a/src/common/chunked_http_framing.h b/src/common/chunked_http_framing.h index cb3aec67..24615d19 100644 --- a/src/common/chunked_http_framing.h +++ b/src/common/chunked_http_framing.h @@ -9,6 +9,14 @@ const size_t ChunkedHttpMaxHeaderSize = Num32BitHexDigits + 2; const opentracing::string_view ChunkedHttpFooter = "\r\n"; +/** + * Write the header part of framing for an http/1.1 chunk. + * @param data start of the buffer to write to. + * @param size the size of the buffer + * @param chunk_size the size of the chunk. + * + * Note: Data is written from the end of the provided buffer. + */ size_t WriteHttpChunkHeader(char* data, size_t size, uint32_t chunk_size) noexcept; } // namespace lightstep diff --git a/src/common/composable_fragment_input_stream.h b/src/common/composable_fragment_input_stream.h index 8c74b50f..99b7a10c 100644 --- a/src/common/composable_fragment_input_stream.h +++ b/src/common/composable_fragment_input_stream.h @@ -5,16 +5,42 @@ #include "common/fragment_input_stream.h" namespace lightstep { +/** + * Maintains an intrusive linked list of ComposableFragmentInputStreams. + */ class ComposableFragmentInputStream : public FragmentInputStream { public: + /** + * Append a ComposableFragmentInputStream. + * @param stream the ComposableFragmentInputStream to append + */ void Append(std::unique_ptr&& stream) noexcept; + /** + * The number of fragments in this node of the linked + * ComposableFragmentInputStreams. + */ virtual int segment_num_fragments() const noexcept = 0; + /** + * Iterate over the fragments in this node of the linked + * ComposableFragmentInputStreams. + * @param callback the callback to call for each fragment + */ virtual bool SegmentForEachFragment(Callback callback) const noexcept = 0; + /** + * Clear this node of the linked ComposableFragmentInputStreams. + */ virtual void SegmentClear() noexcept = 0; + /** + * Seek forward in this node of the linked ComposableFragmentInputStreams. + * @param fragment_index the new fragment index to reposition to relative to + * the current fragment index. + * @param position the position within fragment_index to reposition to + * relative to the current position. + */ virtual void SegmentSeek(int fragment_index, int position) noexcept = 0; // FragmentInputStream diff --git a/src/common/report_request_framing.h b/src/common/report_request_framing.h index 65314ab3..7ae05951 100644 --- a/src/common/report_request_framing.h +++ b/src/common/report_request_framing.h @@ -13,6 +13,14 @@ constexpr size_t ReportRequestSpansMaxHeaderSize = google::protobuf::io::CodedOutputStream::StaticVarintSize32< std::numeric_limits::max()>::value; +/** + * Write framing for a span in a collector::ReportRequest message. + * @param data start of the buffer to write to. + * @param size the size of the buffer + * @param chunk_size the size of the chunk. + * + * Note: Data is written from the end of the provided buffer. + */ size_t WriteReportRequestSpansHeader(char* data, size_t size, uint32_t body_size) noexcept; } // namespace lightstep diff --git a/src/recorder/manual_recorder.h b/src/recorder/manual_recorder.h index 6dcee11f..9ec1a408 100644 --- a/src/recorder/manual_recorder.h +++ b/src/recorder/manual_recorder.h @@ -10,6 +10,9 @@ #include "recorder/metrics_tracker.h" namespace lightstep { +/** + * Implements a Recorder with no background thread and custom Transporter. + */ class ManualRecorder : public ForkAwareRecorder, public AsyncTransporter::Callback, private Noncopyable { diff --git a/src/recorder/serialization/report_request.h b/src/recorder/serialization/report_request.h index 31c22489..2bfc5ddb 100644 --- a/src/recorder/serialization/report_request.h +++ b/src/recorder/serialization/report_request.h @@ -7,15 +7,28 @@ #include "recorder/serialization/embedded_metrics_message.h" namespace lightstep { +/** + * Maintains a BufferChain for a collector::ReportRequest serialization. + */ class ReportRequest final : public BufferChain { public: ReportRequest(std::shared_ptr header, int num_dropped_spans); + /** + * Add a serialized Span to the ReportRequest. + * @param span the serialized span to add. + */ void AddSpan(std::unique_ptr&& span) noexcept; + /** + * @return the number of dropped spans recorded in the ReportRequest + */ int num_dropped_spans() const noexcept; + /** + * @return the number of spans in the ReportRequest + */ int num_spans() const noexcept { return num_spans_; } // BufferChain diff --git a/test/recorder/in_memory_async_transporter.cpp b/test/recorder/in_memory_async_transporter.cpp index f44933af..6311ed14 100644 --- a/test/recorder/in_memory_async_transporter.cpp +++ b/test/recorder/in_memory_async_transporter.cpp @@ -1,8 +1,8 @@ #include "test/recorder/in_memory_async_transporter.h" -#include #include #include +#include namespace lightstep { //-------------------------------------------------------------------------------------------------- @@ -22,7 +22,7 @@ void InMemoryAsyncTransporter::Succeed() noexcept { std::string s(active_message_->num_bytes(), ' '); active_message_->CopyOut(&s[0], s.size()); collector::ReportRequest report; - if(!report.ParseFromString(s)) { + if (!report.ParseFromString(s)) { std::terminate(); } reports_.emplace_back(report); @@ -63,4 +63,4 @@ void InMemoryAsyncTransporter::Send(std::unique_ptr&& message, active_callback_ = &callback; active_message_ = std::move(message); } -} // namespace lightstep +} // namespace lightstep diff --git a/test/recorder/in_memory_async_transporter.h b/test/recorder/in_memory_async_transporter.h index 7b1283de..7c04f2aa 100644 --- a/test/recorder/in_memory_async_transporter.h +++ b/test/recorder/in_memory_async_transporter.h @@ -1,32 +1,48 @@ #pragma once -#include #include +#include #include "lightstep-tracer-common/collector.pb.h" #include "lightstep/transporter.h" namespace lightstep { +/** + * An AsyncTransporter that "transports" spans in to a container. + */ class InMemoryAsyncTransporter final : public AsyncTransporter { public: explicit InMemoryAsyncTransporter( std::function on_span_buffer_full = {}); + /** + * @return the ReportRequests that were transported. + */ const std::vector& reports() const noexcept { return reports_; } + /** + * All the spans that were transported. + */ const std::vector& spans() const noexcept { return spans_; } + /** + * Make the last Send succeed. + */ void Succeed() noexcept; + /** + * Make the last Send fail. + */ void Fail() noexcept; - // AsyncTranspoter + // AsyncTranspoter void OnSpanBufferFull() noexcept override; void Send(std::unique_ptr&& message, - Callback& callback) noexcept override; + Callback& callback) noexcept override; + private: std::function on_span_buffer_full_; @@ -36,4 +52,4 @@ class InMemoryAsyncTransporter final : public AsyncTransporter { Callback* active_callback_{nullptr}; std::unique_ptr active_message_; }; -} // namespace lightstep +} // namespace lightstep diff --git a/test/recorder/manual_recorder_test.cpp b/test/recorder/manual_recorder_test.cpp index a4fdc358..acf43660 100644 --- a/test/recorder/manual_recorder_test.cpp +++ b/test/recorder/manual_recorder_test.cpp @@ -1,11 +1,11 @@ #include "recorder/manual_recorder.h" +#include "3rd_party/catch2/catch.hpp" +#include "lightstep/tracer.h" #include "test/recorder/in_memory_async_transporter.h" +#include "test/utility.h" #include "tracer/counting_metrics_observer.h" #include "tracer/tracer_impl.h" -#include "test/utility.h" -#include "lightstep/tracer.h" -#include "3rd_party/catch2/catch.hpp" using namespace lightstep; TEST_CASE("ManualRecorder") { @@ -23,7 +23,8 @@ TEST_CASE("ManualRecorder") { tracer->Flush(); } }; - auto in_memory_transporter = new InMemoryAsyncTransporter{on_span_buffer_full}; + auto in_memory_transporter = + new InMemoryAsyncTransporter{on_span_buffer_full}; auto recorder = new ManualRecorder{ logger, std::move(options), std::unique_ptr{in_memory_transporter}}; From 6acc881a86f980f69e3498695e3cb3203b9e7337 Mon Sep 17 00:00:00 2001 From: Ryan Burn Date: Mon, 3 Feb 2020 14:05:23 -0800 Subject: [PATCH 66/66] Fix typos --- include/lightstep/buffer_chain.h | 2 +- src/common/chained_stream.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/lightstep/buffer_chain.h b/include/lightstep/buffer_chain.h index cff393be..4d12530e 100644 --- a/include/lightstep/buffer_chain.h +++ b/include/lightstep/buffer_chain.h @@ -4,7 +4,7 @@ namespace lightstep { /** - * BufferChain provides an interface to access a chained sequestion of + * BufferChain provides an interface to access a chained sequence of * contiguous memory buffers. */ class BufferChain { diff --git a/src/common/chained_stream.h b/src/common/chained_stream.h index d965ef1a..5e921c0d 100644 --- a/src/common/chained_stream.h +++ b/src/common/chained_stream.h @@ -13,7 +13,7 @@ namespace lightstep { /** - * Maintains a linked chain of blocks as they aree written + * Maintains a linked chain of blocks as they are written */ class ChainedStream final : public google::protobuf::io::ZeroCopyOutputStream, public ComposableFragmentInputStream,