From de96939ad305f7592fc2384f6bda082c6f618b95 Mon Sep 17 00:00:00 2001 From: Caleb Gilmour Date: Wed, 29 Aug 2018 18:35:16 +0000 Subject: [PATCH 1/8] Move files to retain history --- src/{writer.cpp => agent_writer.cpp} | 0 src/{writer.h => agent_writer.h} | 0 src/{opentracing.cpp => opentracing_agent.cpp} | 0 3 files changed, 0 insertions(+), 0 deletions(-) rename src/{writer.cpp => agent_writer.cpp} (100%) rename src/{writer.h => agent_writer.h} (100%) rename src/{opentracing.cpp => opentracing_agent.cpp} (100%) diff --git a/src/writer.cpp b/src/agent_writer.cpp similarity index 100% rename from src/writer.cpp rename to src/agent_writer.cpp diff --git a/src/writer.h b/src/agent_writer.h similarity index 100% rename from src/writer.h rename to src/agent_writer.h diff --git a/src/opentracing.cpp b/src/opentracing_agent.cpp similarity index 100% rename from src/opentracing.cpp rename to src/opentracing_agent.cpp From 0c1788b84853fdcb9bd72fdbf027d6abb4c43aca Mon Sep 17 00:00:00 2001 From: Caleb Gilmour Date: Sat, 28 Jul 2018 14:27:03 +0000 Subject: [PATCH 2/8] Refactor to support external publishing of traces. --- include/datadog/opentracing.h | 20 +++++++++++++ src/agent_writer.cpp | 53 +++++++++++++++++------------------ src/agent_writer.h | 35 +++++++++++++++-------- src/opentracing_agent.cpp | 5 ++++ src/publisher.cpp | 39 ++++++++++++++++++++++++++ src/publisher.h | 39 ++++++++++++++++++++++++++ src/tracer.cpp | 7 +++++ src/tracer.h | 5 +++- test/opentracing_test.cpp | 7 +++++ test/writer_test.cpp | 38 +++++++++---------------- 10 files changed, 184 insertions(+), 64 deletions(-) create mode 100644 src/publisher.cpp create mode 100644 src/publisher.h diff --git a/include/datadog/opentracing.h b/include/datadog/opentracing.h index cf008e02..7461c8c0 100644 --- a/include/datadog/opentracing.h +++ b/include/datadog/opentracing.h @@ -3,6 +3,9 @@ #include +#include +#include + namespace ot = opentracing; namespace datadog { @@ -32,7 +35,24 @@ struct TracerOptions { std::string operation_name_override = ""; }; +// TracePublisher exposes the data required to publish traces to the +// Datadog Agent. +class TracePublisher { + public: + TracePublisher() {} + virtual ~TracePublisher() {} + + // Returns the Datadog Agent endpoint that traces should be published to. + virtual const std::string path() = 0; + virtual std::size_t pendingTraces() = 0; + virtual void clearTraces() = 0; + virtual const std::map headers() = 0; + virtual const std::string payload() = 0; +}; + std::shared_ptr makeTracer(const TracerOptions &options); +std::shared_ptr makeTracer(const TracerOptions &options, + std::shared_ptr &publisher); } // namespace opentracing } // namespace datadog diff --git a/src/agent_writer.cpp b/src/agent_writer.cpp index 8e86143e..08f5d296 100644 --- a/src/agent_writer.cpp +++ b/src/agent_writer.cpp @@ -1,12 +1,14 @@ #include "writer.h" #include +#include "publisher.h" #include "version_number.h" namespace datadog { namespace opentracing { +Writer::Writer() : trace_publisher_(std::make_shared()) {} + namespace { -const std::string agent_api_path = "/v0.3/traces"; const std::string agent_protocol = "http://"; const size_t max_queued_traces = 7000; // Retry sending traces to agent a couple of times. Any more than that and the agent won't accept @@ -19,15 +21,14 @@ const long default_timeout_ms = 2000L; } // namespace AgentWriter::AgentWriter(std::string host, uint32_t port, std::chrono::milliseconds write_period) - : AgentWriter(std::unique_ptr{new CurlHandle{}}, config::tracer_version, write_period, - max_queued_traces, default_retry_periods, host, port){}; + : AgentWriter(std::unique_ptr{new CurlHandle{}}, write_period, max_queued_traces, + default_retry_periods, host, port){}; -AgentWriter::AgentWriter(std::unique_ptr handle, std::string tracer_version, - std::chrono::milliseconds write_period, size_t max_queued_traces, +AgentWriter::AgentWriter(std::unique_ptr handle, std::chrono::milliseconds write_period, + size_t max_queued_traces, std::vector retry_periods, std::string host, uint32_t port) - : tracer_version_(tracer_version), - write_period_(write_period), + : write_period_(write_period), max_queued_traces_(max_queued_traces), retry_periods_(retry_periods) { setUpHandle(handle, host, port); @@ -38,7 +39,7 @@ void AgentWriter::setUpHandle(std::unique_ptr &handle, std::string host, // Some options are the same for all actions, set them here. // Set the agent URI. std::stringstream agent_uri; - agent_uri << agent_protocol << host << ":" << std::to_string(port) << agent_api_path; + agent_uri << agent_protocol << host << ":" << std::to_string(port) << trace_publisher_->path(); auto rcode = handle->setopt(CURLOPT_URL, agent_uri.str().c_str()); if (rcode != CURLE_OK) { throw std::runtime_error(std::string("Unable to set agent URL: ") + curl_easy_strerror(rcode)); @@ -48,10 +49,6 @@ void AgentWriter::setUpHandle(std::unique_ptr &handle, std::string host, throw std::runtime_error(std::string("Unable to set agent timeout: ") + curl_easy_strerror(rcode)); } - // Set the common HTTP headers. - handle->setHeaders({{"Content-Type", "application/msgpack"}, - {"Datadog-Meta-Lang", "cpp"}, - {"Datadog-Meta-Tracer-Version", tracer_version_}}); } // namespace opentracing AgentWriter::~AgentWriter() { stop(); } @@ -73,10 +70,10 @@ void AgentWriter::write(Trace trace) { if (stop_writing_) { return; } - if (traces_.size() >= max_queued_traces_) { + if (trace_publisher_->pendingTraces() >= max_queued_traces_) { return; } - traces_.push_back(std::move(trace)); + trace_publisher_->addTrace(std::move(trace)); }; void AgentWriter::startWriting(std::unique_ptr handle) { @@ -84,8 +81,9 @@ void AgentWriter::startWriting(std::unique_ptr handle) { // We can capture 'this' because destruction of this stops the thread and the lambda. worker_ = std::make_unique( [this](std::unique_ptr handle) { - std::stringstream buffer; size_t num_traces = 0; + std::map headers; + std::string payload; while (true) { // Encode traces when there are new ones. { @@ -96,18 +94,16 @@ void AgentWriter::startWriting(std::unique_ptr handle) { if (stop_writing_) { return; // Stop the thread. } - num_traces = traces_.size(); + num_traces = trace_publisher_->pendingTraces(); if (num_traces == 0) { continue; } - // Clear the buffer but keep the allocated memory. - buffer.clear(); - buffer.str(std::string{}); - msgpack::pack(buffer, traces_); - traces_.clear(); + headers = trace_publisher_->headers(); + payload = trace_publisher_->payload(); + trace_publisher_->clearTraces(); } // lock on mutex_ ends. // Send spans, not in critical period. - retryFiniteOnFail([&]() { return AgentWriter::postTraces(handle, buffer, num_traces); }); + retryFiniteOnFail([&]() { return AgentWriter::postTraces(handle, headers, payload); }); // Let thread calling 'flush' that we're done flushing. { std::unique_lock lock(mutex_); @@ -146,19 +142,18 @@ void AgentWriter::retryFiniteOnFail(std::function f) const { f(); // Final try after final sleep. } -bool AgentWriter::postTraces(std::unique_ptr &handle, std::stringstream &buffer, - size_t num_traces) try { - handle->setHeaders({{"X-Datadog-Trace-Count", std::to_string(num_traces)}}); +bool AgentWriter::postTraces(std::unique_ptr &handle, + std::map headers, std::string payload) try { + handle->setHeaders(headers); // We have to set the size manually, because msgpack uses null characters. - std::string post_fields = buffer.str(); - CURLcode rcode = handle->setopt(CURLOPT_POSTFIELDSIZE, post_fields.size()); + CURLcode rcode = handle->setopt(CURLOPT_POSTFIELDSIZE, payload.size()); if (rcode != CURLE_OK) { std::cerr << "Error setting agent request size: " << curl_easy_strerror(rcode) << std::endl; return false; } - rcode = handle->setopt(CURLOPT_POSTFIELDS, post_fields.data()); + rcode = handle->setopt(CURLOPT_POSTFIELDS, payload.data()); if (rcode != CURLE_OK) { std::cerr << "Error setting agent request body: " << curl_easy_strerror(rcode) << std::endl; return false; @@ -176,5 +171,7 @@ bool AgentWriter::postTraces(std::unique_ptr &handle, std::stringstream return true; // Don't attempt to retry. } +void ExternalWriter::write(Trace trace) { trace_publisher_->addTrace(std::move(trace)); } + } // namespace opentracing } // namespace datadog diff --git a/src/agent_writer.h b/src/agent_writer.h index f793f1fb..0bc11820 100644 --- a/src/agent_writer.h +++ b/src/agent_writer.h @@ -7,6 +7,7 @@ #include #include #include +#include "publisher.h" #include "span.h" #include "transport.h" @@ -16,15 +17,18 @@ namespace opentracing { class SpanData; using Trace = std::unique_ptr>>; -// Takes Traces and writes them (eg. sends them to Datadog). +// A Writer is used to submit completed traces to the Datadog agent. class Writer { public: - Writer() {} + Writer(); virtual ~Writer() {} // Writes the given Trace. virtual void write(Trace trace) = 0; + + protected: + std::shared_ptr trace_publisher_; }; // A Writer that sends Traces (collections of Spans) to a Datadog agent. @@ -34,10 +38,9 @@ class AgentWriter : public Writer { // runtime_exception. AgentWriter(std::string host, uint32_t port, std::chrono::milliseconds write_period); - AgentWriter(std::unique_ptr handle, std::string tracer_version, - std::chrono::milliseconds write_period, size_t max_queued_traces, - std::vector retry_periods, std::string host, - uint32_t port); + AgentWriter(std::unique_ptr handle, std::chrono::milliseconds write_period, + size_t max_queued_traces, std::vector retry_periods, + std::string host, uint32_t port); // Does not flush on destruction, buffered traces may be lost. Stops all threads. ~AgentWriter() override; @@ -59,13 +62,12 @@ class AgentWriter : public Writer { // or when flush() is called manually. void startWriting(std::unique_ptr handle); // Posts the given Traces to the Agent. Returns true if it succeeds, otherwise false. - static bool postTraces(std::unique_ptr &handle, std::stringstream &buffer, - size_t num_traces); + static bool postTraces(std::unique_ptr &handle, + std::map headers, std::string payload); // Retries the given function a finite number of times according to retry_periods_. Retries when // f() returns false. void retryFiniteOnFail(std::function f) const; - const std::string tracer_version_; // How often to send Traces. const std::chrono::milliseconds write_period_; const size_t max_queued_traces_; @@ -86,8 +88,19 @@ class AgentWriter : public Writer { bool stop_writing_ = false; // If set to true, flushes worker (which sets it false again). Locked by mutex_; bool flush_worker_ = false; - // Multiple producer (potentially), single consumer. Locked by mutex_. - std::deque traces_; +}; + +// A writer that collects trace data but uses an external mechanism to transmit the data +// to the Datadog Agent. +class ExternalWriter : public Writer { + public: + ExternalWriter() {} + ~ExternalWriter() {} + + // Implements Writer methods. + void write(Trace trace) override; + + std::shared_ptr publisher() { return trace_publisher_; } }; } // namespace opentracing diff --git a/src/opentracing_agent.cpp b/src/opentracing_agent.cpp index aaf560fa..2ad9d0ff 100644 --- a/src/opentracing_agent.cpp +++ b/src/opentracing_agent.cpp @@ -10,5 +10,10 @@ std::shared_ptr makeTracer(const TracerOptions &options) { return std::shared_ptr{new Tracer{options}}; } +std::shared_ptr makeTracer(const TracerOptions &options, + std::shared_ptr &publisher) { + return std::shared_ptr{new Tracer{options, publisher}}; +} + } // namespace opentracing } // namespace datadog diff --git a/src/publisher.cpp b/src/publisher.cpp new file mode 100644 index 00000000..b049096a --- /dev/null +++ b/src/publisher.cpp @@ -0,0 +1,39 @@ +#include "publisher.h" +#include "span.h" +#include "version_number.h" + +namespace datadog { +namespace opentracing { + +AgentHttpPublisher::AgentHttpPublisher() { + // Set up common headers and default encoder + common_headers_ = {{"Content-Type", "application/msgpack"}, + {"Datadog-Meta-Lang", "cpp"}, + {"Datadog-Meta-Tracer-Version", config::tracer_version}}; +} + +const std::string agent_api_path = "/v0.3/traces"; + +const std::string AgentHttpPublisher::path() { return agent_api_path; } + +void AgentHttpPublisher::clearTraces() { traces_.clear(); } + +std::size_t AgentHttpPublisher::pendingTraces() { return traces_.size(); } + +const std::map AgentHttpPublisher::headers() { + std::map headers(common_headers_); + headers["X-Datadog-Trace-Count"] = std::to_string(traces_.size()); + return headers; +} + +const std::string AgentHttpPublisher::payload() { + buffer_.clear(); + buffer_.str(std::string{}); + msgpack::pack(buffer_, traces_); + return buffer_.str(); +} + +void AgentHttpPublisher::addTrace(Trace trace) { traces_.push_back(std::move(trace)); } + +} // namespace opentracing +} // namespace datadog diff --git a/src/publisher.h b/src/publisher.h new file mode 100644 index 00000000..fa4665df --- /dev/null +++ b/src/publisher.h @@ -0,0 +1,39 @@ +#ifndef DD_OPENTRACING_PUBLISHER_H +#define DD_OPENTRACING_PUBLISHER_H + +#include + +#include + +namespace datadog { +namespace opentracing { + +class SpanData; +using Trace = std::unique_ptr>>; + +class AgentHttpPublisher : public TracePublisher { + public: + AgentHttpPublisher(); + ~AgentHttpPublisher() override {} + + // Returns the path that is used to submit HTTP requests to the agent. + const std::string path() override; + std::size_t pendingTraces() override; + void clearTraces() override; + // Returns the HTTP headers that are required for the collection of traces. + const std::map headers() override; + // Returns the encoded payload from the collection of traces. + const std::string payload() override; + void addTrace(Trace trace); + + private: + // Holds the headers that are used for all HTTP requests. + std::map common_headers_; + std::deque traces_; + std::stringstream buffer_; +}; + +} // namespace opentracing +} // namespace datadog + +#endif // DD_OPENTRACING_PUBLISHER_H diff --git a/src/tracer.cpp b/src/tracer.cpp index f7c229b5..3379998b 100644 --- a/src/tracer.cpp +++ b/src/tracer.cpp @@ -31,6 +31,13 @@ Tracer::Tracer(TracerOptions options, std::shared_ptr buffer, TimePr get_id_(get_id), sampler_(sampler) {} +Tracer::Tracer(TracerOptions options, std::shared_ptr &publisher) + : opts_(options), get_time_(getRealTime), get_id_(getId), sampler_(KeepAllSampler()) { + auto writer = std::make_shared(); + publisher = writer->publisher(); + buffer_ = std::shared_ptr{new WritingSpanBuffer{writer}}; +} + std::unique_ptr Tracer::StartSpanWithOptions(ot::string_view operation_name, const ot::StartSpanOptions &options) const noexcept try { diff --git a/src/tracer.h b/src/tracer.h index 9184824b..2348cc68 100644 --- a/src/tracer.h +++ b/src/tracer.h @@ -3,10 +3,10 @@ #include #include "clock.h" +#include "publisher.h" #include "sample.h" #include "span.h" #include "span_buffer.h" -#include "writer.h" #include #include @@ -34,6 +34,9 @@ class Tracer : public ot::Tracer, public std::enable_shared_from_this { Tracer(TracerOptions options, std::shared_ptr buffer, TimeProvider get_time, IdProvider get_id, SampleProvider sample); + // Creates a Tracer that exposes data for an external HTTP client to publish + Tracer(TracerOptions options, std::shared_ptr &publisher); + Tracer() = delete; // Starts a new span. diff --git a/test/opentracing_test.cpp b/test/opentracing_test.cpp index 06071122..66e2b303 100644 --- a/test/opentracing_test.cpp +++ b/test/opentracing_test.cpp @@ -1,4 +1,5 @@ #include +#include "mocks.h" #define CATCH_CONFIG_MAIN #include @@ -9,4 +10,10 @@ TEST_CASE("tracer") { auto tracer = makeTracer(TracerOptions{}); REQUIRE(tracer); } + SECTION("can be created with external Writer implementation") { + std::shared_ptr publisher; + auto tracer = makeTracer(TracerOptions{}, publisher); + REQUIRE(tracer); + REQUIRE(publisher); + } } diff --git a/test/writer_test.cpp b/test/writer_test.cpp index 82519c0f..0ff5fdf7 100644 --- a/test/writer_test.cpp +++ b/test/writer_test.cpp @@ -1,6 +1,7 @@ #include "../src/writer.h" #include "../src/writer.cpp" // Otherwise the compiler won't generate AgentWriter for us. #include "mocks.h" +#include "version_number.h" #include @@ -26,7 +27,6 @@ TEST_CASE("writer") { size_t max_queued_traces = 25; std::vector disable_retry; AgentWriter writer{std::move(handle_ptr), - "v0.1.0", only_send_traces_when_we_flush, max_queued_traces, disable_retry, @@ -37,10 +37,6 @@ TEST_CASE("writer") { REQUIRE(handle->options == std::unordered_map{ {CURLOPT_URL, "http://hostname:6319/v0.3/traces"}, {CURLOPT_TIMEOUT_MS, "2000"}}); - REQUIRE(handle->headers == - std::map{{"Content-Type", "application/msgpack"}, - {"Datadog-Meta-Lang", "cpp"}, - {"Datadog-Meta-Tracer-Version", "v0.1.0"}}); } SECTION("traces can be sent") { @@ -69,11 +65,11 @@ TEST_CASE("writer") { {CURLOPT_URL, "http://hostname:6319/v0.3/traces"}, {CURLOPT_TIMEOUT_MS, "2000"}, {CURLOPT_POSTFIELDSIZE, "126"}}); - REQUIRE(handle->headers == - std::map{{"Content-Type", "application/msgpack"}, - {"Datadog-Meta-Lang", "cpp"}, - {"Datadog-Meta-Tracer-Version", "v0.1.0"}, - {"X-Datadog-Trace-Count", "1"}}); + REQUIRE(handle->headers == std::map{ + {"Content-Type", "application/msgpack"}, + {"Datadog-Meta-Lang", "cpp"}, + {"Datadog-Meta-Tracer-Version", config::tracer_version}, + {"X-Datadog-Trace-Count", "1"}}); } SECTION("queue does not grow indefinitely") { @@ -89,7 +85,7 @@ TEST_CASE("writer") { SECTION("bad handle causes constructor to fail") { std::unique_ptr handle_ptr{new MockHandle{}}; handle_ptr->rcode = CURLE_OPERATION_TIMEDOUT; - REQUIRE_THROWS(AgentWriter{std::move(handle_ptr), "v0.1.0", only_send_traces_when_we_flush, + REQUIRE_THROWS(AgentWriter{std::move(handle_ptr), only_send_traces_when_we_flush, max_queued_traces, disable_retry, "hostname", 6319}); } @@ -184,13 +180,8 @@ TEST_CASE("writer") { std::unique_ptr handle_ptr{new MockHandle{}}; MockHandle* handle = handle_ptr.get(); auto write_interval = std::chrono::seconds(2); - AgentWriter writer{std::move(handle_ptr), - "v0.1.0", - write_interval, - max_queued_traces, - disable_retry, - "hostname", - 6319}; + AgentWriter writer{std::move(handle_ptr), write_interval, max_queued_traces, + disable_retry, "hostname", 6319}; // Send 7 traces at 1 trace per second. Since the write period is 2s, there should be 4 // different writes. We don't count the number of writes because that could flake, but we do // check that all 7 traces are written, implicitly testing that multiple writes happen. @@ -223,7 +214,6 @@ TEST_CASE("writer") { std::vector retry_periods{std::chrono::milliseconds(500), std::chrono::milliseconds(2500)}; AgentWriter writer{std::move(handle_ptr), - "v0.1.0", only_send_traces_when_we_flush, max_queued_traces, retry_periods, @@ -262,11 +252,11 @@ TEST_CASE("writer") { writer.write(make_trace( {TestSpanData{"web", "service", "resource", "service.name", 3, 1, 1, 69, 420, 0}})); writer.flush(); - REQUIRE(handle->headers == - std::map{{"Content-Type", "application/msgpack"}, - {"Datadog-Meta-Lang", "cpp"}, - {"Datadog-Meta-Tracer-Version", "v0.1.0"}, - {"X-Datadog-Trace-Count", "3"}}); + REQUIRE(handle->headers == std::map{ + {"Content-Type", "application/msgpack"}, + {"Datadog-Meta-Lang", "cpp"}, + {"Datadog-Meta-Tracer-Version", config::tracer_version}, + {"X-Datadog-Trace-Count", "3"}}); } } } From d8145048e4df6769997948f98450a69d7853ae8f Mon Sep 17 00:00:00 2001 From: Caleb Gilmour Date: Tue, 14 Aug 2018 01:27:06 +0000 Subject: [PATCH 3/8] Change to makeTraceAndPublisher --- include/datadog/opentracing.h | 4 ++-- src/opentracing_agent.cpp | 10 +++++++--- src/tracer.cpp | 4 +--- src/tracer.h | 5 +++-- test/opentracing_test.cpp | 5 +++-- 5 files changed, 16 insertions(+), 12 deletions(-) diff --git a/include/datadog/opentracing.h b/include/datadog/opentracing.h index 7461c8c0..3f582c50 100644 --- a/include/datadog/opentracing.h +++ b/include/datadog/opentracing.h @@ -51,8 +51,8 @@ class TracePublisher { }; std::shared_ptr makeTracer(const TracerOptions &options); -std::shared_ptr makeTracer(const TracerOptions &options, - std::shared_ptr &publisher); +std::tuple, std::shared_ptr> makeTracerAndPublisher( + const TracerOptions &options); } // namespace opentracing } // namespace datadog diff --git a/src/opentracing_agent.cpp b/src/opentracing_agent.cpp index 2ad9d0ff..88a1809b 100644 --- a/src/opentracing_agent.cpp +++ b/src/opentracing_agent.cpp @@ -10,9 +10,13 @@ std::shared_ptr makeTracer(const TracerOptions &options) { return std::shared_ptr{new Tracer{options}}; } -std::shared_ptr makeTracer(const TracerOptions &options, - std::shared_ptr &publisher) { - return std::shared_ptr{new Tracer{options, publisher}}; +std::tuple, std::shared_ptr> makeTracerAndPublisher( + const TracerOptions &options) { + auto xwriter = std::make_shared(); + auto publisher = xwriter->publisher(); + std::shared_ptr writer = xwriter; + return std::tuple, std::shared_ptr>{ + std::shared_ptr{new Tracer{options, writer}}, publisher}; } } // namespace opentracing diff --git a/src/tracer.cpp b/src/tracer.cpp index 3379998b..c2412575 100644 --- a/src/tracer.cpp +++ b/src/tracer.cpp @@ -31,10 +31,8 @@ Tracer::Tracer(TracerOptions options, std::shared_ptr buffer, TimePr get_id_(get_id), sampler_(sampler) {} -Tracer::Tracer(TracerOptions options, std::shared_ptr &publisher) +Tracer::Tracer(TracerOptions options, std::shared_ptr &writer) : opts_(options), get_time_(getRealTime), get_id_(getId), sampler_(KeepAllSampler()) { - auto writer = std::make_shared(); - publisher = writer->publisher(); buffer_ = std::shared_ptr{new WritingSpanBuffer{writer}}; } diff --git a/src/tracer.h b/src/tracer.h index 2348cc68..f4424bb1 100644 --- a/src/tracer.h +++ b/src/tracer.h @@ -34,8 +34,9 @@ class Tracer : public ot::Tracer, public std::enable_shared_from_this { Tracer(TracerOptions options, std::shared_ptr buffer, TimeProvider get_time, IdProvider get_id, SampleProvider sample); - // Creates a Tracer that exposes data for an external HTTP client to publish - Tracer(TracerOptions options, std::shared_ptr &publisher); + // Creates a Tracer using the preconfigured writer, usually for publishing + // trace data using external HTTP clients. + Tracer(TracerOptions options, std::shared_ptr &writer); Tracer() = delete; diff --git a/test/opentracing_test.cpp b/test/opentracing_test.cpp index 66e2b303..980a3da2 100644 --- a/test/opentracing_test.cpp +++ b/test/opentracing_test.cpp @@ -11,8 +11,9 @@ TEST_CASE("tracer") { REQUIRE(tracer); } SECTION("can be created with external Writer implementation") { - std::shared_ptr publisher; - auto tracer = makeTracer(TracerOptions{}, publisher); + auto tp = makeTracerAndPublisher(TracerOptions{}); + auto tracer = std::get<0>(tp); + auto publisher = std::get<1>(tp); REQUIRE(tracer); REQUIRE(publisher); } From 9388e0cdcd8f03bb7c06aacf2a445ae485b816ad Mon Sep 17 00:00:00 2001 From: Caleb Gilmour Date: Tue, 14 Aug 2018 04:23:44 +0000 Subject: [PATCH 4/8] Now curl libs not required. --- BUILD.bazel | 24 +++++++++- src/agent_writer.cpp | 6 +-- src/agent_writer.h | 33 ++----------- src/opentracing_agent.cpp | 15 ++---- src/opentracing_external.cpp | 20 ++++++++ src/tracer.cpp | 12 ++--- src/tracer.h | 8 ++-- src/tracer_factory.cpp | 7 ++- src/writer.cpp | 14 ++++++ src/writer.h | 47 +++++++++++++++++++ test/CMakeLists.txt | 2 +- ...{writer_test.cpp => agent_writer_test.cpp} | 4 +- test/tracer_factory_test.cpp | 2 +- 13 files changed, 130 insertions(+), 64 deletions(-) create mode 100644 src/opentracing_external.cpp create mode 100644 src/writer.cpp create mode 100644 src/writer.h rename test/{writer_test.cpp => agent_writer_test.cpp} (98%) diff --git a/BUILD.bazel b/BUILD.bazel index 82663c60..19ed43bf 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -1,6 +1,28 @@ cc_library( name = "dd_opentracing_cpp", - srcs = glob(["src/*.cpp", "src/*.h"]) + [ + srcs = [ + "src/clock.h", + "src/noopspan.cpp", + "src/noopspan.h", + "src/opentracing_external.cpp", + "src/propagation.cpp", + "src/propagation.h", + "src/publisher.cpp", + "src/publisher.h", + "src/sample.cpp", + "src/sample.h", + "src/span_buffer.cpp", + "src/span_buffer.h", + "src/span.cpp", + "src/span.h", + "src/tracer.cpp", + "src/tracer.h", + "src/transport.cpp", + "src/transport.h", + "src/version_check.cpp", + "src/version_check.h", + "src/writer.cpp", + "src/writer.h", ":version_number.h", ], hdrs = [ diff --git a/src/agent_writer.cpp b/src/agent_writer.cpp index 08f5d296..ad708dd7 100644 --- a/src/agent_writer.cpp +++ b/src/agent_writer.cpp @@ -1,4 +1,4 @@ -#include "writer.h" +#include "agent_writer.h" #include #include "publisher.h" #include "version_number.h" @@ -6,8 +6,6 @@ namespace datadog { namespace opentracing { -Writer::Writer() : trace_publisher_(std::make_shared()) {} - namespace { const std::string agent_protocol = "http://"; const size_t max_queued_traces = 7000; @@ -171,7 +169,5 @@ bool AgentWriter::postTraces(std::unique_ptr &handle, return true; // Don't attempt to retry. } -void ExternalWriter::write(Trace trace) { trace_publisher_->addTrace(std::move(trace)); } - } // namespace opentracing } // namespace datadog diff --git a/src/agent_writer.h b/src/agent_writer.h index 0bc11820..bba8a611 100644 --- a/src/agent_writer.h +++ b/src/agent_writer.h @@ -1,5 +1,5 @@ -#ifndef DD_OPENTRACING_WRITER_H -#define DD_OPENTRACING_WRITER_H +#ifndef DD_OPENTRACING_AGENT_WRITER_H +#define DD_OPENTRACING_AGENT_WRITER_H #include #include @@ -17,20 +17,6 @@ namespace opentracing { class SpanData; using Trace = std::unique_ptr>>; -// A Writer is used to submit completed traces to the Datadog agent. -class Writer { - public: - Writer(); - - virtual ~Writer() {} - - // Writes the given Trace. - virtual void write(Trace trace) = 0; - - protected: - std::shared_ptr trace_publisher_; -}; - // A Writer that sends Traces (collections of Spans) to a Datadog agent. class AgentWriter : public Writer { public: @@ -90,20 +76,7 @@ class AgentWriter : public Writer { bool flush_worker_ = false; }; -// A writer that collects trace data but uses an external mechanism to transmit the data -// to the Datadog Agent. -class ExternalWriter : public Writer { - public: - ExternalWriter() {} - ~ExternalWriter() {} - - // Implements Writer methods. - void write(Trace trace) override; - - std::shared_ptr publisher() { return trace_publisher_; } -}; - } // namespace opentracing } // namespace datadog -#endif // DD_OPENTRACING_WRITER_H +#endif // DD_OPENTRACING_AGENT_WRITER_H diff --git a/src/opentracing_agent.cpp b/src/opentracing_agent.cpp index 88a1809b..5ffeeb26 100644 --- a/src/opentracing_agent.cpp +++ b/src/opentracing_agent.cpp @@ -1,4 +1,5 @@ #include +#include "agent_writer.h" #include "tracer.h" namespace ot = opentracing; @@ -7,16 +8,10 @@ namespace datadog { namespace opentracing { std::shared_ptr makeTracer(const TracerOptions &options) { - return std::shared_ptr{new Tracer{options}}; -} - -std::tuple, std::shared_ptr> makeTracerAndPublisher( - const TracerOptions &options) { - auto xwriter = std::make_shared(); - auto publisher = xwriter->publisher(); - std::shared_ptr writer = xwriter; - return std::tuple, std::shared_ptr>{ - std::shared_ptr{new Tracer{options, writer}}, publisher}; + auto writer = std::shared_ptr{ + new AgentWriter(options.agent_host, options.agent_port, + std::chrono::milliseconds(llabs(options.write_period_ms)))}; + return std::shared_ptr{new Tracer{options, writer}}; } } // namespace opentracing diff --git a/src/opentracing_external.cpp b/src/opentracing_external.cpp new file mode 100644 index 00000000..e22732e6 --- /dev/null +++ b/src/opentracing_external.cpp @@ -0,0 +1,20 @@ +#include +#include "tracer.h" +#include "writer.h" + +namespace ot = opentracing; + +namespace datadog { +namespace opentracing { + +std::tuple, std::shared_ptr> makeTracerAndPublisher( + const TracerOptions &options) { + auto xwriter = std::make_shared(); + auto publisher = xwriter->publisher(); + std::shared_ptr writer = xwriter; + return std::tuple, std::shared_ptr>{ + std::shared_ptr{new Tracer{options, writer}}, publisher}; +} + +} // namespace opentracing +} // namespace datadog diff --git a/src/tracer.cpp b/src/tracer.cpp index c2412575..2c1fcdca 100644 --- a/src/tracer.cpp +++ b/src/tracer.cpp @@ -16,13 +16,6 @@ uint64_t getId() { return distribution(source); } -Tracer::Tracer(TracerOptions options) - : Tracer(options, - std::shared_ptr{new WritingSpanBuffer{std::make_shared( - options.agent_host, options.agent_port, - std::chrono::milliseconds(llabs(options.write_period_ms)))}}, - getRealTime, getId, ConstantRateSampler(options.sample_rate)) {} - Tracer::Tracer(TracerOptions options, std::shared_ptr buffer, TimeProvider get_time, IdProvider get_id, SampleProvider sampler) : opts_(options), @@ -32,7 +25,10 @@ Tracer::Tracer(TracerOptions options, std::shared_ptr buffer, TimePr sampler_(sampler) {} Tracer::Tracer(TracerOptions options, std::shared_ptr &writer) - : opts_(options), get_time_(getRealTime), get_id_(getId), sampler_(KeepAllSampler()) { + : opts_(options), + get_time_(getRealTime), + get_id_(getId), + sampler_(ConstantRateSampler(options.sample_rate)) { buffer_ = std::shared_ptr{new WritingSpanBuffer{writer}}; } diff --git a/src/tracer.h b/src/tracer.h index f4424bb1..8f0f1fd9 100644 --- a/src/tracer.h +++ b/src/tracer.h @@ -27,15 +27,13 @@ uint64_t getId(); class Tracer : public ot::Tracer, public std::enable_shared_from_this { public: - // Creates a Tracer by copying the given options. - Tracer(TracerOptions options); - // Creates a Tracer by copying the given options and injecting the given dependencies. Tracer(TracerOptions options, std::shared_ptr buffer, TimeProvider get_time, IdProvider get_id, SampleProvider sample); - // Creates a Tracer using the preconfigured writer, usually for publishing - // trace data using external HTTP clients. + // Creates a Tracer by copying the given options and using the preconfigured writer. + // The writer is either an AgentWriter that publishes directly to the Datadog Agent, or + // an ExternalWriter that requires an external HTTP client to publish to the Datadog Agent. Tracer(TracerOptions options, std::shared_ptr &writer); Tracer() = delete; diff --git a/src/tracer_factory.cpp b/src/tracer_factory.cpp index 8edc04e4..32a4d306 100644 --- a/src/tracer_factory.cpp +++ b/src/tracer_factory.cpp @@ -2,6 +2,7 @@ #include #include +#include "agent_writer.h" #include "tracer.h" using json = nlohmann::json; @@ -56,7 +57,11 @@ ot::expected> TracerFactory::MakeTracer( error_message = "configuration has an argument with an incorrect type"; return ot::make_unexpected(std::make_error_code(std::errc::invalid_argument)); } - return std::shared_ptr{new TracerImpl{options}}; + auto writer = std::shared_ptr{ + new AgentWriter(options.agent_host, options.agent_port, + std::chrono::milliseconds(llabs(options.write_period_ms)))}; + + return std::shared_ptr{new TracerImpl{options, writer}}; } catch (const std::bad_alloc &) { return ot::make_unexpected(std::make_error_code(std::errc::not_enough_memory)); } diff --git a/src/writer.cpp b/src/writer.cpp new file mode 100644 index 00000000..a56d39a4 --- /dev/null +++ b/src/writer.cpp @@ -0,0 +1,14 @@ +#include "writer.h" +#include +#include "publisher.h" +#include "version_number.h" + +namespace datadog { +namespace opentracing { + +Writer::Writer() : trace_publisher_(std::make_shared()) {} + +void ExternalWriter::write(Trace trace) { trace_publisher_->addTrace(std::move(trace)); } + +} // namespace opentracing +} // namespace datadog diff --git a/src/writer.h b/src/writer.h new file mode 100644 index 00000000..d0310594 --- /dev/null +++ b/src/writer.h @@ -0,0 +1,47 @@ +#ifndef DD_OPENTRACING_WRITER_H +#define DD_OPENTRACING_WRITER_H + +#include +#include +#include +#include +#include +#include +#include "publisher.h" +#include "span.h" +#include "transport.h" + +namespace datadog { +namespace opentracing { + +// A Writer is used to submit completed traces to the Datadog agent. +class Writer { + public: + Writer(); + + virtual ~Writer() {} + + // Writes the given Trace. + virtual void write(Trace trace) = 0; + + protected: + std::shared_ptr trace_publisher_; +}; + +// A writer that collects trace data but uses an external mechanism to transmit the data +// to the Datadog Agent. +class ExternalWriter : public Writer { + public: + ExternalWriter() {} + ~ExternalWriter() override {} + + // Implements Writer methods. + void write(Trace trace) override; + + std::shared_ptr publisher() { return trace_publisher_; } +}; + +} // namespace opentracing +} // namespace datadog + +#endif // DD_OPENTRACING_WRITER_H diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 7c3db75c..357d4c4f 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -14,4 +14,4 @@ _datadog_test(span_test span_test.cpp) _datadog_test(tracer_factory_test tracer_factory_test.cpp) _datadog_test(tracer_test tracer_test.cpp) _datadog_test(sample_test sample_test.cpp) -_datadog_test(writer_test writer_test.cpp) +_datadog_test(agent_writer_test agent_writer_test.cpp) diff --git a/test/writer_test.cpp b/test/agent_writer_test.cpp similarity index 98% rename from test/writer_test.cpp rename to test/agent_writer_test.cpp index 0ff5fdf7..e7614282 100644 --- a/test/writer_test.cpp +++ b/test/agent_writer_test.cpp @@ -1,5 +1,5 @@ -#include "../src/writer.h" -#include "../src/writer.cpp" // Otherwise the compiler won't generate AgentWriter for us. +#include "../src/agent_writer.h" +#include "../src/agent_writer.cpp" // Otherwise the compiler won't generate AgentWriter for us. #include "mocks.h" #include "version_number.h" diff --git a/test/tracer_factory_test.cpp b/test/tracer_factory_test.cpp index 5780e93c..33b3d148 100644 --- a/test/tracer_factory_test.cpp +++ b/test/tracer_factory_test.cpp @@ -12,7 +12,7 @@ using namespace datadog::opentracing; struct MockTracer : public ot::Tracer { TracerOptions opts; - MockTracer(TracerOptions opts_) : opts(opts_) {} + MockTracer(TracerOptions opts_, std::shared_ptr &writer) : opts(opts_) {} std::unique_ptr StartSpanWithOptions(ot::string_view operation_name, const ot::StartSpanOptions &options) const From 9b61949b3750b3eff5f9f399ccb611a5ed714eef Mon Sep 17 00:00:00 2001 From: Caleb Gilmour Date: Mon, 27 Aug 2018 02:10:34 +0000 Subject: [PATCH 5/8] Cleanup and fixes for some warnings --- src/agent_writer.h | 3 --- src/noopspan.h | 2 +- src/propagation.cpp | 6 +++--- src/publisher.h | 2 +- src/span.cpp | 2 +- src/span.h | 9 ++++----- src/span_buffer.h | 2 -- src/tracer.cpp | 6 +++--- src/tracer.h | 3 +-- src/transport.h | 4 ++-- 10 files changed, 16 insertions(+), 23 deletions(-) diff --git a/src/agent_writer.h b/src/agent_writer.h index bba8a611..537c8460 100644 --- a/src/agent_writer.h +++ b/src/agent_writer.h @@ -14,9 +14,6 @@ namespace datadog { namespace opentracing { -class SpanData; -using Trace = std::unique_ptr>>; - // A Writer that sends Traces (collections of Spans) to a Datadog agent. class AgentWriter : public Writer { public: diff --git a/src/noopspan.h b/src/noopspan.h index 17782391..4b7f9236 100644 --- a/src/noopspan.h +++ b/src/noopspan.h @@ -20,7 +20,7 @@ class NoopSpan : public ot::Span { uint64_t parent_id, SpanContext context, const ot::StartSpanOptions &options); NoopSpan() = delete; NoopSpan(NoopSpan &&other); - ~NoopSpan() = default; + ~NoopSpan() override = default; void FinishWithOptions(const ot::FinishSpanOptions &finish_span_options) noexcept override; void SetOperationName(ot::string_view name) noexcept override; diff --git a/src/propagation.cpp b/src/propagation.cpp index 37d63b07..9b377cac 100644 --- a/src/propagation.cpp +++ b/src/propagation.cpp @@ -87,7 +87,7 @@ std::string SpanContext::baggageItem(ot::string_view key) const { SpanContext SpanContext::withId(uint64_t id) const { std::lock_guard lock{mutex_}; auto baggage = baggage_; // (Shallow) copy baggage. - return std::move(SpanContext{id, trace_id_, std::move(baggage)}); + return SpanContext{id, trace_id_, std::move(baggage)}; } ot::expected SpanContext::serialize(const ot::TextMapWriter &writer) const { @@ -149,8 +149,8 @@ ot::expected> SpanContext::deserialize( // Partial context, this shouldn't happen. return ot::make_unexpected(ot::span_context_corrupted_error); } - return std::move( - std::unique_ptr{new SpanContext{parent_id, trace_id, std::move(baggage)}}); + return std::unique_ptr{ + new SpanContext{parent_id, trace_id, std::move(baggage)}}; } catch (const std::bad_alloc &) { return ot::make_unexpected(std::make_error_code(std::errc::not_enough_memory)); } diff --git a/src/publisher.h b/src/publisher.h index fa4665df..fbaaa425 100644 --- a/src/publisher.h +++ b/src/publisher.h @@ -8,7 +8,7 @@ namespace datadog { namespace opentracing { -class SpanData; +struct SpanData; using Trace = std::unique_ptr>>; class AgentHttpPublisher : public TracePublisher { diff --git a/src/span.cpp b/src/span.cpp index 010d8579..d6fd1561 100644 --- a/src/span.cpp +++ b/src/span.cpp @@ -38,7 +38,7 @@ uint64_t SpanData::spanId() const { return span_id; } std::unique_ptr makeSpanData(std::string type, std::string service, ot::string_view resource, std::string name, - uint64_t trace_id, int64_t span_id, uint64_t parent_id, + uint64_t trace_id, uint64_t span_id, uint64_t parent_id, int64_t start) { return std::unique_ptr{ new SpanData(type, service, resource, name, trace_id, span_id, parent_id, start, 0, 0)}; diff --git a/src/span.h b/src/span.h index 25e86a65..ec513f6e 100644 --- a/src/span.h +++ b/src/span.h @@ -12,18 +12,15 @@ namespace opentracing { class Tracer; class SpanBuffer; -class SpanData; typedef std::function IdProvider; // See tracer.h -using Trace = std::unique_ptr>>; - // Contains data that describes a Span. struct SpanData { ~SpanData() = default; friend std::unique_ptr makeSpanData(std::string type, std::string service, ot::string_view resource, std::string name, - uint64_t trace_id, int64_t span_id, + uint64_t trace_id, uint64_t span_id, uint64_t parent_id, int64_t start); friend std::unique_ptr stubSpanData(); @@ -55,9 +52,11 @@ struct SpanData { uint64_t spanId() const; MSGPACK_DEFINE_MAP(name, service, resource, type, start, duration, meta, span_id, trace_id, - parent_id, error); + parent_id, error) }; +using Trace = std::unique_ptr>>; + // A Span, a component of a trace, a single instrumented event. class Span : public ot::Span { public: diff --git a/src/span_buffer.h b/src/span_buffer.h index 9ba83fd4..3f7557d8 100644 --- a/src/span_buffer.h +++ b/src/span_buffer.h @@ -10,9 +10,7 @@ namespace datadog { namespace opentracing { -class SpanData; class Writer; -using Trace = std::unique_ptr>>; struct PendingTrace { PendingTrace() diff --git a/src/tracer.cpp b/src/tracer.cpp index 2c1fcdca..11ac0b24 100644 --- a/src/tracer.cpp +++ b/src/tracer.cpp @@ -58,10 +58,10 @@ std::unique_ptr Tracer::StartSpanWithOptions(ot::string_view operation std::move(span_context), get_time_(), opts_.service, opts_.type, operation_name, operation_name, opts_.operation_name_override}}; sampler_.tag(span); - return std::move(span); + return span; } else { - return std::move(std::unique_ptr{new NoopSpan{ - shared_from_this(), span_id, trace_id, parent_id, std::move(span_context), options}}); + return std::unique_ptr{new NoopSpan{shared_from_this(), span_id, trace_id, parent_id, + std::move(span_context), options}}; } } catch (const std::bad_alloc &) { // At least don't crash. diff --git a/src/tracer.h b/src/tracer.h index 8f0f1fd9..30df6902 100644 --- a/src/tracer.h +++ b/src/tracer.h @@ -7,6 +7,7 @@ #include "sample.h" #include "span.h" #include "span_buffer.h" +#include "writer.h" #include #include @@ -16,8 +17,6 @@ namespace ot = opentracing; namespace datadog { namespace opentracing { -class Writer; -class SpanData; class SpanBuffer; // The interface for providing IDs to spans and traces. diff --git a/src/transport.h b/src/transport.h index f6c4f8c1..c36db197 100644 --- a/src/transport.h +++ b/src/transport.h @@ -11,8 +11,8 @@ namespace opentracing { // An interface to a CURL handle. This interface exists to make testing Recorder easier. class Handle { public: - Handle(){}; - virtual ~Handle(){}; + Handle() {} + virtual ~Handle() {} virtual CURLcode setopt(CURLoption key, const char* value) = 0; virtual CURLcode setopt(CURLoption key, long value) = 0; virtual void setHeaders(std::map headers) = 0; From 6a5d80424e23cfd47c0595f1424ba6cf1af89e80 Mon Sep 17 00:00:00 2001 From: Caleb Gilmour Date: Thu, 30 Aug 2018 05:15:44 +0000 Subject: [PATCH 6/8] Additional comments on the code additions. --- include/datadog/opentracing.h | 8 ++++++++ src/opentracing_agent.cpp | 5 +++++ src/opentracing_external.cpp | 7 +++++++ 3 files changed, 20 insertions(+) diff --git a/include/datadog/opentracing.h b/include/datadog/opentracing.h index 3f582c50..1c929286 100644 --- a/include/datadog/opentracing.h +++ b/include/datadog/opentracing.h @@ -46,11 +46,19 @@ class TracePublisher { virtual const std::string path() = 0; virtual std::size_t pendingTraces() = 0; virtual void clearTraces() = 0; + // Returns the HTTP headers that are required for the collection of traces. virtual const std::map headers() = 0; + // Returns the encoded payload from the collection of traces. virtual const std::string payload() = 0; }; +// makeTracer returns an opentracing::Tracer that submits traces to the Datadog Agent. +// This should be used when control over the HTTP requests to the Datadog Agent is not required. std::shared_ptr makeTracer(const TracerOptions &options); +// makeTracerAndPublisher initializes an opentracing::Tracer and provides a publisher +// to use when submitting traces to the Datadog Agent. +// This should be used in applications that need to also control the HTTP requests to the Datadog +// Agent. std::tuple, std::shared_ptr> makeTracerAndPublisher( const TracerOptions &options); diff --git a/src/opentracing_agent.cpp b/src/opentracing_agent.cpp index 5ffeeb26..76766d37 100644 --- a/src/opentracing_agent.cpp +++ b/src/opentracing_agent.cpp @@ -1,3 +1,8 @@ +// Implementation of the exposed makeTracer function. +// This is kept separately to isolate the AgentWriter and its cURL dependency. +// Users of the library that do not use this tracer are able to avoid the +// additional dependency and implementation details. + #include #include "agent_writer.h" #include "tracer.h" diff --git a/src/opentracing_external.cpp b/src/opentracing_external.cpp index e22732e6..63f2185d 100644 --- a/src/opentracing_external.cpp +++ b/src/opentracing_external.cpp @@ -1,3 +1,10 @@ +// Implementation of the exposed makeTracerAndPublisher function. +// This is intentionally kept separate from the makeTracer function, which has additional +// dependencies. It allows the library to be used with an external HTTP implementation for sending +// traces to the Datadog Agent. +// +// See BAZEL.build for the files required to build this library using makeTracerAndPublisher. + #include #include "tracer.h" #include "writer.h" From 5bd13fc551a240618b80367c4ad1274610380b48 Mon Sep 17 00:00:00 2001 From: Caleb Gilmour Date: Thu, 30 Aug 2018 05:39:07 +0000 Subject: [PATCH 7/8] Use make_unique --- src/propagation.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/propagation.cpp b/src/propagation.cpp index 9b377cac..44ea4933 100644 --- a/src/propagation.cpp +++ b/src/propagation.cpp @@ -149,8 +149,8 @@ ot::expected> SpanContext::deserialize( // Partial context, this shouldn't happen. return ot::make_unexpected(ot::span_context_corrupted_error); } - return std::unique_ptr{ - new SpanContext{parent_id, trace_id, std::move(baggage)}}; + return std::unique_ptr( + std::make_unique(parent_id, trace_id, std::move(baggage))); } catch (const std::bad_alloc &) { return ot::make_unexpected(std::make_error_code(std::errc::not_enough_memory)); } From 06a51ae405fd3c563286b7970b7eeb94ee264989 Mon Sep 17 00:00:00 2001 From: Caleb Gilmour Date: Tue, 4 Sep 2018 05:01:53 +0000 Subject: [PATCH 8/8] Publisher -> Encoder --- BUILD.bazel | 4 ++-- include/datadog/opentracing.h | 14 +++++++------- src/agent_writer.cpp | 16 ++++++++-------- src/agent_writer.h | 2 +- src/{publisher.cpp => encoder.cpp} | 16 ++++++++-------- src/{publisher.h => encoder.h} | 12 ++++++------ src/opentracing_external.cpp | 12 ++++++------ src/tracer.h | 7 ++++--- src/writer.cpp | 6 +++--- src/writer.h | 6 +++--- test/opentracing_test.cpp | 6 +++--- 11 files changed, 51 insertions(+), 50 deletions(-) rename src/{publisher.cpp => encoder.cpp} (59%) rename src/{publisher.h => encoder.h} (80%) diff --git a/BUILD.bazel b/BUILD.bazel index 19ed43bf..f11d9cab 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -7,8 +7,8 @@ cc_library( "src/opentracing_external.cpp", "src/propagation.cpp", "src/propagation.h", - "src/publisher.cpp", - "src/publisher.h", + "src/encoder.cpp", + "src/encoder.h", "src/sample.cpp", "src/sample.h", "src/span_buffer.cpp", diff --git a/include/datadog/opentracing.h b/include/datadog/opentracing.h index 1c929286..145c2995 100644 --- a/include/datadog/opentracing.h +++ b/include/datadog/opentracing.h @@ -35,14 +35,14 @@ struct TracerOptions { std::string operation_name_override = ""; }; -// TracePublisher exposes the data required to publish traces to the +// TraceEncoder exposes the data required to encode and submit traces to the // Datadog Agent. -class TracePublisher { +class TraceEncoder { public: - TracePublisher() {} - virtual ~TracePublisher() {} + TraceEncoder() {} + virtual ~TraceEncoder() {} - // Returns the Datadog Agent endpoint that traces should be published to. + // Returns the Datadog Agent endpoint that traces should be sent to. virtual const std::string path() = 0; virtual std::size_t pendingTraces() = 0; virtual void clearTraces() = 0; @@ -55,11 +55,11 @@ class TracePublisher { // makeTracer returns an opentracing::Tracer that submits traces to the Datadog Agent. // This should be used when control over the HTTP requests to the Datadog Agent is not required. std::shared_ptr makeTracer(const TracerOptions &options); -// makeTracerAndPublisher initializes an opentracing::Tracer and provides a publisher +// makeTracerAndEncoder initializes an opentracing::Tracer and provides an encoder // to use when submitting traces to the Datadog Agent. // This should be used in applications that need to also control the HTTP requests to the Datadog // Agent. -std::tuple, std::shared_ptr> makeTracerAndPublisher( +std::tuple, std::shared_ptr> makeTracerAndEncoder( const TracerOptions &options); } // namespace opentracing diff --git a/src/agent_writer.cpp b/src/agent_writer.cpp index ad708dd7..42abb85b 100644 --- a/src/agent_writer.cpp +++ b/src/agent_writer.cpp @@ -1,6 +1,6 @@ #include "agent_writer.h" #include -#include "publisher.h" +#include "encoder.h" #include "version_number.h" namespace datadog { @@ -37,7 +37,7 @@ void AgentWriter::setUpHandle(std::unique_ptr &handle, std::string host, // Some options are the same for all actions, set them here. // Set the agent URI. std::stringstream agent_uri; - agent_uri << agent_protocol << host << ":" << std::to_string(port) << trace_publisher_->path(); + agent_uri << agent_protocol << host << ":" << std::to_string(port) << trace_encoder_->path(); auto rcode = handle->setopt(CURLOPT_URL, agent_uri.str().c_str()); if (rcode != CURLE_OK) { throw std::runtime_error(std::string("Unable to set agent URL: ") + curl_easy_strerror(rcode)); @@ -68,10 +68,10 @@ void AgentWriter::write(Trace trace) { if (stop_writing_) { return; } - if (trace_publisher_->pendingTraces() >= max_queued_traces_) { + if (trace_encoder_->pendingTraces() >= max_queued_traces_) { return; } - trace_publisher_->addTrace(std::move(trace)); + trace_encoder_->addTrace(std::move(trace)); }; void AgentWriter::startWriting(std::unique_ptr handle) { @@ -92,13 +92,13 @@ void AgentWriter::startWriting(std::unique_ptr handle) { if (stop_writing_) { return; // Stop the thread. } - num_traces = trace_publisher_->pendingTraces(); + num_traces = trace_encoder_->pendingTraces(); if (num_traces == 0) { continue; } - headers = trace_publisher_->headers(); - payload = trace_publisher_->payload(); - trace_publisher_->clearTraces(); + headers = trace_encoder_->headers(); + payload = trace_encoder_->payload(); + trace_encoder_->clearTraces(); } // lock on mutex_ ends. // Send spans, not in critical period. retryFiniteOnFail([&]() { return AgentWriter::postTraces(handle, headers, payload); }); diff --git a/src/agent_writer.h b/src/agent_writer.h index 537c8460..ec897bf5 100644 --- a/src/agent_writer.h +++ b/src/agent_writer.h @@ -7,7 +7,7 @@ #include #include #include -#include "publisher.h" +#include "encoder.h" #include "span.h" #include "transport.h" diff --git a/src/publisher.cpp b/src/encoder.cpp similarity index 59% rename from src/publisher.cpp rename to src/encoder.cpp index b049096a..254cad31 100644 --- a/src/publisher.cpp +++ b/src/encoder.cpp @@ -1,11 +1,11 @@ -#include "publisher.h" +#include "encoder.h" #include "span.h" #include "version_number.h" namespace datadog { namespace opentracing { -AgentHttpPublisher::AgentHttpPublisher() { +AgentHttpEncoder::AgentHttpEncoder() { // Set up common headers and default encoder common_headers_ = {{"Content-Type", "application/msgpack"}, {"Datadog-Meta-Lang", "cpp"}, @@ -14,26 +14,26 @@ AgentHttpPublisher::AgentHttpPublisher() { const std::string agent_api_path = "/v0.3/traces"; -const std::string AgentHttpPublisher::path() { return agent_api_path; } +const std::string AgentHttpEncoder::path() { return agent_api_path; } -void AgentHttpPublisher::clearTraces() { traces_.clear(); } +void AgentHttpEncoder::clearTraces() { traces_.clear(); } -std::size_t AgentHttpPublisher::pendingTraces() { return traces_.size(); } +std::size_t AgentHttpEncoder::pendingTraces() { return traces_.size(); } -const std::map AgentHttpPublisher::headers() { +const std::map AgentHttpEncoder::headers() { std::map headers(common_headers_); headers["X-Datadog-Trace-Count"] = std::to_string(traces_.size()); return headers; } -const std::string AgentHttpPublisher::payload() { +const std::string AgentHttpEncoder::payload() { buffer_.clear(); buffer_.str(std::string{}); msgpack::pack(buffer_, traces_); return buffer_.str(); } -void AgentHttpPublisher::addTrace(Trace trace) { traces_.push_back(std::move(trace)); } +void AgentHttpEncoder::addTrace(Trace trace) { traces_.push_back(std::move(trace)); } } // namespace opentracing } // namespace datadog diff --git a/src/publisher.h b/src/encoder.h similarity index 80% rename from src/publisher.h rename to src/encoder.h index fbaaa425..113029bc 100644 --- a/src/publisher.h +++ b/src/encoder.h @@ -1,5 +1,5 @@ -#ifndef DD_OPENTRACING_PUBLISHER_H -#define DD_OPENTRACING_PUBLISHER_H +#ifndef DD_OPENTRACING_ENCODER_H +#define DD_OPENTRACING_ENCODER_H #include @@ -11,10 +11,10 @@ namespace opentracing { struct SpanData; using Trace = std::unique_ptr>>; -class AgentHttpPublisher : public TracePublisher { +class AgentHttpEncoder : public TraceEncoder { public: - AgentHttpPublisher(); - ~AgentHttpPublisher() override {} + AgentHttpEncoder(); + ~AgentHttpEncoder() override {} // Returns the path that is used to submit HTTP requests to the agent. const std::string path() override; @@ -36,4 +36,4 @@ class AgentHttpPublisher : public TracePublisher { } // namespace opentracing } // namespace datadog -#endif // DD_OPENTRACING_PUBLISHER_H +#endif // DD_OPENTRACING_ENCODER_H diff --git a/src/opentracing_external.cpp b/src/opentracing_external.cpp index 63f2185d..59bbef62 100644 --- a/src/opentracing_external.cpp +++ b/src/opentracing_external.cpp @@ -1,9 +1,9 @@ -// Implementation of the exposed makeTracerAndPublisher function. +// Implementation of the exposed makeTracerAndEncoder function. // This is intentionally kept separate from the makeTracer function, which has additional // dependencies. It allows the library to be used with an external HTTP implementation for sending // traces to the Datadog Agent. // -// See BAZEL.build for the files required to build this library using makeTracerAndPublisher. +// See BAZEL.build for the files required to build this library using makeTracerAndEncoder. #include #include "tracer.h" @@ -14,13 +14,13 @@ namespace ot = opentracing; namespace datadog { namespace opentracing { -std::tuple, std::shared_ptr> makeTracerAndPublisher( +std::tuple, std::shared_ptr> makeTracerAndEncoder( const TracerOptions &options) { auto xwriter = std::make_shared(); - auto publisher = xwriter->publisher(); + auto encoder = xwriter->encoder(); std::shared_ptr writer = xwriter; - return std::tuple, std::shared_ptr>{ - std::shared_ptr{new Tracer{options, writer}}, publisher}; + return std::tuple, std::shared_ptr>{ + std::shared_ptr{new Tracer{options, writer}}, encoder}; } } // namespace opentracing diff --git a/src/tracer.h b/src/tracer.h index 30df6902..45d58882 100644 --- a/src/tracer.h +++ b/src/tracer.h @@ -3,7 +3,7 @@ #include #include "clock.h" -#include "publisher.h" +#include "encoder.h" #include "sample.h" #include "span.h" #include "span_buffer.h" @@ -31,8 +31,9 @@ class Tracer : public ot::Tracer, public std::enable_shared_from_this { IdProvider get_id, SampleProvider sample); // Creates a Tracer by copying the given options and using the preconfigured writer. - // The writer is either an AgentWriter that publishes directly to the Datadog Agent, or - // an ExternalWriter that requires an external HTTP client to publish to the Datadog Agent. + // The writer is either an AgentWriter that sends trace data directly to the Datadog Agent, or + // an ExternalWriter that requires an external HTTP client to encode and submit to the Datadog + // Agent. Tracer(TracerOptions options, std::shared_ptr &writer); Tracer() = delete; diff --git a/src/writer.cpp b/src/writer.cpp index a56d39a4..3cd40cb4 100644 --- a/src/writer.cpp +++ b/src/writer.cpp @@ -1,14 +1,14 @@ #include "writer.h" #include -#include "publisher.h" +#include "encoder.h" #include "version_number.h" namespace datadog { namespace opentracing { -Writer::Writer() : trace_publisher_(std::make_shared()) {} +Writer::Writer() : trace_encoder_(std::make_shared()) {} -void ExternalWriter::write(Trace trace) { trace_publisher_->addTrace(std::move(trace)); } +void ExternalWriter::write(Trace trace) { trace_encoder_->addTrace(std::move(trace)); } } // namespace opentracing } // namespace datadog diff --git a/src/writer.h b/src/writer.h index d0310594..09307840 100644 --- a/src/writer.h +++ b/src/writer.h @@ -7,7 +7,7 @@ #include #include #include -#include "publisher.h" +#include "encoder.h" #include "span.h" #include "transport.h" @@ -25,7 +25,7 @@ class Writer { virtual void write(Trace trace) = 0; protected: - std::shared_ptr trace_publisher_; + std::shared_ptr trace_encoder_; }; // A writer that collects trace data but uses an external mechanism to transmit the data @@ -38,7 +38,7 @@ class ExternalWriter : public Writer { // Implements Writer methods. void write(Trace trace) override; - std::shared_ptr publisher() { return trace_publisher_; } + std::shared_ptr encoder() { return trace_encoder_; } }; } // namespace opentracing diff --git a/test/opentracing_test.cpp b/test/opentracing_test.cpp index 980a3da2..b275f469 100644 --- a/test/opentracing_test.cpp +++ b/test/opentracing_test.cpp @@ -11,10 +11,10 @@ TEST_CASE("tracer") { REQUIRE(tracer); } SECTION("can be created with external Writer implementation") { - auto tp = makeTracerAndPublisher(TracerOptions{}); + auto tp = makeTracerAndEncoder(TracerOptions{}); auto tracer = std::get<0>(tp); - auto publisher = std::get<1>(tp); + auto encoder = std::get<1>(tp); REQUIRE(tracer); - REQUIRE(publisher); + REQUIRE(encoder); } }