circular_buffer.h fails to compile on Win32 because of a conversion from uint64_t to size_t

[BuildMan-Alpha]

Describe your environment
Win32 Compile using Visual Studio 2019 (16.10.0)

Steps to reproduce
Run CMake with -G "Visual Studio 16 2019" -A "Win32" "-DBUILD_TESTING=OFF" "c:\dev\AA3rdPartyExt\opentelemetry-cpp" "-DCMAKE_TOOLCHAIN_FILE=C:\dev\vcpkg\scripts\buildsystems\vcpkg.cmake" "-DCMAKE_PREFIX_PATH=c:\dev\AA3rdPartyExt\Build\nlohmann_json\VS16\Win32;c:\dev\AA3rdPartyExt\Build\googletest\VS16\Win32" -DCMAKE_POSITION_INDEPENDENT_CODE=ON -DWITH_PROMETHEUS=ON

What is the expected behavior?
Clean compile

What is the actual behavior?
C:\dev\AA3rdPartyExt\opentelemetry-cpp\sdk\include\opentelemetry/sdk/common/circular_buffer.h(146,17): warning C4244: 'return': conversion from 'uint64_t' to 'size_t', possible loss of data [C:\dev\AA3rdPartyExt\Build\opentelemetry-cpp\VS16\Win32\sdk\src\trace\opentelemetry_trace.vcxproj]
C:\dev\AA3rdPartyExt\opentelemetry-cpp\sdk\include\opentelemetry/sdk/common/circular_buffer.h(142): message : while compiling class template member function 'size_t opentelemetry::v0::sdk::common::CircularBufferopentelemetry::v0::sdk::trace::Recordable::size(void) noexcept const' [C:\dev\AA3rdPartyExt\Build\opentelemetry-cpp\VS16\Win32\sdk\src\trace\opentelemetry_trace.vcxproj]
C:\dev\AA3rdPartyExt\opentelemetry-cpp\sdk\src\trace\batch_span_processor.cc(51): message : see reference to function template instantiation 'size_t opentelemetry::v0::sdk::common::CircularBufferopentelemetry::v0::sdk::trace::Recordable::size(void) noexcept const' being compiled [C:\dev\AA3rdPartyExt\Build\opentelemetry-cpp\VS16\Win32\sdk\src\trace\opentelemetry_trace.vcxproj]
C:\dev\AA3rdPartyExt\opentelemetry-cpp\sdk\include\opentelemetry/sdk/trace/batch_span_processor.h(145): message : see reference to class template instantiation 'opentelemetry::v0::sdk::common::CircularBufferopentelemetry::v0::sdk::trace::Recordable' being compiled [C:\dev\AA3rdPartyExt\Build\opentelemetry-cpp\VS16\Win32\sdk\src\trace\opentelemetry_trace.vcxproj]
C:\dev\AA3rdPartyExt\opentelemetry-cpp\sdk\include\opentelemetry/sdk/common/circular_buffer.h(96,17): warning C4244: 'argument': conversion from 'uint64_t' to 'size_t', possible loss of data [C:\dev\AA3rdPartyExt\Build\opentelemetry-cpp\VS16\Win32\sdk\src\trace\opentelemetry_trace.vcxproj]
C:\dev\AA3rdPartyExt\opentelemetry-cpp\sdk\include\opentelemetry/sdk/common/circular_buffer.h(83): message : while compiling class template member function 'bool opentelemetry::v0::sdk::common::CircularBufferopentelemetry::v0::sdk::trace::Recordable::Add(std::unique_ptr<opentelemetry::v0::sdk::trace::Recordable,std::default_deleteopentelemetry::v0::sdk::trace::Recordable> &) noexcept' [C:\dev\AA3rdPartyExt\Build\opentelemetry-cpp\VS16\Win32\sdk\src\trace\opentelemetry_trace.vcxproj]
C:\dev\AA3rdPartyExt\opentelemetry-cpp\sdk\src\trace\batch_span_processor.cc(44): message : see reference to function template instantiation 'bool opentelemetry::v0::sdk::common::CircularBufferopentelemetry::v0::sdk::trace::Recordable::Add(std::unique_ptr<opentelemetry::v0::sdk::trace::Recordable,std::default_deleteopentelemetry::v0::sdk::trace::Recordable> &) noexcept' being compiled [C:\dev\AA3rdPartyExt\Build\opentelemetry-cpp\VS16\Win32\sdk\src\trace\opentelemetry_trace.vcxproj]
C:\dev\AA3rdPartyExt\opentelemetry-cpp\sdk\include\opentelemetry/sdk/common/circular_buffer.h(112,15): warning C4244: 'argument': conversion from 'uint64_t' to 'size_t', possible loss of data [C:\dev\AA3rdPartyExt\Build\opentelemetry-cpp\VS16\Win32\sdk\src\trace\opentelemetry_trace.vcxproj]
C:\dev\AA3rdPartyExt\opentelemetry-cpp\sdk\include\opentelemetry/sdk/common/circular_buffer.h(177,73): error C2398: Element '2': conversion from 'uint64_t' to 'size_t' requires a narrowing conversion [C:\dev\AA3rdPartyExt\Build\opentelemetry-cpp\VS16\Win32\sdk\src\trace\opentelemetry_trace.vcxproj]
C:\dev\AA3rdPartyExt\opentelemetry-cpp\sdk\include\opentelemetry/sdk/common/circular_buffer.h(166): message : while compiling class template member function 'opentelemetry::v0::sdk::common::CircularBufferRange<opentelemetry::v0::sdk::common::AtomicUniquePtr> opentelemetry::v0::sdk::common::CircularBuffer::PeekImpl(void) noexcept' [C:\dev\AA3rdPartyExt\Build\opentelemetry-cpp\VS16\Win32\sdk\src\trace\opentelemetry_trace.vcxproj]
with
[
T=opentelemetry::v0::sdk::trace::Recordable
]
C:\dev\AA3rdPartyExt\opentelemetry-cpp\sdk\include\opentelemetry/sdk/common/circular_buffer.h(55): message : see reference to function template instantiation 'opentelemetry::v0::sdk::common::CircularBufferRange<opentelemetry::v0::sdk::common::AtomicUniquePtr> opentelemetry::v0::sdk::common::CircularBuffer::PeekImpl(void) noexcept' being compiled [C:\dev\AA3rdPartyExt\Build\opentelemetry-cpp\VS16\Win32\sdk\src\trace\opentelemetry_trace.vcxproj]
with
[
T=opentelemetry::v0::sdk::trace::Recordable
]
C:\dev\AA3rdPartyExt\opentelemetry-cpp\sdk\include\opentelemetry/sdk/common/circular_buffer.h(177,73): warning C4244: 'argument': conversion from 'uint64_t' to 'size_t', possible loss of data [C:\dev\AA3rdPartyExt\Build\opentelemetry-cpp\VS16\Win32\sdk\src\trace\opentelemetry_trace.vcxproj]
C:\dev\AA3rdPartyExt\opentelemetry-cpp\sdk\include\opentelemetry/sdk/common/circular_buffer.h(179,74): error C2398: Element '2': conversion from 'uint64_t' to 'size_t' requires a narrowing conversion [C:\dev\AA3rdPartyExt\Build\opentelemetry-cpp\VS16\Win32\sdk\src\trace\opentelemetry_trace.vcxproj]
C:\dev\AA3rdPartyExt\opentelemetry-cpp\sdk\include\opentelemetry/sdk/common/circular_buffer.h(179,74): warning C4244: 'argument': conversion from 'uint64_t' to 'size_t', possible loss of data [C:\dev\AA3rdPartyExt\Build\opentelemetry-cpp\VS16\Win32\sdk\src\trace\opentelemetry_trace.vcxproj]
C:\dev\AA3rdPartyExt\opentelemetry-cpp\sdk\include\opentelemetry/sdk/common/circular_buffer.h(180,51): error C2398: Element '2': conversion from 'uint64_t' to 'size_t' requires a narrowing conversion [C:\dev\AA3rdPartyExt\Build\opentelemetry-cpp\VS16\Win32\sdk\src\trace\opentelemetry_trace.vcxproj]
C:\dev\AA3rdPartyExt\opentelemetry-cpp\sdk\include\opentelemetry/sdk/common/circular_buffer.h(180,51): warning C4244: 'argument': conversion from 'uint64_t' to 'size_t', possible loss of data [C:\dev\AA3rdPartyExt\Build\opentelemetry-cpp\VS16\Win32\sdk\src\trace\opentelemetry_trace.vcxproj]
Additional context

[maxgolov]

OpenTelemetry C++ SDK v1.0 will not have support for Metrics. These features are experimental and were turned off by WITH_METRICS_PREVIEW=OFF, which in turn does not enable WITH_PROMETHEUS. Please compile without prometheus. We'd be able to tackle this after v1.0 GA.

@lalitb

[maxgolov]

Re. Win32 vs x64 issue - please use x64 for now. We'd address the x86 on Windows promptly (without prometheus support though!).

[ThomsonTan]

@maxgolov is prometheus 64-bit only?

[maxgolov]

@ThomsonTan - no, it appears like Visual Studio 2019 is rather strict with type conversion. There's common issue with Win32.

All of our CI so far has been x64 only (for Linux, Mac, Windows). And we never caught a few issues in common core code with Win32 / x86... Some of these are new warnings, that were quite lax in older version of the compiler.

I think there are two separate tasks here:

• P1: clean-up x86 building without Prometheus, without Metrics, without Logs Preview. This has to happen for v1.0 GA, because we do not deliver Prometheus for v1.

• P∞: clean-up x86 for the rest of optional code, including experimental + Prometheus. That is after v1.0 by the end of this year. I expect there would be more clean-up needed in additional things we are not intending to release for v1 ( for WITH_METRICS_PREVIEW , WITH_LOGS_PREVIEW , WITH_PROMETHEUS ).

I'll send out the fix for basic / common core components for Win32 / x86. Almost done. Again - cleaning up the core only for Win32, without metrics / prometheus pieces. It also needs a few patches to CMakeLists.txt to auto-install the missing deps via vcpkg. The scripts already support ARCH variable to install the proper parts, just need to pass the right variable (ARCH=x86) in function(install_windows_deps) , that'll satisfy the deps even if building Win32 configuration via Visual Studio 2019 IDE.

[BuildMan-Alpha]

Thanks Max! Looking forward to metrics being added. From: 'Max Golovanov' via GithubAdmin ***@***.*** Sent: Thursday, June 10, 2021 6:40 PM To: open-telemetry/opentelemetry-cpp Cc: BuildMan-Alpha; Author Subject: Re: [open-telemetry/opentelemetry-cpp] circular_buffer.h fails to compile on Win32 because of a conversion from uint64_t to size_t (#842) OpenTelemetry C++ SDK v1.0 will not have support for Metrics. These features are experimental and were turned off by WITH_METRICS_PREVIEW=OFF, which in turn does not enable WITH_PROMETHEUS. Please compile without prometheus. We'd be able to tackle this after v1.0 GA. @lalitb <https://github.com/lalitb> — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#842 (comment)> , or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAOEBJQGOMBWKQ3SEPAQ7ALTSE5NJANCNFSM46OMVCHA> . <https://github.com/notifications/beacon/AAOEBJTBUS75WVHEEUFST7LTSE5NJA5CNFSM46OMVCHKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOGM2UP6Q.gif>

[BuildMan-Alpha]

Thanks Max! 32-bit isn’t critical for this project as we only build that to integrate with an existing 32-bit client environment. We do have some 32-bit server code that could really use opentelemetry-cpp though . Sadly, moving that large code base to 64-bit is work that is ahead of us. From: 'Max Golovanov' via GithubAdmin ***@***.*** Sent: Thursday, June 10, 2021 7:17 PM To: open-telemetry/opentelemetry-cpp Cc: BuildMan-Alpha; Author Subject: Re: [open-telemetry/opentelemetry-cpp] circular_buffer.h fails to compile on Win32 because of a conversion from uint64_t to size_t (#842) Re. Win32 vs x64 issue - please use x64 for now. We'd address the x86 on Windows promptly (without prometheus support though!). — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#842 (comment)> , or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAOEBJR5U2YPGY5MPOS5PWTTSFBWVANCNFSM46OMVCHA> . <https://github.com/notifications/beacon/AAOEBJXCL2BVAEZHSUYWTJDTSFBWVA5CNFSM46OMVCHKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOGM2Y3FI.gif>

[maxgolov]

@BuildMan-Alpha - we'll make the core work well with 32-bit before July 1st. All core tests work and pass with the fix I sent for review. #847 ... Once we get there, we'll set up the build loop for Win32 / x86, and incrementally add the other components, including Logs and Metrics. I hit some minor issue with Prometheus (Metrics) on Win32. Thus, I want to take staged approach. Fixing core first. Then fixing Metrics right after.