From 8d1dd2f341c00d24cccc1a5f0236422d265b1c5c Mon Sep 17 00:00:00 2001 From: paullegranddc <82819397+paullegranddc@users.noreply.github.com> Date: Fri, 3 Jun 2022 13:56:28 +0200 Subject: [PATCH 1/5] Upgrade all crates to edition 2021 (#21) --- ddcommon/Cargo.toml | 2 +- ddprof/Cargo.toml | 2 +- ddtelemetry/Cargo.toml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ddcommon/Cargo.toml b/ddcommon/Cargo.toml index 09c4e1493..dce19f2f4 100644 --- a/ddcommon/Cargo.toml +++ b/ddcommon/Cargo.toml @@ -2,7 +2,7 @@ # This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2021-Present Datadog, Inc. [package] -edition = "2018" +edition = "2021" license = "Apache-2.0" name = "ddcommon" version = "0.7.0-rc.1" diff --git a/ddprof/Cargo.toml b/ddprof/Cargo.toml index fae899d71..ed49dc754 100644 --- a/ddprof/Cargo.toml +++ b/ddprof/Cargo.toml @@ -4,7 +4,7 @@ [package] name = "ddprof" version = "0.7.0-rc.1" -edition = "2018" +edition = "2021" license = "Apache-2.0" [lib] diff --git a/ddtelemetry/Cargo.toml b/ddtelemetry/Cargo.toml index 7bbf62c3a..928ab424b 100644 --- a/ddtelemetry/Cargo.toml +++ b/ddtelemetry/Cargo.toml @@ -1,5 +1,5 @@ [package] -edition = "2018" +edition = "2021" license = "Apache 2.0" name = "ddtelemetry" version = "0.7.0-rc.1" From 996f295e4f7c5a163fbacee50684dd5d4961aa2d Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Wed, 8 Jun 2022 11:25:48 +0200 Subject: [PATCH 2/5] Disable certain new clippy lints - including clippy::significant_drop_in_scrutinee (#23) --- ddprof-ffi/src/vec.rs | 1 + ddtelemetry/src/worker.rs | 3 +++ 2 files changed, 4 insertions(+) diff --git a/ddprof-ffi/src/vec.rs b/ddprof-ffi/src/vec.rs index 456350fbd..8258d1c19 100644 --- a/ddprof-ffi/src/vec.rs +++ b/ddprof-ffi/src/vec.rs @@ -124,6 +124,7 @@ impl Default for Vec { } } +#[allow(clippy::get_first)] #[cfg(test)] mod test { use crate::vec::*; diff --git a/ddtelemetry/src/worker.rs b/ddtelemetry/src/worker.rs index fed005dc8..0468026a6 100644 --- a/ddtelemetry/src/worker.rs +++ b/ddtelemetry/src/worker.rs @@ -256,6 +256,8 @@ impl TelemetryWorker { let mut series = Vec::new(); for (context_key, extra_tags, points) in self.data.metric_buckets.flush_series() { let context_guard = self.data.metric_contexts.get_context(context_key); + + #[allow(clippy::significant_drop_in_scrutinee)] let context = match context_guard.read() { Some(context) => context, None => { @@ -268,6 +270,7 @@ impl TelemetryWorker { continue; } }; + let mut tags = extra_tags; tags.extend(context.tags.iter().cloned()); series.push(data::metrics::Serie { From 5ea0aee012be50bc66ee3d84008c7a716d2805ba Mon Sep 17 00:00:00 2001 From: r1viollet Date: Wed, 8 Jun 2022 09:39:34 +0200 Subject: [PATCH 3/5] Following alpine upgrade, Bump rust version to 1.60 --- .github/workflows/lint.yml | 2 +- .github/workflows/test.yml | 2 +- README.md | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index fd5c09022..3bd389fb0 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -18,7 +18,7 @@ jobs: name: "clippy #${{ matrix.version }}" strategy: matrix: - version: [nightly, "1.56.1"] + version: [nightly, "1.60"] runs-on: ubuntu-latest steps: - name: Checkout sources diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index ea3e000f5..daa434e21 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -13,7 +13,7 @@ jobs: rust_version: [""] include: - platform: "ubuntu-latest" - rust_version: "1.56.1" + rust_version: "1.60" steps: - name: Checkout sources uses: actions/checkout@v2 diff --git a/README.md b/README.md index f32ab230c..64f2e987b 100644 --- a/README.md +++ b/README.md @@ -27,4 +27,4 @@ bash ffi-build.sh /opt/libdatadog #### Build Dependencies -Rust 1.56 or newer with cargo. Some platforms may need protoc; others have it shipped in prost-build. +Rust 1.60 or newer with cargo. Some platforms may need protoc; others have it shipped in prost-build. From 6a2c981f0e6e6ea3954590f8751e77fdf5e49c3b Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Tue, 28 Jun 2022 07:28:42 -0600 Subject: [PATCH 4/5] Build FFI in CI, including Windows (#17) Co-authored-by: Pawel Chojnacki Co-authored-by: Gregory LEOCADIE --- .github/actions/cache/action.yaml | 7 +++- .github/workflows/test.yml | 65 ++++++++++++++++++++++++++++++- ddprof-ffi/cbindgen.toml | 20 +++++++++- examples/ffi/CMakeLists.txt | 21 ++++++++-- examples/ffi/exporter.cpp | 2 +- examples/ffi/profiles.c | 2 +- ffi-build.sh | 36 ++++++++++++----- 7 files changed, 132 insertions(+), 21 deletions(-) diff --git a/.github/actions/cache/action.yaml b/.github/actions/cache/action.yaml index b401ef8be..484794b70 100644 --- a/.github/actions/cache/action.yaml +++ b/.github/actions/cache/action.yaml @@ -5,16 +5,19 @@ inputs: rust_version: default: "latest" required: false + build_profile: + default: "unspecified" + required: false runs: using: composite steps: - name: ⚡ Cache - uses: actions/cache@v2 + uses: actions/cache@v3 with: path: | ~/.cargo/registry/ ~/.cargo/git/db/ ~/.cargo/bin/ target/ - key: v1-${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.toml') }}-${{ inputs.rust_version }} + key: v1-${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.toml') }}-${{ inputs.rust_version }}-${{ inputs.build_profile }} diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index daa434e21..c4d56aade 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -2,6 +2,7 @@ name: Test on: [push] env: CARGO_TERM_COLOR: always + RUST_VERSION: 1.60.0 jobs: test: @@ -9,11 +10,11 @@ jobs: runs-on: ${{ matrix.platform }} strategy: matrix: - platform: [windows-latest, ubuntu-latest, macos-latest] + platform: [windows-latest, ubuntu-latest, macos-latest] rust_version: [""] include: - platform: "ubuntu-latest" - rust_version: "1.60" + rust_version: "${RUST_VERSION}" steps: - name: Checkout sources uses: actions/checkout@v2 @@ -30,3 +31,63 @@ jobs: run: cargo build --workspace --verbose - name: "[${{ steps.rust-version.outputs.version}}] cargo test --workspace --verbose" run: cargo test --workspace --verbose + + ffi: + name: "FFI #${{ matrix.platform }} ${{ matrix.rust_version }}" + runs-on: ${{ matrix.platform }} + strategy: + matrix: + platform: [windows-latest, ubuntu-latest, macos-latest] + rust_version: [""] + include: + - platform: "ubuntu-latest" + rust_version: "${RUST_VERSION}" + steps: + + - name: Checkout sources + uses: actions/checkout@v2 + + - name: Setup output dir + shell: bash + run: | + WORKSPACE_PATH=${{ github.workspace }} + if [[ "${{ matrix.platform }}" == "windows-latest" ]]; then + WORKSPACE_PATH=$(cygpath -ua '${{ github.workspace }}') + fi + echo "OUTPUT_FOLDER=$WORKSPACE_PATH/artifacts" >> $GITHUB_ENV + + - name: Cache + uses: ./.github/actions/cache + with: + rust_version: ${{ matrix.rust_version }} + build_profile: "release" + + - name: Install Rust ${{ matrix.rust_version }} + if: ${{ matrix.rust_version != '' }} + run: rustup install ${{ matrix.rust_version }} && rustup default ${{ matrix.rust_version }} + + - id: rust-version + run: "echo ::set-output name=version::$(rustc --version)" + + - name: "Generate FFI" + shell: bash + run: | + chmod +x ffi-build.sh + ./ffi-build.sh ${OUTPUT_FOLDER} + + - name: 'Publish libdatadog' + uses: actions/upload-artifact@v2 + if: '${{ always() }}' + with: + if-no-files-found: error + name: libdatadog.${{ matrix.platform }} + path: ${{ github.workspace }}/artifacts + retention-days: 1 + + - name: "Test building C bindings" + shell: bash + run: | + mkdir examples/ffi/build + cd examples/ffi/build + cmake -S .. -DDDProf_ROOT=$OUTPUT_FOLDER + cmake --build . diff --git a/ddprof-ffi/cbindgen.toml b/ddprof-ffi/cbindgen.toml index 25b044b33..e418aa509 100644 --- a/ddprof-ffi/cbindgen.toml +++ b/ddprof-ffi/cbindgen.toml @@ -14,8 +14,24 @@ sys_includes = ["stdbool.h", "stddef.h", "stdint.h"] after_includes = """ +#if defined(_MSC_VER) #define DDPROF_FFI_CHARSLICE_C(string) \\ - /* NOTE: Compilation fails if you pass in a char* instead of a literal */ ((ddprof_ffi_CharSlice) {.ptr = "" string, .len = sizeof(string) - 1})""" +/* NOTE: Compilation fails if you pass in a char* instead of a literal */ {.ptr = "" string, .len = sizeof(string) - 1} +#else +#define DDPROF_FFI_CHARSLICE_C(string) \\ +/* NOTE: Compilation fails if you pass in a char* instead of a literal */ ((ddprof_ffi_CharSlice){ .ptr = "" string, .len = sizeof(string) - 1 }) +#endif + +#if defined(__cplusplus) && (__cplusplus >= 201703L) +# define DD_CHECK_RETURN [[nodiscard]] +#elif defined(_Check_return_) /* SAL */ +# define DD_CHECK_RETURN _Check_return_ +#elif (defined(__has_attribute) && __has_attribute(warn_unused_result)) || \\ + (defined(__GNUC__) && (__GNUC__ >= 4)) +# define DD_CHECK_RETURN __attribute__((__warn_unused_result__)) +#else +# define DD_CHECK_RETURN +#endif""" [export] prefix = "ddprof_ffi_" @@ -28,7 +44,7 @@ prefix_with_name = true rename_variants = "ScreamingSnakeCase" [fn] -must_use = "__attribute__((warn_unused_result))" +must_use = "DD_CHECK_RETURN" [parse] parse_deps = true diff --git a/examples/ffi/CMakeLists.txt b/examples/ffi/CMakeLists.txt index 3ceffc5a6..ce39f0253 100644 --- a/examples/ffi/CMakeLists.txt +++ b/examples/ffi/CMakeLists.txt @@ -4,7 +4,22 @@ cmake_minimum_required(VERSION 3.19) find_package(DDProf) add_executable(exporter exporter.cpp) -target_compile_features(exporter PRIVATE cxx_std_11) -target_link_libraries(exporter DDProf::FFI) + add_executable(profiles profiles.c) -target_link_libraries(profiles DDProf::FFI) + +set(EXTERNAL_LIBS DDProf::FFI) + +if (CMAKE_CXX_COMPILER_ID MATCHES "MSVC") + set(EXTERNAL_LIBS "${EXTERNAL_LIBS};NtDll;UserEnv;Bcrypt;crypt32;wsock32;ws2_32;shlwapi") +endif() + +if (CMAKE_CXX_COMPILER_ID MATCHES "MSVC") + target_compile_features(exporter PRIVATE cxx_std_20) # needed for designated initializers + target_compile_definitions(exporter PUBLIC _CRT_SECURE_NO_WARNINGS) +else() + target_compile_features(exporter PRIVATE cxx_std_11) +endif() + +target_link_libraries(exporter ${EXTERNAL_LIBS}) + +target_link_libraries(profiles ${EXTERNAL_LIBS}) diff --git a/examples/ffi/exporter.cpp b/examples/ffi/exporter.cpp index a1f18a176..197f64321 100644 --- a/examples/ffi/exporter.cpp +++ b/examples/ffi/exporter.cpp @@ -9,7 +9,7 @@ extern "C" { #include static ddprof_ffi_Slice_c_char to_slice_c_char(const char *s) { - return (ddprof_ffi_Slice_c_char){.ptr = s, .len = strlen(s)}; + return {.ptr = s, .len = strlen(s)}; } struct Deleter { diff --git a/examples/ffi/profiles.c b/examples/ffi/profiles.c index 9c5292947..d184d637a 100644 --- a/examples/ffi/profiles.c +++ b/examples/ffi/profiles.c @@ -31,7 +31,7 @@ int main(void) { struct ddprof_ffi_Location root_location = { // yes, a zero-initialized mapping is valid - .mapping = (struct ddprof_ffi_Mapping){}, + .mapping = (struct ddprof_ffi_Mapping){0}, .lines = (struct ddprof_ffi_Slice_line){&root_line, 1}, }; int64_t value = 10; diff --git a/ffi-build.sh b/ffi-build.sh index a8d422edc..c09319629 100755 --- a/ffi-build.sh +++ b/ffi-build.sh @@ -13,6 +13,8 @@ mkdir -v -p "$destdir/include/ddprof" "$destdir/lib/pkgconfig" "$destdir/cmake" version=$(awk -F\" '$1 ~ /^version/ { print $2 }' < ddprof-ffi/Cargo.toml) target="$(rustc -vV | awk '/^host:/ { print $2 }')" shared_library_suffix=".so" +static_library_suffix=".a" +library_prefix="lib" remove_rpath=0 fix_macos_rpath=0 @@ -32,6 +34,7 @@ case "$target" in # on alpine musl, Rust adds some weird runpath to cdylibs remove_rpath=1 ;; + "x86_64-apple-darwin") expected_native_static_libs=" -framework Security -framework CoreFoundation -liconv -lSystem -lresolv -lc -lm -liconv" native_static_libs="${expected_native_static_libs}" @@ -39,10 +42,20 @@ case "$target" in # fix usage of library in macos via rpath fix_macos_rpath=1 ;; + "x86_64-unknown-linux-gnu"|"aarch64-unknown-linux-gnu") expected_native_static_libs=" -ldl -lrt -lpthread -lgcc_s -lc -lm -lrt -lpthread -lutil -ldl -lutil" native_static_libs=" -ldl -lrt -lpthread -lc -lm -lrt -lpthread -lutil -ldl -lutil" ;; + + "x86_64-pc-windows-msvc") + expected_native_static_libs="" # I don't know what to expect + native_static_libs="" # I don't know what to expect + shared_library_suffix=".dll" + static_library_suffix=".lib" + library_prefix="" + ;; + *) >&2 echo "Unknown platform '${target}'" exit 1 @@ -71,28 +84,31 @@ cp -v LICENSE LICENSE-3rdparty.yml NOTICE "$destdir/" export RUSTFLAGS="${RUSTFLAGS:- -C relocation-model=pic}" -echo "Building the libddprof_ffi library (may take some time)..." +echo "Building the ddprof_ffi library (may take some time)..." cargo build --release --target "${target}" -cp -v "target/${target}/release/libddprof_ffi.a" "target/${target}/release/libddprof_ffi${shared_library_suffix}" "$destdir/lib/" + +shared_library_name="${library_prefix}ddprof_ffi${shared_library_suffix}" +static_library_name="${library_prefix}ddprof_ffi${static_library_suffix}" +cp -v "target/${target}/release/$static_library_name" "target/${target}/release/$shared_library_name" "$destdir/lib/" if [[ "$remove_rpath" -eq 1 ]]; then - patchelf --remove-rpath "$destdir/lib/libddprof_ffi${shared_library_suffix}" + patchelf --remove-rpath "$destdir/lib/${shared_library_name}" fi if [[ "$fix_macos_rpath" -eq 1 ]]; then - install_name_tool -id @rpath/libddprof_ffi${shared_library_suffix} "$destdir/lib/libddprof_ffi${shared_library_suffix}" + install_name_tool -id @rpath/${shared_library_name} "$destdir/lib/${shared_library_name}" fi # objcopy might not be available on macOS -if command -v objcopy > /dev/null; then +if command -v objcopy > /dev/null && [[ "$target" != "x86_64-pc-windows-msvc" ]]; then # Remove .llvmbc section which is not useful for clients - objcopy --remove-section .llvmbc "$destdir/lib/libddprof_ffi.a" + objcopy --remove-section .llvmbc "$destdir/lib/${static_library_name}" # Ship debug information separate from shared library, so that downstream packages can selectively include it # https://sourceware.org/gdb/onlinedocs/gdb/Separate-Debug-Files.html - objcopy --only-keep-debug "$destdir/lib/libddprof_ffi${shared_library_suffix}" "$destdir/lib/libddprof_ffi${shared_library_suffix}.debug" - strip -S "$destdir/lib/libddprof_ffi${shared_library_suffix}" - objcopy --add-gnu-debuglink="$destdir/lib/libddprof_ffi${shared_library_suffix}.debug" "$destdir/lib/libddprof_ffi${shared_library_suffix}" + objcopy --only-keep-debug "$destdir/lib/$shared_library_name" "$destdir/lib/$shared_library_name.debug" + strip -S "$destdir/lib/$shared_library_name" + objcopy --add-gnu-debuglink="$destdir/lib/$shared_library_name.debug" "$destdir/lib/$shared_library_name" fi echo "Checking that native-static-libs are as expected for this platform..." @@ -118,7 +134,7 @@ if [ -n "$unexpected_native_libs" ]; then fi cd - -echo "Generating the ddprof/ffi.h header..." +echo "Generating the $destdir/include/ddprof/ffi.h header..." cbindgen --crate ddprof-ffi --config ddprof-ffi/cbindgen.toml --output "$destdir/include/ddprof/ffi.h" # CI doesn't have any clang tooling From 76b1f600ee35f472e2a60dcf7708a00c2f040c08 Mon Sep 17 00:00:00 2001 From: Ivo Anjo Date: Fri, 8 Jul 2022 10:32:41 +0100 Subject: [PATCH 5/5] Package 0.7.0 Ruby gem I'm following the release instructions described in --- ruby/README.md | 2 +- ruby/Rakefile | 10 +++++----- ruby/lib/libdatadog/version.rb | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/ruby/README.md b/ruby/README.md index 4f7515616..585bbdf0d 100644 --- a/ruby/README.md +++ b/ruby/README.md @@ -22,7 +22,7 @@ Note: No Ruby needed to run this! It all runs inside docker :) Note: Publishing new releases to rubygems.org can only be done by Datadog employees. 1. [ ] Locate the new libdatadog release on GitHub: -2. [ ] Update the `LIB_GITHUB_RELEASES` section of the with the new version +2. [ ] Update the `LIB_VERSION_TO_PACKAGE` and `LIB_GITHUB_RELEASES` sections of the `Rakefile` with the new version 3. [ ] Update the file with the `LIB_VERSION` and `VERSION` to use 4. [ ] Commit change, open PR, get it merged 5. [ ] Release by running `docker-compose run push_to_rubygems`. diff --git a/ruby/Rakefile b/ruby/Rakefile index 57e14ccb9..d96ccf7bf 100644 --- a/ruby/Rakefile +++ b/ruby/Rakefile @@ -11,7 +11,7 @@ require "rubygems/package" RSpec::Core::RakeTask.new(:spec) -LIB_VERSION_TO_PACKAGE = "0.7.0-rc.1" +LIB_VERSION_TO_PACKAGE = "0.7.0" unless LIB_VERSION_TO_PACKAGE.start_with?(Libdatadog::LIB_VERSION) raise "`LIB_VERSION_TO_PACKAGE` setting in (#{LIB_VERSION_TO_PACKAGE}) does not match " \ "`LIB_VERSION` setting in (#{Libdatadog::LIB_VERSION})" @@ -20,22 +20,22 @@ end LIB_GITHUB_RELEASES = [ { file: "libdatadog-aarch64-alpine-linux-musl.tar.gz", - sha256: "d3a6d71f45ce0f95978dcb31525389ee3ea02ea0170f00be466ff6ce1a1f6047", + sha256: "de0ba9c95da07d89b487d99b36f767f763f1272ebdff5b532b576473d64f3c66", ruby_platform: "aarch64-linux-musl" }, { file: "libdatadog-aarch64-unknown-linux-gnu.tar.gz", - sha256: "e792c923d5cdc6d581da87d12ab789ae578fa588fb2a220f72660f8d25df6de8", + sha256: "256750fe9ebcd9bf8426b83f89f52572f01559ae5f4add3a4c46df84fc122eb6", ruby_platform: "aarch64-linux" }, { file: "libdatadog-x86_64-alpine-linux-musl.tar.gz", - sha256: "402a1f40b55e1bad022d1391793a4ed79c8b41bfd9265cad721960be77c12f1e", + sha256: "0ae6b3d9d37e6af8e31a44286424c34ddd4945022efbaed6978fc60a8b923ba6", ruby_platform: "x86_64-linux-musl" }, { file: "libdatadog-x86_64-unknown-linux-gnu.tar.gz", - sha256: "9b43711b23e42e76684eeced9e8d25183d350060d087d755622fa6748fa79aa5", + sha256: "b5f617d08e637e9a201437198e715b2f3688d5367d0af572988c80dd3c2e6b81", ruby_platform: "x86_64-linux" } ] diff --git a/ruby/lib/libdatadog/version.rb b/ruby/lib/libdatadog/version.rb index 5c12a60e5..448380605 100644 --- a/ruby/lib/libdatadog/version.rb +++ b/ruby/lib/libdatadog/version.rb @@ -6,7 +6,7 @@ module Libdatadog GEM_MAJOR_VERSION = "1" GEM_MINOR_VERSION = "0" - GEM_PRERELEASE_VERSION = ".rc1" # remember to include dot prefix, if needed! + GEM_PRERELEASE_VERSION = "" # remember to include dot prefix, if needed! private_constant :GEM_MAJOR_VERSION, :GEM_MINOR_VERSION, :GEM_PRERELEASE_VERSION # The gem version scheme is lib_version.gem_major.gem_minor[.prerelease].