Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HPCC-30866 Remove fork() unsafe code from open-telemetry random number generator #18038

Merged
merged 1 commit into from
Nov 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions vcpkg_overlays/opentelemetry-cpp/add-missing-dependencies.patch
Original file line number Diff line number Diff line change
@@ -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)
13 changes: 13 additions & 0 deletions vcpkg_overlays/opentelemetry-cpp/add-missing-find-dependency.patch
Original file line number Diff line number Diff line change
@@ -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@")
56 changes: 56 additions & 0 deletions vcpkg_overlays/opentelemetry-cpp/hpcc-remove-unsafe-onfork.patch
Original file line number Diff line number Diff line change
@@ -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
75 changes: 75 additions & 0 deletions vcpkg_overlays/opentelemetry-cpp/portfile.cmake
Original file line number Diff line number Diff line change
@@ -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")
26 changes: 26 additions & 0 deletions vcpkg_overlays/opentelemetry-cpp/use-default-cxx-version.patch
Original file line number Diff line number Diff line change
@@ -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
87 changes: 87 additions & 0 deletions vcpkg_overlays/opentelemetry-cpp/vcpkg.json
Original file line number Diff line number Diff line change
@@ -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"
}
}
}
Loading