Skip to content

Commit 552b0ef

Browse files
bwoebibwoebi
authored
Get rid of some bitwise checking in ddog_shall_log (#2539)
1 parent 48ba7d7 commit 552b0ef

29 files changed

Lines changed: 222 additions & 223 deletions

‎Cargo.lock‎

Lines changed: 13 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Makefile‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ RUN_TESTS_CMD := REPORT_EXIT_STATUS=1 TEST_PHP_SRCDIR=$(PROJECT_ROOT) USE_TRACKE
3636

3737
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 }' )
3838
TEST_FILES = $(shell find tests/ext -name '*.php*' -o -name '*.inc' -o -name '*.json' -o -name 'CONFLICTS' | awk '{ printf "$(BUILD_DIR)/%s\n", $$1 }' )
39-
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")
39+
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")
4040
TEST_OPCACHE_FILES = $(shell find tests/opcache -name '*.php*' -o -name '.gitkeep' | awk '{ printf "$(BUILD_DIR)/%s\n", $$1 }' )
4141
TEST_STUB_FILES = $(shell find tests/ext -type d -name 'stubs' -exec find '{}' -type f \; | awk '{ printf "$(BUILD_DIR)/%s\n", $$1 }' )
4242
INIT_HOOK_TEST_FILES = $(shell find tests/C2PHP -name '*.phpt' -o -name '*.inc' | awk '{ printf "$(BUILD_DIR)/%s\n", $$1 }' )

‎cbindgen.toml‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,3 @@ rename_variants = "ScreamingSnakeCase"
2626
[parse]
2727
parse_deps = true
2828
include = ["ddcommon", "ddtelemetry", "ddcommon-ffi", "ddtelemetry-ffi", "datadog-sidecar", "datadog-ipc", "uuid"]
29-
30-
[macro_expansion]
31-
bitflags = true

‎components-rs/Cargo.toml‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,13 @@ path = "lib.rs"
99

1010
[dependencies]
1111
ddcommon = { path = "../libdatadog/ddcommon" }
12-
ddcommon-ffi = { path = "../libdatadog/ddcommon-ffi" }
12+
ddcommon-ffi = { path = "../libdatadog/ddcommon-ffi", default-features = false }
1313
ddtelemetry = { path = "../libdatadog/ddtelemetry" }
14-
ddtelemetry-ffi = { path = "../libdatadog/ddtelemetry-ffi" }
14+
ddtelemetry-ffi = { path = "../libdatadog/ddtelemetry-ffi", default-features = false }
1515
datadog-sidecar = { path = "../libdatadog/sidecar" }
1616
datadog-sidecar-ffi = { path = "../libdatadog/sidecar-ffi" }
1717
spawn_worker = { path = "../libdatadog/spawn_worker" }
1818
anyhow = { version = "1.0" }
19-
bitflags = "2.3.3"
2019
const-str = "0.5.6"
2120
json = "0.12.4"
2221
lazy_static = "1.4"
@@ -34,4 +33,4 @@ tracing-subscriber = { version = "0.3", default-features = false, features = [
3433
] }
3534

3635
[build-dependencies]
37-
cbindgen = "0.24"
36+
cbindgen = "0.26"

‎components-rs/common.h‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,8 @@ typedef struct ddog_Vec_Tag_ParseResult {
108108
struct ddog_Error *error_message;
109109
} ddog_Vec_Tag_ParseResult;
110110

111+
#define ddog_LOG_ONCE (1 << 3)
112+
111113
typedef enum ddog_ConfigurationOrigin {
112114
DDOG_CONFIGURATION_ORIGIN_ENV_VAR,
113115
DDOG_CONFIGURATION_ORIGIN_CODE,
@@ -116,15 +118,26 @@ typedef enum ddog_ConfigurationOrigin {
116118
DDOG_CONFIGURATION_ORIGIN_DEFAULT,
117119
} ddog_ConfigurationOrigin;
118120

121+
typedef enum ddog_Log {
122+
DDOG_LOG_ERROR = 1,
123+
DDOG_LOG_WARN = 2,
124+
DDOG_LOG_INFO = 3,
125+
DDOG_LOG_DEBUG = 4,
126+
DDOG_LOG_TRACE = 5,
127+
DDOG_LOG_DEPRECATED = (3 | ddog_LOG_ONCE),
128+
DDOG_LOG_STARTUP = (3 | (2 << 4)),
129+
DDOG_LOG_STARTUP_WARN = (1 | (2 << 4)),
130+
DDOG_LOG_SPAN = (4 | (3 << 4)),
131+
DDOG_LOG_SPAN_TRACE = (5 | (3 << 4)),
132+
DDOG_LOG_HOOK_TRACE = (5 | (4 << 4)),
133+
} ddog_Log;
134+
119135
typedef struct ddog_BlockingTransport_SidecarInterfaceResponse__SidecarInterfaceRequest ddog_BlockingTransport_SidecarInterfaceResponse__SidecarInterfaceRequest;
120136

121137
typedef struct ddog_InstanceId ddog_InstanceId;
122138

123139
typedef struct ddog_TelemetryActionsBuffer ddog_TelemetryActionsBuffer;
124140

125-
typedef struct ddog_Log {
126-
uint32_t bits;
127-
} ddog_Log;
128141
typedef struct ddog_BlockingTransport_SidecarInterfaceResponse__SidecarInterfaceRequest ddog_SidecarTransport;
129142

130143
typedef enum ddog_Option_VecU8_Tag {

‎components-rs/ddtrace.h‎

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -8,20 +8,6 @@
88
#include "telemetry.h"
99
#include "sidecar.h"
1010

11-
#define ddog_Log_Error (ddog_Log){ .bits = (uint32_t)1 }
12-
#define ddog_Log_Warn (ddog_Log){ .bits = (uint32_t)2 }
13-
#define ddog_Log_Info (ddog_Log){ .bits = (uint32_t)3 }
14-
#define ddog_Log_Debug (ddog_Log){ .bits = (uint32_t)4 }
15-
#define ddog_Log_Trace (ddog_Log){ .bits = (uint32_t)5 }
16-
#define ddog_Log_Once (ddog_Log){ .bits = (uint32_t)(1 << 3) }
17-
#define ddog_Log__Deprecated (ddog_Log){ .bits = (uint32_t)(3 | (1 << 4)) }
18-
#define ddog_Log_Deprecated (ddog_Log){ .bits = (uint32_t)((3 | (1 << 4)) | (1 << 3)) }
19-
#define ddog_Log_Startup (ddog_Log){ .bits = (uint32_t)(3 | (2 << 4)) }
20-
#define ddog_Log_Startup_Warn (ddog_Log){ .bits = (uint32_t)(1 | (2 << 4)) }
21-
#define ddog_Log_Span (ddog_Log){ .bits = (uint32_t)(4 | (3 << 4)) }
22-
#define ddog_Log_Span_Trace (ddog_Log){ .bits = (uint32_t)(5 | (3 << 4)) }
23-
#define ddog_Log_Hook_Trace (ddog_Log){ .bits = (uint32_t)(5 | (4 << 4)) }
24-
2511
typedef uint64_t ddog_QueueId;
2612

2713
/**
@@ -149,13 +135,13 @@ ddog_CharSlice ddtrace_get_container_id(void);
149135

150136
void ddtrace_set_container_cgroup_path(ddog_CharSlice path);
151137

152-
bool ddog_shall_log(struct ddog_Log category);
138+
bool ddog_shall_log(enum ddog_Log category);
153139

154140
void ddog_set_error_log_level(bool once);
155141

156142
void ddog_set_log_level(ddog_CharSlice level, bool once);
157143

158-
void ddog_log(struct ddog_Log category, ddog_CharSlice msg);
144+
void ddog_log(enum ddog_Log category, bool once, ddog_CharSlice msg);
159145

160146
void ddog_reset_log_once(void);
161147

‎components-rs/log.rs‎

Lines changed: 20 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ use std::cell::RefCell;
22
use std::collections::BTreeSet;
33
use std::ffi::c_char;
44
use std::fmt::Debug;
5-
use bitflags::bitflags;
65
use tracing::Level;
76
use tracing_core::{Event, Field, LevelFilter, Subscriber};
87
use tracing_subscriber::EnvFilter;
@@ -13,24 +12,23 @@ use tracing_subscriber::util::SubscriberInitExt;
1312
use ddcommon_ffi::CharSlice;
1413
use ddcommon_ffi::slice::AsBytes;
1514

16-
bitflags! {
17-
#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
18-
#[repr(C)]
19-
pub struct Log: u32 {
20-
const Error = 1;
21-
const Warn = 2;
22-
const Info = 3;
23-
const Debug = 4;
24-
const Trace = 5;
25-
const Once = 1 << 3; // I.e. once per request
26-
const _Deprecated = 3 | (1 << 4);
27-
const Deprecated = 3 | (1 << 4) | (1 << 3) /* Once */;
28-
const Startup = 3 | (2 << 4);
29-
const Startup_Warn = 1 | (2 << 4);
30-
const Span = 4 | (3 << 4);
31-
const Span_Trace = 5 | (3 << 4);
32-
const Hook_Trace = 5 | (4 << 4);
33-
}
15+
pub const LOG_ONCE: isize = 1 << 3;
16+
17+
#[allow(non_camel_case_types)]
18+
#[derive(Clone, Copy)]
19+
#[repr(C)]
20+
pub enum Log {
21+
Error = 1,
22+
Warn = 2,
23+
Info = 3,
24+
Debug = 4,
25+
Trace = 5,
26+
Deprecated = 3 | LOG_ONCE,
27+
Startup = 3 | (2 << 4),
28+
Startup_Warn = 1 | (2 << 4),
29+
Span = 4 | (3 << 4),
30+
Span_Trace = 5 | (3 << 4),
31+
Hook_Trace = 5 | (4 << 4),
3432
}
3533

3634
#[no_mangle]
@@ -51,7 +49,7 @@ macro_rules! with_target {
5149
Log::Info => tracing::$p!(target: "ddtrace", Level::INFO, $($t)*),
5250
Log::Debug => tracing::$p!(target: "ddtrace", Level::DEBUG, $($t)*),
5351
Log::Trace => tracing::$p!(target: "ddtrace", Level::TRACE, $($t)*),
54-
Log::_Deprecated => tracing::$p!(target: "deprecated", Level::INFO, $($t)*),
52+
Log::Deprecated => tracing::$p!(target: "deprecated", Level::INFO, $($t)*),
5553
Log::Startup => tracing::$p!(target: "startup", Level::INFO, $($t)*),
5654
Log::Span => tracing::$p!(target: "span", Level::DEBUG, $($t)*),
5755
Log::Span_Trace => tracing::$p!(target: "span", Level::TRACE, $($t)*),
@@ -63,13 +61,11 @@ macro_rules! with_target {
6361

6462
#[no_mangle]
6563
pub extern "C" fn ddog_shall_log(category: Log) -> bool {
66-
let category = category & !Log::Once;
6764
with_target!(category, tracing::event_enabled!())
6865
}
6966

7067
pub fn log<S>(category: Log, msg: S) where S: AsRef<str> + tracing::Value {
71-
let once = !(category & Log::Once).is_empty();
72-
let category = category & !Log::Once;
68+
let once = (category as isize & LOG_ONCE) != 0;
7369
if once {
7470
with_target!(category, tracing::event!(once = true, msg));
7571
} else {
@@ -177,9 +173,7 @@ fn set_log_subscriber<S>(subscriber: S) where S: SubscriberInitExt {
177173
}
178174

179175
#[no_mangle]
180-
pub unsafe extern "C" fn ddog_log(category: Log, msg: CharSlice) {
181-
let once = !(category & Log::Once).is_empty();
182-
let category = category & !Log::Once;
176+
pub unsafe extern "C" fn ddog_log(category: Log, once: bool, msg: CharSlice) {
183177
if once {
184178
with_target!(category, tracing::event!(once = true, "{}", msg.to_utf8_lossy()));
185179
} else {

‎components/log/log.c‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,32 +9,32 @@ __thread ddog_Log _ddog_log_source_value;
99
__declspec(thread) ddog_Log _ddog_log_source_value;
1010
#endif
1111

12-
static void ddog_logf_va(ddog_Log source, const char *format, va_list va) {
12+
static void ddog_logf_va(ddog_Log source, bool once, const char *format, va_list va) {
1313
char buf[0x100];
1414
va_list va2;
1515
va_copy(va2, va);
1616
int len = vsnprintf(buf, sizeof(buf), format, va);
1717
if (len > (int)sizeof(buf)) {
1818
char *msg = malloc(len + 1);
1919
len = vsnprintf(msg, len + 1, format, va2);
20-
ddog_log(source, (ddog_CharSlice){ .ptr = msg, .len = (uintptr_t)len });
20+
ddog_log(source, once || (source & ddog_LOG_ONCE), (ddog_CharSlice){ .ptr = msg, .len = (uintptr_t)len });
2121
free(msg);
2222
} else {
23-
ddog_log(source, (ddog_CharSlice){ .ptr = buf, .len = (uintptr_t)len });
23+
ddog_log(source, once || (source & ddog_LOG_ONCE), (ddog_CharSlice){ .ptr = buf, .len = (uintptr_t)len });
2424
}
2525
va_end(va2);
2626
}
2727

28-
void ddog_logf(ddog_Log source, const char *format, ...) {
28+
void ddog_logf(ddog_Log source, bool once, const char *format, ...) {
2929
va_list va;
3030
va_start(va, format);
31-
ddog_logf_va(source, format, va);
31+
ddog_logf_va(source, once, format, va);
3232
va_end(va);
3333
}
3434

3535
void _ddog_log_source(const char *format, ...) {
3636
va_list va;
3737
va_start(va, format);
38-
ddog_logf_va(_ddog_log_source_value, format, va);
38+
ddog_logf_va(_ddog_log_source_value, false, format, va);
3939
va_end(va);
4040
}

‎components/log/log.h‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,14 @@ extern __thread ddog_Log _ddog_log_source_value;
88
#else
99
extern __declspec(thread) ddog_Log _ddog_log_source_value;
1010
#endif
11-
void ddog_logf(ddog_Log source, const char *format, ...);
11+
void ddog_logf(ddog_Log source, bool once, const char *format, ...);
1212
void _ddog_log_source(const char *format, ...);
1313

14-
#define LOG(source, format, ...) do { if (ddog_shall_log(ddog_Log_##source)) { ddog_logf(ddog_Log_##source, format, ##__VA_ARGS__); } } while (0)
15-
#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)
14+
#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)
15+
#define LOG_ONCE(source, format, ...) do { if (ddog_shall_log(DDOG_LOG_##source)) { ddog_logf(DDOG_LOG_##source, true, format, ##__VA_ARGS__); } } while (0)
1616
#define LOGEV(source, body) { \
17-
if (ddog_shall_log(ddog_Log_##source)) { \
18-
_ddog_log_source_value = ddog_Log_##source; \
17+
if (ddog_shall_log(DDOG_LOG_##source)) { \
18+
_ddog_log_source_value = DDOG_LOG_##source; \
1919
void (*log)(const char *format, ...) = _ddog_log_source; \
2020
body \
2121
} \
@@ -25,19 +25,19 @@ void _ddog_log_source(const char *format, ...);
2525
do { \
2626
const char *message_ = message; \
2727
ZEND_ASSERT(0 && message_); \
28-
LOG(Error, message_); \
28+
LOG(ERROR, message_); \
2929
} while (0)
3030

3131
#define LOG_LINE(source, format, ...) \
3232
do { \
33-
if (ddog_shall_log(ddog_Log_##source)) { \
34-
ddog_logf(ddog_Log_##source, format " in %s on line %d", ##__VA_ARGS__, zend_get_executed_filename(), (int)zend_get_executed_lineno()); \
33+
if (ddog_shall_log(DDOG_LOG_##source)) { \
34+
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()); \
3535
} \
3636
} while (0)
3737
#define LOG_LINE_ONCE(source, format, ...) \
3838
do { \
39-
if (ddog_shall_log(ddog_Log_##source)) { \
40-
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()); \
39+
if (ddog_shall_log(DDOG_LOG_##source)) { \
40+
ddog_logf(DDOG_LOG_##source, true, format " in %s on line %d", ##__VA_ARGS__, zend_get_executed_filename(), (int)zend_get_executed_lineno()); \
4141
} \
4242
} while (0)
4343

‎ext/auto_flush.c‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ ZEND_RESULT_CODE ddtrace_flush_tracer(bool force_on_startup, bool collect_cycles
3434

3535
if (zend_hash_num_elements(Z_ARR(trace)) == 0) {
3636
zend_array_destroy(Z_ARR(trace));
37-
LOG(Info, "No finished traces to be sent to the agent");
37+
LOG(INFO, "No finished traces to be sent to the agent");
3838
return SUCCESS;
3939
}
4040

@@ -75,14 +75,14 @@ ZEND_RESULT_CODE ddtrace_flush_tracer(bool force_on_startup, bool collect_cycles
7575
shm = ddog_unmap_shm(mapped_shm);
7676
ddog_drop_anon_shm_handle(shm);
7777
if (ddtrace_ffi_try("Failed sending traces to the sidecar", retry_error)) {
78-
LOG(Debug, "Failed sending traces via shm to sidecar: %.*s", (int) send_error.some.len, send_error.some.ptr);
78+
LOG(DEBUG, "Failed sending traces via shm to sidecar: %.*s", (int) send_error.some.len, send_error.some.ptr);
7979
} else {
8080
break;
8181
}
8282
}
8383

8484
char *url = ddtrace_agent_url();
85-
LOG(Info, "Flushing trace of size %d to send-queue for %s",
85+
LOG(INFO, "Flushing trace of size %d to send-queue for %s",
8686
zend_hash_num_elements(Z_ARR(trace)), url);
8787
free(url);
8888
} while (0);
@@ -92,7 +92,7 @@ ZEND_RESULT_CODE ddtrace_flush_tracer(bool force_on_startup, bool collect_cycles
9292
}
9393
}
9494
} else {
95-
LOG(Info, "Skipping flushing trace of size %d as connection to sidecar failed",
95+
LOG(INFO, "Skipping flushing trace of size %d as connection to sidecar failed",
9696
zend_hash_num_elements(Z_ARR(trace)));
9797
}
9898
} else {
@@ -101,13 +101,13 @@ ZEND_RESULT_CODE ddtrace_flush_tracer(bool force_on_startup, bool collect_cycles
101101
size_t size;
102102
if (ddtrace_serialize_simple_array_into_c_string(&traces, &payload, &size)) {
103103
if (size > limit) {
104-
LOG(Error, "Agent request payload of %zu bytes exceeds configured %zu byte limit; dropping request", size, limit);
104+
LOG(ERROR, "Agent request payload of %zu bytes exceeds configured %zu byte limit; dropping request", size, limit);
105105
success = false;
106106
} else {
107107
success = ddtrace_send_traces_via_thread(1, payload, size);
108108
if (success) {
109109
char *url = ddtrace_agent_url();
110-
LOG(Info, "Flushing trace of size %d to send-queue for %s",
110+
LOG(INFO, "Flushing trace of size %d to send-queue for %s",
111111
zend_hash_num_elements(Z_ARR(trace)), url);
112112
free(url);
113113
}

0 commit comments

Comments
 (0)