Merge branch 'main' into FixJaegerIds

Author: lalitb

.bazelrc

@@ -19,3 +19,6 @@ common:tsan --copt -fsanitize=thread
 common:tsan --copt -DTHREAD_SANITIZER
 common:tsan --linkopt -fsanitize=thread
 common:tsan --cc_output_directory_tag=tsan
+# This is needed to address false positive problem with abseil.The same setting as gRPC
+# https://github.com/google/sanitizers/issues/953
+common:tsan --test_env=TSAN_OPTIONS=report_atomic_races=0

CMakeLists.txt

@@ -50,10 +50,10 @@ option(WITH_STL "Whether to use Standard Library for C++latest features" OFF)
 option(WITH_ABSEIL "Whether to use Abseil for C++latest features" OFF)
 
 if(WITH_ABSEIL)
-  add_definitions(-DHAVE_ABSEIL)
   find_package(absl CONFIG REQUIRED)
 
-  set(CORE_RUNTIME_LIBS absl::any absl::base absl::bits absl::city)
+  set(CORE_RUNTIME_LIBS absl::bad_variant_access absl::any absl::base
+                        absl::bits absl::city)
 
   # target_link_libraries(main PRIVATE absl::any absl::base absl::bits
   # absl::city)
@@ -67,7 +67,7 @@ if(WITH_STL)
   add_definitions(-DHAVE_CPP_STDLIB)
   add_definitions(-DHAVE_GSL)
   # Require at least C++17. C++20 is needed to avoid gsl::span
-  if(CMAKE_MINOR_VERSION VERSION_GREATER "3.18")
+  if(CMAKE_VERSION VERSION_GREATER 3.18.0)
     # Ask for 20, may get anything below
     set(CMAKE_CXX_STANDARD 20)
   else()
@@ -92,8 +92,7 @@ if(WITH_STL)
         set(MSVC_CXX_OPT_FLAG "/O2")
       endif()
     endif()
-    set(CMAKE_CXX_FLAGS
-        "${CMAKE_CXX_FLAGS} /Zc:__cplusplus ${MSVC_CXX_OPT_FLAG}")
+    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${MSVC_CXX_OPT_FLAG}")
   endif()
 endif()
 
@@ -168,8 +167,10 @@ if(MSVC)
   # Options for Visual C++ compiler: /Zc:__cplusplus - report an updated value
   # for recent C++ language standards. Without this option MSVC returns the
   # value of __cplusplus="199711L"
-  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /Zc:__cplusplus")
-
+  if(MSVC_VERSION GREATER 1900)
+    # __cplusplus flag is not supported by Visual Studio 2015
+    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /Zc:__cplusplus")
+  endif()
   # When using vcpkg, all targets build with the same runtime
   if(VCPKG_TOOLCHAIN)
     set(CMAKE_MSVC_RUNTIME_LIBRARY

api/CMakeLists.txt

@@ -41,6 +41,10 @@ if(BUILD_TESTING)
   add_subdirectory(test)
 endif()
 
+if(WITH_ABSEIL)
+  target_compile_definitions(opentelemetry_api INTERFACE HAVE_ABSEIL)
+endif()
+
 if(WITH_STL)
   message("Building with standard library types...")
   target_compile_definitions(opentelemetry_api INTERFACE HAVE_CPP_STDLIB

api/include/opentelemetry/nostd/variant.h

@@ -47,7 +47,7 @@ OPENTELEMETRY_END_NAMESPACE
 #  endif
 
 #  ifdef HAVE_ABSEIL
-#    include "absl/types/variant.h"
+#    include <absl/types/variant.h>
 #  else
 #    include "./absl/types/variant.h"
 #  endif

api/include/opentelemetry/trace/propagation/b3_propagator.h

@@ -139,11 +139,11 @@ class B3Propagator : public B3PropagatorExtractor
 
     char trace_identity[kTraceIdHexStrLength + kSpanIdHexStrLength + 3];
     static_assert(sizeof(trace_identity) == 51, "b3 trace identity buffer size mismatch");
-    span_context.trace_id().ToLowerBase16(
-        nostd::span<char, 2 * TraceId::kSize>{&trace_identity[0], kTraceIdHexStrLength});
+    span_context.trace_id().ToLowerBase16(nostd::span<char, 2 * TraceId::kSize>{
+        &trace_identity[0], static_cast<std::size_t>(kTraceIdHexStrLength)});
     trace_identity[kTraceIdHexStrLength] = '-';
     span_context.span_id().ToLowerBase16(nostd::span<char, 2 * SpanId::kSize>{
-        &trace_identity[kTraceIdHexStrLength + 1], kSpanIdHexStrLength});
+        &trace_identity[kTraceIdHexStrLength + 1], static_cast<std::size_t>(kSpanIdHexStrLength)});
     trace_identity[kTraceIdHexStrLength + kSpanIdHexStrLength + 1] = '-';
     trace_identity[kTraceIdHexStrLength + kSpanIdHexStrLength + 2] =
         span_context.trace_flags().IsSampled() ? '1' : '0';

api/test/nostd/variant_test.cc

@@ -27,6 +27,7 @@ TEST(VariantSizeTest, GetVariantSize)
   EXPECT_EQ((nostd::variant_size<nostd::variant<int, double>>::value), 2);
 }
 
+#if 0  // Disable this test for now. It does not compile with Visual Studio 2015.
 TEST(VariantAlternativeTest, GetVariantSize)
 {
   EXPECT_TRUE((std::is_same<nostd::variant_alternative_t<0, nostd::variant<int>>, int>::value));
@@ -35,6 +36,7 @@ TEST(VariantAlternativeTest, GetVariantSize)
   EXPECT_TRUE((std::is_same<nostd::variant_alternative_t<1, const nostd::variant<int, double>>,
                             const double>::value));
 }
+#endif
 
 TEST(VariantTest, Get)
 {

examples/grpc/client.cpp

@@ -1,12 +1,14 @@
+// Make sure to include GRPC headers first because otherwise Abseil may create
+// ambiguity with `nostd::variant` if compiled with Visual Studio 2015. Other
+// modern compilers are unaffected.
+#include <grpcpp/grpcpp.h>
+#include "messages.grpc.pb.h"
+
 #include "tracer_common.h"
 #include <iostream>
 #include <memory>
 #include <string>
 
-#include <grpcpp/grpcpp.h>
-
-#include "messages.grpc.pb.h"
-
 using grpc::Channel;
 using grpc::ClientContext;
 using grpc::ClientReader;

exporters/etw/include/opentelemetry/exporters/etw/etw_traceloggingdynamic.h

@@ -8,7 +8,8 @@
 #    endif
 #  endif
 #else
-#  ifdef HAVE_TLD
-#    include "TraceLoggingDynamic.h"
+#  ifndef HAVE_TLD
+#    define HAVE_TLD
 #  endif
+#  include "TraceLoggingDynamic.h"
 #endif

exporters/etw/include/opentelemetry/exporters/etw/etw_tracer.h

@@ -291,8 +291,12 @@ static inline bool CopySpanIdToActivityId(const trace::SpanContext &spanContext,
   {
     return false;
   }
-  auto spanId = spanContext.span_id().Id().data();
-  std::copy(spanId, spanId + 8, reinterpret_cast<uint8_t *>(&outGuid));
+  auto spanId      = spanContext.span_id().Id().data();
+  uint8_t *guidPtr = reinterpret_cast<uint8_t *>(&outGuid);
+  for (size_t i = 0; i < 8; i++)
+  {
+    guidPtr[i] = spanId[i];
+  }
   return true;
 };
 

exporters/etw/test/etw_perf_test.cc

@@ -14,9 +14,7 @@
 
 using namespace OPENTELEMETRY_NAMESPACE;
 
-using Properties       = opentelemetry::exporter::etw::Properties;
-using PropertyValue    = opentelemetry::exporter::etw::PropertyValue;
-using PropertyValueMap = opentelemetry::exporter::etw::PropertyValueMap;
+using namespace opentelemetry::exporter::etw;
 
 namespace
 {

exporters/etw/test/etw_tracer_test.cc

@@ -12,9 +12,7 @@
 
 using namespace OPENTELEMETRY_NAMESPACE;
 
-using Properties       = opentelemetry::exporter::etw::Properties;
-using PropertyValue    = opentelemetry::exporter::etw::PropertyValue;
-using PropertyValueMap = opentelemetry::exporter::etw::PropertyValueMap;
+using namespace opentelemetry::exporter::etw;
 
 std::string getTemporaryValue()
 {

sdk/include/opentelemetry/sdk/trace/multi_span_processor.h

@@ -148,7 +148,7 @@ class MultiSpanProcessor : public SpanProcessor
     ProcessorNode(std::unique_ptr<SpanProcessor> &&value,
                   ProcessorNode *prev = nullptr,
                   ProcessorNode *next = nullptr)
-        : value_(std::move(value)), prev_(prev), next_(next)
+        : value_(std::move(value)), next_(next), prev_(prev)
     {}
   };
 

sdk/src/common/core.cc

@@ -6,31 +6,3 @@
 #include "opentelemetry/version.h"
 #include "opentelemetry/nostd/variant.h"
 // clang-format on
-
-#if defined(HAVE_ABSEIL)
-/* The option of building and linking with Abseil library implies that Abseil
- * may already provide the ThrowBadVariantAccess implementation if its own.
- * Reconsider the implementation below: we are potentially introducing
- * a function that is already implemented in the Abseil. Most likely the code
- * below needs to be removed entirely.
- */
-#  if defined(__GNUC__) || defined(__GNUG__)
-#    ifndef __cdecl
-// see https://gcc.gnu.org/onlinedocs/gcc/x86-Function-Attributes.html
-// Intel x86 architecture specific calling conventions
-#      ifdef _M_IX86
-#        define __cdecl __attribute__((__cdecl__))
-#      else
-#        define __cdecl
-#      endif
-#    endif
-#  endif
-
-namespace absl
-{
-namespace variant_internal
-{
-void __cdecl ThrowBadVariantAccess(){/* TODO: std::terminate or re-throw? */};
-}
-}  // namespace absl
-#endif