Merge pull request #18038 from ghalliday/issue30866

HPCC-30866 Remove fork() unsafe code from open-telemetry random number generator Reviewed-by: Mark Kelly [email protected] Merged-by: Gavin Halliday <[email protected]>
hpcc-systems · Nov 16, 2023 · e081c75 · e081c75
2 parents 11c0506 + bbb02b3
commit e081c75
Show file tree

Hide file tree

Showing 6 changed files with 272 additions and 0 deletions.
diff --git a/vcpkg_overlays/opentelemetry-cpp/add-missing-dependencies.patch b/vcpkg_overlays/opentelemetry-cpp/add-missing-dependencies.patch
@@ -0,0 +1,15 @@
+diff --git a/cmake/opentelemetry-proto.cmake b/cmake/opentelemetry-proto.cmake
+index 34b33d3..19e67e9 100644
+--- a/cmake/opentelemetry-proto.cmake
++++ b/cmake/opentelemetry-proto.cmake
+@@ -311,6 +311,10 @@ if(WITH_OTLP_GRPC)
+   endif()
+ endif()
+
++if(TARGET gRPC::grpc++)
++  target_link_libraries(opentelemetry_proto PUBLIC gRPC::grpc++)
++endif()
++
+ if(BUILD_SHARED_LIBS)
+   foreach(proto_target ${OPENTELEMETRY_PROTO_TARGETS})
+     set_property(TARGET ${proto_target} PROPERTY POSITION_INDEPENDENT_CODE ON)
diff --git a/vcpkg_overlays/opentelemetry-cpp/add-missing-find-dependency.patch b/vcpkg_overlays/opentelemetry-cpp/add-missing-find-dependency.patch
@@ -0,0 +1,13 @@
+diff --git a/cmake/opentelemetry-cpp-config.cmake.in b/cmake/opentelemetry-cpp-config.cmake.in
+index adae58d..2642772 100644
+--- a/cmake/opentelemetry-cpp-config.cmake.in
++++ b/cmake/opentelemetry-cpp-config.cmake.in
+@@ -69,6 +69,8 @@ set(OPENTELEMETRY_VERSION
+ # ##############################################################################
+
+ find_package(Threads)
++include(CMakeFindDependencyMacro)
++find_dependency(absl)
+
+ set_and_check(OPENTELEMETRY_CPP_INCLUDE_DIRS "@PACKAGE_INCLUDE_INSTALL_DIR@")
+ set_and_check(OPENTELEMETRY_CPP_LIBRARY_DIRS "@PACKAGE_CMAKE_INSTALL_LIBDIR@")
diff --git a/vcpkg_overlays/opentelemetry-cpp/hpcc-remove-unsafe-onfork.patch b/vcpkg_overlays/opentelemetry-cpp/hpcc-remove-unsafe-onfork.patch
@@ -0,0 +1,56 @@
+diff --git a/sdk/src/common/random.cc b/sdk/src/common/random.cc
+index 77b88cfa..14e94d0c 100644
+--- a/sdk/src/common/random.cc
++++ b/sdk/src/common/random.cc
+@@ -13,11 +13,10 @@ namespace sdk
+ {
+ namespace common
+ {
+-// Wraps a thread_local random number generator, but adds a fork handler so that
+-// the generator will be correctly seeded after forking.
+-//
+-// See https://stackoverflow.com/q/51882689/4447365 and
+-//     https://github.com/opentracing-contrib/nginx-opentracing/issues/52
++// Wraps a thread_local random number generator.
++// The previous fork handler is removed because it was not safe and was solving a problem that did
++// not need to be solved since there should be no logic in the child() before it calls exec().
++
+ namespace
+ {
+ class TlsRandomNumberGenerator
+@@ -26,17 +25,14 @@ class TlsRandomNumberGenerator
+   TlsRandomNumberGenerator() noexcept
+   {
+     Seed();
+-    platform::AtFork(nullptr, nullptr, OnFork);
+   }
+
+-  static FastRandomNumberGenerator &engine() noexcept { return engine_; }
++  FastRandomNumberGenerator & engine() noexcept { return engine_; }
+
+ private:
+-  static thread_local FastRandomNumberGenerator engine_;
+-
+-  static void OnFork() noexcept { Seed(); }
++  FastRandomNumberGenerator engine_;
+
+-  static void Seed() noexcept
++  void Seed() noexcept
+   {
+     std::random_device random_device;
+     std::seed_seq seed_seq{random_device(), random_device(), random_device(), random_device()};
+@@ -44,13 +40,12 @@ class TlsRandomNumberGenerator
+   }
+ };
+
+-thread_local FastRandomNumberGenerator TlsRandomNumberGenerator::engine_{};
+ }  // namespace
+
+ FastRandomNumberGenerator &Random::GetRandomNumberGenerator() noexcept
+ {
+   static thread_local TlsRandomNumberGenerator random_number_generator{};
+-  return TlsRandomNumberGenerator::engine();
++  return random_number_generator.engine();
+ }
+
+ uint64_t Random::GenerateRandom64() noexcept
diff --git a/vcpkg_overlays/opentelemetry-cpp/portfile.cmake b/vcpkg_overlays/opentelemetry-cpp/portfile.cmake
@@ -0,0 +1,75 @@
+if(VCPKG_TARGET_IS_WINDOWS)
+    vcpkg_check_linkage(ONLY_STATIC_LIBRARY)
+endif()
+
+vcpkg_from_github(
+    OUT_SOURCE_PATH SOURCE_PATH
+    REPO open-telemetry/opentelemetry-cpp
+    REF "v${VERSION}"
+    SHA512 86cf0320f9ee50bc1aa2b7a8b254fb0df25d1bd1f5f01ebc3630ab7fe2f6ca5e53ca8e042518b4e7096dbb102c0b880e9a25fcdf5f668d24ff57d9247237bf62
+    HEAD_REF main
+    PATCHES
+        # Use the compiler's default C++ version. Picking a version with
+        # CMAKE_CXX_STANDARD is not needed as the Abseil port already picked
+        # one and propagates that version across all its downstream deps.
+        use-default-cxx-version.patch
+        # When compiling code generated by gRPC we need to link the gRPC library
+        # too.
+        add-missing-dependencies.patch
+        # Missing find_dependency for Abseil
+        add-missing-find-dependency.patch
+        # HPCC-fix: Remove code that reinitialised the random number generator on fork()
+        hpcc-remove-unsafe-onfork.patch
+)
+
+vcpkg_check_features(OUT_FEATURE_OPTIONS FEATURE_OPTIONS
+    FEATURES
+        etw WITH_ETW
+        zipkin WITH_ZIPKIN
+        prometheus WITH_PROMETHEUS
+        elasticsearch WITH_ELASTICSEARCH
+        jaeger WITH_JAEGER
+        otlp WITH_OTLP
+        otlp-http WITH_OTLP_HTTP
+        zpages WITH_ZPAGES
+        otlp-grpc WITH_OTLP_GRPC
+)
+
+# opentelemetry-proto is a third party submodule and opentelemetry-cpp release did not pack it.
+if(WITH_OTLP)
+    set(OTEL_PROTO_VERSION "0.19.0")
+    vcpkg_download_distfile(ARCHIVE
+        URLS "https://github.com/open-telemetry/opentelemetry-proto/archive/v${OTEL_PROTO_VERSION}.tar.gz"
+        FILENAME "opentelemetry-proto-${OTEL_PROTO_VERSION}.tar.gz"
+        SHA512 b6d47aaa90ff934eb24047757d5fdb8a5be62963a49b632460511155f09a725937fb7535cf34f738b81cc799600adbbc3809442aba584d760891c0a1f0ce8c03
+    )
+
+    vcpkg_extract_source_archive(src ARCHIVE "${ARCHIVE}")
+    file(REMOVE_RECURSE "${SOURCE_PATH}/third_party/opentelemetry-proto")
+    file(COPY "${src}/." DESTINATION "${SOURCE_PATH}/third_party/opentelemetry-proto")
+    # Create empty .git directory to prevent opentelemetry from cloning it during build time
+    file(MAKE_DIRECTORY "${SOURCE_PATH}/third_party/opentelemetry-proto/.git")
+    list(APPEND FEATURE_OPTIONS -DCMAKE_CXX_STANDARD=14)
+    list(APPEND FEATURE_OPTIONS -DgRPC_CPP_PLUGIN_EXECUTABLE=${CURRENT_HOST_INSTALLED_DIR}/tools/grpc/grpc_cpp_plugin${VCPKG_HOST_EXECUTABLE_SUFFIX})
+endif()
+
+vcpkg_cmake_configure(
+    SOURCE_PATH "${SOURCE_PATH}"
+    OPTIONS
+        -DBUILD_TESTING=OFF
+        -DWITH_EXAMPLES=OFF
+        -DWITH_LOGS_PREVIEW=ON
+        -DOPENTELEMETRY_INSTALL=ON
+        -DWITH_ABSEIL=ON
+        ${FEATURE_OPTIONS}
+    MAYBE_UNUSED_VARIABLES
+        WITH_OTLP_GRPC
+)
+
+vcpkg_cmake_install()
+vcpkg_cmake_config_fixup(CONFIG_PATH lib/cmake/${PORT})
+vcpkg_copy_pdbs()
+
+file(REMOVE_RECURSE "${CURRENT_PACKAGES_DIR}/debug/include")
+file(REMOVE_RECURSE "${CURRENT_PACKAGES_DIR}/debug/share")
+vcpkg_install_copyright(FILE_LIST "${SOURCE_PATH}/LICENSE")
diff --git a/vcpkg_overlays/opentelemetry-cpp/use-default-cxx-version.patch b/vcpkg_overlays/opentelemetry-cpp/use-default-cxx-version.patch
@@ -0,0 +1,26 @@
+diff --git a/CMakeLists.txt b/CMakeLists.txt
+index f4fa064..a868106 100644
+--- a/CMakeLists.txt
++++ b/CMakeLists.txt
+@@ -126,21 +126,6 @@ endif()
+ option(OPENTELEMETRY_INSTALL "Whether to install opentelemetry targets"
+        ${OPENTELEMETRY_INSTALL_default})
+
+-if(NOT DEFINED CMAKE_CXX_STANDARD)
+-  if(WITH_STL)
+-    # Require at least C++17. C++20 is needed to avoid gsl::span
+-    if(CMAKE_VERSION VERSION_GREATER 3.11.999)
+-      # Ask for 20, may get anything below
+-      set(CMAKE_CXX_STANDARD 20)
+-    else()
+-      # Ask for 17, may get anything below
+-      set(CMAKE_CXX_STANDARD 17)
+-    endif()
+-  else()
+-    set(CMAKE_CXX_STANDARD 11)
+-  endif()
+-endif()
+-
+ if(WITH_STL)
+   # These definitions are needed for test projects that do not link against
+   # opentelemetry-api library directly. We ensure that variant implementation
diff --git a/vcpkg_overlays/opentelemetry-cpp/vcpkg.json b/vcpkg_overlays/opentelemetry-cpp/vcpkg.json
@@ -0,0 +1,87 @@
+{
+  "$schema": "https://raw.githubusercontent.com/microsoft/vcpkg-tool/main/docs/vcpkg.schema.json",
+  "name": "opentelemetry-cpp",
+  "version-semver": "1.9.1",
+  "description": [
+    "OpenTelemetry is a collection of tools, APIs, and SDKs.",
+    "You use it to instrument, generate, collect, and export telemetry data (metrics, logs, and traces) for analysis in order to understand your software's performance and behavior."
+  ],
+  "homepage": "https://github.com/open-telemetry/opentelemetry-cpp",
+  "license": "Apache-2.0",
+  "dependencies": [
+    "abseil",
+    "curl",
+    "nlohmann-json",
+    {
+      "name": "vcpkg-cmake",
+      "host": true
+    },
+    {
+      "name": "vcpkg-cmake-config",
+      "host": true
+    }
+  ],
+  "features": {
+    "elasticsearch": {
+      "description": "Whether to include the Elasticsearch Client in the SDK"
+    },
+    "etw": {
+      "description": "Whether to include the ETW Exporter in the SDK",
+      "supports": "windows"
+    },
+    "jaeger": {
+      "description": "Whether to include the Jaeger exporter",
+      "dependencies": [
+        "thrift"
+      ]
+    },
+    "otlp": {
+      "description": "Whether to include the OpenTelemetry Protocol in the SDK",
+      "dependencies": [
+        "protobuf"
+      ]
+    },
+    "otlp-grpc": {
+      "description": "Whether to include the OTLP gRPC exporter in the SDK",
+      "dependencies": [
+        "grpc",
+        {
+          "name": "grpc",
+          "host": true
+        },
+        {
+          "name": "opentelemetry-cpp",
+          "default-features": false,
+          "features": [
+            "otlp"
+          ]
+        }
+      ]
+    },
+    "otlp-http": {
+      "description": "Whether to include the OpenTelemetry Protocol over HTTP in the SDK",
+      "dependencies": [
+        "curl",
+        {
+          "name": "opentelemetry-cpp",
+          "default-features": false,
+          "features": [
+            "otlp"
+          ]
+        }
+      ]
+    },
+    "prometheus": {
+      "description": "Whether to include the Prometheus Client in the SDK",
+      "dependencies": [
+        "prometheus-cpp"
+      ]
+    },
+    "zipkin": {
+      "description": "Whether to include the Zipkin exporter in the SDK"
+    },
+    "zpages": {
+      "description": "Whether to include the Zpages Server in the SDK"
+    }
+  }
+}