From d7bb00e650387ded2e0a6c1f038c984e6241c5d7 Mon Sep 17 00:00:00 2001 From: Damien MEHALA Date: Tue, 16 Apr 2024 23:23:46 +0200 Subject: [PATCH 1/5] feat: add windows support - refactor: the codebase to use `substr` instead of `range`. - refactor: CMake targets. - refactor: add bazelrc to build using either abseil or std::string STD. - ci: build and run on windows using CMake and bazel. - update: add platform support section and update building instructions. --- .bazelrc | 1 + .bazelrc.absl | 15 ++ .bazelrc.std | 15 ++ .circleci/config.yml | 80 +++++++++-- BUILD.bazel | 8 -- CMakeLists.txt | 198 ++++++++++++++------------- README.md | 89 +++++++----- benchmark/CMakeLists.txt | 7 - bin/README.md | 2 +- bin/benchmark | 2 +- bin/hasher-example | 2 +- bin/test | 6 +- bin/with-toolchain | 2 +- cmake/compiler/clang.cmake | 103 ++++++++++++++ cmake/compiler/clang_apple.cmake | 10 ++ cmake/compiler/gcc.cmake | 83 +++++++++++ cmake/compiler/msvc.cmake | 71 ++++++++++ cmake/{ => deps}/curl.cmake | 3 +- examples/hasher/hasher.cpp | 6 +- examples/http-server/Dockerfile | 2 +- fuzz/README.md | 2 +- src/datadog/base64.cpp | 18 +-- src/datadog/curl.cpp | 50 +++---- src/datadog/expected.h | 56 ++++---- src/datadog/extraction_util.cpp | 10 +- src/datadog/http_client.cpp | 33 +++-- src/datadog/limiter.cpp | 5 +- src/datadog/msgpack.h | 2 +- src/datadog/parse_util.cpp | 17 +-- src/datadog/platform_util.cpp | 145 ++++++++++++++++---- src/datadog/remote_config.cpp | 8 +- src/datadog/sampling_util.h | 3 +- src/datadog/span.cpp | 1 - src/datadog/string_util.cpp | 20 ++- src/datadog/string_util.h | 6 +- src/datadog/tag_propagation.cpp | 40 ++++-- src/datadog/trace_id.cpp | 7 +- src/datadog/trace_sampler_config.cpp | 2 +- src/datadog/tracer.cpp | 4 +- src/datadog/tracer_config.cpp | 12 +- src/datadog/version.cpp | 7 +- src/datadog/w3c_propagation.cpp | 102 +++++++------- test/CMakeLists.txt | 10 +- test/mocks/collectors.h | 4 +- test/mocks/http_clients.h | 2 +- test/mocks/loggers.cpp | 2 +- test/mocks/loggers.h | 12 +- test/system-tests/CMakeLists.txt | 5 +- test/system-tests/main.cpp | 2 +- test/test_limiter.cpp | 6 + test/test_remote_config.cpp | 4 +- test/test_tracer_config.cpp | 42 +++--- test/test_tracer_telemetry.cpp | 2 +- 53 files changed, 916 insertions(+), 430 deletions(-) create mode 100644 .bazelrc create mode 100644 .bazelrc.absl create mode 100644 .bazelrc.std create mode 100644 cmake/compiler/clang.cmake create mode 100644 cmake/compiler/clang_apple.cmake create mode 100644 cmake/compiler/gcc.cmake create mode 100644 cmake/compiler/msvc.cmake rename cmake/{ => deps}/curl.cmake (91%) diff --git a/.bazelrc b/.bazelrc new file mode 100644 index 00000000..c2210dda --- /dev/null +++ b/.bazelrc @@ -0,0 +1 @@ +.bazelrc.absl diff --git a/.bazelrc.absl b/.bazelrc.absl new file mode 100644 index 00000000..3fd91881 --- /dev/null +++ b/.bazelrc.absl @@ -0,0 +1,15 @@ +# In order to support both Unixen and Windows, different styles of compiler +# flags must be used. +# +# This .bazelrc file specifies different compiler flags for Linux/Darwin versus +# Windows. +# +# This bazelrc defines the `DD_USE_ABSEIL_FOR_ENVOY` preprocessor macro, and so +# the resulting library will use `absl::string_view` and `absl::optional` +# instead of their standard (`std`) equivalents. + +build --enable_platform_specific_config + +build:linux --cxxopt='-std=c++17' --cxxopt='-Wall' --cxxopt='-Wextra' --cxxopt='-pedantic' --cxxopt='-DDD_USE_ABSEIL_FOR_ENVOY' +build:macos --cxxopt='-std=c++17' --cxxopt='-Wall' --cxxopt='-Wextra' --cxxopt='-pedantic' --cxxopt='-DDD_USE_ABSEIL_FOR_ENVOY' +build:windows --cxxopt='/std:c++17' --cxxopt='/DDD_USE_ABSEIL_FOR_ENVOY' --linkopt='ws2_32.lib' diff --git a/.bazelrc.std b/.bazelrc.std new file mode 100644 index 00000000..802f014d --- /dev/null +++ b/.bazelrc.std @@ -0,0 +1,15 @@ +# In order to support both Unixen and Windows, different styles of compiler +# flags must be used. +# +# This .bazelrc file specifies different compiler flags for Linux/Darwin versus +# Windows. +# +# This bazelrc does _not_ define the `DD_USE_ABSEIL_FOR_ENVOY` preprocessor +# macro, and so the resulting library will use `std::string_view` and +# `std::optional` instead of their Abseil equivalents. + +build --enable_platform_specific_config + +build:linux --cxxopt='-std=c++17' --cxxopt='-Wall' --cxxopt='-Wextra' --cxxopt='-pedantic' +build:macos --cxxopt='-std=c++17' --cxxopt='-Wall' --cxxopt='-Wextra' --cxxopt='-pedantic' +build:windows --cxxopt='/std:c++17' --linkopt='ws2_32.lib' diff --git a/.circleci/config.yml b/.circleci/config.yml index 9963a8ef..819ffd2b 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -33,18 +33,65 @@ jobs: type: string arch: type: string + bazelrc: + type: string executor: docker-<< parameters.arch >> environment: MAKE_JOB_COUNT: 8 steps: - checkout - - run: bin/with-toolchain << parameters.toolchain >> bazelisk build --jobs $MAKE_JOB_COUNT dd_trace_cpp + - run: bin/with-toolchain << parameters.toolchain >> bazelisk --bazelrc=<< parameters.bazelrc >> build --jobs $MAKE_JOB_COUNT dd_trace_cpp - test-cmake: + build-and-test-windows-bazel: parameters: - toolchain: + # `bazelrc` is the path to the .bazelrc file to use in the build/test. + # The repository has two flavors: one that uses Abseil types (for use with + # Envoy), and one that uses std types. + bazelrc: + type: string + machine: + image: "windows-server-2022-gui:current" + resource_class: windows.medium + shell: powershell.exe -ExecutionPolicy Bypass + environment: + MAKE_JOB_COUNT: 4 + steps: + - checkout + - run: choco install -y bazelisk + - run: bazelisk.exe --bazelrc=<< parameters.bazelrc >> build --jobs $env:MAKE_JOB_COUNT dd_trace_cpp + + build-and-test-windows-cmake: + parameters: + arch: type: string - sanitize: + machine: + image: "windows-server-2022-gui:current" + shell: powershell.exe -ExecutionPolicy Bypass + resource_class: "windows.large" + environment: + MAKE_JOB_COUNT: 8 + steps: + - checkout + - run: + name: Install dependencies + command: | + choco install -y cmake --installargs 'ADD_CMAKE_TO_PATH=System' + choco install -y ninja + - run: + name: Building + command: | + & 'C:\\Program Files\\Microsoft Visual Studio\\2022\\Community\\Common7\\Tools\\Launch-VsDevShell.ps1' -arch << parameters.arch >> + cmake -B build -DCMAKE_BUILD_TYPE=Debug -DDD_TRACE_BUILD_TESTING=1 -G Ninja . + cmake --build build -j $env:MAKE_JOB_COUNT -v + - run: + name: Testing + command: | + & 'C:\\Program Files\\Microsoft Visual Studio\\2022\\Community\\Common7\\Tools\\Launch-VsDevShell.ps1' -arch << parameters.arch >> + .\build\test\tests.exe + + build-and-test-cmake: + parameters: + toolchain: type: string arch: type: string @@ -55,7 +102,7 @@ jobs: ASAN_OPTIONS: alloc_dealloc_mismatch=0 steps: - checkout - - run: bin/with-toolchain << parameters.toolchain >> cmake . -B .build -DBUILD_TESTING=1 -DSANITIZE=<< parameters.sanitize >> + - run: bin/with-toolchain << parameters.toolchain >> cmake . -B .build -DCMAKE_BUILD_TYPE=Debug -DDD_TRACE_BUILD_TESTING=1 - run: cmake --build .build -j ${MAKE_JOB_COUNT} -v - run: cd .build && test/tests @@ -127,19 +174,34 @@ workflows: jobs: - format - shellcheck - - test-cmake: + - build-and-test-cmake: matrix: parameters: toolchain: ["gnu", "llvm"] - sanitize: ["on", "off"] arch: ["amd64", "arm64"] - build-bazel: matrix: parameters: - arch: ["amd64", "arm64"] toolchain: ["gnu", "llvm"] - - coverage + arch: ["amd64", "arm64"] + bazelrc: [".bazelrc.absl", ".bazelrc.std"] + - build-and-test-windows-cmake: + matrix: + parameters: + arch: ["amd64"] + - build-and-test-windows-bazel: + matrix: + parameters: + bazelrc: [".bazelrc.absl", ".bazelrc.std"] + - coverage: + requires: + - build-and-test-cmake - system-tests: + requires: + - build-bazel + - build-and-test-cmake + - build-and-test-windows-bazel + - build-and-test-windows-cmake filters: branches: only: diff --git a/BUILD.bazel b/BUILD.bazel index ae4a9631..85010c9a 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -117,14 +117,6 @@ cc_library( "src/datadog/version.h", "src/datadog/w3c_propagation.h", ], - copts = [ - "-Wall", - "-Wextra", - "-Werror", - "-pedantic", - "-std=c++17", - "-DDD_USE_ABSEIL_FOR_ENVOY", - ], strip_include_prefix = "src/", visibility = ["//visibility:public"], deps = [ diff --git a/CMakeLists.txt b/CMakeLists.txt index c1838193..6f33e850 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -4,89 +4,79 @@ cmake_minimum_required(VERSION 3.24) project(dd-trace-cpp) -option(BUILD_COVERAGE "Build code with code coverage profiling instrumentation" OFF) -option(BUILD_EXAMPLES "Build example programs" OFF) -option(BUILD_TESTING "Build the unit tests (test/)" OFF) -option(BUILD_FUZZERS "Build fuzzers" OFF) -option(BUILD_BENCHMARK "Build benchmark binaries" OFF) -option(SANITIZE "Build with address sanitizer and undefined behavior sanitizer" OFF) +option(BUILD_SHARED_LIBS "Build shared libraries" OFF) +option(BUILD_STATIC_LIBS "Build static libraries" OFF) + +# Consumer of the library using FetchContent do not need +# to build unit tests, fuzzers and examples. +if(CMAKE_CURRENT_SOURCE_DIR STREQUAL CMAKE_SOURCE_DIR) + option(DD_TRACE_BUILD_EXAMPLES "Build example programs" OFF) + option(DD_TRACE_BUILD_TESTING "Build the unit tests (test/)" OFF) + option(DD_TRACE_BUILD_FUZZERS "Build fuzzers" OFF) + option(DD_TRACE_BUILD_BENCHMARK "Build benchmark binaries" OFF) + option(DD_TRACE_ENABLE_COVERAGE "Build code with code coverage profiling instrumentation" OFF) + option(DD_TRACE_ENABLE_SANITIZE "Build with address sanitizer and undefined behavior sanitizer" OFF) +endif() if (NOT CMAKE_BUILD_TYPE) set(CMAKE_BUILD_TYPE "RelWithDebInfo") +elseif (CMAKE_BUILD_TYPE STREQUAL "Debug") + set(DD_TRACE_ENABLE_SANITIZE ON) endif () +# Linking this library requires libcurl and threads. +find_package(Threads REQUIRED) +include(cmake/deps/curl.cmake) + set(CMAKE_POSITION_INDEPENDENT_CODE ON) set(CMAKE_EXPORT_COMPILE_COMMANDS ON) set(CMAKE_CXX_STANDARD 17) set(CMAKE_CXX_STANDARD_REQUIRED ON) set(CMAKE_CXX_EXTENSIONS OFF) -# This warning has a false positive. See -# . -add_compile_options(-Wno-error=free-nonheap-object) - -# This warning has a false positive with clang. See -# . -if (CMAKE_CXX_COMPILER_ID STREQUAL "Clang") - add_compile_options(-Wno-error=unused-lambda-capture) - - # If we're building with clang, then use the libc++ version of the standard - # library instead of libstdc++. Better coverage of build configurations. - # - # But there's one exception: libfuzzer is built with libstdc++ on Ubuntu, - # and so won't link to libc++. So, if any of the FUZZ_* variables are set, - # keep to libstdc++ (the default on most systems). - if (NOT ${BUILD_FUZZERS}) - add_compile_options(-stdlib=libc++) - add_link_options(-stdlib=libc++) - endif () -endif() +file(STRINGS ${CMAKE_CURRENT_SOURCE_DIR}/src/datadog/version.cpp DD_TRACE_VERSION_CPP_CONTENTS REGEX "#define DD_TRACE_VERSION( |_NUM )") +string(REGEX MATCH "#define DD_TRACE_VERSION \"[^\"]*" DD_TRACE_VERSION ${DD_TRACE_VERSION_CPP_CONTENTS}) +string(REGEX REPLACE "[^\"]+\"" "" DD_TRACE_VERSION ${DD_TRACE_VERSION}) +message(STATUS "dd-trace-cpp version=[${DD_TRACE_VERSION}]") +unset(DD_TRACE_VERSION_CPP_CONTENTS) -function(add_sanitizers) - add_compile_options(-fsanitize=address) - add_link_options(-fsanitize=address) - add_compile_options(-fsanitize=undefined) - add_link_options(-fsanitize=undefined) -endfunction() - -if(BUILD_FUZZERS) - set(BUILD_TESTING OFF) - add_compile_options(-fsanitize=fuzzer) - add_link_options(-fsanitize=fuzzer) - add_sanitizers() - add_subdirectory(fuzz) -endif() +if (CMAKE_CXX_COMPILER_ID STREQUAL "MSVC") + message(STATUS "Compiler: MSVC ${CMAKE_CXX_COMPILER_VERSION}") + include(cmake/compiler/msvc.cmake) +elseif (CMAKE_CXX_COMPILER_ID STREQUAL "Clang") + message(STATUS "Compiler: clang ${CMAKE_CXX_COMPILER_VERSION}") + include(cmake/compiler/clang.cmake) +elseif (CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang") + message(STATUS "Compiler: clang-apple ${CMAKE_CXX_COMPILER_VERSION}") + include(cmake/compiler/clang_apple.cmake) +elseif (CMAKE_CXX_COMPILER_ID STREQUAL "GNU") + message(STATUS "Compiler: GCC ${CMAKE_CXX_COMPILER_VERSION}") + include(cmake/compiler/gcc.cmake) +endif () -if (SANITIZE) - add_sanitizers() -endif() +if (DD_TRACE_BUILD_FUZZERS) + add_subdirectory(fuzz) +endif () -if (BUILD_TESTING) - set(CMAKE_BUILD_TYPE "Debug") +if (DD_TRACE_BUILD_TESTING) + add_subdirectory(test) endif() -include(cmake/curl.cmake) +if (DD_TRACE_BUILD_EXAMPLES) + add_subdirectory(examples) +endif () -if (APPLE) - find_library(COREFOUNDATION_LIBRARY CoreFoundation) - find_library(SYSTEMCONFIGURATION_LIBRARY SystemConfiguration) +if (DD_TRACE_BUILD_BENCHMARK) + add_subdirectory(benchmark) endif () -if (MSVC) - add_compile_options(/W4 /WX) -else() - add_compile_options(-Wall -Wextra -pedantic -Werror) - if (BUILD_COVERAGE) - add_compile_options(-g -O0 -fprofile-arcs -ftest-coverage) - endif() -endif() +add_library(dd_trace_cpp-objects OBJECT) +add_library(dd_trace::obj ALIAS dd_trace_cpp-objects) -if(BUILD_COVERAGE) - set(COVERAGE_LIBRARIES gcov) -endif() +add_dependencies(dd_trace_cpp-objects libcurl) -add_library(dd_trace_cpp-objects OBJECT) -target_sources(dd_trace_cpp-objects PRIVATE +target_sources(dd_trace_cpp-objects + PRIVATE src/datadog/base64.cpp src/datadog/cerr_logger.cpp src/datadog/clock.cpp @@ -210,43 +200,65 @@ target_sources(dd_trace_cpp-objects PUBLIC src/datadog/w3c_propagation.h ) -add_dependencies(dd_trace_cpp-objects libcurl) +# TODO: define public/private +# Headers location are different depending of whether we are building +# or installing the library. +# target_include_directories(dd_trace_cpp-objects +# PUBLIC +# $ +# $ +# ) -# Linking this library requires libcurl and threads. -find_package(Threads REQUIRED) target_link_libraries(dd_trace_cpp-objects - PUBLIC + PRIVATE libcurl Threads::Threads - ${COVERAGE_LIBRARIES} - ${COREFOUNDATION_LIBRARY} - ${SYSTEMCONFIGURATION_LIBRARY} + dd_trace::specs ) # Produce both shared and static versions of the library. -add_library(dd_trace_cpp-shared SHARED $) -set_target_properties(dd_trace_cpp-shared PROPERTIES OUTPUT_NAME "dd_trace_cpp") -add_dependencies(dd_trace_cpp-shared dd_trace_cpp-objects) -target_link_libraries(dd_trace_cpp-shared dd_trace_cpp-objects) - -add_library(dd_trace_cpp-static STATIC $) -set_target_properties(dd_trace_cpp-static PROPERTIES OUTPUT_NAME "dd_trace_cpp") -add_dependencies(dd_trace_cpp-static dd_trace_cpp-objects) -target_link_libraries(dd_trace_cpp-static dd_trace_cpp-objects) - -# When installing, install the static library, the shared library, and the -# public headers. -install(TARGETS dd_trace_cpp-static dd_trace_cpp-shared dd_trace_cpp-objects - FILE_SET public_headers) - -if(BUILD_TESTING) - add_subdirectory(test) -endif() +if (BUILD_SHARED_LIBS) + add_library(dd_trace_cpp-shared SHARED $) + add_library(dd_trace::shared ALIAS dd_trace_cpp-shared) -if(BUILD_EXAMPLES) - add_subdirectory(examples) -endif() + add_dependencies(dd_trace_cpp-shared dd_trace_cpp-objects) -if(BUILD_BENCHMARK) - add_subdirectory(benchmark) -endif() + target_link_libraries(dd_trace_cpp-shared + PUBLIC + dd_trace::obj + PRIVATE + dd_trace::specs + ) + + install(TARGETS dd_trace_cpp-objects dd_trace_cpp-shared + EXPORT dd_trace_cpp-export + FILE_SET public_headers DESTINATION ${CMAKE_INSTALL_INCLUDEDIR} + LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR} + ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR} + # For Windows + RUNTIME DESTINATION ${CMAKE_INSTALL_LIBDIR} + ) +endif () + +if (BUILD_STATIC_LIBS) + add_library(dd_trace_cpp-static STATIC $) + add_library(dd_trace::static ALIAS dd_trace_cpp-static) + + add_dependencies(dd_trace_cpp-static dd_trace_cpp-objects) + + target_link_libraries(dd_trace_cpp-static + PUBLIC + dd_trace::obj + PRIVATE + dd_trace::specs + ) + + install(TARGETS dd_trace_cpp-objects dd_trace_cpp-static + EXPORT dd_trace_cpp-export + FILE_SET public_headers DESTINATION ${CMAKE_INSTALL_INCLUDEDIR} + LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR} + ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR} + # For Windows + RUNTIME DESTINATION ${CMAKE_INSTALL_LIBDIR} + ) +endif () diff --git a/README.md b/README.md index 73a823a9..8d505d10 100644 --- a/README.md +++ b/README.md @@ -38,64 +38,81 @@ int main() { ``` See the [examples](examples) directory for more extensive usage examples. -Build ------ -### `cmake && make && make install` Style Build -Build this library from source using [CMake][1]. Installation places a shared -library and public headers into the appropriate system directories -(`/usr/local/[...]`), or to a specified installation prefix. +## Platform Support +The library is currently compatible with and has been tested on the following CPU architecture, OS and compiler combinations: +- x86_64 and arm64 Linux with GCC 11.4. +- x86_64 and arm64 Linux with Clang 14. +- x86_64 Windows with MSVC 2022. +- arm64 macOS with Apple Clang 15. + +All possible configurations cannot be tested. However, any conforming C++17 compiler should be supported. + +## Building and Installation + +### Requirements +`dd-trace-cpp` requires a [supported](#platform-support) C++17 compiler. -A recent version of CMake is required (3.24), which might not be in your +A recent version of CMake is required (`3.24`), which might not be in your system's package manager. [bin/install-cmake](bin/install-cmake) is an installer for a recent CMake. -dd-trace-cpp requires a C++17 compiler. - -Here is how to install dd-trace-cpp into `.install/` within the source -repository. -```console -$ git clone 'https://github.com/datadog/dd-trace-cpp' -$ cd dd-trace-cpp -$ bin/install-cmake -$ mkdir .install -$ mkdir .build -$ cd .build -$ cmake -DCMAKE_INSTALL_PREFIX=../.install .. -$ make -j $(nproc) -$ make install -$ find ../.install -type d +### Building +Build this library from source using [CMake][1]. + +```shell +git clone 'https://github.com/datadog/dd-trace-cpp' +cd dd-trace-cpp +cmake -B build . +cmake --build build -j +``` + +By default CMake will generate both static and shared libraries. To build either on of them use +as configuration parameter `BUILD_SHARED_LIBS` or `BUILD_STATIC_LIBS`. Example: + +```shell +cmake -B build -DBUILD_SHARED_LIBS=1 . +``` + +### Installation +Installation places a shared library and public headers into the appropriate system directories +(`/usr/local/[...]`), or to a specified installation prefix. + +```shell +cmake --install + +# Here is how to install dd-trace-cpp into `.install/` within the source +# repository. +# cmake --install build --prefix=.install ``` -To instead install into `/usr/local/`, omit the `.install` directory and the -`-DCMAKE_INSTALL_PREFIX=../.install` option. +### Optional: Linking to the shared library +In case you decided to build the shared library: -Then, when building an executable that uses `dd-trace-cpp`, specify the path to +When building an executable that uses `dd-trace-cpp`, specify the path to the installed headers using an appropriate `-I` option. If the library was installed into the default system directories, then the `-I` option is not needed. -```console -$ c++ -I/path/to/dd-trace-cpp/.install/include -c -o my_app.o my_app.cpp +```shell +c++ -I/path/to/dd-trace-cpp/.install/include -c -o my_app.o my_app.cpp ``` When linking an executable that uses `dd-trace-cpp`, specify linkage to the built library using the `-ldd_trace_cpp` option and an appropriate `-L` option. If the library was installed into the default system directories, then the `-L` options is not needed. The `-ldd_trace_cpp` option is always needed. -```console -$ c++ -o my_app my_app.o -L/path/to/dd-trace-cpp/.install/lib -ldd_trace_cpp +```shell +c++ -o my_app my_app.o -L/path/to/dd-trace-cpp/.install/lib -ldd_trace_cpp ``` Test ---- -Pass `-DBUILD_TESTING=1` to `cmake` to include the unit tests in the build. +Pass `-DDD_TRACE_BUILD_TESTING=1` to `cmake` to include the unit tests in the build. The resulting unit test executable is `test/tests` within the build directory. -```console -$ mkdir .build -$ cd .build -$ cmake -DBUILD_TESTING=1 .. -$ make -j $(nproc) -$ ./test/tests +```shell +cmake -Bbuild -DDD_TRACE_BUILD_TESTING=1 .. +cmake --build build -j +./build/test/tests ``` Alternatively, [bin/test](bin/test) is provided for convenience. diff --git a/benchmark/CMakeLists.txt b/benchmark/CMakeLists.txt index 4dbb0c6d..75ebcb03 100644 --- a/benchmark/CMakeLists.txt +++ b/benchmark/CMakeLists.txt @@ -3,13 +3,6 @@ # # It's intended to be used as part of Datadog's internal benchmarking platform. # See `../.gitlab/benchmarks.yml`. - -set(CMAKE_POSITION_INDEPENDENT_CODE ON) -set(CMAKE_EXPORT_COMPILE_COMMANDS ON) -set(CMAKE_CXX_STANDARD 17) -set(CMAKE_CXX_STANDARD_REQUIRED ON) -set(CMAKE_CXX_EXTENSIONS OFF) - add_executable(dd_trace_cpp-benchmark benchmark.cpp hasher.cpp diff --git a/bin/README.md b/bin/README.md index ac1fba01..ed7f565d 100644 --- a/bin/README.md +++ b/bin/README.md @@ -35,7 +35,7 @@ This directory contains scripts that are useful during development. - [with-toolchain](with-toolchain) is a command wrapper that sets the `CC` and `CXX` environment variables based on its first argument, which is either "gnu" (to use the gcc/g++ toolchain) or "llvm" (to use the clang/clang++ toolchain). - For example: `with-toolchain llvm cmake -DBUILD_TESTING=1 ..`. + For example: `with-toolchain llvm cmake -DDD_TRACE_BUILD_TESTING=1 ..`. [1]: https://bazel.build/ [2]: https://github.com/bazelbuild/bazelisk diff --git a/bin/benchmark b/bin/benchmark index 9d2014f8..576ed4fc 100755 --- a/bin/benchmark +++ b/bin/benchmark @@ -8,7 +8,7 @@ cd "$(dirname "$0")"/.. mkdir -p .build cd .build -cmake .. -DBUILD_BENCHMARK=1 -DCMAKE_BUILD_TYPE=Release +cmake .. -DDD_TRACE_BUILD_BENCHMARK=1 -DCMAKE_BUILD_TYPE=Release make -j cd ../ diff --git a/bin/hasher-example b/bin/hasher-example index 4f07d78b..a6fd3557 100755 --- a/bin/hasher-example +++ b/bin/hasher-example @@ -19,7 +19,7 @@ cd "$(dirname "$0")"/.. mkdir -p .build cd .build -cmake .. -DBUILD_HASHER_EXAMPLE=1 +cmake .. -DDD_TRACE_BUILD_EXAMPLES=1 # shellcheck disable=SC2086 make -j "$(nproc)" $verbosity_flags cd ../ diff --git a/bin/test b/bin/test index 46ca7707..66adb741 100755 --- a/bin/test +++ b/bin/test @@ -5,7 +5,7 @@ set -e MAKE_JOB_COUNT=${MAKE_JOB_COUNT:-$(nproc)} if [ "$1" = '--coverage' ]; then - coverage_flags=-DBUILD_COVERAGE=1 + coverage_flags=-DDD_TRACE_ENABLE_COVERAGE=1 shift else coverage_flags='' @@ -20,7 +20,7 @@ fi if [ "$1" = '--show-coverage' ]; then # implies --coverage - coverage_flags=-DBUILD_COVERAGE=1 + coverage_flags=-DDD_TRACE_ENABLE_COVERAGE=1 show_coverage=1 shift else @@ -38,7 +38,7 @@ cd "$(dirname "$0")"/.. mkdir -p .build cd .build # shellcheck disable=SC2086 -cmake .. $coverage_flags -DBUILD_TESTING=1 +cmake .. $coverage_flags -DDD_TRACE_BUILD_TESTING=1 # Clean up any code coverage artifacts. find . -type f -name '*.gcda' -print0 | xargs -0 rm -f # shellcheck disable=SC2086 diff --git a/bin/with-toolchain b/bin/with-toolchain index f01c1e36..21af8762 100755 --- a/bin/with-toolchain +++ b/bin/with-toolchain @@ -15,7 +15,7 @@ usage: Print this message. example: - $ ../bin/with-toolchain llvm cmake .. -DBUILD_TESTING=1 + $ ../bin/with-toolchain llvm cmake .. -DDD_TRACE_BUILD_TESTING=1 END_USAGE } diff --git a/cmake/compiler/clang.cmake b/cmake/compiler/clang.cmake new file mode 100644 index 00000000..e751af38 --- /dev/null +++ b/cmake/compiler/clang.cmake @@ -0,0 +1,103 @@ +# This target interface encapsulates compiler and linker options for the project. +# It provides a convenient way to apply these options to multiple targets. +# ---- +# Usage: Link `dd_trace_cpp-specs` to targets using target_link_libraries. +add_library(dd_trace_cpp-specs INTERFACE) +add_library(dd_trace::specs ALIAS dd_trace_cpp-specs) + +target_compile_options(dd_trace_cpp-specs + INTERFACE + -Wall + -Wextra + # -Wformat + # -Wformat=2 + # -Wconversion + # -Wsign-conversion + -Wimplicit-fallthrough + -Werror + -Werror=format-security + # This warning has a false positive with clang. See + # . + -Wno-error=unused-lambda-capture + -fno-omit-frame-pointer + -fno-delete-null-pointer-checks + -fno-strict-overflow + -fno-strict-aliasing + -pedantic +) + +if (CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 16) + target_compile_options(dd_trace_cpp-specs + INTERFACE + -fstrict-flex-arrays=3 + ) +endif () + +if (CMAKE_BUILD_TYPE STREQUAL "Release|RelWithDebInfo") + # -ftrivial-auto-var-init can interfere with other tools + target_compile_options(dd_trace_cpp-specs + INTERFACE + -ftrivial-auto-var-init=zero + ) +endif () + +function(add_sanitizers) + target_compile_options(dd_trace_cpp-specs + INTERFACE + -fsanitize=address,undefined + ) + + target_link_libraries(dd_trace_cpp-specs + INTERFACE + -fsanitize=address,undefined + ) +endfunction() + +if (DD_TRACE_ENABLE_COVERAGE) + find_program(GCOV_PATH gcov) + if (NOT GCOV_PATH) + message(FATAL_ERROR "gcov not found. Cannot build with -DDD_TRACE_ENABLE_COVERAGE") + endif () + + target_compile_options(dd_trace_cpp-specs + INTERFACE + -g + -O0 + -fprofile-arcs + -ftest-coverage + ) + + target_link_libraries(dd_trace_cpp-specs + INTERFACE + gcov + ) +endif() + +if (DD_TRACE_ENABLE_SANITIZE) + add_sanitizers() +endif() + +if (DD_TRACE_BUILD_FUZZERS) + add_sanitizers() + + target_compile_options(dd_trace_cpp-specs + INTERFACE + -fsanitize=fuzzer + ) + + target_link_libraries(dd_trace_cpp-specs + INTERFACE + -fsanitize=fuzzer + ) +else () + # If we're building with clang, then use the libc++ version of the standard + # library instead of libstdc++. Better coverage of build configurations. + # + # But there's one exception: libfuzzer is built with libstdc++ on Ubuntu, + # and so won't link to libc++. So, if any of the FUZZ_* variables are set, + # keep to libstdc++ (the default on most systems). + message(STATUS "C++ standard library: libc++") + add_compile_options(-stdlib=libc++) + add_link_options(-stdlib=libc++) +endif() + diff --git a/cmake/compiler/clang_apple.cmake b/cmake/compiler/clang_apple.cmake new file mode 100644 index 00000000..3d4b0b22 --- /dev/null +++ b/cmake/compiler/clang_apple.cmake @@ -0,0 +1,10 @@ +include(cmake/compiler/clang.cmake) + +find_library(COREFOUNDATION_LIBRARY CoreFoundation) +find_library(SYSTEMCONFIGURATION_LIBRARY SystemConfiguration) + +target_link_libraries(dd_trace_cpp-specs + INTERFACE + ${COREFOUNDATION_LIBRARY} + ${SYSTEMCONFIGURATION_LIBRARY} +) diff --git a/cmake/compiler/gcc.cmake b/cmake/compiler/gcc.cmake new file mode 100644 index 00000000..eb953630 --- /dev/null +++ b/cmake/compiler/gcc.cmake @@ -0,0 +1,83 @@ +# This target interface encapsulates compiler and linker options for the project. +# It provides a convenient way to apply these options to multiple targets. +# ---- +# Usage: Link `dd_trace_cpp-specs` to targets using target_link_libraries. +add_library(dd_trace_cpp-specs INTERFACE) +add_library(dd_trace::specs ALIAS dd_trace_cpp-specs) + +target_compile_options(dd_trace_cpp-specs + INTERFACE + -Wall + -Wextra + # -Wformat + # -Wformat=2 + # -Wconversion + # -Wsign-conversion + -Wtrampolines + -Wimplicit-fallthrough + -Werror + -Werror=format-security + # This warning has a false positive. See + # . + -Wno-error=free-nonheap-object + -fno-omit-frame-pointer + -fno-delete-null-pointer-checks + -fno-strict-overflow + -fno-strict-aliasing + -pedantic +) + +target_link_options(dd_trace_cpp-specs + INTERFACE + -Wl,-z,noexecstack +) + +if (CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 13) + target_compile_options(dd_trace_cpp-specs + INTERFACE + -Wbidi-chars=any + -fstrict-flex-arrays=3 + ) + + if (CMAKE_BUILD_TYPE STREQUAL "Release|RelWithDebInfo") + # -ftrivial-auto-var-init can interfere with other tools + target_compile_options(dd_trace_cpp-specs + INTERFACE + -ftrivial-auto-var-init=zero + ) + endif () +endif () + +if (DD_TRACE_ENABLE_COVERAGE) + find_program(GCOV_PATH gcov) + if (NOT GCOV_PATH) + message(FATAL_ERROR "gcov not found. Cannot build with -DDD_TRACE_ENABLE_COVERAGE") + endif () + + target_compile_options(dd_trace_cpp-specs + INTERFACE + -g -O0 -fprofile-arcs -ftest-coverage + ) + + target_link_libraries(dd_trace_cpp-specs + INTERFACE + gcov + ) +endif() + +if (DD_TRACE_ENABLE_SANITIZE) + target_compile_options(dd_trace_cpp-specs + INTERFACE + -fsanitize=address,undefined + ) + + target_link_libraries(dd_trace_cpp-specs + INTERFACE + -fsanitize=address,undefined + ) +endif() + +if (DD_TRACE_BUILD_FUZZERS) + message(FATAL_ERROR "Fuzzers are not support for GCC builds") +endif () + diff --git a/cmake/compiler/msvc.cmake b/cmake/compiler/msvc.cmake new file mode 100644 index 00000000..d115d068 --- /dev/null +++ b/cmake/compiler/msvc.cmake @@ -0,0 +1,71 @@ +macro(get_WIN32_WINNT version) + if(CMAKE_SYSTEM_VERSION) + set(ver ${CMAKE_SYSTEM_VERSION}) + string(REGEX MATCH "^([0-9]+).([0-9])" ver ${ver}) + string(REGEX MATCH "^([0-9]+)" verMajor ${ver}) + + # Check for Windows 10, b/c we'll need to convert to hex 'A'. + if("${verMajor}" MATCHES "10") + set(verMajor "A") + string(REGEX REPLACE "^([0-9]+)" ${verMajor} ver ${ver}) + endif() + + string(REPLACE "." "" ver ${ver}) + # Prepend each digit with a zero. + string(REGEX REPLACE "([0-9A-Z])" "0\\1" ver ${ver}) + set(${version} "0x${ver}") + endif() +endmacro() + +get_WIN32_WINNT(win_ver) + +# Automatically export all symbols +# It avoid the need for explicit dllimport/dllexport +# +# NOTE: when the library will have a public API. We should enforce +# which symbol to export (even with gcc) +set(WINDOWS_EXPORT_ALL_SYMBOLS ON) + +# This target interface encapsulates compiler and linker options for the project. +# It provides a convenient way to apply these options to multiple targets. +# ---- +# Usage: Link `dd_trace_cpp-specs` to targets using target_link_libraries. +add_library(dd_trace_cpp-specs INTERFACE) +add_library(dd_trace::specs ALIAS dd_trace_cpp-specs) + +target_compile_options(dd_trace_cpp-specs + INTERFACE + /W4 + /wd4706 + /D_CRT_SECURE_NO_WARNINGS + /D_WIN32_WINNT=${win_ver} +) + +if (CMAKE_BUILD_TYPE STREQUAL "Debug|RelWithDebInfo") + target_compile_options(dd_trace_cpp-specs + INTERFACE + # Embedded debug information in binaries (no pdb) + /Z7 + ) +endif () + +if (DD_TRACE_ENABLE_COVERAGE) + message(FATAL_ERROR "BUILD_COVERAGE is not supported for MSVC builds.") +endif () + +if (DD_TRACE_BUILD_FUZZERS) + message(FATAL_ERROR "Fuzzers are not support for MSVC builds.") +endif () + +if (DD_TRACE_ENABLE_SANITIZE) + target_compile_options(dd_trace_cpp-specs + INTERFACE + /fsanitize=address + /RTC1 + ) + target_link_options(dd_trace_cpp-specs + INTERFACE + /fsanitize=address + ) +endif () + diff --git a/cmake/curl.cmake b/cmake/deps/curl.cmake similarity index 91% rename from cmake/curl.cmake rename to cmake/deps/curl.cmake index a07f4fd7..edd9e762 100644 --- a/cmake/curl.cmake +++ b/cmake/deps/curl.cmake @@ -1,4 +1,3 @@ - include(FetchContent) # No need to build curl executable @@ -17,6 +16,8 @@ SET(USE_LIBIDN2 OFF) SET(CURL_USE_LIBSSH2 OFF) SET(CURL_USE_LIBPSL OFF) SET(CURL_DISABLE_HSTS ON) +set(CURL_CA_PATH "none") +set(CURL_CA_PATH_SET FALSE) FetchContent_Declare( curl diff --git a/examples/hasher/hasher.cpp b/examples/hasher/hasher.cpp index 8b96b68f..d030a832 100644 --- a/examples/hasher/hasher.cpp +++ b/examples/hasher/hasher.cpp @@ -150,9 +150,9 @@ int main() { const fs::path path(input_path); // Create a root span for the current request. - dd::SpanConfig config; - config.name = "sha256.request"; - auto root = tracer.create_span(config); + dd::SpanConfig span_config; + span_config.name = "sha256.request"; + auto root = tracer.create_span(span_config); root.set_tag("path", path.u8string()); if (!fs::exists(path)) { diff --git a/examples/http-server/Dockerfile b/examples/http-server/Dockerfile index 33c3d649..6f1fa754 100644 --- a/examples/http-server/Dockerfile +++ b/examples/http-server/Dockerfile @@ -10,6 +10,6 @@ run apt update -y \ && git clone --branch "${BRANCH}" 'https://github.com/datadog/dd-trace-cpp' . \ && bin/install-cmake \ && mkdir dist \ - && cmake -B .build -DBUILD_EXAMPLES=1 . \ + && cmake -B .build -DDD_TRACE_BUILD_EXAMPLES=1 . \ && cmake --build .build -j \ && cmake --install .build --prefix=dist diff --git a/fuzz/README.md b/fuzz/README.md index 648d3e40..1929ebd7 100644 --- a/fuzz/README.md +++ b/fuzz/README.md @@ -9,7 +9,7 @@ executables to the build: `BUILD_FUZZERS`. When building the fuzzers, the toolchain must be clang-based. For example: ```console $ rm -rf .build # if toolchain needs clearing -$ bin/with-toolchain llvm bin/cmake-build -DBUILD_FUZZERS=1 +$ bin/with-toolchain llvm bin/cmake-build -DDD_TRACE_BUILD_FUZZERS=1 $ .build/fuzz/w3c-propagation/w3c-propagation-fuzz [... fuzzer output ...] diff --git a/src/datadog/base64.cpp b/src/datadog/base64.cpp index decc6df4..35aa4833 100644 --- a/src/datadog/base64.cpp +++ b/src/datadog/base64.cpp @@ -47,9 +47,9 @@ std::string base64_decode(StringView input) { return ""; } - output.push_back(c0 << 2 | (c1 & 0xF0) >> 4); - output.push_back((c1 & 0x0F) << 4 | ((c2 & 0x3C) >> 2)); - output.push_back(((c2 & 0x03) << 6) | (c3 & 0x3F)); + output.push_back(static_cast(c0 << 2 | (c1 & 0xF0) >> 4)); + output.push_back(static_cast((c1 & 0x0F) << 4 | ((c2 & 0x3C) >> 2))); + output.push_back(static_cast(((c2 & 0x03) << 6) | (c3 & 0x3F))); } // If padding is missing, return the empty string in lieu of an Error. @@ -68,17 +68,17 @@ std::string base64_decode(StringView input) { if (c2 == k_eol) { // The last quadruplet is of the form "xx==", where only one character needs // to be decoded. - output.push_back(c0 << 2 | (c1 & 0xF0) >> 4); + output.push_back(static_cast(c0 << 2 | (c1 & 0xF0) >> 4)); } else if (c3 == k_eol) { // The last quadruplet is of the form "xxx=", where only two character needs // to be decoded. - output.push_back(c0 << 2 | (c1 & 0xF0) >> 4); - output.push_back((c1 & 0x0F) << 4 | ((c2 & 0x3C) >> 2)); + output.push_back(static_cast(c0 << 2 | (c1 & 0xF0) >> 4)); + output.push_back(static_cast((c1 & 0x0F) << 4 | ((c2 & 0x3C) >> 2))); } else { // The last quadruplet is not padded -> common use case - output.push_back(c0 << 2 | (c1 & 0xF0) >> 4); - output.push_back((c1 & 0x0F) << 4 | ((c2 & 0x3C) >> 2)); - output.push_back(((c2 & 0x03) << 6) | (c3 & 0x3F)); + output.push_back(static_cast(c0 << 2 | (c1 & 0xF0) >> 4)); + output.push_back(static_cast((c1 & 0x0F) << 4 | ((c2 & 0x3C) >> 2))); + output.push_back(static_cast(((c2 & 0x03) << 6) | (c3 & 0x3F))); } return output; diff --git a/src/datadog/curl.cpp b/src/datadog/curl.cpp index d1ef8d3d..0f828bc1 100644 --- a/src/datadog/curl.cpp +++ b/src/datadog/curl.cpp @@ -223,8 +223,6 @@ class CurlImpl { void *user_data); static std::size_t on_read_body(char *data, std::size_t, std::size_t length, void *user_data); - static bool is_non_whitespace(unsigned char); - static char to_lower(unsigned char); public: explicit CurlImpl(const std::shared_ptr &, const Clock &, @@ -367,8 +365,8 @@ Expected CurlImpl::post( throw_on_error( curl_.easy_setopt_errorbuffer(handle.get(), request->error_buffer)); throw_on_error(curl_.easy_setopt_post(handle.get(), 1)); - throw_on_error(curl_.easy_setopt_postfieldsize(handle.get(), - request->request_body.size())); + throw_on_error(curl_.easy_setopt_postfieldsize( + handle.get(), static_cast(request->request_body.size()))); throw_on_error( curl_.easy_setopt_postfields(handle.get(), request->request_body.data())); throw_on_error( @@ -431,29 +429,19 @@ std::size_t CurlImpl::on_read_header(char *data, std::size_t, // https://curl.se/libcurl/c/CURLOPT_HEADERFUNCTION.html // - const char *const begin = data; - const char *const end = begin + length; - const char *const colon = std::find(begin, end, ':'); - if (colon == end) { + StringView sv(data, length); + auto colon_idx = sv.find(":"); + if (colon_idx == StringView::npos) { return length; } - const auto key = trim(range(begin, colon)); - const auto value = trim(range(colon + 1, end)); + const auto key = trim(sv.substr(0, colon_idx)); + const auto value = trim(sv.substr(colon_idx + 1)); - std::string key_lower; - key_lower.reserve(key.size()); - std::transform(key.begin(), key.end(), std::back_inserter(key_lower), - &to_lower); - - request->response_headers_lower.emplace(std::move(key_lower), value); + request->response_headers_lower.emplace(to_lower(key), value); return length; } -bool CurlImpl::is_non_whitespace(unsigned char ch) { return !std::isspace(ch); } - -char CurlImpl::to_lower(unsigned char ch) { return std::tolower(ch); } - std::size_t CurlImpl::on_read_body(char *data, std::size_t, std::size_t length, void *user_data) { const auto request = static_cast(user_data); @@ -480,7 +468,7 @@ CURLMcode CurlImpl::log_on_error(CURLMcode result) { void CurlImpl::run() { int num_messages_remaining; CURLMsg *message; - const int max_wait_milliseconds = 10000; + constexpr int max_wait_milliseconds = 10000; std::unique_lock lock(mutex_); for (;;) { @@ -513,18 +501,18 @@ void CurlImpl::run() { auto *request = reinterpret_cast(user_data); const auto timeout = request->deadline - clock_().tick; if (timeout <= std::chrono::steady_clock::time_point::duration::zero()) { - std::string message; - message += + std::string error_message; + error_message += "Request deadline exceeded before request was even added to " "libcurl " "event loop. Deadline was "; - message += std::to_string( + error_message += std::to_string( -std::chrono::duration_cast(timeout) .count()); - message += " nanoseconds ago."; + error_message += " nanoseconds ago."; request->on_error( Error{Error::CURL_DEADLINE_EXCEEDED_BEFORE_REQUEST_START, - std::move(message)}); + std::move(error_message)}); curl_.easy_cleanup(handle); delete request; @@ -533,8 +521,10 @@ void CurlImpl::run() { } log_on_error(curl_.easy_setopt_timeout_ms( - handle, std::chrono::duration_cast(timeout) - .count())); + handle, + static_cast( + std::chrono::duration_cast(timeout) + .count()))); log_on_error(curl_.multi_add_handle(multi_handle_, handle)); request_handles_.insert(handle); } @@ -631,9 +621,7 @@ CurlImpl::HeaderReader::HeaderReader( : response_headers_lower_(response_headers_lower) {} Optional CurlImpl::HeaderReader::lookup(StringView key) const { - buffer_.clear(); - std::transform(key.begin(), key.end(), std::back_inserter(buffer_), - &to_lower); + buffer_ = to_lower(key); const auto found = response_headers_lower_->find(buffer_); if (found == response_headers_lower_->end()) { diff --git a/src/datadog/expected.h b/src/datadog/expected.h index f442fc55..05d5add6 100644 --- a/src/datadog/expected.h +++ b/src/datadog/expected.h @@ -52,16 +52,16 @@ class Expected { public: Expected() = default; Expected(const Expected&) = default; - Expected(Expected&) = default; Expected(Expected&&) = default; + + Expected(const Value&); + Expected(Value&&); + Expected(const Error&); + Expected(Error&&); + Expected& operator=(const Expected&) = default; Expected& operator=(Expected&&) = default; - template - Expected(Other&&); - template - Expected& operator=(Other&&); - // Return whether this object holds a `Value` (as opposed to an `Error`). bool has_value() const noexcept; explicit operator bool() const noexcept; @@ -99,15 +99,21 @@ class Expected { }; template -template -Expected::Expected(Other&& other) : data_(std::forward(other)) {} +Expected::Expected(const Value& value) : data_(value) {} template -template -Expected& Expected::operator=(Other&& other) { - data_ = std::forward(other); - return *this; -} +Expected::Expected(Value&& value) : data_(std::move(value)) {} + +template +Expected::Expected(const Error& error) : data_(error) {} + +template +Expected::Expected(Error&& error) : data_(std::move(error)) {} + +// Expected& Expected::operator=(Other&& other) { +// data_ = std::forward(other); +// return *this; +// } template bool Expected::has_value() const noexcept { @@ -194,15 +200,15 @@ class Expected { public: Expected() = default; Expected(const Expected&) = default; - Expected(Expected&) = default; Expected(Expected&&) = default; Expected& operator=(const Expected&) = default; Expected& operator=(Expected&&) = default; - template - Expected(Other&&); - template - Expected& operator=(Other&&); + Expected(const Error&); + Expected(Error&&); + Expected(decltype(nullopt)); + explicit Expected(const Optional&); + explicit Expected(Optional&&); void swap(Expected& other); @@ -221,14 +227,12 @@ class Expected { const Error* if_error() const&& = delete; }; -template -Expected::Expected(Other&& other) : data_(std::forward(other)) {} - -template -Expected& Expected::operator=(Other&& other) { - data_ = std::forward(other); - return *this; -} +inline Expected::Expected(const Error& error) : data_(error) {} +inline Expected::Expected(Error&& error) : data_(std::move(error)) {} +inline Expected::Expected(decltype(nullopt)) : data_(nullopt) {} +inline Expected::Expected(const Optional& data) : data_(data) {} +inline Expected::Expected(Optional&& data) + : data_(std::move(data)) {} inline void Expected::swap(Expected& other) { data_.swap(other.data_); } diff --git a/src/datadog/extraction_util.cpp b/src/datadog/extraction_util.cpp index 0e57e840..9eaf6484 100644 --- a/src/datadog/extraction_util.cpp +++ b/src/datadog/extraction_util.cpp @@ -70,12 +70,13 @@ Expected> extract_id_header(const DictReader& headers, StringView header_kind, StringView style_name, int base) { + Optional result; auto found = headers.lookup(header); if (!found) { - return nullopt; + return result; } - auto result = parse_uint64(*found, base); - if (auto* error = result.if_error()) { + auto parsed_id = parse_uint64(*found, base); + if (auto* error = parsed_id.if_error()) { std::string prefix; prefix += "Could not extract "; append(prefix, style_name); @@ -88,7 +89,8 @@ Expected> extract_id_header(const DictReader& headers, prefix += ' '; return error->with_prefix(prefix); } - return *result; + result = std::move(*parsed_id); + return result; } } // namespace diff --git a/src/datadog/http_client.cpp b/src/datadog/http_client.cpp index 227d1b59..07d72476 100644 --- a/src/datadog/http_client.cpp +++ b/src/datadog/http_client.cpp @@ -1,15 +1,19 @@ #include "http_client.h" +#include + #include "string_util.h" namespace datadog { namespace tracing { +constexpr StringView k_scheme_separator = "://"; +constexpr StringView k_supported[] = {"http", "https", "unix", "http+unix", + "https+unix"}; + Expected HTTPClient::URL::parse(StringView input) { - const StringView separator = "://"; - const auto after_scheme = std::search(input.begin(), input.end(), - separator.begin(), separator.end()); - if (after_scheme == input.end()) { + const auto after_scheme = input.find(k_scheme_separator); + if (after_scheme == StringView::npos) { std::string message; message += "Datadog Agent URL is missing the \"://\" separator: \""; append(message, input); @@ -17,19 +21,17 @@ Expected HTTPClient::URL::parse(StringView input) { return Error{Error::URL_MISSING_SEPARATOR, std::move(message)}; } - const StringView scheme = range(input.begin(), after_scheme); - const StringView supported[] = {"http", "https", "unix", "http+unix", - "https+unix"}; + const StringView scheme = input.substr(0, after_scheme); const auto found = - std::find(std::begin(supported), std::end(supported), scheme); - if (found == std::end(supported)) { + std::find(std::begin(k_supported), std::end(k_supported), scheme); + if (found == std::end(k_supported)) { std::string message; message += "Unsupported URI scheme \""; append(message, scheme); message += "\" in Datadog Agent URL \""; append(message, input); message += "\". The following are supported:"; - for (const auto& supported_scheme : supported) { + for (const auto& supported_scheme : k_supported) { message += ' '; append(message, supported_scheme); } @@ -37,7 +39,7 @@ Expected HTTPClient::URL::parse(StringView input) { } const StringView authority_and_path = - range(after_scheme + separator.size(), input.end()); + input.substr(after_scheme + k_scheme_separator.size()); // If the scheme is for unix domain sockets, then there's no way to // distinguish the path-to-socket from the path-to-resource. Some // implementations require that the forward slashes in the path-to-socket @@ -69,12 +71,13 @@ Expected HTTPClient::URL::parse(StringView input) { // Again, though, we're only parsing URLs that designate the location of // the Datadog Agent service, and so they will not have a resource // location. Still, let's parse it properly. - const auto after_authority = - std::find(authority_and_path.begin(), authority_and_path.end(), '/'); + const auto after_authority = authority_and_path.find('/'); return HTTPClient::URL{ std::string(scheme), - std::string(range(authority_and_path.begin(), after_authority)), - std::string(range(after_authority, authority_and_path.end()))}; + std::string(authority_and_path.substr(0, after_authority)), + (after_authority == StringView::npos) + ? "" + : std::string(authority_and_path.substr(after_authority))}; } } // namespace tracing diff --git a/src/datadog/limiter.cpp b/src/datadog/limiter.cpp index 80c0f01f..dfe0d488 100644 --- a/src/datadog/limiter.cpp +++ b/src/datadog/limiter.cpp @@ -70,11 +70,10 @@ Limiter::Result Limiter::allow(int tokens_requested) { num_requested_++; // refill "tokens" if (now >= next_refresh_) { - auto intervals = - (now - next_refresh_).count() / refresh_interval_.count() + 1; + intervals = (now - next_refresh_).count() / refresh_interval_.count() + 1; if (intervals > 0) { next_refresh_ += refresh_interval_ * intervals; - num_tokens_ += intervals * tokens_per_refresh_; + num_tokens_ += static_cast(intervals * tokens_per_refresh_); if (num_tokens_ > max_tokens_) { num_tokens_ = max_tokens_; } diff --git a/src/datadog/msgpack.h b/src/datadog/msgpack.h index 2df96f1c..52b8a919 100644 --- a/src/datadog/msgpack.h +++ b/src/datadog/msgpack.h @@ -160,7 +160,7 @@ inline void pack_integer(std::string& buffer, std::int32_t value) { } inline Expected pack_string(std::string& buffer, StringView value) { - return pack_string(buffer, value.begin(), value.size()); + return pack_string(buffer, value.data(), value.size()); } } // namespace msgpack diff --git a/src/datadog/parse_util.cpp b/src/datadog/parse_util.cpp index 7377d1ff..b444cd8d 100644 --- a/src/datadog/parse_util.cpp +++ b/src/datadog/parse_util.cpp @@ -18,14 +18,15 @@ namespace { template Expected parse_integer(StringView input, int base, StringView kind) { Integer value; - const auto status = std::from_chars(input.begin(), input.end(), value, base); + const char *const end = input.data() + input.size(); + const auto status = std::from_chars(input.data(), end, value, base); if (status.ec == std::errc::invalid_argument) { std::string message; message += "Is not a valid integer: \""; append(message, input); message += '\"'; return Error{Error::INVALID_INTEGER, std::move(message)}; - } else if (status.ptr != input.end()) { + } else if (status.ptr != end) { std::string message; message += "Integer has trailing characters in: \""; append(message, input); @@ -101,9 +102,9 @@ std::vector parse_list(StringView input) { return items; } - const char *const end = input.end(); + const char *const end = input.data() + input.size(); + const char *current = input.data(); - const char *current = input.begin(); const char *begin_delim; do { const char *begin_item = @@ -135,16 +136,16 @@ Expected> parse_tags( std::string value; for (const StringView &token : list) { - const auto separator = std::find(token.begin(), token.end(), ':'); + const auto separator = token.find(':'); - if (separator == token.end()) { + if (separator == std::string::npos) { key = std::string{trim(token)}; } else { - key = std::string{trim(range(token.begin(), separator))}; + key = std::string{trim(token.substr(0, separator))}; if (key.empty()) { continue; } - value = std::string{trim(range(separator + 1, token.end()))}; + value = std::string{trim(token.substr(separator + 1))}; } // If there are duplicate values, then the last one wins. diff --git a/src/datadog/platform_util.cpp b/src/datadog/platform_util.cpp index f074fbe0..aa32bf3e 100644 --- a/src/datadog/platform_util.cpp +++ b/src/datadog/platform_util.cpp @@ -1,34 +1,37 @@ #include "platform_util.h" -#include -#include -#include -#include - -#include - -#include "string_util.h" - +// clang-format off #if defined(__x86_64__) || defined(_M_X64) -#define DD_SDK_CPU_ARCH "x86_64" +# define DD_SDK_CPU_ARCH "x86_64" #elif defined(i386) || defined(__i386__) || defined(__i386) || defined(_M_IX86) -#define DD_SDK_CPU_ARCH "x86" +# define DD_SDK_CPU_ARCH "x86" #elif defined(__aarch64__) || defined(_M_ARM64) -#define DD_SDK_CPU_ARCH "arm64" +# define DD_SDK_CPU_ARCH "arm64" #else -#define DD_SDK_CPU_ARCH "unknown" +# define DD_SDK_CPU_ARCH "unknown" #endif -#if defined(__APPLE__) -#include -#define DD_SDK_OS "Darwin" -#define DD_SDK_KERNEL "Darwin" -#elif defined(__linux__) || defined(__unix__) -#define DD_SDK_OS "GNU/Linux" -#define DD_SDK_KERNEL "Linux" -#else -#define DD_SDK_OS "unknown" +#if defined(__APPLE__) || defined(__linux__) || defined(__unix__) +# include +# include +# include +# include +# if defined(__APPLE__) +# include +# define DD_SDK_OS "Darwin" +# define DD_SDK_KERNEL "Darwin" +# elif defined(__linux__) || defined(__unix__) +# define DD_SDK_OS "GNU/Linux" +# define DD_SDK_KERNEL "Linux" +# include "string_util.h" +# include +# endif +#elif defined(_MSC_VER) +# include +# include +# include #endif +// clang-format on namespace datadog { namespace tracing { @@ -67,12 +70,9 @@ std::string get_os_version() { return ""; } -#else -std::string get_os_version() { return ""; } #endif -#if defined(__APPLE__) || defined(__linux__) - +#if defined(__APPLE__) || defined(__linux__) || defined(__unix__) HostInfo _get_host_info() { HostInfo res; @@ -91,7 +91,86 @@ HostInfo _get_host_info() { return res; } +#elif defined(_MSC_VER) +std::tuple get_windows_info() { + // NOTE(@dmehala): Retrieving the Windows version has been complicated since + // Windows 8.1. The `GetVersion` function and its variants depend on the + // application manifest, which is the lowest version supported by the + // application. Use `RtlGetVersion` to obtain the accurate OS version + // regardless of the manifest. + using RtlGetVersion = auto (*)(LPOSVERSIONINFOEXW)->NTSTATUS; + + RtlGetVersion func = + (RtlGetVersion)GetProcAddress(GetModuleHandleA("ntdll"), "RtlGetVersion"); + + if (func) { + OSVERSIONINFOEXW os_info; + ZeroMemory(&os_info, sizeof(OSVERSIONINFO)); + os_info.dwOSVersionInfoSize = sizeof(os_info); + + if (func(&os_info) == 0) { + // major, minor, build -> name, version + switch (os_info.dwMajorVersion) { + case 5: { + switch (os_info.dwMinorVersion) { + case 0: + return {"Windows 2000", "NT 5.0"}; + case 1: + return {"Windows XP", "NT 5.1"}; + case 2: + return {"Windows XP", "NT 5.2"}; + default: + return {"Windows XP", "NT 5.x"}; + } + }; break; + case 6: { + switch (os_info.dwMinorVersion) { + case 0: + return {"Windows Vista", "NT 6.0"}; + case 1: + return {"Windows 7", "NT 6.1"}; + case 2: + return {"Windows 8", "NT 6.2"}; + case 3: + return {"Windows 8.1", "NT 6.3"}; + default: + return {"Windows 8.1", "NT 6.x"}; + } + }; break; + case 10: { + if (os_info.dwBuildNumber >= 10240 && os_info.dwBuildNumber < 22000) { + return {"Windows 10", "NT 10.0"}; + } else if (os_info.dwBuildNumber >= 22000) { + return {"Windows 11", "21H2"}; + } + }; break; + } + } + } + + return {"", ""}; +} + +HostInfo _get_host_info() { + HostInfo host; + host.cpu_architecture = DD_SDK_CPU_ARCH; + + auto [os, os_version] = get_windows_info(); + host.os = std::move(os); + host.os_version = std::move(os_version); + char buffer[256]; + if (0 == gethostname(buffer, sizeof(buffer))) { + host.hostname = buffer; + } + + return host; +} +#else +HostInfo _get_host_info() { + HostInfo res; + return res; +} #endif } // namespace @@ -103,12 +182,24 @@ HostInfo get_host_info() { std::string get_hostname() { return get_host_info().hostname; } -int get_process_id() { return ::getpid(); } +int get_process_id() { +#if defined(_MSC_VER) + return GetCurrentProcessId(); +#else + return ::getpid(); +#endif +} int at_fork_in_child(void (*on_fork)()) { +#if defined(_MSC_VER) + // Windows does not have `fork`, and so this is not relevant there. + (void)on_fork; + return 0; +#else // https://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_atfork.html return pthread_atfork(/*before fork*/ nullptr, /*in parent*/ nullptr, /*in child*/ on_fork); +#endif } } // namespace tracing diff --git a/src/datadog/remote_config.cpp b/src/datadog/remote_config.cpp index e26084d2..a7be6d82 100644 --- a/src/datadog/remote_config.cpp +++ b/src/datadog/remote_config.cpp @@ -6,9 +6,9 @@ #include #include "base64.h" -#include "datadog/string_view.h" #include "json.hpp" #include "random.h" +#include "string_view.h" #include "version.h" using namespace nlohmann::literals; @@ -38,7 +38,7 @@ constexpr std::array capabilities_byte_array( std::size_t j = sizeof(in) - 1; std::array res{}; for (std::size_t i = 0; i < sizeof(in); ++i) { - res[j--] = in >> (i * 8); + res[j--] = static_cast(in >> (i * 8)); } return res; @@ -56,12 +56,12 @@ ConfigUpdate parse_dynamic_config(const nlohmann::json& j) { if (auto sampling_rate_it = j.find("tracing_sampling_rate"); sampling_rate_it != j.cend() && sampling_rate_it->is_number()) { - config_update.trace_sampling_rate = *sampling_rate_it; + config_update.trace_sampling_rate = sampling_rate_it->get(); } if (auto tags_it = j.find("tracing_tags"); tags_it != j.cend() && tags_it->is_array()) { - config_update.tags = *tags_it; + config_update.tags = tags_it->get>(); } if (auto tracing_enabled_it = j.find("tracing_enabled"); diff --git a/src/datadog/sampling_util.h b/src/datadog/sampling_util.h index 825484ae..162c571b 100644 --- a/src/datadog/sampling_util.h +++ b/src/datadog/sampling_util.h @@ -35,7 +35,8 @@ inline std::uint64_t max_id_from_rate(Rate rate) { return std::numeric_limits::max(); } - return rate * static_cast(std::numeric_limits::max()); + return static_cast( + rate * static_cast(std::numeric_limits::max())); } } // namespace tracing diff --git a/src/datadog/span.cpp b/src/datadog/span.cpp index 5a6a12cb..3522923b 100644 --- a/src/datadog/span.cpp +++ b/src/datadog/span.cpp @@ -3,7 +3,6 @@ #include #include -#include "datadog/sampling_mechanism.h" #include "dict_writer.h" #include "optional.h" #include "span_config.h" diff --git a/src/datadog/string_util.cpp b/src/datadog/string_util.cpp index 8ff173d5..90e99832 100644 --- a/src/datadog/string_util.cpp +++ b/src/datadog/string_util.cpp @@ -34,6 +34,15 @@ void to_lower(std::string& text) { [](unsigned char ch) { return std::tolower(ch); }); } +std::string to_lower(StringView sv) { + std::string s; + s.reserve(sv.size()); + std::transform(sv.begin(), sv.end(), std::back_inserter(s), + [](char c) { return std::tolower(c); }); + + return s; +} + std::string to_string(bool b) { return b ? "true" : "false"; } std::string to_string(double d, size_t precision) { @@ -79,12 +88,15 @@ std::string join_tags( } bool starts_with(StringView subject, StringView prefix) { - if (prefix.size() > subject.size()) { - return false; + auto s = subject.data(); + auto p = prefix.data(); + const auto prefix_end = p + prefix.size(); + while (*s == *p) { + ++s; + ++p; } - return std::mismatch(subject.begin(), subject.end(), prefix.begin()).second == - prefix.end(); + return p == prefix_end; } StringView trim(StringView str) { diff --git a/src/datadog/string_util.h b/src/datadog/string_util.h index bd50e32c..090b1018 100644 --- a/src/datadog/string_util.h +++ b/src/datadog/string_util.h @@ -9,13 +9,9 @@ namespace datadog { namespace tracing { -// Return a `string_view` over the specified range of characters `[begin, end)`. -inline StringView range(const char* begin, const char* end) { - return StringView{begin, std::size_t(end - begin)}; -} - // Convert the specified `text` to lower case in-place. void to_lower(std::string& text); +std::string to_lower(StringView sv); // Return a string representation of the specified boolean `value`. // The result is "true" for `true` and "false" for `false`. diff --git a/src/datadog/tag_propagation.cpp b/src/datadog/tag_propagation.cpp index 58dac1a0..2d41a4a6 100644 --- a/src/datadog/tag_propagation.cpp +++ b/src/datadog/tag_propagation.cpp @@ -42,8 +42,10 @@ Expected decode_tag( return Error{Error::MALFORMED_TRACE_TAGS, std::move(message)}; } - const StringView key = range(entry.begin(), separator); - const StringView value = range(separator + 1, entry.end()); + const StringView key = entry.substr(0, separator - entry.cbegin()); + const StringView value = + entry.substr(separator - entry.cbegin() + 1, + separator - entry.cbegin() + entry.size()); destination.emplace_back(std::string(key), std::string(value)); return nullopt; @@ -61,18 +63,29 @@ void append_tag(std::string& serialized_tags, StringView tag_key, Expected>> decode_tags( StringView header_value) { std::vector> tags; - - auto iter = header_value.begin(); - const auto end = header_value.end(); - if (iter == end) { - // An empty string means no tags. - return tags; + tags.reserve(header_value.size()); + + if (header_value.empty()) return tags; + + std::size_t beg = 0; + for (std::size_t i = 0; i < header_value.size(); ++i) { + if (header_value[i] == ',') { + auto result = decode_tag(tags, header_value.substr(beg, i - beg)); + if (auto* error = result.if_error()) { + std::string prefix; + prefix += "Error decoding trace tags \""; + append(prefix, header_value); + prefix += "\": "; + return error->with_prefix(prefix); + } + + beg = i + 1; + } } - decltype(iter) next; - do { - next = std::find(iter, end, ','); - auto result = decode_tag(tags, range(iter, next)); + if (beg != header_value.size()) { + auto result = + decode_tag(tags, header_value.substr(beg, header_value.size() - beg)); if (auto* error = result.if_error()) { std::string prefix; prefix += "Error decoding trace tags \""; @@ -80,8 +93,7 @@ Expected>> decode_tags( prefix += "\": "; return error->with_prefix(prefix); } - iter = next + 1; - } while (next != end); + } return tags; } diff --git a/src/datadog/trace_id.cpp b/src/datadog/trace_id.cpp index 9667b30a..b8ba90f1 100644 --- a/src/datadog/trace_id.cpp +++ b/src/datadog/trace_id.cpp @@ -49,9 +49,10 @@ Expected TraceID::parse_hex(StringView input) { } // Parse the lower part and the higher part separately. - const auto divider = input.begin() + (input.size() - 16); - const auto high_hex = range(input.begin(), divider); - const auto low_hex = range(divider, input.end()); + const auto divider = input.size() - 16; + const auto high_hex = input.substr(0, divider); + const auto low_hex = input.substr(divider); + TraceID trace_id; auto result = parse_hex_piece(low_hex); diff --git a/src/datadog/trace_sampler_config.cpp b/src/datadog/trace_sampler_config.cpp index ac4afcd6..4a2cc229 100644 --- a/src/datadog/trace_sampler_config.cpp +++ b/src/datadog/trace_sampler_config.cpp @@ -226,7 +226,7 @@ Expected finalize_config( message += "Trace sampling max_per_second must be greater than zero, but the " "following value was given: "; - message += std::to_string(*config.max_per_second); + message += std::to_string(max_per_second); return Error{Error::MAX_PER_SECOND_OUT_OF_RANGE, std::move(message)}; } result.max_per_second = max_per_second; diff --git a/src/datadog/tracer.cpp b/src/datadog/tracer.cpp index 6d3bd980..76bb19a5 100644 --- a/src/datadog/tracer.cpp +++ b/src/datadog/tracer.cpp @@ -3,8 +3,6 @@ #include #include -#include "datadog/runtime_id.h" -#include "datadog/trace_sampler_config.h" #include "datadog_agent.h" #include "dict_reader.h" #include "environment.h" @@ -15,6 +13,7 @@ #include "logger.h" #include "parse_util.h" #include "platform_util.h" +#include "runtime_id.h" #include "span.h" #include "span_config.h" #include "span_data.h" @@ -22,6 +21,7 @@ #include "tag_propagation.h" #include "tags.h" #include "trace_sampler.h" +#include "trace_sampler_config.h" #include "trace_segment.h" #include "tracer_signature.h" #include "version.h" diff --git a/src/datadog/tracer_config.cpp b/src/datadog/tracer_config.cpp index d3177c4a..95e52a27 100644 --- a/src/datadog/tracer_config.cpp +++ b/src/datadog/tracer_config.cpp @@ -211,19 +211,19 @@ Expected load_tracer_env_config(Logger &logger) { const auto global_styles = styles_from_env(environment::DD_TRACE_PROPAGATION_STYLE); - if (auto propagation_value = + if (auto trace_extraction_styles = styles_from_env(environment::DD_TRACE_PROPAGATION_STYLE_EXTRACT)) { - env_cfg.extraction_styles = std::move(*propagation_value); - } else if (auto propagation_value = + env_cfg.extraction_styles = std::move(*trace_extraction_styles); + } else if (auto extraction_styles = styles_from_env(environment::DD_PROPAGATION_STYLE_EXTRACT)) { - env_cfg.extraction_styles = std::move(*propagation_value); + env_cfg.extraction_styles = std::move(*extraction_styles); } else { env_cfg.extraction_styles = global_styles; } - if (auto injection_styles = + if (auto trace_injection_styles = styles_from_env(environment::DD_TRACE_PROPAGATION_STYLE_INJECT)) { - env_cfg.injection_styles = std::move(*injection_styles); + env_cfg.injection_styles = std::move(*trace_injection_styles); } else if (auto injection_styles = styles_from_env(environment::DD_PROPAGATION_STYLE_INJECT)) { env_cfg.injection_styles = std::move(*injection_styles); diff --git a/src/datadog/version.cpp b/src/datadog/version.cpp index a795d9a5..036f81d0 100644 --- a/src/datadog/version.cpp +++ b/src/datadog/version.cpp @@ -3,10 +3,11 @@ namespace datadog { namespace tracing { -#define VERSION "v0.2.0" +#define DD_TRACE_VERSION "v0.2.0" -const char* const tracer_version = VERSION; -const char* const tracer_version_string = "[dd-trace-cpp version " VERSION "]"; +const char* const tracer_version = DD_TRACE_VERSION; +const char* const tracer_version_string = + "[dd-trace-cpp version " DD_TRACE_VERSION "]"; } // namespace tracing } // namespace datadog diff --git a/src/datadog/w3c_propagation.cpp b/src/datadog/w3c_propagation.cpp index e8d96d18..0795ccc0 100644 --- a/src/datadog/w3c_propagation.cpp +++ b/src/datadog/w3c_propagation.cpp @@ -59,40 +59,41 @@ Optional extract_traceparent(ExtractedData& result, // group 5) static const std::regex regex{pattern}; - std::match_results match; - if (!std::regex_match(traceparent.begin(), traceparent.end(), match, regex)) { + std::cmatch match; + if (!std::regex_match(traceparent.data(), match, regex)) { return "malformed_traceparent"; } assert(match.ready()); assert(match.size() == 5 + 1); - const auto to_string_view = [](const auto& submatch) { - assert(submatch.first <= submatch.second); - return StringView{submatch.first, - std::size_t(submatch.second - submatch.first)}; + const auto to_string_view = [traceparent](const std::cmatch& match, + const std::size_t index) { + assert(index < match.size()); + return StringView(traceparent.data() + match.position(index), + std::size_t(match.length(index))); }; - const auto version = to_string_view(match[1]); + const auto version = to_string_view(match, 1); if (version == "ff") { return "invalid_version"; } - if (version == "00" && !to_string_view(match[5]).empty()) { + if (version == "00" && !to_string_view(match, 5).empty()) { return "malformed_traceparent"; } - result.trace_id = *TraceID::parse_hex(to_string_view(match[2])); + result.trace_id = *TraceID::parse_hex(to_string_view(match, 2)); if (result.trace_id == 0) { return "trace_id_zero"; } - result.parent_id = *parse_uint64(to_string_view(match[3]), 16); + result.parent_id = *parse_uint64(to_string_view(match, 3), 16); if (*result.parent_id == 0) { return "parent_id_zero"; } - const auto flags = *parse_uint64(to_string_view(match[4]), 16); + const auto flags = *parse_uint64(to_string_view(match, 4), 16); result.sampling_priority = int(flags & 1); return nullopt; @@ -109,61 +110,58 @@ struct PartiallyParsedTracestate { // specified `tracestate`. If `tracestate` does not have a Datadog-specific // portion, return `nullopt`. Optional parse_tracestate(StringView tracestate) { - Optional result; - - const char* const begin = tracestate.begin(); - const char* const end = tracestate.end(); - const char* pair_begin = begin; - while (pair_begin != end) { - const char* const pair_end = std::find(pair_begin, end, ','); + const std::size_t begin = 0; + const std::size_t end = tracestate.size(); + std::size_t pair_begin = begin; + while (pair_begin < end) { + const std::size_t pair_end = tracestate.find(',', pair_begin); // Note that since this `pair` is `strip`ped, `pair_begin` is not // necessarily equal to `pair.begin()` (similarly for the ends). - const auto pair = trim(range(pair_begin, pair_end)); + const auto pair = + trim(tracestate.substr(pair_begin, pair_end - pair_begin)); if (pair.empty()) { - pair_begin = pair_end == end ? end : pair_end + 1; + pair_begin = (pair_end == StringView::npos) ? end : pair_end + 1; continue; } - const auto kv_separator = std::find(pair.begin(), pair.end(), '='); - if (kv_separator == pair.end()) { + const auto kv_separator = pair.find('='); + if (kv_separator == StringView::npos) { // This is an invalid entry because it contains a non-whitespace character // but not a "=". // Let's move on to the next entry. - pair_begin = pair_end == end ? end : pair_end + 1; + pair_begin = (pair_end == StringView::npos) ? end : pair_end + 1; continue; } - const auto key = range(pair.begin(), kv_separator); + const auto key = pair.substr(0, kv_separator); if (key != "dd") { // On to the next. - pair_begin = pair_end == end ? end : pair_end + 1; + pair_begin = (pair_end == StringView::npos) ? end : pair_end + 1; continue; } - // We found the "dd" entry. - result.emplace(); - result->datadog_value = range(kv_separator + 1, pair.end()); + PartiallyParsedTracestate result; + result.datadog_value = pair.substr(kv_separator + 1); // `result->other_entries` is whatever was before the "dd" entry and // whatever is after the "dd" entry, but without an extra comma in the // middle. - if (pair_begin != begin) { + if (pair_begin != 0) { // There's a prefix - append(result->other_entries, range(begin, pair_begin - 1)); - if (pair_end != end) { + append(result.other_entries, tracestate.substr(0, pair_begin - 1)); + if (pair_end != StringView::npos && pair_end + 1 < end) { // and a suffix - append(result->other_entries, range(pair_end, end)); + append(result.other_entries, tracestate.substr(pair_end)); } - } else if (pair_end != end) { + } else if (pair_end != StringView::npos && pair_end + 1 < end) { // There's just a suffix - append(result->other_entries, range(pair_end + 1, end)); + append(result.other_entries, tracestate.substr(pair_end + 1)); } - break; + return result; } - return result; + return nullopt; } - // Fill the specified `result` with information parsed from the specified // `datadog_value`. `datadog_value` is the value of the "dd" entry in the // "tracestate" header. @@ -175,27 +173,23 @@ Optional parse_tracestate(StringView tracestate) { // - `sampling_priority` // - `additional_datadog_w3c_tracestate` void parse_datadog_tracestate(ExtractedData& result, StringView datadog_value) { - const char* const begin = datadog_value.begin(); - const char* const end = datadog_value.end(); - const char* pair_begin = begin; - while (pair_begin != end) { - const char* const pair_end = std::find(pair_begin, end, ';'); - const auto pair = range(pair_begin, pair_end); + const std::size_t end = datadog_value.size(); + std::size_t pair_begin = 0; + while (pair_begin < end) { + const std::size_t pair_end = datadog_value.find(';', pair_begin); + const auto pair = datadog_value.substr(pair_begin, pair_end - pair_begin); + pair_begin = (pair_end == StringView::npos) ? end : pair_end + 1; if (pair.empty()) { - // chaff! - pair_begin = pair_end == end ? end : pair_end + 1; continue; } - const auto kv_separator = std::find(pair_begin, pair_end, ':'); - if (kv_separator == pair_end) { - // chaff! - pair_begin = pair_end == end ? end : pair_end + 1; + const auto kv_separator = pair.find(':'); + if (kv_separator == StringView::npos) { continue; } - const auto key = range(pair_begin, kv_separator); - const auto value = range(kv_separator + 1, pair_end); + const auto key = pair.substr(0, kv_separator); + const auto value = pair.substr(kv_separator + 1); if (key == "o") { result.origin = std::string{value}; // Equal signs are allowed in the value of "origin," but equal signs are @@ -206,8 +200,6 @@ void parse_datadog_tracestate(ExtractedData& result, StringView datadog_value) { } else if (key == "s") { const auto maybe_priority = parse_int(value, 10); if (!maybe_priority) { - // chaff! - pair_begin = pair_end == end ? end : pair_end + 1; continue; } const int priority = *maybe_priority; @@ -253,8 +245,6 @@ void parse_datadog_tracestate(ExtractedData& result, StringView datadog_value) { } append(*entries, pair); } - - pair_begin = pair_end == end ? end : pair_end + 1; } } @@ -266,7 +256,7 @@ void parse_datadog_tracestate(ExtractedData& result, StringView datadog_value) { // `parse_datadog_tracestate`. void extract_tracestate(ExtractedData& result, const DictReader& headers) { const auto maybe_tracestate = headers.lookup("tracestate"); - if (!maybe_tracestate) { + if (!maybe_tracestate || maybe_tracestate->empty()) { return; } diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 528459d8..c4ae657e 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -38,9 +38,11 @@ add_executable(tests test_trace_sampler.cpp ) -target_link_libraries(tests dd_trace_cpp-static ${COVERAGE_LIBRARIES}) -if(BUILD_COVERAGE) - target_link_options(tests PRIVATE -fprofile-arcs -ftest-coverage) -endif() +target_link_libraries(tests + PRIVATE + libcurl + dd_trace_cpp-static + dd_trace::specs +) add_subdirectory(system-tests) diff --git a/test/mocks/collectors.h b/test/mocks/collectors.h index ef48e3a4..3d0495a2 100644 --- a/test/mocks/collectors.h +++ b/test/mocks/collectors.h @@ -7,9 +7,9 @@ #include #include -#include #include #include +#include #include #include #include @@ -39,7 +39,7 @@ struct MockCollector : public Collector { } std::size_t span_count() const { - return std::accumulate(chunks.begin(), chunks.end(), 0, + return std::accumulate(chunks.begin(), chunks.end(), std::size_t(0), [](std::size_t total, const auto& chunk) { return total + chunk.size(); }); diff --git a/test/mocks/http_clients.h b/test/mocks/http_clients.h index ad59006c..3459ce90 100644 --- a/test/mocks/http_clients.h +++ b/test/mocks/http_clients.h @@ -50,7 +50,7 @@ struct MockHTTPClient : public HTTPClient { on_error_ = on_error; set_headers(request_headers); } - return post_error; + return Expected(post_error); } void drain(std::chrono::steady_clock::time_point /*deadline*/) override { diff --git a/test/mocks/loggers.cpp b/test/mocks/loggers.cpp index 85a1d308..f669b617 100644 --- a/test/mocks/loggers.cpp +++ b/test/mocks/loggers.cpp @@ -10,7 +10,7 @@ std::ostream& operator<<(std::ostream& stream, for (; i < entries.size(); ++i) { const auto& entry = entries[i]; const auto kind_name = - entry.kind == MockLogger::Entry::ERROR ? "ERROR" : "STARTUP"; + entry.kind == MockLogger::Entry::DD_ERROR ? "ERROR" : "STARTUP"; stream << '\n' << (i + 1) << ". " << kind_name << ": "; std::visit([&](const auto& value) { stream << value; }, entry.payload); } diff --git a/test/mocks/loggers.h b/test/mocks/loggers.h index eeed55b2..016a90a2 100644 --- a/test/mocks/loggers.h +++ b/test/mocks/loggers.h @@ -25,7 +25,7 @@ struct NullLogger : public Logger { struct MockLogger : public Logger { struct Entry { - enum Kind { ERROR, STARTUP } kind; + enum Kind { DD_ERROR, STARTUP } kind; std::variant payload; }; @@ -49,7 +49,7 @@ struct MockLogger : public Logger { if (echo) { *echo << stream.str() << '\n'; } - entries.push_back(Entry{Entry::ERROR, stream.str()}); + entries.push_back(Entry{Entry::DD_ERROR, stream.str()}); } void log_startup(const LogFunc& write) override { @@ -67,7 +67,7 @@ struct MockLogger : public Logger { if (echo) { *echo << error << '\n'; } - entries.push_back(Entry{Entry::ERROR, error}); + entries.push_back(Entry{Entry::DD_ERROR, error}); } void log_error(StringView message) override { @@ -75,10 +75,10 @@ struct MockLogger : public Logger { if (echo) { *echo << message << '\n'; } - entries.push_back(Entry{Entry::ERROR, std::string(message)}); + entries.push_back(Entry{Entry::DD_ERROR, std::string(message)}); } - int error_count() const { return count(Entry::ERROR); } + int error_count() const { return count(Entry::DD_ERROR); } int startup_count() const { return count(Entry::STARTUP); } @@ -94,7 +94,7 @@ struct MockLogger : public Logger { std::lock_guard lock{mutex}; auto found = std::find_if( entries.begin(), entries.end(), - [](const Entry& entry) { return entry.kind == Entry::ERROR; }); + [](const Entry& entry) { return entry.kind == Entry::DD_ERROR; }); return std::get(found->payload); } diff --git a/test/system-tests/CMakeLists.txt b/test/system-tests/CMakeLists.txt index de8b7e0b..d563da59 100644 --- a/test/system-tests/CMakeLists.txt +++ b/test/system-tests/CMakeLists.txt @@ -1,3 +1,6 @@ +# add_library(httplib INTERFACE) +# target_include_directories(httplib SYSTEM INTERFACE .) + add_executable(parametric-http-server) target_sources(parametric-http-server @@ -12,7 +15,7 @@ target_include_directories(parametric-http-server ../../examples/http-server/common ) -target_link_libraries(parametric-http-server dd_trace_cpp-static) +target_link_libraries(parametric-http-server dd_trace_cpp-static dd_trace::specs) install( TARGETS parametric-http-server diff --git a/test/system-tests/main.cpp b/test/system-tests/main.cpp index 05f5774e..6b2b9051 100644 --- a/test/system-tests/main.cpp +++ b/test/system-tests/main.cpp @@ -160,7 +160,7 @@ int main(int argc, char* argv[]) { res.status = 500; }); - std::signal(SIGTERM, hard_stop); + // std::signal(SIGTERM, hard_stop); svr.listen("0.0.0.0", *port); return 0; } diff --git a/test/test_limiter.cpp b/test/test_limiter.cpp index 2ffb42f8..0564ac5a 100644 --- a/test/test_limiter.cpp +++ b/test/test_limiter.cpp @@ -8,6 +8,12 @@ using namespace datadog::tracing; +// clang-format off +#if defined(_MSC_VER) +# define timegm _mkgmtime +#endif +// clang-format on + TEST_CASE("limiter") { // Starting calendar time 2007-03-12 00:00:00 std::tm start{}; diff --git a/test/test_remote_config.cpp b/test/test_remote_config.cpp index 6c59fc59..2800a1c7 100644 --- a/test/test_remote_config.cpp +++ b/test/test_remote_config.cpp @@ -19,7 +19,7 @@ REMOTE_CONFIG_TEST("first payload") { TracerConfig tracer_cfg; const std::time_t mock_time = 1672484400; - const Clock clock = []() { + const Clock clock = [mock_time]() { TimePoint result; result.wall = std::chrono::system_clock::from_time_t(mock_time); return result; @@ -53,7 +53,7 @@ REMOTE_CONFIG_TEST("response processing") { TracerConfig tracer_cfg; const std::time_t mock_time = 1672484400; - const Clock clock = []() { + const Clock clock = [mock_time]() { TimePoint result; result.wall = std::chrono::system_clock::from_time_t(mock_time); return result; diff --git a/test/test_tracer_config.cpp b/test/test_tracer_config.cpp index fb9f72c0..42c93bc1 100644 --- a/test/test_tracer_config.cpp +++ b/test/test_tracer_config.cpp @@ -23,11 +23,6 @@ #include "mocks/event_schedulers.h" #include "mocks/loggers.h" #include "test.h" -#ifdef _MSC_VER -#include // SetEnvironmentVariable -#else -#include // setenv, unsetenv -#endif namespace datadog { namespace tracing { @@ -68,7 +63,10 @@ class EnvGuard { void set_value(const std::string& value) { #ifdef _MSC_VER - ::SetEnvironmentVariable(name_.c_str(), value.c_str()); + std::string envstr{name_}; + envstr += "="; + envstr += value; + assert(_putenv(envstr.c_str()) == 0); #else const bool overwrite = true; ::setenv(name_.c_str(), value.c_str(), overwrite); @@ -77,7 +75,9 @@ class EnvGuard { void unset() { #ifdef _MSC_VER - ::SetEnvironmentVariable(name_.c_str(), NULL); + std::string envstr{name_}; + envstr += "="; + assert(_putenv(envstr.c_str()) == 0); #else ::unsetenv(name_.c_str()); #endif @@ -212,7 +212,6 @@ TEST_CASE("TracerConfig::defaults") { }; auto test_case = GENERATE(values({ - {"empty", "", {}, nullopt}, {"missing colon", "foo", { @@ -561,7 +560,6 @@ TEST_CASE("TracerConfig::agent") { // during configuration. For the purposes of configuration, any // value is accepted. {"we don't parse port", x, "bogus", x, "http", "localhost:bogus"}, - {"even empty is ok", x, "", x, "http", "localhost:"}, {"URL", x, x, "http://dd-agent:8080", "http", "dd-agent:8080"}, {"URL overrides scheme", x, x, "https://dd-agent:8080", "https", "dd-agent:8080"}, @@ -683,7 +681,6 @@ TEST_CASE("TracerConfig::trace_sampler") { }; auto test_case = GENERATE(values({ - {"empty", "", {Error::INVALID_DOUBLE}}, {"nonsense", "nonsense", {Error::INVALID_DOUBLE}}, {"trailing space", "0.23 ", {Error::INVALID_DOUBLE}}, {"out of range of double", "123e9999999999", {Error::INVALID_DOUBLE}}, @@ -749,7 +746,6 @@ TEST_CASE("TracerConfig::trace_sampler") { }; auto test_case = GENERATE(values({ - {"empty", "", {Error::INVALID_DOUBLE}}, {"nonsense", "nonsense", {Error::INVALID_DOUBLE}}, {"trailing space", "23 ", {Error::INVALID_DOUBLE}}, {"out of range of double", "123e9999999999", {Error::INVALID_DOUBLE}}, @@ -1060,13 +1056,17 @@ TEST_CASE("TracerConfig::span_sampler") { SECTION("failed usage") { SECTION("unable to open") { - std::filesystem::path defunct; - { - SomewhatSecureTemporaryFile file; - REQUIRE(file.is_open()); - defunct = file.path(); - } - const EnvGuard guard{"DD_SPAN_SAMPLING_RULES_FILE", defunct.string()}; + // It's not elegant, but neither an empty path nor a path to a + // deleted file work for this test on Windows. + // + // On Windows, deleting the file doesn't delete the file, and an + // empty path deletes the environment variable rather than set the + // environment variable empty. + // + // An easy workaround is to choose a path that is very likely not on + // the file system. + const std::string invalid = "ooga/booga/booga/booga"; + const EnvGuard guard{"DD_SPAN_SAMPLING_RULES_FILE", invalid}; auto finalized = finalize_config(config); REQUIRE(!finalized); REQUIRE(finalized.error().code == Error::SPAN_SAMPLING_RULES_FILE_IO); @@ -1169,8 +1169,9 @@ TEST_CASE("TracerConfig propagation styles") { }; // brevity - const auto datadog = PropagationStyle::DATADOG, - b3 = PropagationStyle::B3, none = PropagationStyle::NONE; + static const auto datadog = PropagationStyle::DATADOG, + b3 = PropagationStyle::B3, + none = PropagationStyle::NONE; // clang-format off auto test_case = GENERATE(values({ {__LINE__, "Datadog", x, {datadog}}, @@ -1346,7 +1347,6 @@ TEST_CASE("configure 128-bit trace IDs") { {__LINE__, "no", false}, {__LINE__, "nein", true}, {__LINE__, "0", false}, - {__LINE__, "", true}, })); // clang-format on diff --git a/test/test_tracer_telemetry.cpp b/test/test_tracer_telemetry.cpp index 534a17d4..2b0a7b21 100644 --- a/test/test_tracer_telemetry.cpp +++ b/test/test_tracer_telemetry.cpp @@ -16,7 +16,7 @@ using namespace datadog::tracing; TEST_CASE("Tracer telemetry", "[telemetry]") { const std::time_t mock_time = 1672484400; - const Clock clock = []() { + const Clock clock = [mock_time]() { TimePoint result; result.wall = std::chrono::system_clock::from_time_t(mock_time); return result; From 02d927b17a64d1d1f62ac58aa31e3f3517e4b467 Mon Sep 17 00:00:00 2001 From: Damien Mehala Date: Mon, 20 May 2024 22:04:10 +0200 Subject: [PATCH 2/5] fix: rebase and potential UB --- src/datadog/platform_util.cpp | 2 +- src/datadog/string_util.cpp | 10 +++++----- src/datadog/w3c_propagation.cpp | 2 -- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/datadog/platform_util.cpp b/src/datadog/platform_util.cpp index aa32bf3e..9ea57753 100644 --- a/src/datadog/platform_util.cpp +++ b/src/datadog/platform_util.cpp @@ -98,7 +98,7 @@ std::tuple get_windows_info() { // application manifest, which is the lowest version supported by the // application. Use `RtlGetVersion` to obtain the accurate OS version // regardless of the manifest. - using RtlGetVersion = auto (*)(LPOSVERSIONINFOEXW)->NTSTATUS; + using RtlGetVersion = auto(*)(LPOSVERSIONINFOEXW)->NTSTATUS; RtlGetVersion func = (RtlGetVersion)GetProcAddress(GetModuleHandleA("ntdll"), "RtlGetVersion"); diff --git a/src/datadog/string_util.cpp b/src/datadog/string_util.cpp index 90e99832..0455a24f 100644 --- a/src/datadog/string_util.cpp +++ b/src/datadog/string_util.cpp @@ -91,14 +91,14 @@ bool starts_with(StringView subject, StringView prefix) { auto s = subject.data(); auto p = prefix.data(); const auto prefix_end = p + prefix.size(); - while (*s == *p) { - ++s; - ++p; + while (p < prefix_end) { + if (*s++ != *p++) { + return false; + } } - return p == prefix_end; + return true; } - StringView trim(StringView str) { str.remove_prefix( std::min(str.find_first_not_of(k_spaces_characters), str.size())); diff --git a/src/datadog/w3c_propagation.cpp b/src/datadog/w3c_propagation.cpp index 0795ccc0..6d139e74 100644 --- a/src/datadog/w3c_propagation.cpp +++ b/src/datadog/w3c_propagation.cpp @@ -215,8 +215,6 @@ void parse_datadog_tracestate(ExtractedData& result, StringView datadog_value) { } } else if (key == "p") { if (value.size() != 16) { - // chaff! - pair_begin = pair_end == end ? end : pair_end + 1; continue; } From 4526b199d66cc4554779167e37416233c963524f Mon Sep 17 00:00:00 2001 From: Damien Mehala Date: Mon, 20 May 2024 22:34:46 +0200 Subject: [PATCH 3/5] Apply suggestions from code review --- README.md | 5 ++--- src/datadog/expected.h | 5 ----- src/datadog/platform_util.cpp | 1 - test/system-tests/CMakeLists.txt | 3 --- 4 files changed, 2 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 8d505d10..cbe532aa 100644 --- a/README.md +++ b/README.md @@ -39,13 +39,12 @@ int main() { See the [examples](examples) directory for more extensive usage examples. ## Platform Support -The library is currently compatible with and has been tested on the following CPU architecture, OS and compiler combinations: +The library has been tested and is compatible on the following CPU architecture, OS and compiler combinations: - x86_64 and arm64 Linux with GCC 11.4. - x86_64 and arm64 Linux with Clang 14. - x86_64 Windows with MSVC 2022. - arm64 macOS with Apple Clang 15. -All possible configurations cannot be tested. However, any conforming C++17 compiler should be supported. ## Building and Installation @@ -67,7 +66,7 @@ cmake --build build -j ``` By default CMake will generate both static and shared libraries. To build either on of them use -as configuration parameter `BUILD_SHARED_LIBS` or `BUILD_STATIC_LIBS`. Example: +either `BUILD_SHARED_LIBS` or `BUILD_STATIC_LIBS`. Example: ```shell cmake -B build -DBUILD_SHARED_LIBS=1 . diff --git a/src/datadog/expected.h b/src/datadog/expected.h index 05d5add6..3b7f3e6f 100644 --- a/src/datadog/expected.h +++ b/src/datadog/expected.h @@ -110,11 +110,6 @@ Expected::Expected(const Error& error) : data_(error) {} template Expected::Expected(Error&& error) : data_(std::move(error)) {} -// Expected& Expected::operator=(Other&& other) { -// data_ = std::forward(other); -// return *this; -// } - template bool Expected::has_value() const noexcept { return std::holds_alternative(data_); diff --git a/src/datadog/platform_util.cpp b/src/datadog/platform_util.cpp index 9ea57753..600f4309 100644 --- a/src/datadog/platform_util.cpp +++ b/src/datadog/platform_util.cpp @@ -109,7 +109,6 @@ std::tuple get_windows_info() { os_info.dwOSVersionInfoSize = sizeof(os_info); if (func(&os_info) == 0) { - // major, minor, build -> name, version switch (os_info.dwMajorVersion) { case 5: { switch (os_info.dwMinorVersion) { diff --git a/test/system-tests/CMakeLists.txt b/test/system-tests/CMakeLists.txt index d563da59..5a5223ba 100644 --- a/test/system-tests/CMakeLists.txt +++ b/test/system-tests/CMakeLists.txt @@ -1,6 +1,3 @@ -# add_library(httplib INTERFACE) -# target_include_directories(httplib SYSTEM INTERFACE .) - add_executable(parametric-http-server) target_sources(parametric-http-server From 8fd333d270d72b49433efc72c884addd5821cd66 Mon Sep 17 00:00:00 2001 From: Damien Mehala Date: Mon, 20 May 2024 22:46:23 +0200 Subject: [PATCH 4/5] uncomment std::signal --- test/system-tests/main.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/system-tests/main.cpp b/test/system-tests/main.cpp index 6b2b9051..05f5774e 100644 --- a/test/system-tests/main.cpp +++ b/test/system-tests/main.cpp @@ -160,7 +160,7 @@ int main(int argc, char* argv[]) { res.status = 500; }); - // std::signal(SIGTERM, hard_stop); + std::signal(SIGTERM, hard_stop); svr.listen("0.0.0.0", *port); return 0; } From 7355dcc7b814268b3266b330438868676f43031f Mon Sep 17 00:00:00 2001 From: Damien Mehala Date: Mon, 20 May 2024 23:11:26 +0200 Subject: [PATCH 5/5] include csignal --- test/system-tests/main.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/test/system-tests/main.cpp b/test/system-tests/main.cpp index 05f5774e..6e0cd3c3 100644 --- a/test/system-tests/main.cpp +++ b/test/system-tests/main.cpp @@ -4,6 +4,7 @@ #include #include +#include #include #include #include