[C++] ENH: enable building against system opentelemetry on windows

[h-vetinari]

Describe the enhancement requested

In #44982, I ran into some problems while trying to build arrow against a local opentelemetry on windows. Failure looks as follows

[115/935] Building CXX object src\arrow\CMakeFiles\arrow_util_shared.dir\util\thread_pool.cc.obj
FAILED: src/arrow/CMakeFiles/arrow_util_shared.dir/util/thread_pool.cc.obj 
C:\PROGRA~1\MICROS~2\2022\ENTERP~1\VC\Tools\MSVC\1442~1.344\bin\Hostx64\x64\cl.exe  /nologo /TP -DABSL_CONSUME_DLL -DARROW_EXPORTING -DARROW_HAVE_RUNTIME_AVX2 -DARROW_HAVE_RUNTIME_AVX512 -DARROW_HAVE_RUNTIME_BMI2 -DARROW_HAVE_RUNTIME_SSE4_2 -DBOOST_ALL_DYN_LINK -DBOOST_ALL_NO_LIB -DHAVE_ABSEIL -DHAVE_MSGPACK -DOPENTELEMETRY_ABI_VERSION_NO=1 -DPROTOBUF_USE_DLLS -DURI_STATIC_BUILD -DUSE_IMPORT_EXPORT -D_CRT_SECURE_NO_WARNINGS -D_ENABLE_EXTENDED_ALIGNED_STORAGE -I%SRC_DIR%\cpp\build\src -I%SRC_DIR%\cpp\src -I%SRC_DIR%\cpp\src\generated -external:I%PREFIX%\Library\include -external:W0 /DWIN32 /D_WINDOWS /GR /EHsc /D_SILENCE_TR1_NAMESPACE_DEPRECATION_WARNING  /EHsc /wd5105 /bigobj /utf-8 /W3 /wd4800 /wd4996 /wd4065  /O2 /Ob2 /DNDEBUG -std:c++17 -MD /showIncludes /Fosrc\arrow\CMakeFiles\arrow_util_shared.dir\util\thread_pool.cc.obj /Fdsrc\arrow\CMakeFiles\arrow_util_shared.dir\ /FS -c %SRC_DIR%\cpp\src\arrow\util\thread_pool.cc
%SRC_DIR%\cpp\src\arrow/util/async_generator.h(1636): warning C4003: not enough arguments for function-like macro invocation 'max'

I commented that this might have something to do with NOMINMAX; resp. that it only happens when enabling otel-support on windows.

Going back further (since I had been trying to enable otel-support for a long time), I had at some point needed the following patch

diff --git a/cpp/src/arrow/util/tracing_internal.h b/cpp/src/arrow/util/tracing_internal.h
index 6ed731599..79907cf0c 100644
--- a/cpp/src/arrow/util/tracing_internal.h
+++ b/cpp/src/arrow/util/tracing_internal.h
@@ -122,13 +122,13 @@ struct Scope {
   opentelemetry::trace::Scope scope_impl;
 };

-opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>& UnwrapSpan(
+ARROW_EXPORT opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>& UnwrapSpan(
     ::arrow::util::tracing::SpanDetails* span);

-const opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>& UnwrapSpan(
+ARROW_EXPORT const opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>& UnwrapSpan(
     const ::arrow::util::tracing::SpanDetails* span);

-opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>& RewrapSpan(
+ARROW_EXPORT opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>& RewrapSpan(
     ::arrow::util::tracing::SpanDetails* span,
     opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span> ot_span);

@kou said:

The ARROW_EXPORT/max problems are that our OpenTelemetry integration doesn't work on Windows. It doesn't depend on bundled opentelemetry-cpp nor system opentelemetry-cpp.

So this issue is for making arrow compatible with an existing otel on the system (no problem if this requires a very recent version)

Component(s)

C++, Packaging

[h-vetinari]

OK, now that azure got sorted I came back to this; with -DNOMINMAX in CXXFLAGS and the patch above, I got libarrow to build in conda-forge/arrow-cpp-feedstock#1924; however, the tests all fail with

E0000 00:00:1770442241.390220    6520 descriptor_database.cc:678] File already exists in database: opentelemetry/proto/common/v1/common.proto

This has already come up in conda-forge/arrow-cpp-feedstock#1048

@h-vetinari: it sounds like arrow shouldn't be re-loading the otel proto definitions?

@h-vetinari: To top things off, it looks like there's a third potential source for this (at least in conda-forge), in addition to opentelemetry & arrow: https://github.com/conda-forge/proto-opentelemetry-proto-feedstock

@xhochy: This happens if two libraries load the same protobuf definition in a different, conflicting version. We have the same problem with all the onnx-* packages. The workaround for this is build all (except at most one) with static protobuf so that each library gets its own protobuf (global) namespace.

@lidavidm: Hmm, something seems wrong. Arrow itself shouldn't be loading the OpenTelemetry Protobuf generated code. Either we actually built a bundled copy of OpenTelemetry (doesn't seem so) or we still statically linked OpenTelemetry somehow? (Shouldn't be possible?) Or possibly multiple OpenTelemetry libraries included the Protobuf generated code?

That problem was never solved AFAICT, because we ended up dropping windows from that PR. We do run into this problem in conda-forge occasionally, and I don't remember a durable solution (e.g. in conda-forge/tensorflow-feedstock#437, we gave up, c.f. this patch). In contrast to e.g. tensorflow (where we're not building for windows at all), the problem only occurs on windows, unix+otel works fine.

[raulcd]

I have come to something really similar enabling OpenTelemetry on PyArrow wheels:

• GH-49382: [Python] Enable OpenTelemetry on PyArrow wheels #49383

I was able to build OpenTelemetry on Windows wheels basically with a patch really similar to what is being proposed here. I am wondering about @kou 's comment about OpenTelemetry integration not working on Windows. Should I drop the patches for Windows on that PR?

[kou]

I just wanted to say that "let's fix missing ARROW_EXPORT and max conflict as a separated issue".

If GH-49383 fixes it, let's use it.