Get rid of some bitwise checking in ddog_shall_log (#2539)

Makefile

@@ -36,7 +36,7 @@ RUN_TESTS_CMD := REPORT_EXIT_STATUS=1 TEST_PHP_SRCDIR=$(PROJECT_ROOT) USE_TRACKE
 
 C_FILES = $(shell find components components-rs ext src/dogstatsd zend_abstract_interface -name '*.c' -o -name '*.h' | awk '{ printf "$(BUILD_DIR)/%s\n", $$1 }' )
 TEST_FILES = $(shell find tests/ext -name '*.php*' -o -name '*.inc' -o -name '*.json' -o -name 'CONFLICTS' | awk '{ printf "$(BUILD_DIR)/%s\n", $$1 }' )
-RUST_FILES = $(BUILD_DIR)/Cargo.toml $(shell find components-rs -name '*.c' -o -name '*.rs' -o -name 'Cargo.toml' | awk '{ printf "$(BUILD_DIR)/%s\n", $$1 }' ) $(shell find libdatadog/{ddcommon,ddcommon-ffi,ddtelemetry,ddtelemetry-ffi,ipc,sidecar,sidecar-ffi,spawn_worker,tools/{cc_utils,sidecar_mockgen},trace-*,Cargo.toml} -type f \( -path "*/src*" -o -path "*/examples*" -o -path "*Cargo.toml" -o -path "*/build.rs" -o -path "*/tests/dataservice.rs" -o -path "*/tests/service_functional.rs" \) -not -path "*/ipc/build.rs" -not -path "*/sidecar-ffi/build.rs")
+RUST_FILES = $(BUILD_DIR)/Cargo.toml $(shell find components-rs -name '*.c' -o -name '*.rs' -o -name 'Cargo.toml' | awk '{ printf "$(BUILD_DIR)/%s\n", $$1 }' ) $(shell find libdatadog/{build-common,ddcommon,ddcommon-ffi,ddtelemetry,ddtelemetry-ffi,ipc,sidecar,sidecar-ffi,spawn_worker,tools/{cc_utils,sidecar_mockgen},trace-*,Cargo.toml} -type f \( -path "*/src*" -o -path "*/examples*" -o -path "*Cargo.toml" -o -path "*/build.rs" -o -path "*/tests/dataservice.rs" -o -path "*/tests/service_functional.rs" \) -not -path "*/ipc/build.rs" -not -path "*/sidecar-ffi/build.rs")
 TEST_OPCACHE_FILES = $(shell find tests/opcache -name '*.php*' -o -name '.gitkeep' | awk '{ printf "$(BUILD_DIR)/%s\n", $$1 }' )
 TEST_STUB_FILES = $(shell find tests/ext -type d -name 'stubs' -exec find '{}' -type f \; | awk '{ printf "$(BUILD_DIR)/%s\n", $$1 }' )
 INIT_HOOK_TEST_FILES = $(shell find tests/C2PHP -name '*.phpt' -o -name '*.inc' | awk '{ printf "$(BUILD_DIR)/%s\n", $$1 }' )

cbindgen.toml

@@ -26,6 +26,3 @@ rename_variants = "ScreamingSnakeCase"
 [parse]
 parse_deps = true
 include = ["ddcommon", "ddtelemetry", "ddcommon-ffi", "ddtelemetry-ffi", "datadog-sidecar", "datadog-ipc", "uuid"]
-
-[macro_expansion]
-bitflags = true

components-rs/Cargo.toml

@@ -9,14 +9,13 @@ path = "lib.rs"
 
 [dependencies]
 ddcommon = { path = "../libdatadog/ddcommon" }
-ddcommon-ffi = { path = "../libdatadog/ddcommon-ffi" }
+ddcommon-ffi = { path = "../libdatadog/ddcommon-ffi", default-features = false }
 ddtelemetry = { path = "../libdatadog/ddtelemetry" }
-ddtelemetry-ffi = { path = "../libdatadog/ddtelemetry-ffi" }
+ddtelemetry-ffi = { path = "../libdatadog/ddtelemetry-ffi", default-features = false }
 datadog-sidecar = { path = "../libdatadog/sidecar" }
 datadog-sidecar-ffi = { path = "../libdatadog/sidecar-ffi" }
 spawn_worker = { path = "../libdatadog/spawn_worker" }
 anyhow = { version = "1.0" }
-bitflags = "2.3.3"
 const-str = "0.5.6"
 json = "0.12.4"
 lazy_static = "1.4"
@@ -34,4 +33,4 @@ tracing-subscriber = { version = "0.3", default-features = false, features = [
 ] }
 
 [build-dependencies]
-cbindgen = "0.24"
+cbindgen = "0.26"

components-rs/common.h

@@ -108,6 +108,8 @@ typedef struct ddog_Vec_Tag_ParseResult {
   struct ddog_Error *error_message;
 } ddog_Vec_Tag_ParseResult;
 
+#define ddog_LOG_ONCE (1 << 3)
+
 typedef enum ddog_ConfigurationOrigin {
   DDOG_CONFIGURATION_ORIGIN_ENV_VAR,
   DDOG_CONFIGURATION_ORIGIN_CODE,
@@ -116,15 +118,26 @@ typedef enum ddog_ConfigurationOrigin {
   DDOG_CONFIGURATION_ORIGIN_DEFAULT,
 } ddog_ConfigurationOrigin;
 
+typedef enum ddog_Log {
+  DDOG_LOG_ERROR = 1,
+  DDOG_LOG_WARN = 2,
+  DDOG_LOG_INFO = 3,
+  DDOG_LOG_DEBUG = 4,
+  DDOG_LOG_TRACE = 5,
+  DDOG_LOG_DEPRECATED = (3 | ddog_LOG_ONCE),
+  DDOG_LOG_STARTUP = (3 | (2 << 4)),
+  DDOG_LOG_STARTUP_WARN = (1 | (2 << 4)),
+  DDOG_LOG_SPAN = (4 | (3 << 4)),
+  DDOG_LOG_SPAN_TRACE = (5 | (3 << 4)),
+  DDOG_LOG_HOOK_TRACE = (5 | (4 << 4)),
+} ddog_Log;
+
 typedef struct ddog_BlockingTransport_SidecarInterfaceResponse__SidecarInterfaceRequest ddog_BlockingTransport_SidecarInterfaceResponse__SidecarInterfaceRequest;
 
 typedef struct ddog_InstanceId ddog_InstanceId;
 
 typedef struct ddog_TelemetryActionsBuffer ddog_TelemetryActionsBuffer;
 
-typedef struct ddog_Log {
-  uint32_t bits;
-} ddog_Log;
 typedef struct ddog_BlockingTransport_SidecarInterfaceResponse__SidecarInterfaceRequest ddog_SidecarTransport;
 
 typedef enum ddog_Option_VecU8_Tag {

components-rs/ddtrace.h

@@ -8,20 +8,6 @@
 #include "telemetry.h"
 #include "sidecar.h"
 
-#define ddog_Log_Error (ddog_Log){ .bits = (uint32_t)1 }
-#define ddog_Log_Warn (ddog_Log){ .bits = (uint32_t)2 }
-#define ddog_Log_Info (ddog_Log){ .bits = (uint32_t)3 }
-#define ddog_Log_Debug (ddog_Log){ .bits = (uint32_t)4 }
-#define ddog_Log_Trace (ddog_Log){ .bits = (uint32_t)5 }
-#define ddog_Log_Once (ddog_Log){ .bits = (uint32_t)(1 << 3) }
-#define ddog_Log__Deprecated (ddog_Log){ .bits = (uint32_t)(3 | (1 << 4)) }
-#define ddog_Log_Deprecated (ddog_Log){ .bits = (uint32_t)((3 | (1 << 4)) | (1 << 3)) }
-#define ddog_Log_Startup (ddog_Log){ .bits = (uint32_t)(3 | (2 << 4)) }
-#define ddog_Log_Startup_Warn (ddog_Log){ .bits = (uint32_t)(1 | (2 << 4)) }
-#define ddog_Log_Span (ddog_Log){ .bits = (uint32_t)(4 | (3 << 4)) }
-#define ddog_Log_Span_Trace (ddog_Log){ .bits = (uint32_t)(5 | (3 << 4)) }
-#define ddog_Log_Hook_Trace (ddog_Log){ .bits = (uint32_t)(5 | (4 << 4)) }
-
 typedef uint64_t ddog_QueueId;
 
 /**
@@ -149,13 +135,13 @@ ddog_CharSlice ddtrace_get_container_id(void);
 
 void ddtrace_set_container_cgroup_path(ddog_CharSlice path);
 
-bool ddog_shall_log(struct ddog_Log category);
+bool ddog_shall_log(enum ddog_Log category);
 
 void ddog_set_error_log_level(bool once);
 
 void ddog_set_log_level(ddog_CharSlice level, bool once);
 
-void ddog_log(struct ddog_Log category, ddog_CharSlice msg);
+void ddog_log(enum ddog_Log category, bool once, ddog_CharSlice msg);
 
 void ddog_reset_log_once(void);
 

components-rs/log.rs

@@ -2,7 +2,6 @@ use std::cell::RefCell;
 use std::collections::BTreeSet;
 use std::ffi::c_char;
 use std::fmt::Debug;
-use bitflags::bitflags;
 use tracing::Level;
 use tracing_core::{Event, Field, LevelFilter, Subscriber};
 use tracing_subscriber::EnvFilter;
@@ -13,24 +12,23 @@ use tracing_subscriber::util::SubscriberInitExt;
 use ddcommon_ffi::CharSlice;
 use ddcommon_ffi::slice::AsBytes;
 
-bitflags! {
-    #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
-    #[repr(C)]
-    pub struct Log: u32 {
-        const Error = 1;
-        const Warn = 2;
-        const Info = 3;
-        const Debug = 4;
-        const Trace = 5;
-        const Once = 1 << 3; // I.e. once per request
-        const _Deprecated = 3 | (1 << 4);
-        const Deprecated = 3 | (1 << 4) | (1 << 3) /* Once */;
-        const Startup = 3 | (2 << 4);
-        const Startup_Warn = 1 | (2 << 4);
-        const Span = 4 | (3 << 4);
-        const Span_Trace = 5 | (3 << 4);
-        const Hook_Trace = 5 | (4 << 4);
-    }
+pub const LOG_ONCE: isize = 1 << 3;
+
+#[allow(non_camel_case_types)]
+#[derive(Clone, Copy)]
+#[repr(C)]
+pub enum Log {
+    Error = 1,
+    Warn = 2,
+    Info = 3,
+    Debug = 4,
+    Trace = 5,
+    Deprecated = 3 | LOG_ONCE,
+    Startup = 3 | (2 << 4),
+    Startup_Warn = 1 | (2 << 4),
+    Span = 4 | (3 << 4),
+    Span_Trace = 5 | (3 << 4),
+    Hook_Trace = 5 | (4 << 4),
 }
 
 #[no_mangle]
@@ -51,7 +49,7 @@ macro_rules! with_target {
             Log::Info => tracing::$p!(target: "ddtrace", Level::INFO, $($t)*),
             Log::Debug => tracing::$p!(target: "ddtrace", Level::DEBUG, $($t)*),
             Log::Trace => tracing::$p!(target: "ddtrace", Level::TRACE, $($t)*),
-            Log::_Deprecated => tracing::$p!(target: "deprecated", Level::INFO, $($t)*),
+            Log::Deprecated => tracing::$p!(target: "deprecated", Level::INFO, $($t)*),
             Log::Startup => tracing::$p!(target: "startup", Level::INFO, $($t)*),
             Log::Span => tracing::$p!(target: "span", Level::DEBUG, $($t)*),
             Log::Span_Trace => tracing::$p!(target: "span", Level::TRACE, $($t)*),
@@ -63,13 +61,11 @@ macro_rules! with_target {
 
 #[no_mangle]
 pub extern "C" fn ddog_shall_log(category: Log) -> bool {
-    let category = category & !Log::Once;
     with_target!(category, tracing::event_enabled!())
 }
 
 pub fn log<S>(category: Log, msg: S) where S: AsRef<str> + tracing::Value {
-    let once = !(category & Log::Once).is_empty();
-    let category = category & !Log::Once;
+    let once = (category as isize & LOG_ONCE) != 0;
     if once {
         with_target!(category, tracing::event!(once = true, msg));
     } else {
@@ -177,9 +173,7 @@ fn set_log_subscriber<S>(subscriber: S) where S: SubscriberInitExt {
 }
 
 #[no_mangle]
-pub unsafe extern "C" fn ddog_log(category: Log, msg: CharSlice) {
-    let once = !(category & Log::Once).is_empty();
-    let category = category & !Log::Once;
+pub unsafe extern "C" fn ddog_log(category: Log, once: bool, msg: CharSlice) {
     if once {
         with_target!(category, tracing::event!(once = true, "{}", msg.to_utf8_lossy()));
     } else {

components/log/log.c

@@ -9,32 +9,32 @@ __thread ddog_Log _ddog_log_source_value;
 __declspec(thread) ddog_Log _ddog_log_source_value;
 #endif
 
-static void ddog_logf_va(ddog_Log source, const char *format, va_list va) {
+static void ddog_logf_va(ddog_Log source, bool once, const char *format, va_list va) {
     char buf[0x100];
     va_list va2;
     va_copy(va2, va);
     int len = vsnprintf(buf, sizeof(buf), format, va);
     if (len > (int)sizeof(buf)) {
         char *msg = malloc(len + 1);
         len = vsnprintf(msg, len + 1, format, va2);
-        ddog_log(source, (ddog_CharSlice){ .ptr = msg, .len = (uintptr_t)len });
+        ddog_log(source, once || (source & ddog_LOG_ONCE), (ddog_CharSlice){ .ptr = msg, .len = (uintptr_t)len });
         free(msg);
     } else {
-        ddog_log(source, (ddog_CharSlice){ .ptr = buf, .len = (uintptr_t)len });
+        ddog_log(source, once || (source & ddog_LOG_ONCE), (ddog_CharSlice){ .ptr = buf, .len = (uintptr_t)len });
     }
     va_end(va2);
 }
 
-void ddog_logf(ddog_Log source, const char *format, ...) {
+void ddog_logf(ddog_Log source, bool once, const char *format, ...) {
     va_list va;
     va_start(va, format);
-    ddog_logf_va(source, format, va);
+    ddog_logf_va(source, once, format, va);
     va_end(va);
 }
 
 void _ddog_log_source(const char *format, ...) {
     va_list va;
     va_start(va, format);
-    ddog_logf_va(_ddog_log_source_value, format, va);
+    ddog_logf_va(_ddog_log_source_value, false, format, va);
     va_end(va);
 }

components/log/log.h

@@ -8,14 +8,14 @@ extern __thread ddog_Log _ddog_log_source_value;
 #else
 extern __declspec(thread) ddog_Log _ddog_log_source_value;
 #endif
-void ddog_logf(ddog_Log source, const char *format, ...);
+void ddog_logf(ddog_Log source, bool once, const char *format, ...);
 void _ddog_log_source(const char *format, ...);
 
-#define LOG(source, format, ...) do { if (ddog_shall_log(ddog_Log_##source)) { ddog_logf(ddog_Log_##source, format, ##__VA_ARGS__); } } while (0)
-#define LOG_ONCE(source, format, ...) do { if (ddog_shall_log(ddog_Log_##source)) { ddog_logf((ddog_Log){ .bits = (ddog_Log_##source).bits | (ddog_Log_Once).bits }, format, ##__VA_ARGS__); } } while (0)
+#define LOG(source, format, ...) do { if (ddog_shall_log(DDOG_LOG_##source)) { ddog_logf(DDOG_LOG_##source, (DDOG_LOG_##source & ddog_LOG_ONCE) != 0, format, ##__VA_ARGS__); } } while (0)
+#define LOG_ONCE(source, format, ...) do { if (ddog_shall_log(DDOG_LOG_##source)) { ddog_logf(DDOG_LOG_##source, true, format, ##__VA_ARGS__); } } while (0)
 #define LOGEV(source, body) { \
-    if (ddog_shall_log(ddog_Log_##source)) { \
-        _ddog_log_source_value = ddog_Log_##source; \
+    if (ddog_shall_log(DDOG_LOG_##source)) { \
+        _ddog_log_source_value = DDOG_LOG_##source; \
         void (*log)(const char *format, ...) = _ddog_log_source; \
         body \
     } \
@@ -25,19 +25,19 @@ void _ddog_log_source(const char *format, ...);
     do {                                  \
         const char *message_ = message;   \
         ZEND_ASSERT(0 && message_);       \
-        LOG(Error, message_);      \
+        LOG(ERROR, message_);      \
     } while (0)
 
 #define LOG_LINE(source, format, ...) \
     do { \
-        if (ddog_shall_log(ddog_Log_##source)) { \
-            ddog_logf(ddog_Log_##source, format " in %s on line %d", ##__VA_ARGS__, zend_get_executed_filename(), (int)zend_get_executed_lineno()); \
+        if (ddog_shall_log(DDOG_LOG_##source)) { \
+            ddog_logf(DDOG_LOG_##source, (DDOG_LOG_##source & ddog_LOG_ONCE) != 0, format " in %s on line %d", ##__VA_ARGS__, zend_get_executed_filename(), (int)zend_get_executed_lineno()); \
         } \
     } while (0)
 #define LOG_LINE_ONCE(source, format, ...) \
     do { \
-        if (ddog_shall_log(ddog_Log_##source)) { \
-            ddog_logf((ddog_Log){ .bits = (ddog_Log_##source).bits | (ddog_Log_Once).bits }, format " in %s on line %d", ##__VA_ARGS__, zend_get_executed_filename(), (int)zend_get_executed_lineno()); \
+        if (ddog_shall_log(DDOG_LOG_##source)) { \
+            ddog_logf(DDOG_LOG_##source, true, format " in %s on line %d", ##__VA_ARGS__, zend_get_executed_filename(), (int)zend_get_executed_lineno()); \
         } \
     } while (0)
 

ext/auto_flush.c

@@ -34,7 +34,7 @@ ZEND_RESULT_CODE ddtrace_flush_tracer(bool force_on_startup, bool collect_cycles
 
     if (zend_hash_num_elements(Z_ARR(trace)) == 0) {
         zend_array_destroy(Z_ARR(trace));
-        LOG(Info, "No finished traces to be sent to the agent");
+        LOG(INFO, "No finished traces to be sent to the agent");
         return SUCCESS;
     }
 
@@ -75,14 +75,14 @@ ZEND_RESULT_CODE ddtrace_flush_tracer(bool force_on_startup, bool collect_cycles
                                 shm = ddog_unmap_shm(mapped_shm);
                                 ddog_drop_anon_shm_handle(shm);
                                 if (ddtrace_ffi_try("Failed sending traces to the sidecar", retry_error)) {
-                                    LOG(Debug, "Failed sending traces via shm to sidecar: %.*s", (int) send_error.some.len, send_error.some.ptr);
+                                    LOG(DEBUG, "Failed sending traces via shm to sidecar: %.*s", (int) send_error.some.len, send_error.some.ptr);
                                 } else {
                                     break;
                                 }
                             }
 
                             char *url = ddtrace_agent_url();
-                            LOG(Info, "Flushing trace of size %d to send-queue for %s",
+                            LOG(INFO, "Flushing trace of size %d to send-queue for %s",
                                 zend_hash_num_elements(Z_ARR(trace)), url);
                             free(url);
                         } while (0);
@@ -92,7 +92,7 @@ ZEND_RESULT_CODE ddtrace_flush_tracer(bool force_on_startup, bool collect_cycles
                 }
             }
         } else {
-            LOG(Info, "Skipping flushing trace of size %d as connection to sidecar failed",
+            LOG(INFO, "Skipping flushing trace of size %d as connection to sidecar failed",
                                zend_hash_num_elements(Z_ARR(trace)));
         }
     } else {
@@ -101,13 +101,13 @@ ZEND_RESULT_CODE ddtrace_flush_tracer(bool force_on_startup, bool collect_cycles
         size_t size;
         if (ddtrace_serialize_simple_array_into_c_string(&traces, &payload, &size)) {
             if (size > limit) {
-                LOG(Error, "Agent request payload of %zu bytes exceeds configured %zu byte limit; dropping request", size, limit);
+                LOG(ERROR, "Agent request payload of %zu bytes exceeds configured %zu byte limit; dropping request", size, limit);
                 success = false;
             } else {
                 success = ddtrace_send_traces_via_thread(1, payload, size);
                 if (success) {
                     char *url = ddtrace_agent_url();
-                    LOG(Info, "Flushing trace of size %d to send-queue for %s",
+                    LOG(INFO, "Flushing trace of size %d to send-queue for %s",
                                        zend_hash_num_elements(Z_ARR(trace)), url);
                     free(url);
                 }