diff --git a/cpp/cmake_modules/FindThrift.cmake b/cpp/cmake_modules/FindThrift.cmake index 5d195844e20..07028971d9f 100644 --- a/cpp/cmake_modules/FindThrift.cmake +++ b/cpp/cmake_modules/FindThrift.cmake @@ -47,9 +47,17 @@ endfunction(EXTRACT_THRIFT_VERSION) if(MSVC_TOOLCHAIN AND NOT DEFINED THRIFT_MSVC_LIB_SUFFIX) if(NOT ARROW_THRIFT_USE_SHARED) if(ARROW_USE_STATIC_CRT) - set(THRIFT_MSVC_LIB_SUFFIX "mt") + if("${CMAKE_BUILD_TYPE}" STREQUAL "DEBUG") + set(THRIFT_MSVC_LIB_SUFFIX "mtd") + else() + set(THRIFT_MSVC_LIB_SUFFIX "mt") + endif() else() - set(THRIFT_MSVC_LIB_SUFFIX "md") + if("${CMAKE_BUILD_TYPE}" STREQUAL "DEBUG") + set(THRIFT_MSVC_LIB_SUFFIX "mdd") + else() + set(THRIFT_MSVC_LIB_SUFFIX "md") + endif() endif() endif() endif() diff --git a/cpp/src/arrow/compute/exec/exec_plan.cc b/cpp/src/arrow/compute/exec/exec_plan.cc index bb197f8db84..a23d6d5bae6 100644 --- a/cpp/src/arrow/compute/exec/exec_plan.cc +++ b/cpp/src/arrow/compute/exec/exec_plan.cc @@ -85,9 +85,11 @@ struct ExecPlanImpl : public ExecPlan { #ifdef ARROW_WITH_OPENTELEMETRY if (HasMetadata()) { auto pairs = metadata().get()->sorted_pairs(); + opentelemetry::nostd::shared_ptr span = + ::arrow::internal::tracing::UnwrapSpan(span_.details.get()); std::for_each(std::begin(pairs), std::end(pairs), - [this](std::pair const& pair) { - span_.Get().span->SetAttribute(pair.first, pair.second); + [span](std::pair const& pair) { + span->SetAttribute(pair.first, pair.second); }); } #endif diff --git a/cpp/src/arrow/compute/exec/hash_join.cc b/cpp/src/arrow/compute/exec/hash_join.cc index 63d9522c443..391c8b2cd4b 100644 --- a/cpp/src/arrow/compute/exec/hash_join.cc +++ b/cpp/src/arrow/compute/exec/hash_join.cc @@ -29,6 +29,7 @@ #include "arrow/compute/exec/key_hash.h" #include "arrow/compute/exec/task_util.h" #include "arrow/compute/kernels/row_encoder.h" +#include "arrow/util/tracing_internal.h" namespace arrow { namespace compute { diff --git a/cpp/src/arrow/compute/exec/hash_join.h b/cpp/src/arrow/compute/exec/hash_join.h index 9739cbc6436..63b6e51f1d3 100644 --- a/cpp/src/arrow/compute/exec/hash_join.h +++ b/cpp/src/arrow/compute/exec/hash_join.h @@ -28,7 +28,7 @@ #include "arrow/result.h" #include "arrow/status.h" #include "arrow/type.h" -#include "arrow/util/tracing_internal.h" +#include "arrow/util/tracing.h" namespace arrow { namespace compute { diff --git a/cpp/src/arrow/compute/exec/hash_join_node.cc b/cpp/src/arrow/compute/exec/hash_join_node.cc index e47d6095542..c8e6c6ffb2f 100644 --- a/cpp/src/arrow/compute/exec/hash_join_node.cc +++ b/cpp/src/arrow/compute/exec/hash_join_node.cc @@ -28,6 +28,7 @@ #include "arrow/util/future.h" #include "arrow/util/make_unique.h" #include "arrow/util/thread_pool.h" +#include "arrow/util/tracing_internal.h" namespace arrow { diff --git a/cpp/src/arrow/compute/function.cc b/cpp/src/arrow/compute/function.cc index 2b3d4e6feb9..fd80de4dbb3 100644 --- a/cpp/src/arrow/compute/function.cc +++ b/cpp/src/arrow/compute/function.cc @@ -215,6 +215,7 @@ Result Function::Execute(const std::vector& args, } util::tracing::Span span; + START_COMPUTE_SPAN(span, name(), {{"function.name", name()}, {"function.options", options ? options->ToString() : ""}, diff --git a/cpp/src/arrow/compute/light_array_test.cc b/cpp/src/arrow/compute/light_array_test.cc index 3f6d4780352..dcc7841a091 100644 --- a/cpp/src/arrow/compute/light_array_test.cc +++ b/cpp/src/arrow/compute/light_array_test.cc @@ -18,6 +18,7 @@ #include "arrow/compute/light_array.h" #include +#include #include "arrow/compute/exec/test_util.h" #include "arrow/testing/generator.h" diff --git a/cpp/src/arrow/util/tracing.cc b/cpp/src/arrow/util/tracing.cc index b8bddcd5052..8bf21f688c4 100644 --- a/cpp/src/arrow/util/tracing.cc +++ b/cpp/src/arrow/util/tracing.cc @@ -16,30 +16,28 @@ // under the License. #include "arrow/util/tracing.h" + +#include "arrow/util/config.h" #include "arrow/util/make_unique.h" #include "arrow/util/tracing_internal.h" namespace arrow { + +using internal::make_unique; namespace util { namespace tracing { #ifdef ARROW_WITH_OPENTELEMETRY -Span::Impl& Span::Set(const Impl& impl) { - inner_impl.reset(new Impl(impl)); - return *inner_impl; -} +Span::Span() noexcept { details = make_unique<::arrow::internal::tracing::SpanImpl>(); } -Span::Impl& Span::Set(Impl&& impl) { - inner_impl.reset(new Impl(std::move(impl))); - return *inner_impl; +#else + +Span::Span() noexcept { /* details is left a nullptr */ } #endif -// Default destructor when impl type is complete. -Span::~Span() = default; - } // namespace tracing } // namespace util } // namespace arrow diff --git a/cpp/src/arrow/util/tracing.h b/cpp/src/arrow/util/tracing.h index 15f7fca1eee..c6968219b6e 100644 --- a/cpp/src/arrow/util/tracing.h +++ b/cpp/src/arrow/util/tracing.h @@ -19,49 +19,21 @@ #include -#include "arrow/util/logging.h" +#include "arrow/util/visibility.h" namespace arrow { - -namespace internal { -namespace tracing { - -// Forward declaration SpanImpl. -class SpanImpl; - -} // namespace tracing -} // namespace internal - namespace util { namespace tracing { -class ARROW_EXPORT Span { +class ARROW_EXPORT SpanDetails { public: - using Impl = arrow::internal::tracing::SpanImpl; - - Span() = default; // Default constructor. The inner_impl is a nullptr. - ~Span(); // Destructor. Default destructor defined in tracing.cc where impl is a - // complete type. - - Impl& Set(const Impl&); - Impl& Set(Impl&&); - - const Impl& Get() const { - ARROW_CHECK(inner_impl) - << "Attempted to dereference a null pointer. Use Span::Set before " - "dereferencing."; - return *inner_impl; - } - - Impl& Get() { - ARROW_CHECK(inner_impl) - << "Attempted to dereference a null pointer. Use Span::Set before " - "dereferencing."; - return *inner_impl; - } + virtual ~SpanDetails() {} +}; - private: - std::unique_ptr inner_impl; +class ARROW_EXPORT Span { + public: + Span() noexcept; + std::unique_ptr details; }; } // namespace tracing diff --git a/cpp/src/arrow/util/tracing_internal.cc b/cpp/src/arrow/util/tracing_internal.cc index 904a1fd76a8..668a2aaba8b 100644 --- a/cpp/src/arrow/util/tracing_internal.cc +++ b/cpp/src/arrow/util/tracing_internal.cc @@ -202,14 +202,38 @@ opentelemetry::trace::Tracer* GetTracer() { return tracer.get(); } -#ifdef ARROW_WITH_OPENTELEMETRY +opentelemetry::nostd::shared_ptr& UnwrapSpan( + ::arrow::util::tracing::SpanDetails* span) { + SpanImpl* span_impl = checked_cast(span); + ARROW_CHECK(span_impl->ot_span) + << "Attempted to dereference a null pointer. Use Span::Set before " + "dereferencing."; + return span_impl->ot_span; +} + +const opentelemetry::nostd::shared_ptr& UnwrapSpan( + const ::arrow::util::tracing::SpanDetails* span) { + const SpanImpl* span_impl = checked_cast(span); + ARROW_CHECK(span_impl->ot_span) + << "Attempted to dereference a null pointer. Use Span::Set before " + "dereferencing."; + return span_impl->ot_span; +} + +opentelemetry::nostd::shared_ptr& RewrapSpan( + ::arrow::util::tracing::SpanDetails* span, + opentelemetry::nostd::shared_ptr ot_span) { + SpanImpl* span_impl = checked_cast(span); + span_impl->ot_span = std::move(ot_span); + return span_impl->ot_span; +} + opentelemetry::trace::StartSpanOptions SpanOptionsWithParent( const util::tracing::Span& parent_span) { opentelemetry::trace::StartSpanOptions options; - options.parent = parent_span.Get().span->GetContext(); + options.parent = UnwrapSpan(parent_span.details.get())->GetContext(); return options; } -#endif } // namespace tracing } // namespace internal diff --git a/cpp/src/arrow/util/tracing_internal.h b/cpp/src/arrow/util/tracing_internal.h index d0d6062e6e2..2898fd245fb 100644 --- a/cpp/src/arrow/util/tracing_internal.h +++ b/cpp/src/arrow/util/tracing_internal.h @@ -106,48 +106,63 @@ AsyncGenerator PropagateSpanThroughAsyncGenerator(AsyncGenerator wrapped) return PropagateSpanThroughAsyncGenerator(std::move(wrapped), std::move(span)); } -class SpanImpl { +class SpanImpl : public ::arrow::util::tracing::SpanDetails { public: - opentelemetry::nostd::shared_ptr span; + ~SpanImpl() override = default; + opentelemetry::nostd::shared_ptr ot_span; }; +opentelemetry::nostd::shared_ptr& UnwrapSpan( + ::arrow::util::tracing::SpanDetails* span); + +const opentelemetry::nostd::shared_ptr& UnwrapSpan( + const ::arrow::util::tracing::SpanDetails* span); + +opentelemetry::nostd::shared_ptr& RewrapSpan( + ::arrow::util::tracing::SpanDetails* span, + opentelemetry::nostd::shared_ptr ot_span); + opentelemetry::trace::StartSpanOptions SpanOptionsWithParent( const util::tracing::Span& parent_span); -#define START_SPAN(target_span, ...) \ - auto opentelemetry_scope##__LINE__ = \ - ::arrow::internal::tracing::GetTracer()->WithActiveSpan( \ - target_span \ - .Set(::arrow::util::tracing::Span::Impl{ \ - ::arrow::internal::tracing::GetTracer()->StartSpan(__VA_ARGS__)}) \ - .span) - -#define START_SPAN_WITH_PARENT(target_span, parent_span, ...) \ - auto opentelemetry_scope##__LINE__ = \ - ::arrow::internal::tracing::GetTracer()->WithActiveSpan( \ - target_span \ - .Set(::arrow::util::tracing::Span::Impl{ \ - ::arrow::internal::tracing::GetTracer()->StartSpan( \ - __VA_ARGS__, \ - ::arrow::internal::tracing::SpanOptionsWithParent(parent_span))}) \ - .span) - -#define START_COMPUTE_SPAN(target_span, ...) \ - START_SPAN(target_span, __VA_ARGS__); \ - target_span.Get().span->SetAttribute( \ - "arrow.memory_pool_bytes", ::arrow::default_memory_pool()->bytes_allocated()) +#define START_SPAN(target_span, ...) \ + auto opentelemetry_scope##__LINE__ = \ + ::arrow::internal::tracing::GetTracer()->WithActiveSpan( \ + ::arrow::internal::tracing::RewrapSpan( \ + target_span.details.get(), \ + ::arrow::internal::tracing::GetTracer()->StartSpan(__VA_ARGS__))) + +#define START_SPAN_WITH_PARENT(target_span, parent_span, ...) \ + auto opentelemetry_scope##__LINE__ = \ + ::arrow::internal::tracing::GetTracer()->WithActiveSpan( \ + ::arrow::internal::tracing::RewrapSpan( \ + target_span.details.get(), \ + \ + ::arrow::internal::tracing::GetTracer()->StartSpan( \ + __VA_ARGS__, \ + ::arrow::internal::tracing::SpanOptionsWithParent(parent_span)))) + +#define START_COMPUTE_SPAN(target_span, ...) \ + START_SPAN(target_span, __VA_ARGS__); \ + ::arrow::internal::tracing::UnwrapSpan(target_span.details.get()) \ + ->SetAttribute("arrow.memory_pool_bytes", \ + ::arrow::default_memory_pool()->bytes_allocated()) #define START_COMPUTE_SPAN_WITH_PARENT(target_span, parent_span, ...) \ START_SPAN_WITH_PARENT(target_span, parent_span, __VA_ARGS__); \ - target_span.Get().span->SetAttribute( \ - "arrow.memory_pool_bytes", ::arrow::default_memory_pool()->bytes_allocated()) + ::arrow::internal::tracing::UnwrapSpan(target_span.details.get()) \ + ->SetAttribute("arrow.memory_pool_bytes", \ + ::arrow::default_memory_pool()->bytes_allocated()) -#define EVENT(target_span, ...) target_span.Get().span->AddEvent(__VA_ARGS__) +#define EVENT(target_span, ...) \ + ::arrow::internal::tracing::UnwrapSpan(target_span.details.get())->AddEvent(__VA_ARGS__) -#define MARK_SPAN(target_span, status) \ - ::arrow::internal::tracing::MarkSpan(status, target_span.Get().span.get()) +#define MARK_SPAN(target_span, status) \ + ::arrow::internal::tracing::MarkSpan( \ + status, ::arrow::internal::tracing::UnwrapSpan(target_span.details.get()).get()) -#define END_SPAN(target_span) target_span.Get().span->End() +#define END_SPAN(target_span) \ + ::arrow::internal::tracing::UnwrapSpan(target_span.details.get())->End() #define END_SPAN_ON_FUTURE_COMPLETION(target_span, target_future, target_capture) \ target_future = target_future.Then( \