Merge branch 'main' into paullgdc/factorise_generic_ffi_code

.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 }}

.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

.github/workflows/test.yml

@@ -2,18 +2,19 @@ name: Test
 on: [push]
 env: 
   CARGO_TERM_COLOR: always
+  RUST_VERSION: 1.60.0
 
 jobs:
   test:
     name: "cargo test --workspace #${{ matrix.platform }} ${{ matrix.rust_version }}"
     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.56.1"
+            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 .

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.

ddcommon-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_"

ddcommon-ffi/src/vec.rs

@@ -124,6 +124,7 @@ impl<T> Default for Vec<T> {
     }
 }
 
+#[allow(clippy::get_first)]
 #[cfg(test)]
 mod test {
     use crate::vec::*;

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"

ddprof-ffi/cbindgen.toml

@@ -24,7 +24,7 @@ prefix_with_name = true
 rename_variants = "ScreamingSnakeCase"
 
 [fn]
-must_use = "__attribute__((warn_unused_result))"
+must_use = "DD_CHECK_RETURN"
 
 [parse]
 parse_deps = true

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]

ddtelemetry/Cargo.toml

@@ -1,5 +1,5 @@
 [package]
-edition = "2018"
+edition = "2021"
 license = "Apache 2.0"
 name = "ddtelemetry"
 version = "0.7.0-rc.1"

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 {

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})

examples/ffi/exporter.cpp

@@ -9,7 +9,7 @@ extern "C" {
 #include <thread>
 
 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 {

examples/ffi/profiles.c

@@ -32,7 +32,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;

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,17 +34,28 @@ 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}"
         shared_library_suffix=".dylib"
         # 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..."
@@ -121,7 +137,7 @@ cd -
 echo "Building tools"
 cargo build --package tools --bins
 
-echo "Generating the ddprof/ffi.h header..."
+echo "Generating $destdir/include/libdatadog headers...
 cbindgen --crate ddcommon-ffi --config ddcommon-ffi/cbindgen.toml --output "$destdir/include/libdatadog/ddcommon.h"
 cbindgen --crate ddprof-ffi --config ddprof-ffi/cbindgen.toml --output "$destdir/include/libdatadog/ddprof.h"
 ./target/debug/dedup_headers "$destdir/include/libdatadog/ddcommon.h" "$destdir/include/libdatadog/ddprof.h"

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: <https://github.com/datadog/libdatadog/releases>
-2. [ ] Update the `LIB_GITHUB_RELEASES` section of the <Rakefile> 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 <lib/libdatadog/version.rb> 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`.

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 <Rakefile> (#{LIB_VERSION_TO_PACKAGE}) does not match " \
     "`LIB_VERSION` setting in <lib/libdatadog/version.rb> (#{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"
   }
 ]

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].