Skip to content

POC: Rust Wrapper for 1DS C++ SDK #1225

New issue

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

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

Already on GitHub? Sign in to your account

Draft
wants to merge 18 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ fi

# Evaluate switches
LINK_TYPE=
CMAKE_OPTS="${CMAKE_OPTS:--DBUILD_SHARED_LIBS=OFF}"
CMAKE_OPTS="${CMAKE_OPTS:-DBUILD_SHARED_LIBS=OFF}"
while getopts "h?vl:D:" opt; do
case "$opt" in
h|\?) usage
Expand Down
1 change: 1 addition & 0 deletions clean-repo.cmd
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
git clean -xd -f
25 changes: 25 additions & 0 deletions wrappers/rust/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# Generated by Cargo
# will have compiled files and executables
debug/
target/
out/

# Remove Cargo.lock from gitignore if creating an executable, leave it for libraries
# More information here https://doc.rust-lang.org/cargo/guide/cargo-toml-vs-cargo-lock.html
Cargo.lock

# These are backup files generated by rustfmt
**/*.rs.bk

# MSVC Windows builds of rustc generate these, which store debugging information
*.pdb
*.exe
*.exe
*.o
*.lib
*.so
*.dll

# Need to rename this to something better ...
lib/
include/
4 changes: 4 additions & 0 deletions wrappers/rust/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
[workspace]
resolver = "2"

members = ["cpp-client-telemetry-sys", "oneds-telemetry", "telemetry-sample"]
13 changes: 13 additions & 0 deletions wrappers/rust/build.ps1
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# @echo off

# set VSTOOLS_VERSION=vs2019
# cd %~dp0

# call ..\..\tools\vcvars.cmd
#![allow(non_upper_case_globals)]
#![allow(non_camel_case_types)]
#![allow(non_snake_case)]
#deploy-dll.cmd
#generate-bindings.cmd

cargo run
12 changes: 12 additions & 0 deletions wrappers/rust/cpp-client-telemetry-sys/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
[package]
name = "cpp-client-telemetry-sys"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]

[build-dependencies]
bindgen = { version = "0.65.1", features = ["experimental"] }
subprocess = "0.2.9"
76 changes: 76 additions & 0 deletions wrappers/rust/cpp-client-telemetry-sys/build.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
use std::env;
use std::fs;
use std::path::PathBuf;
use subprocess::Exec;

static PROJECT_ROOT: &str = "../../../";

fn copy_static_libs() {
// TODO add compatibility for x86 and ARM
let cpp_project_out = PathBuf::from(PROJECT_ROOT).join("Solutions/out/Release/x64");
let out_dir = PathBuf::from(env::var("OUT_DIR").unwrap());

std::fs::copy(cpp_project_out.join("win32-lib/ClientTelemetry.lib"), PathBuf::from(&out_dir).join("ClientTelemetry.lib"))
.expect("Failed to copy ClientTelemetry lib");
std::fs::copy(cpp_project_out.join("sqlite/sqlite.lib"), out_dir.join("sqlite.lib"))
.expect("Failed to copy sqlite native library");
std::fs::copy(cpp_project_out.join("zlib/zlib.lib"), out_dir.join("zlib.lib"))
.expect("Failed to copy zlib native library");

// Tell cargo to look for shared libraries in the specified directory
println!("cargo:rustc-link-search={}", out_dir.display());
println!("cargo:rustc-link-lib=ClientTelemetry");
println!("cargo:rustc-link-lib=wininet");
println!("cargo:rustc-link-lib=crypt32");
println!("cargo:rustc-link-lib=sqlite");
println!("cargo:rustc-link-lib=zlib");
}

fn write_bindings() {
// Precompile header with the appropriate preprocessor options
let mat_h_location = PathBuf::from(PROJECT_ROOT).join("lib/include/public/mat.h");
println!("cargo:rerun-if-changed={}", mat_h_location.to_string_lossy());

// TODO use clang crate instead of invoking CLI directly
let header_out = Exec::cmd("clang")
.arg("-E")
.arg(mat_h_location)
.arg("-D")
.arg("MATSDK_STATIC_LIB=1")
.capture()
.expect("Failed to open clang process")
.stdout_str();

let out_dir = PathBuf::from(env::var("OUT_DIR").unwrap());
let mat_out_path = out_dir.join("mat.out.h");

fs::write(&mat_out_path, header_out).unwrap();

// The bindgen::Builder is the main entry point
// to bindgen, and lets you build up options for
// the resulting bindings.
let bindings = bindgen::Builder::default()
// The input header we would like to generate
// bindings for.
.parse_callbacks(Box::new(bindgen::CargoCallbacks))
.rust_target(bindgen::RustTarget::Stable_1_68)
.header(PathBuf::from(out_dir).join("mat.out.h").to_string_lossy())
.allowlist_type("evt_.*")
.allowlist_function("evt_.*")
.allowlist_var("evt_.*")
// Finish the builder and generate the bindings.
.generate()
// Unwrap the Result and panic on failure.
.expect("Unable to generate bindings");

// // Write the bindings to the $OUT_DIR/bindings.rs file.
let out_path = PathBuf::from(env::var("OUT_DIR").unwrap());
bindings
.write_to_file(out_path.join("bindings.rs"))
.expect("Couldn't write bindings!");
}

fn main() {
write_bindings();
copy_static_libs();
}
90 changes: 90 additions & 0 deletions wrappers/rust/cpp-client-telemetry-sys/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
#![allow(non_upper_case_globals)]
#![allow(non_camel_case_types)]
#![allow(non_snake_case)]
include!(concat!(env!("OUT_DIR"), "/bindings.rs"));

use std::ffi::*;

fn evt_api_call_wrapper(evt_context: Box<evt_context_t>) -> (evt_status_t, Box<evt_context_t>) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this take ownership of the Box<evt_context_t> and then return it? My naive reading of this would be that evt_context: &evt_context_t would be the right type.

let raw_pointer = Box::into_raw(evt_context);

let mut result: evt_status_t = -1;
unsafe {
result = evt_api_call_default(raw_pointer);
}

let out_context = unsafe { Box::from_raw(raw_pointer) };
(result, out_context)
}

pub fn evt_open(config: &CString) -> Option<evt_handle_t> {
let config_bytes = config.to_bytes_with_nul().to_vec();

let context: Box<evt_context_t> = Box::new(evt_context_t {
call: evt_call_t_EVT_OP_OPEN,
handle: 0,
data: config_bytes.as_ptr() as *mut c_void,
result: 0,
size: 0,
});

let (result, context) = evt_api_call_wrapper(context);

if result == -1 {
return Option::None;
}

return Some(context.handle.clone());
}

pub fn evt_close(handle: &evt_handle_t) -> evt_status_t {
let context: Box<evt_context_t> = Box::new(evt_context_t {
call: evt_call_t_EVT_OP_CLOSE,
handle: *handle,
data: std::ptr::null_mut(),
result: 0,
size: 0,
});

let (result, _) = evt_api_call_wrapper(context);
result
}

pub fn evt_upload(handle: &evt_handle_t) -> evt_status_t {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because you are directly exposing evt_handle_t outside your crate, and this type has unsafe pointers in it, these functions are actually unsafe to call. They allow a safe caller to violate type safety.

You can fix this in one of several ways:

  1. Don't expose these functions outside this crate.
  2. Don't expose evt_context_t, and instead define an EvtContext wrapper type which contains evt_context_t but does not expose it outside of the crate.
  3. Mark these functions unsafe.

let context: Box<evt_context_t> = Box::new(evt_context_t {
call: evt_call_t_EVT_OP_UPLOAD,
handle: *handle,
data: std::ptr::null_mut(),
result: 0,
size: 0,
});

evt_api_call_wrapper(context).0
}

pub fn evt_flush(handle: &evt_handle_t) -> evt_status_t {
let context: Box<evt_context_t> = Box::new(evt_context_t {
call: evt_call_t_EVT_OP_FLUSH,
handle: *handle,
data: std::ptr::null_mut(),
result: 0,
size: 0,
});

evt_api_call_wrapper(context).0
}

pub fn evt_log(handle: &evt_handle_t, data: &mut [evt_prop]) -> evt_status_t {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function must have an unsafe signature, i.e. pub unsafe fn evt_log, because the values inside evt_prop use raw pointers and the implementation of this function dereferences those raw pointers. As written now, this function could be used to violate type safety.

A better solution could ensure that evt_prop was using type-safe types. You could have a type which represents an owned pointer to a C-style string, that has the same representation as the underlying evt_* structure. That would greatly reduce your exposure to unsafe code semantics.

If you can point out where the C headers are that define those structures, I can help write the safe Rust projections for these.

let data_len = data.len() as u32;
let data_pointer = data.as_mut_ptr() as *mut c_void;

let context: Box<evt_context_t> = Box::new(evt_context_t {
call: evt_call_t_EVT_OP_LOG,
handle: *handle,
data: data_pointer,
result: 0,
size: data_len,
});

evt_api_call_wrapper(context).0
}
25 changes: 25 additions & 0 deletions wrappers/rust/cpp-client-telemetry-sys/tests/sdk_tests.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
use cpp_client_telemetry_sys::*;
use std::ffi::CString;

#[test]
fn test_open_close() {
// Simple sanity check to ensure the SDK was linked correctly.
let config = r#"{
"eventCollectorUri": "http://localhost:64099/OneCollector/track",
"cacheFilePath":"hackathon_storage.db",
"config":{"host": "*"},
"name":"Rust-API-Client-0",
"version":"1.0.0",
"primaryToken":"99999999999999999999999999999999-99999999-9999-9999-9999-999999999999-9999",
"maxTeardownUploadTimeInSec":5,
"hostMode":false,
"traceLevelMask": 4294967295,
"minimumTraceLevel":0,
"sdkmode":0,
"compat": {"customTypePrefix": "compat_event"}
}"#;

let handle = evt_open(&CString::new(config).unwrap()).expect("Failed to get SDK handle");

assert_eq!(evt_close(&handle), 0);
}
13 changes: 13 additions & 0 deletions wrappers/rust/deploy-dll.cmd
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
@echo off
set PROJECT_DIR=%~dp0

@mkdir %PROJECT_DIR%\include
copy %PROJECT_DIR%..\..\lib\include\public\mat.h %PROJECT_DIR%\include
copy %PROJECT_DIR%..\..\lib\include\public\Version.h %PROJECT_DIR%\include
copy %PROJECT_DIR%..\..\lib\include\public\ctmacros.hpp %PROJECT_DIR%\include

@mkdir %PROJECT_DIR%\lib\%1\%2
copy %PROJECT_DIR%..\..\Solutions\out\%1\%2\win32-dll\*.lib %PROJECT_DIR%\lib\%1\%2
copy %PROJECT_DIR%..\..\Solutions\out\Release\x64\lib\Release\*.lib %PROJECT_DIR%\lib\%1\%2

exit /b 0
17 changes: 17 additions & 0 deletions wrappers/rust/oneds-telemetry/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
[package]
name = "oneds-telemetry"
version = "0.1.0"
edition = "2021"
publish = false
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
chrono = { version = "0.4.30" }
log = { version = "0.4.20" }
once_cell = "1.12.0"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason to use once_cell, now that Once and OnceLock are part of the Rust standard library?

cpp-client-telemetry-sys = { path = "../cpp-client-telemetry-sys" }

[dev-dependencies]

[build-dependencies]
bindgen = { version = "0.65.1", features = ["experimental"] }
58 changes: 58 additions & 0 deletions wrappers/rust/oneds-telemetry/src/appender.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
use log::{Level, Metadata, Record};

use crate::log_manager_provider;

pub static LOGGER: TelemetryCollectorLogBridge = TelemetryCollectorLogBridge;
pub static mut CONSOLE_ENABLED: bool = true;
pub static mut COLLECTOR_ENABLED: bool = true;

pub struct TelemetryCollectorLogBridge;

impl log::Log for TelemetryCollectorLogBridge {
fn enabled(&self, metadata: &Metadata) -> bool {
metadata.level() <= Level::Debug
}

fn log(&self, record: &Record) {
if unsafe { CONSOLE_ENABLED } && self.enabled(record.metadata()) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These fields should probably be changed to use std::sync::atomc::AtomicBool, which will allow this code to be safe.

Using static mut is unsafe because it allows for race conditions, which are undefined behavior.

let utc_now = chrono::Utc::now();
let nanos = utc_now.timestamp_subsec_nanos();

println!(
"[{} {}.{}] {} <{}> - {}: {}",
utc_now.date_naive().to_string(),
utc_now.time().format("%H:%M:%S"),
nanos,
record.target(),
record.module_path().unwrap(),
record.level(),
record.args()
);
}

if unsafe { COLLECTOR_ENABLED }
// Default
&& record.target() != "oneds_telemetry"
// Items from deeper in the `oneds_telemetry` crate
&& !record.target().starts_with("oneds_telemetry::")
{
log_manager_provider().trace(format!("{}", record.args()).as_str());
}
}

fn flush(&self) {
if unsafe { COLLECTOR_ENABLED } {
log_manager_provider().flush(false);
}
}
}

// fn map_severity_to_otel_severity(level: Level) -> Severity {
// match level {
// Level::Error => Severity::Error,
// Level::Warn => Severity::Warn,
// Level::Info => Severity::Info,
// Level::Debug => Severity::Debug,
// Level::Trace => Severity::Trace,
// }
// }
Loading