diff --git a/.circleci/config.yml b/.circleci/config.yml index de70ce74..37e90f29 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -3,7 +3,7 @@ version: 2 defaults: &defaults working_directory: ~/dd-opentracing-cpp docker: - - image: datadog/docker-library:dd_opentracing_cpp_build_0_3_2 + - image: datadog/docker-library:dd_opentracing_cpp_build_0_3_3 jobs: build: @@ -17,12 +17,12 @@ jobs: - run: name: Run clang-format command: | - find ./ -iname *.h -o -iname *.cpp | while read fname; do + find include src test -iname '*.h' -o -iname '*.cpp' | while read fname; do changes=$(clang-format-5.0 -output-replacements-xml $fname | grep -c " tracer, uint64_t span_id, uint64_t trace_id, - uint64_t parent_id, SpanContext context, const ot::StartSpanOptions &options) + uint64_t parent_id, SpanContext context, + const ot::StartSpanOptions & /* options */) : tracer_(std::move(tracer)), span_id_(span_id), trace_id_(trace_id), @@ -21,19 +22,22 @@ NoopSpan::NoopSpan(NoopSpan &&other) parent_id_(other.parent_id_), context_(std::move(other.context_)) {} -void NoopSpan::FinishWithOptions(const ot::FinishSpanOptions &finish_span_options) noexcept {} +void NoopSpan::FinishWithOptions( + const ot::FinishSpanOptions & /* finish_span_options */) noexcept {} -void NoopSpan::SetOperationName(ot::string_view operation_name) noexcept {} +void NoopSpan::SetOperationName(ot::string_view /* operation_name */) noexcept {} -void NoopSpan::SetTag(ot::string_view key, const ot::Value &value) noexcept {} +void NoopSpan::SetTag(ot::string_view /* key */, const ot::Value & /* value */) noexcept {} -void NoopSpan::SetBaggageItem(ot::string_view restricted_key, ot::string_view value) noexcept {} +void NoopSpan::SetBaggageItem(ot::string_view /* restricted_key */, + ot::string_view /* value */) noexcept {} std::string NoopSpan::BaggageItem(ot::string_view restricted_key) const noexcept { return context_.baggageItem(restricted_key); } -void NoopSpan::Log(std::initializer_list> fields) noexcept {} +void NoopSpan::Log( + std::initializer_list> /* fields */) noexcept {} const ot::SpanContext &NoopSpan::context() const noexcept { return context_; } @@ -43,7 +47,7 @@ uint64_t NoopSpan::traceId() const { return trace_id_; } uint64_t NoopSpan::spanId() const { return span_id_; } OptionalSamplingPriority NoopSpan::setSamplingPriority( - std::unique_ptr priority) { + std::unique_ptr /* priority */) { return nullptr; }; OptionalSamplingPriority NoopSpan::getSamplingPriority() const { return nullptr; }; diff --git a/src/propagation.cpp b/src/propagation.cpp index 33960195..6bea0241 100644 --- a/src/propagation.cpp +++ b/src/propagation.cpp @@ -48,7 +48,8 @@ bool has_prefix(const std::string &str, const std::string &prefix) { } // namespace OptionalSamplingPriority asSamplingPriority(int i) { - if (i < (int)SamplingPriority::MinimumValue || i > (int)SamplingPriority::MaximumValue) { + if (i < static_cast(SamplingPriority::MinimumValue) || + i > static_cast(SamplingPriority::MaximumValue)) { return nullptr; } return std::make_unique(static_cast(i)); diff --git a/src/sample.cpp b/src/sample.cpp index f5fc2143..04f133f9 100644 --- a/src/sample.cpp +++ b/src/sample.cpp @@ -41,13 +41,13 @@ bool DiscardRateSampler::discard(const SpanContext& context) const { return true; } -OptionalSamplingPriority DiscardRateSampler::sample(const std::string& environment, - const std::string& service, - uint64_t trace_id) const { +OptionalSamplingPriority DiscardRateSampler::sample(const std::string& /* environment */, + const std::string& /* service */, + uint64_t /* trace_id */) const { return nullptr; } -bool PrioritySampler::discard(const SpanContext& context) const { return false; } +bool PrioritySampler::discard(const SpanContext& /* context */) const { return false; } OptionalSamplingPriority PrioritySampler::sample(const std::string& environment, const std::string& service, diff --git a/src/span.cpp b/src/span.cpp index bf84501b..6925a93e 100644 --- a/src/span.cpp +++ b/src/span.cpp @@ -112,7 +112,8 @@ void audit(SpanData *span) { } } -void Span::FinishWithOptions(const ot::FinishSpanOptions &finish_span_options) noexcept try { +void Span::FinishWithOptions( + const ot::FinishSpanOptions & /* finish_span_options */) noexcept try { if (is_finished_.exchange(true)) { return; } @@ -309,7 +310,8 @@ std::string Span::BaggageItem(ot::string_view restricted_key) const noexcept { return context_.baggageItem(restricted_key); } -void Span::Log(std::initializer_list> fields) noexcept {} +void Span::Log( + std::initializer_list> /* fields */) noexcept {} OptionalSamplingPriority Span::setSamplingPriority( std::unique_ptr user_priority) { diff --git a/src/span_buffer.cpp b/src/span_buffer.cpp index d620ec6d..9c954f76 100644 --- a/src/span_buffer.cpp +++ b/src/span_buffer.cpp @@ -11,7 +11,7 @@ namespace { const std::string sampling_priority_metric = "_sampling_priority_v1"; } // namespace -void PendingTrace::finish(const std::shared_ptr& sampler) { +void PendingTrace::finish() { if (finished_spans->size() == 0) { std::cerr << "finish called on trace with no spans" << std::endl; return; // I don't know why this would ever happen. @@ -58,13 +58,12 @@ void WritingSpanBuffer::finishSpan(std::unique_ptr span, trace.finished_spans->push_back(std::move(span)); if (trace.finished_spans->size() == trace.all_spans.size()) { assignSamplingPriorityImpl(sampler, trace.finished_spans->back().get()); - trace.finish(sampler); - unbufferAndWriteTrace(trace_id, sampler); + trace.finish(); + unbufferAndWriteTrace(trace_id); } } -void WritingSpanBuffer::unbufferAndWriteTrace(uint64_t trace_id, - const std::shared_ptr& sampler) { +void WritingSpanBuffer::unbufferAndWriteTrace(uint64_t trace_id) { auto trace_iter = traces_.find(trace_id); if (trace_iter == traces_.end()) { return; diff --git a/src/span_buffer.h b/src/span_buffer.h index 69c9bfc4..5e42afae 100644 --- a/src/span_buffer.h +++ b/src/span_buffer.h @@ -20,7 +20,7 @@ struct PendingTrace { PendingTrace() : finished_spans(Trace{new std::vector>()}), all_spans() {} - void finish(const std::shared_ptr& sampler); + void finish(); Trace finished_spans; std::unordered_set all_spans; @@ -72,8 +72,7 @@ class WritingSpanBuffer : public SpanBuffer { protected: // Exists to make it easy for a subclass (ie, our testing mock) to override on-trace-finish // behaviour. - virtual void unbufferAndWriteTrace(uint64_t trace_id, - const std::shared_ptr& sampler); + virtual void unbufferAndWriteTrace(uint64_t trace_id); std::unordered_map traces_; }; diff --git a/src/tracer.h b/src/tracer.h index d298ad3d..061c71e5 100644 --- a/src/tracer.h +++ b/src/tracer.h @@ -43,6 +43,10 @@ class Tracer : public ot::Tracer, public std::enable_shared_from_this { const ot::StartSpanOptions &options) const noexcept override; + // Ensure we aren't hiding methods from ot::Tracer due to the overloaded overrides. + using ot::Tracer::Extract; + using ot::Tracer::Inject; + ot::expected Inject(const ot::SpanContext &sc, std::ostream &writer) const override; ot::expected Inject(const ot::SpanContext &sc, diff --git a/src/transport.cpp b/src/transport.cpp index b03dd735..64f9f5f1 100644 --- a/src/transport.cpp +++ b/src/transport.cpp @@ -7,7 +7,7 @@ namespace datadog { namespace opentracing { size_t write_callback(char* ptr, size_t size, size_t nmemb, void* userdata) { - CurlHandle* handle = (CurlHandle*)userdata; + CurlHandle* handle = static_cast(userdata); handle->response_buffer_.write(ptr, size * nmemb); if (!handle->response_buffer_) { @@ -40,7 +40,7 @@ CurlHandle::CurlHandle() { throw std::runtime_error(std::string("Unable to set curl write callback: ") + curl_easy_strerror(rcode)); } - rcode = curl_easy_setopt(handle_, CURLOPT_WRITEDATA, (void*)this); + rcode = curl_easy_setopt(handle_, CURLOPT_WRITEDATA, static_cast(this)); if (rcode != CURLE_OK) { tearDownHandle(); throw std::runtime_error(std::string("Unable to set curl write callback userdata: ") + diff --git a/test/mocks.h b/test/mocks.h index 461b3415..258e99d0 100644 --- a/test/mocks.h +++ b/test/mocks.h @@ -35,9 +35,10 @@ struct TestSpanData : public SpanData { struct MockSampler : public PrioritySampler { MockSampler() {} - bool discard(const SpanContext& context) const override { return discard_spans; } - OptionalSamplingPriority sample(const std::string& environment, const std::string& service, - uint64_t trace_id) const override { + bool discard(const SpanContext& /* context */) const override { return discard_spans; } + OptionalSamplingPriority sample(const std::string& /* environment */, + const std::string& /* service */, + uint64_t /* trace_id */) const override { if (sampling_priority == nullptr) { return nullptr; } @@ -73,8 +74,7 @@ struct MockBuffer : public WritingSpanBuffer { MockBuffer() : WritingSpanBuffer(std::make_shared(std::make_shared())){}; - void unbufferAndWriteTrace(uint64_t trace_id, - const std::shared_ptr& sampler) override { + void unbufferAndWriteTrace(uint64_t /* trace_id */) { // Haha NOPE. // Leave the trace inside the traces map instead of deleting it. } @@ -215,7 +215,7 @@ struct MockTextMapCarrier : ot::TextMapReader, ot::TextMapWriter { return {}; } - ot::expected LookupKey(ot::string_view key) const override { + ot::expected LookupKey(ot::string_view /* key */) const override { return ot::make_unexpected(ot::lookup_key_not_supported_error); } diff --git a/test/sample_test.cpp b/test/sample_test.cpp index e92a42fe..07c94ebf 100644 --- a/test/sample_test.cpp +++ b/test/sample_test.cpp @@ -11,8 +11,12 @@ using namespace datadog::opentracing; TEST_CASE("sample") { - int id = 100; // Starting span id. - std::tm start{0, 0, 0, 12, 2, 107}; // Starting calendar time 2007-03-12 00:00:00 + int id = 100; // Starting span id. + // Starting calendar time 2007-03-12 00:00:00 + std::tm start{}; + start.tm_mday = 12; + start.tm_mon = 2; + start.tm_year = 107; TimePoint time{std::chrono::system_clock::from_time_t(timegm(&start)), std::chrono::steady_clock::time_point{}}; auto buffer = std::make_shared(); @@ -133,7 +137,7 @@ TEST_CASE("priority sampler unit test") { REQUIRE(((*p == SamplingPriority::SamplerKeep) || (*p == SamplingPriority::SamplerDrop))); count_sampled += *p == SamplingPriority::SamplerKeep ? 1 : 0; } - double sample_rate = count_sampled / (double)total; + double sample_rate = count_sampled / static_cast(total); REQUIRE((sample_rate < 0.85 && sample_rate > 0.75)); // Case 2, service:nginx,env:prod => 0.2 count_sampled = 0; @@ -144,7 +148,7 @@ TEST_CASE("priority sampler unit test") { REQUIRE(((*p == SamplingPriority::SamplerKeep) || (*p == SamplingPriority::SamplerDrop))); count_sampled += *p == SamplingPriority::SamplerKeep ? 1 : 0; } - sample_rate = count_sampled / (double)total; + sample_rate = count_sampled / static_cast(total); REQUIRE((sample_rate < 0.85 && sample_rate > 0.75)); } } @@ -225,7 +229,7 @@ TEST_CASE("priority sampler \"integration\" test") { REQUIRE(((*p == SamplingPriority::SamplerKeep) || (*p == SamplingPriority::SamplerDrop))); count_sampled += *p == SamplingPriority::SamplerKeep ? 1 : 0; } - double sample_rate = count_sampled / (double)total; + double sample_rate = count_sampled / static_cast(total); REQUIRE((sample_rate < 0.35 && sample_rate > 0.25)); } } diff --git a/test/span_test.cpp b/test/span_test.cpp index 2e12eddd..311ff395 100644 --- a/test/span_test.cpp +++ b/test/span_test.cpp @@ -11,8 +11,12 @@ using namespace datadog::opentracing; using json = nlohmann::json; TEST_CASE("span") { - int id = 100; // Starting span id. - std::tm start{0, 0, 0, 12, 2, 107}; // Starting calendar time 2007-03-12 00:00:00 + int id = 100; // Starting span id. + // Starting calendar time 2007-03-12 00:00:00 + std::tm start{}; + start.tm_mday = 12; + start.tm_mon = 2; + start.tm_year = 107; TimePoint time{std::chrono::system_clock::from_time_t(timegm(&start)), std::chrono::steady_clock::time_point{}}; auto sampler = std::make_shared(); diff --git a/test/tracer_factory_test.cpp b/test/tracer_factory_test.cpp index 248a75d6..d0443e1e 100644 --- a/test/tracer_factory_test.cpp +++ b/test/tracer_factory_test.cpp @@ -12,41 +12,47 @@ using namespace datadog::opentracing; struct MockTracer : public ot::Tracer { TracerOptions opts; - MockTracer(TracerOptions opts_, std::shared_ptr &writer, - std::shared_ptr sampler) + MockTracer(TracerOptions opts_, std::shared_ptr & /* writer */, + std::shared_ptr /* sampler */) : opts(opts_) {} - std::unique_ptr StartSpanWithOptions(ot::string_view operation_name, - const ot::StartSpanOptions &options) const + std::unique_ptr StartSpanWithOptions(ot::string_view /* operation_name */, + const ot::StartSpanOptions & /* options */) const noexcept override { return nullptr; } - ot::expected Inject(const ot::SpanContext &sc, std::ostream &writer) const override { + // This is here to avoid a warning about hidden overloaded-virtual methods. + using ot::Tracer::Extract; + using ot::Tracer::Inject; + + ot::expected Inject(const ot::SpanContext & /* sc */, + std::ostream & /* writer */) const override { return ot::make_unexpected(ot::invalid_carrier_error); } - ot::expected Inject(const ot::SpanContext &sc, - const ot::TextMapWriter &writer) const override { + ot::expected Inject(const ot::SpanContext & /* sc */, + const ot::TextMapWriter & /* writer */) const override { return ot::make_unexpected(ot::invalid_carrier_error); } - ot::expected Inject(const ot::SpanContext &sc, - const ot::HTTPHeadersWriter &writer) const override { + ot::expected Inject(const ot::SpanContext & /* sc */, + const ot::HTTPHeadersWriter & /* writer */) const override { return ot::make_unexpected(ot::invalid_carrier_error); } - ot::expected> Extract(std::istream &reader) const override { + ot::expected> Extract( + std::istream & /* reader */) const override { return ot::make_unexpected(ot::invalid_carrier_error); } ot::expected> Extract( - const ot::TextMapReader &reader) const override { + const ot::TextMapReader & /* reader */) const override { return ot::make_unexpected(ot::invalid_carrier_error); } ot::expected> Extract( - const ot::HTTPHeadersReader &reader) const override { + const ot::HTTPHeadersReader & /* reader */) const override { return ot::make_unexpected(ot::invalid_carrier_error); } diff --git a/test/tracer_test.cpp b/test/tracer_test.cpp index 90906bf4..fd06c0c4 100644 --- a/test/tracer_test.cpp +++ b/test/tracer_test.cpp @@ -9,8 +9,12 @@ using namespace datadog::opentracing; TEST_CASE("tracer") { - int id = 100; // Starting span id. - std::tm start{0, 0, 0, 12, 2, 107}; // Starting calendar time 2007-03-12 00:00:00 + int id = 100; // Starting span id. + // Starting calendar time 2007-03-12 00:00:00 + std::tm start{}; + start.tm_mday = 12; + start.tm_mon = 2; + start.tm_year = 107; TimePoint time{std::chrono::system_clock::from_time_t(timegm(&start)), std::chrono::steady_clock::time_point{}}; auto buffer = std::make_shared();