Skip to content

Commit

Permalink
make exception message collection optional and default disable
Browse files Browse the repository at this point in the history
  • Loading branch information
realFlowControl committed Feb 2, 2024
1 parent b62bd07 commit ddf48a3
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 8 deletions.
29 changes: 29 additions & 0 deletions profiling/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ pub struct SystemSettings {
pub profiling_allocation_enabled: bool,
pub profiling_timeline_enabled: bool,
pub profiling_exception_enabled: bool,
pub profiling_exception_message_enabled: bool,

// todo: can't this be Option<String>? I don't think the string can ever be static.
pub output_pprof: Option<Cow<'static, str>>,
Expand All @@ -42,6 +43,7 @@ impl SystemSettings {
self.profiling_allocation_enabled = false;
self.profiling_timeline_enabled = false;
self.profiling_exception_enabled = false;
self.profiling_exception_message_enabled = false;
}

/// # Safety
Expand All @@ -62,6 +64,7 @@ impl SystemSettings {
profiling_allocation_enabled: profiling_allocation_enabled(),
profiling_timeline_enabled: profiling_timeline_enabled(),
profiling_exception_enabled: profiling_exception_enabled(),
profiling_exception_message_enabled: profiling_exception_message_enabled(),
output_pprof: profiling_output_pprof(),
profiling_exception_sampling_distance: profiling_exception_sampling_distance(),
profiling_log_level: profiling_log_level(),
Expand Down Expand Up @@ -122,6 +125,7 @@ impl SystemSettings {
profiling_allocation_enabled: false,
profiling_timeline_enabled: false,
profiling_exception_enabled: false,
profiling_exception_message_enabled: false,
output_pprof: None,
profiling_exception_sampling_distance: 0,
profiling_log_level: LevelFilter::Off,
Expand All @@ -138,6 +142,7 @@ impl SystemSettings {
system_settings.profiling_allocation_enabled = false;
system_settings.profiling_timeline_enabled = false;
system_settings.profiling_exception_enabled = false;
system_settings.profiling_exception_message_enabled = false;
}
}

Expand Down Expand Up @@ -332,6 +337,7 @@ pub(crate) enum ConfigId {
ProfilingAllocationEnabled,
ProfilingTimelineEnabled,
ProfilingExceptionEnabled,
ProfilingExceptionMessageEnabled,
ProfilingExceptionSamplingDistance,
ProfilingLogLevel,
ProfilingOutputPprof,
Expand All @@ -358,6 +364,7 @@ impl ConfigId {
ProfilingAllocationEnabled => b"DD_PROFILING_ALLOCATION_ENABLED\0",
ProfilingTimelineEnabled => b"DD_PROFILING_TIMELINE_ENABLED\0",
ProfilingExceptionEnabled => b"DD_PROFILING_EXCEPTION_ENABLED\0",
ProfilingExceptionMessageEnabled => b"DD_PROFILING_EXCEPTION_MESSAGE_ENABLED\0",
ProfilingExceptionSamplingDistance => b"DD_PROFILING_EXCEPTION_SAMPLING_DISTANCE\0",
ProfilingLogLevel => b"DD_PROFILING_LOG_LEVEL\0",

Expand Down Expand Up @@ -400,6 +407,7 @@ lazy_static::lazy_static! {
profiling_allocation_enabled: false,
profiling_timeline_enabled: false,
profiling_exception_enabled: false,
profiling_exception_message_enabled: false,
output_pprof: None,
profiling_exception_sampling_distance: u32::MAX,
profiling_log_level: LevelFilter::Off,
Expand All @@ -415,6 +423,7 @@ lazy_static::lazy_static! {
profiling_allocation_enabled: true,
profiling_timeline_enabled: true,
profiling_exception_enabled: true,
profiling_exception_message_enabled: false,
output_pprof: None,
profiling_exception_sampling_distance: 100,
profiling_log_level: LevelFilter::Off,
Expand Down Expand Up @@ -496,6 +505,16 @@ unsafe fn profiling_exception_enabled() -> bool {
)
}

/// # Safety
/// This function must only be called after config has been initialized in
/// rinit, and before it is uninitialized in mshutdown.
unsafe fn profiling_exception_message_enabled() -> bool {
get_system_bool(
ProfilingExceptionMessageEnabled,
DEFAULT_SYSTEM_SETTINGS.profiling_exception_message_enabled,
)
}

/// # Safety
/// This function must only be called after config has been initialized in
/// rinit, and before it is uninitialized in mshutdown.
Expand Down Expand Up @@ -811,6 +830,16 @@ pub(crate) fn minit(module_number: libc::c_int) {
ini_change: Some(zai_config_system_ini_change),
parser: None,
},
zai_config_entry {
id: transmute(ProfilingExceptionMessageEnabled),
name: ProfilingExceptionMessageEnabled.env_var_name(),
type_: ZAI_CONFIG_TYPE_BOOL,
default_encoded_value: ZaiStr::literal(b"0\0"),
aliases: ptr::null_mut(),
aliases_count: 0,
ini_change: Some(zai_config_system_ini_change),
parser: None,
},
zai_config_entry {
id: transmute(ProfilingExceptionSamplingDistance),
name: ProfilingExceptionSamplingDistance.env_var_name(),
Expand Down
16 changes: 13 additions & 3 deletions profiling/src/exception.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,23 @@ impl ExceptionProfilingStats {
#[cfg(php8)]
let exception_name = unsafe { (*exception).class_name() };

let message = unsafe {
let collect_message = REQUEST_LOCALS.with(|cell| {
cell.try_borrow()
.map(|locals| locals.system_settings().profiling_exception_message_enabled)
.unwrap_or(false)
});

let message = if collect_message {
#[cfg(php7)]
let exception_obj = (*exception).value.obj;
#[cfg(php8)]
let exception_obj = exception;
zend::zai_str_from_zstr(zend::zai_exception_message(exception_obj).as_mut())
.into_string()
Some(unsafe {
zend::zai_str_from_zstr(zend::zai_exception_message(exception_obj).as_mut())
.into_string()
})
} else {
None
};

self.next_sampling_interval();
Expand Down
12 changes: 7 additions & 5 deletions profiling/src/profiling/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -758,7 +758,7 @@ impl Profiler {
&self,
execute_data: *mut zend_execute_data,
exception: String,
message: String,
message: Option<String>,
) {
let result = collect_stack_sample(execute_data);
match result {
Expand All @@ -771,10 +771,12 @@ impl Profiler {
value: LabelValue::Str(exception.clone().into()),
});

labels.push(Label {
key: "exception message",
value: LabelValue::Str(message.into()),
});
if message.is_some() {
labels.push(Label {
key: "exception message",
value: LabelValue::Str(message.unwrap().into()),
});
}

let n_labels = labels.len();

Expand Down

0 comments on commit ddf48a3

Please sign in to comment.