This is an automated email from the ASF dual-hosted git repository.
jmjoy pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/skywalking-php.git
The following commit(s) were added to refs/heads/master by this push:
new c9c8af2 Optionally enable zend observer api for auto instrumentation.
(#81)
c9c8af2 is described below
commit c9c8af2e007e47090a095504d5574112626aed10
Author: jmjoy <[email protected]>
AuthorDate: Tue Jul 4 19:06:43 2023 +0800
Optionally enable zend observer api for auto instrumentation. (#81)
---
.github/workflows/rust.yml | 19 +++
Cargo.lock | 5 +-
Cargo.toml | 3 +
README.md | 2 +-
build.rs | 6 +-
dist-material/LICENSE | 2 +-
docs/en/configuration/ini-settings.md | 1 +
docs/en/configuration/zend-observer.md | 32 +++++
docs/menu.yml | 2 +
src/execute.rs | 213 +++++++++++++++++++++++++++------
src/lib.rs | 6 +
src/module.rs | 22 ++--
src/plugin/mod.rs | 39 +++++-
src/plugin/plugin_pdo.rs | 6 +-
src/plugin/plugin_predis.rs | 40 +++++--
src/tag.rs | 4 +-
tests/common/mod.rs | 21 ++++
tests/data/expected_context.yaml | 6 +-
tests/e2e.rs | 2 +-
19 files changed, 354 insertions(+), 77 deletions(-)
diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml
index bf88d86..3136a71 100644
--- a/.github/workflows/rust.yml
+++ b/.github/workflows/rust.yml
@@ -56,16 +56,31 @@ jobs:
# Many composer dependencies need PHP 7.2+
- php: "7.2"
swoole: "4.6.7"
+ enable_zend_observer: "Off"
- php: "7.3"
swoole: "4.7.1"
+ enable_zend_observer: "Off"
- php: "7.4"
swoole: "4.8.10"
+ enable_zend_observer: "Off"
- php: "8.0"
swoole: "5.0.0"
+ enable_zend_observer: "Off"
+ - php: "8.0"
+ swoole: "5.0.0"
+ enable_zend_observer: "On"
+ - php: "8.1"
+ swoole: "5.0.0"
+ enable_zend_observer: "Off"
- php: "8.1"
swoole: "5.0.0"
+ enable_zend_observer: "On"
+ - php: "8.2"
+ swoole: "5.0.0"
+ enable_zend_observer: "Off"
- php: "8.2"
swoole: "5.0.0"
+ enable_zend_observer: "On"
runs-on: ${{ matrix.os }}
steps:
@@ -164,6 +179,8 @@ jobs:
toolchain: ${{ env.RUST_STABLE_TOOLCHAIN }}
command: test
args: --release --workspace
+ env:
+ ENABLE_ZEND_OBSERVER: ${{ matrix.version.enable_zend_observer }}
continue-on-error: true
# Rebuild the mixture when cargo test failed.
@@ -189,6 +206,8 @@ jobs:
toolchain: ${{ env.RUST_STABLE_TOOLCHAIN }}
command: test
args: --release --workspace
+ env:
+ ENABLE_ZEND_OBSERVER: ${{ matrix.version.enable_zend_observer }}
- name: View logs
if: always()
diff --git a/Cargo.lock b/Cargo.lock
index e49b7f9..49394f5 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1528,9 +1528,9 @@ dependencies = [
[[package]]
name = "phper-sys"
-version = "0.12.0"
+version = "0.12.1"
source = "registry+https://github.com/rust-lang/crates.io-index";
-checksum = "b2538b854618f97f39c0612a9ad97c5980b8d6f32d3a01a9de605851dcaf9507"
+checksum = "fda67a4b0389183e564f08797db31b635eff8bb2b30cfe5cc2394acaa35cd6a7"
dependencies = [
"bindgen",
"cc",
@@ -2099,6 +2099,7 @@ dependencies = [
"libc",
"once_cell",
"phper",
+ "phper-build",
"prost",
"reqwest",
"serde_json",
diff --git a/Cargo.toml b/Cargo.toml
index 7164e6e..47debd0 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -60,3 +60,6 @@ url = "2.3.1"
axum = "0.6.18"
fastcgi-client = "0.8.0"
reqwest = { version = "0.11.18", features = ["trust-dns", "json", "stream"] }
+
+[build-dependencies]
+phper-build = "0.12.0"
diff --git a/README.md b/README.md
index 0f92d0c..a3b339a 100644
--- a/README.md
+++ b/README.md
@@ -37,7 +37,7 @@ SkyWalking PHP Agent requires SkyWalking 8.4+ and PHP 7.2+
* Swoole Ecosystem
- *The components of the PHP-FPM ecosystem can also be used in Swoole
regardless of the flag `SWOOLE_HOOK_ALL`.
+ *The components of the PHP-FPM ecosystem can also be used in Swoole
regardless of the flag `SWOOLE_HOOK_ALL`.*
## Contact Us
diff --git a/build.rs b/build.rs
index dd029df..e7c34a8 100644
--- a/build.rs
+++ b/build.rs
@@ -14,9 +14,5 @@
// limitations under the License.
fn main() {
- #[cfg(target_os = "macos")]
- {
- println!("cargo:rustc-link-arg=-undefined");
- println!("cargo:rustc-link-arg=dynamic_lookup");
- }
+ phper_build::register_all();
}
diff --git a/dist-material/LICENSE b/dist-material/LICENSE
index 57055e1..79cc092 100644
--- a/dist-material/LICENSE
+++ b/dist-material/LICENSE
@@ -599,7 +599,7 @@ The text of each license is also included in
licenses/LICENSE-[project].txt.
https://crates.io/crates/phper-alloc/0.12.0 0.12.0 MulanPSL-2.0
https://crates.io/crates/phper-build/0.12.0 0.12.0 MulanPSL-2.0
https://crates.io/crates/phper-macros/0.12.0 0.12.0 MulanPSL-2.0
- https://crates.io/crates/phper-sys/0.12.0 0.12.0 MulanPSL-2.0
+ https://crates.io/crates/phper-sys/0.12.1 0.12.1 MulanPSL-2.0
========================================================================
Unlicense licenses
diff --git a/docs/en/configuration/ini-settings.md
b/docs/en/configuration/ini-settings.md
index 9106753..bc48b4d 100644
--- a/docs/en/configuration/ini-settings.md
+++ b/docs/en/configuration/ini-settings.md
@@ -19,3 +19,4 @@ This is the configuration list supported in `php.ini`.
| skywalking_agent.ssl_cert_chain_path | The certificate file.
Enable mTLS when `ssl_key_path` and `ssl_cert_chain_path` exist.
| |
| skywalking_agent.heartbeat_period | Agent heartbeat report
period. Unit, second.
| 30 |
| skywalking_agent.properties_report_period_factor | The agent sends the
instance properties to the backend every heartbeat_period *
properties_report_period_factor seconds. | 10 |
+| skywalking_agent.enable_zend_observer | Whether to use `zend
observer` instead of `zend_execute_ex` to hook the functions, this feature is
only available for PHP8+. | Off |
diff --git a/docs/en/configuration/zend-observer.md
b/docs/en/configuration/zend-observer.md
new file mode 100644
index 0000000..c6deeb8
--- /dev/null
+++ b/docs/en/configuration/zend-observer.md
@@ -0,0 +1,32 @@
+# Zend observer
+
+> Refer to:
<https://www.datadoghq.com/blog/engineering/php-8-observability-baked-right-in/#the-observability-landscape-before-php-8>
+
+By default, skywalking-php hooks the `zend_execute_internal` and
`zend_execute_ex` functions to implement auto instrumentation.
+
+But there are some drawbacks:
+
+- All PHP function calls are placed on the native C stack, which is limited by
the value set in `ulimit -s`.
+- Not compatible with the new JIT added in PHP 8.
+
+## The observer API in PHP 8+
+
+Now, zend observer api is a new generation method, and it is also a method
currently recommended by PHP8.
+
+This method has no stack problem and will not affect JIT.
+
+## Configuration
+
+The following configuration example enables JIT in PHP8 and zend observer
support in skywalking-php at the same time.
+
+```ini
+[opcache]
+zend_extension = opcache
+; Enable JIT
+opcache.jit = tracing
+
+[skywalking_agent]
+extension = skywalking_agent.so
+; Switch to use zend observer api to implement auto instrumentation.
+skywalking_agent.enable_zend_observer = On
+```
diff --git a/docs/menu.yml b/docs/menu.yml
index 63878fc..7e9c135 100644
--- a/docs/menu.yml
+++ b/docs/menu.yml
@@ -26,6 +26,8 @@ catalog:
catalog:
- name: "INI Settings"
path: "/en/configuration/ini-settings"
+ - name: "Zend Observer"
+ path: "/en/configuration/zend-observer"
- name: "Contribution"
catalog:
- name: "Compiling Guidance"
diff --git a/src/execute.rs b/src/execute.rs
index 4bc8e68..779afbd 100644
--- a/src/execute.rs
+++ b/src/execute.rs
@@ -14,7 +14,8 @@
// limitations under the License.
use crate::{
- plugin::select_plugin,
+ module::{ENABLE_ZEND_OBSERVER, IS_ZEND_OBSERVER_CALLED_FOR_INTERNAL},
+ plugin::select_plugin_hook,
request::{HACK_SWOOLE_ON_REQUEST_FUNCTION_NAME, IS_SWOOLE},
util::catch_unwind_result,
};
@@ -28,11 +29,10 @@ use phper::{
use std::{any::Any, panic::AssertUnwindSafe, ptr::null_mut,
sync::atomic::Ordering};
use tracing::{error, trace};
-pub type BeforeExecuteHook =
- dyn FnOnce(Option<i64>, &mut ExecuteData) -> crate::Result<Box<dyn Any>>;
+pub type BeforeExecuteHook = dyn Fn(Option<i64>, &mut ExecuteData) ->
crate::Result<Box<dyn Any>>;
pub type AfterExecuteHook =
- dyn FnOnce(Option<i64>, Box<dyn Any>, &mut ExecuteData, &mut ZVal) ->
crate::Result<()>;
+ dyn Fn(Option<i64>, Box<dyn Any>, &mut ExecuteData, &mut ZVal) ->
crate::Result<()>;
pub trait Noop {
fn noop() -> Self;
@@ -109,21 +109,9 @@ unsafe extern "C" fn execute_internal(
return;
}
- let plugin = select_plugin(class_name.as_deref(), &function_name);
- let plugin = match plugin {
- Some(plugin) => plugin,
- None => {
- ori_execute_internal(Some(execute_data), Some(return_value));
- return;
- }
- };
-
- let (before, after) = match plugin.hook(class_name.as_deref(),
&function_name) {
- Some(hook) => hook,
- None => {
- ori_execute_internal(Some(execute_data), Some(return_value));
- return;
- }
+ let Some((before, after)) = select_plugin_hook(class_name.as_deref(),
&function_name) else {
+ ori_execute_internal(Some(execute_data), Some(return_value));
+ return;
};
let request_id = infer_request_id(execute_data);
@@ -183,21 +171,9 @@ unsafe extern "C" fn execute_ex(execute_data: *mut
sys::zend_execute_data) {
}
};
- let plugin = select_plugin(class_name.as_deref(), &function_name);
- let plugin = match plugin {
- Some(plugin) => plugin,
- None => {
- ori_execute_ex(Some(execute_data));
- return;
- }
- };
-
- let (before, after) = match plugin.hook(class_name.as_deref(),
&function_name) {
- Some(hook) => hook,
- None => {
- ori_execute_ex(Some(execute_data));
- return;
- }
+ let Some((before, after)) = select_plugin_hook(class_name.as_deref(),
&function_name) else {
+ ori_execute_ex(Some(execute_data));
+ return;
};
let request_id = infer_request_id(execute_data);
@@ -257,11 +233,15 @@ fn ori_execute_ex(execute_data: Option<&mut ExecuteData>)
{
pub fn register_execute_functions() {
unsafe {
- ORI_EXECUTE_INTERNAL = sys::zend_execute_internal;
- sys::zend_execute_internal = Some(execute_internal);
+ if !*ENABLE_ZEND_OBSERVER || !IS_ZEND_OBSERVER_CALLED_FOR_INTERNAL {
+ ORI_EXECUTE_INTERNAL = sys::zend_execute_internal;
+ sys::zend_execute_internal = Some(execute_internal);
+ }
- ORI_EXECUTE_EX = sys::zend_execute_ex;
- sys::zend_execute_ex = Some(execute_ex);
+ if !*ENABLE_ZEND_OBSERVER {
+ ORI_EXECUTE_EX = sys::zend_execute_ex;
+ sys::zend_execute_ex = Some(execute_ex);
+ }
}
}
@@ -324,3 +304,160 @@ fn infer_request_id(execute_data: &mut ExecuteData) ->
Option<i64> {
}
}
}
+
+pub fn register_observer_handlers() {
+ #[cfg(phper_major_version = "8")]
+ unsafe {
+ if *ENABLE_ZEND_OBSERVER {
+ tracing::info!("register zend observer handlers");
+
sys::zend_observer_fcall_register(Some(self::observer::observer_handler));
+ }
+ }
+}
+
+#[cfg(phper_major_version = "8")]
+pub mod observer {
+ use super::*;
+ use dashmap::DashMap;
+ use once_cell::sync::Lazy;
+ use phper::sys;
+
+ /// Key is the pointer address of execute_data, value is the result of
+ /// before hook.
+ static mut RESULT_MAP: Lazy<DashMap<*const sys::zend_execute_data, Box<dyn
Any>>> =
+ Lazy::new(DashMap::new);
+
+ pub unsafe extern "C" fn observer_handler(
+ execute_data: *mut sys::zend_execute_data,
+ ) -> sys::zend_observer_fcall_handlers {
+ let Some(execute_data) = ExecuteData::try_from_mut_ptr(execute_data)
else {
+ return Default::default()
+ };
+
+ let (function_name, class_name) = match
get_function_and_class_name(execute_data) {
+ Ok(x) => x,
+ Err(err) => {
+ error!(?err, "get function and class name failed");
+ return Default::default();
+ }
+ };
+
+ trace!(
+ ?function_name,
+ ?class_name,
+ "observer_handler function and class name"
+ );
+
+ let Some(function_name) = function_name else {
+ return Default::default();
+ };
+
+ if function_name == HACK_SWOOLE_ON_REQUEST_FUNCTION_NAME {
+ return Default::default();
+ }
+
+ if select_plugin_hook(class_name.as_deref(), &function_name).is_none()
{
+ return Default::default();
+ }
+
+ sys::zend_observer_fcall_handlers {
+ begin: Some(observer_begin),
+ end: Some(observer_end),
+ }
+ }
+
+ unsafe extern "C" fn observer_begin(execute_data: *mut
sys::zend_execute_data) {
+ let Some(execute_data) = ExecuteData::try_from_mut_ptr(execute_data)
else {
+ return;
+ };
+ trace!(execute_data_ptr=?execute_data.as_ptr(), "start
observer_begin");
+
+ let Ok((function_name, class_name)) =
get_function_and_class_name(execute_data) else {
+ return;
+ };
+
+ trace!(
+ ?function_name,
+ ?class_name,
+ "observer_begin function and class name"
+ );
+
+ let Some(function_name) = function_name else {
+ return;
+ };
+
+ let Some((before, _)) = select_plugin_hook(class_name.as_deref(),
&function_name) else {
+ return;
+ };
+
+ let request_id = infer_request_id(execute_data);
+ trace!(
+ ?request_id,
+ ?function_name,
+ ?class_name,
+ "observer_begin infer request id"
+ );
+
+ let result =
+ match catch_unwind_result(AssertUnwindSafe(|| before(request_id,
execute_data))) {
+ Ok(result) => result,
+ Err(err) => {
+ error!(?err, "run observer_begin hook failed");
+ return;
+ }
+ };
+
+ RESULT_MAP.insert(execute_data.as_ptr(), result);
+ }
+
+ unsafe extern "C" fn observer_end(
+ execute_data: *mut sys::zend_execute_data, retval: *mut sys::zval,
+ ) {
+ let Some(execute_data) = ExecuteData::try_from_mut_ptr(execute_data)
else {
+ return;
+ };
+ trace!(execute_data_ptr=?execute_data.as_ptr(), "start observer_end");
+
+ let Some((_, result)) = RESULT_MAP.remove(&execute_data.as_ptr()) else
{
+ return;
+ };
+
+ let mut null = ZVal::from(());
+ let ret = match ZVal::try_from_mut_ptr(retval) {
+ Some(ret) => ret,
+ None => &mut null,
+ };
+
+ let Ok((function_name, class_name)) =
get_function_and_class_name(execute_data) else {
+ return;
+ };
+
+ trace!(
+ ?function_name,
+ ?class_name,
+ "observer_end function and class name"
+ );
+
+ let Some(function_name) = function_name else {
+ return;
+ };
+
+ let Some((_, after)) = select_plugin_hook(class_name.as_deref(),
&function_name) else {
+ return;
+ };
+
+ let request_id = infer_request_id(execute_data);
+ trace!(
+ ?request_id,
+ ?function_name,
+ ?class_name,
+ "observer_end infer request id"
+ );
+
+ if let Err(err) = catch_unwind_result(AssertUnwindSafe(|| {
+ after(request_id, result, execute_data, ret)
+ })) {
+ error!(?err, "run observer_end hook failed");
+ };
+ }
+}
diff --git a/src/lib.rs b/src/lib.rs
index 75e683e..5a16850 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -83,6 +83,11 @@ const SKYWALKING_AGENT_HEARTBEAT_PERIOD: &str =
"skywalking_agent.heartbeat_peri
const SKYWALKING_AGENT_PROPERTIES_REPORT_PERIOD_FACTOR: &str =
"skywalking_agent.properties_report_period_factor";
+/// Whether to use zend observer instead of zend_execute_ex to hook the
+/// functions. This feature is only available for PHP8+, and can work with
+/// PHP8's jit.
+const SKYWALKING_AGENT_ENABLE_ZEND_OBSERVER: &str =
"skywalking_agent.enable_zend_observer";
+
#[php_get_module]
pub fn get_module() -> Module {
let mut module = Module::new(
@@ -147,6 +152,7 @@ pub fn get_module() -> Module {
10i64,
Policy::System,
);
+ module.add_ini(SKYWALKING_AGENT_ENABLE_ZEND_OBSERVER, false,
Policy::System);
// Hooks.
module.on_module_init(module::init);
diff --git a/src/module.rs b/src/module.rs
index 37a41fe..662044e 100644
--- a/src/module.rs
+++ b/src/module.rs
@@ -15,15 +15,10 @@
use crate::{
channel::Reporter,
- execute::register_execute_functions,
+ execute::{register_execute_functions, register_observer_handlers},
util::{get_sapi_module_name, IPS},
worker::init_worker,
- SKYWALKING_AGENT_AUTHENTICATION, SKYWALKING_AGENT_ENABLE,
SKYWALKING_AGENT_ENABLE_TLS,
- SKYWALKING_AGENT_HEARTBEAT_PERIOD, SKYWALKING_AGENT_LOG_FILE,
SKYWALKING_AGENT_LOG_LEVEL,
- SKYWALKING_AGENT_PROPERTIES_REPORT_PERIOD_FACTOR,
SKYWALKING_AGENT_RUNTIME_DIR,
- SKYWALKING_AGENT_SERVICE_NAME, SKYWALKING_AGENT_SKYWALKING_VERSION,
- SKYWALKING_AGENT_SSL_CERT_CHAIN_PATH, SKYWALKING_AGENT_SSL_KEY_PATH,
- SKYWALKING_AGENT_SSL_TRUSTED_CA_PATH,
+ *,
};
use anyhow::bail;
use once_cell::sync::Lazy;
@@ -117,6 +112,18 @@ pub static HEARTBEAT_PERIOD: Lazy<i64> =
pub static PROPERTIES_REPORT_PERIOD_FACTOR: Lazy<i64> =
Lazy::new(||
ini_get::<i64>(SKYWALKING_AGENT_PROPERTIES_REPORT_PERIOD_FACTOR));
+/// Zend observer is only support in PHP8+.
+pub static ENABLE_ZEND_OBSERVER: Lazy<bool> = Lazy::new(|| {
+ sys::PHP_MAJOR_VERSION >= 8 &&
ini_get::<bool>(SKYWALKING_AGENT_ENABLE_ZEND_OBSERVER)
+});
+
+/// For PHP 8.2+, zend observer api are now also called for internal functions.
+///
+/// Refer to this commit:
<https://github.com/php/php-src/commit/625f1649639c2b9a9d76e4d42f88c264ddb8447d>
+#[allow(clippy::absurd_extreme_comparisons)]
+pub const IS_ZEND_OBSERVER_CALLED_FOR_INTERNAL: bool =
+ sys::PHP_MAJOR_VERSION > 8 || (sys::PHP_MAJOR_VERSION == 8 &&
sys::PHP_MINOR_VERSION >= 2);
+
pub fn init() {
if !is_enable() {
return;
@@ -184,6 +191,7 @@ pub fn init() {
// Hook functions.
register_execute_functions();
+ register_observer_handlers();
}
pub fn shutdown() {
diff --git a/src/plugin/mod.rs b/src/plugin/mod.rs
index 53adeef..71acd06 100644
--- a/src/plugin/mod.rs
+++ b/src/plugin/mod.rs
@@ -26,6 +26,8 @@ use crate::execute::{AfterExecuteHook, BeforeExecuteHook};
use once_cell::sync::Lazy;
use phper::{eg, objects::ZObj};
use skywalking::trace::span::AbstractSpan;
+use std::{collections::HashMap, ops::Deref, sync::Mutex};
+use tracing::error;
// Register plugins here.
static PLUGINS: Lazy<Vec<Box<DynPlugin>>> = Lazy::new(|| {
@@ -54,7 +56,35 @@ pub trait Plugin {
) -> Option<(Box<BeforeExecuteHook>, Box<AfterExecuteHook>)>;
}
-pub fn select_plugin(class_name: Option<&str>, function_name: &str) ->
Option<&'static DynPlugin> {
+pub fn select_plugin_hook(
+ class_name: Option<&str>, function_name: &str,
+) -> Option<(&'static BeforeExecuteHook, &'static AfterExecuteHook)> {
+ type HookMap =
+ HashMap<(Option<String>, String), Option<(Box<BeforeExecuteHook>,
Box<AfterExecuteHook>)>>;
+
+ static LOCK: Lazy<Mutex<()>> = Lazy::new(Default::default);
+ static mut HOOK_MAP: Lazy<HookMap> = Lazy::new(HashMap::new);
+
+ let _guard = match LOCK.lock() {
+ Ok(guard) => guard,
+ Err(err) => {
+ error!(?err, "get lock failed");
+ return None;
+ }
+ };
+ unsafe {
+ HOOK_MAP
+ .entry((class_name.map(ToOwned::to_owned),
function_name.to_owned()))
+ .or_insert_with(|| {
+ select_plugin(class_name, function_name)
+ .and_then(|plugin| plugin.hook(class_name, function_name))
+ })
+ .as_ref()
+ .map(|(before, after)| (before.deref(), after.deref()))
+ }
+}
+
+fn select_plugin(class_name: Option<&str>, function_name: &str) ->
Option<&'static DynPlugin> {
let mut selected_plugin = None;
for plugin in &*PLUGINS {
@@ -77,9 +107,9 @@ pub fn select_plugin(class_name: Option<&str>,
function_name: &str) -> Option<&'
selected_plugin.map(AsRef::as_ref)
}
-fn log_exception(span: &mut impl AbstractSpan) {
- let ex = unsafe { ZObj::try_from_mut_ptr(eg!(exception)) };
- if let Some(ex) = ex {
+fn log_exception(span: &mut impl AbstractSpan) -> Option<&mut ZObj> {
+ let mut ex = unsafe { ZObj::try_from_mut_ptr(eg!(exception)) };
+ if let Some(ex) = ex.as_mut() {
let mut span_object = span.span_object_mut();
span_object.is_error = true;
@@ -101,4 +131,5 @@ fn log_exception(span: &mut impl AbstractSpan) {
span_object.add_log(logs);
}
}
+ ex
}
diff --git a/src/plugin/plugin_pdo.rs b/src/plugin/plugin_pdo.rs
index 9d33b6d..c4a32b9 100644
--- a/src/plugin/plugin_pdo.rs
+++ b/src/plugin/plugin_pdo.rs
@@ -202,6 +202,10 @@ fn after_hook(
) -> crate::Result<()> {
let mut span = span.downcast::<Span>().unwrap();
+ if log_exception(&mut *span).is_some() {
+ return Ok(());
+ }
+
if let Some(b) = return_value.as_bool() {
if !b {
return after_hook_when_false(get_this_mut(execute_data)?, &mut
span);
@@ -217,8 +221,6 @@ fn after_hook(
}
}
- log_exception(&mut *span);
-
Ok(())
}
diff --git a/src/plugin/plugin_predis.rs b/src/plugin/plugin_predis.rs
index bcf9e36..ca7ab7e 100644
--- a/src/plugin/plugin_predis.rs
+++ b/src/plugin/plugin_predis.rs
@@ -164,9 +164,7 @@ impl Plugin for PredisPlugin {
Box<crate::execute::AfterExecuteHook>,
)> {
match (class_name, function_name) {
- (Some(class_name @ "Predis\\Client"), function_name)
- if
REDIS_ALL_COMMANDS.contains(&*function_name.to_ascii_uppercase()) =>
- {
+ (Some(class_name @ "Predis\\Client"), "__call") => {
Some(self.hook_predis_execute_command(class_name,
function_name))
}
_ => None,
@@ -181,22 +179,32 @@ enum ConnectionType {
impl PredisPlugin {
fn hook_predis_execute_command(
- &self, class_name: &str, function_name: &str,
+ &self, class_name: &str, _function_name: &str,
) -> (Box<BeforeExecuteHook>, Box<AfterExecuteHook>) {
let class_name = class_name.to_owned();
- let function_name = function_name.to_owned();
(
Box::new(move |request_id, execute_data| {
validate_num_args(execute_data, 1)?;
+ let function_name = execute_data
+ .get_parameter(0)
+ .as_z_str()
+ .and_then(|s| s.to_str().ok())
+ .unwrap_or("")
+ .to_string();
+
+ let cmd = function_name.to_uppercase();
+
+ if !REDIS_ALL_COMMANDS.contains(&*cmd) {
+ return Ok(Box::new(()));
+ }
+
let this = get_this_mut(execute_data)?;
let handle = this.handle();
let connection = this.call("getConnection", [])?;
let peer = Self::get_peer(connection)?;
- let cmd = function_name.to_ascii_uppercase();
-
let op = if REDIS_READ_COMMANDS.contains(&*cmd) {
Some("read")
} else if REDIS_WRITE_COMMANDS.contains(&*cmd) {
@@ -206,14 +214,20 @@ impl PredisPlugin {
};
let key = op
- .and_then(|_| execute_data.get_parameter(0).as_z_str())
+ .and_then(|_| execute_data.get_parameter(1).as_z_arr())
+ .and_then(|params| params.get(0))
+ .and_then(|key| key.as_z_str())
.and_then(|s| s.to_str().ok());
debug!(handle, cmd, key, op, "call redis command");
- let mut span = RequestContext::try_with_global_ctx(request_id,
|ctx| {
- Ok(ctx.create_exit_span(&format!("{}->{}", class_name,
function_name), &peer))
- })?;
+ let mut span =
+ RequestContext::try_with_global_ctx(request_id, |ctx| {
+ Ok(ctx.create_exit_span(
+ &format!("{}->{}", class_name, &function_name,),
+ &peer,
+ ))
+ })?;
let mut span_object = span.span_object_mut();
span_object.set_span_layer(SpanLayer::Cache);
@@ -230,6 +244,10 @@ impl PredisPlugin {
Ok(Box::new(span))
}),
Box::new(move |_, span, _, return_value| {
+ if span.downcast_ref::<()>().is_some() {
+ return Ok(());
+ }
+
let mut span = span.downcast::<Span>().unwrap();
let exception = unsafe { eg!(exception) };
diff --git a/src/tag.rs b/src/tag.rs
index c408390..16a1347 100644
--- a/src/tag.rs
+++ b/src/tag.rs
@@ -17,11 +17,11 @@
//!
//! Virtual Cache
//!
-//!
https://skywalking.apache.org/docs/main/next/en/setup/service-agent/virtual-cache/
+//!
<https://skywalking.apache.org/docs/main/next/en/setup/service-agent/virtual-cache/>
//!
//! Virtual Database
//!
-//!
https://skywalking.apache.org/docs/main/next/en/setup/service-agent/virtual-database/
+//!
<https://skywalking.apache.org/docs/main/next/en/setup/service-agent/virtual-database/>
use std::fmt::Display;
diff --git a/tests/common/mod.rs b/tests/common/mod.rs
index 91dbee1..a691108 100644
--- a/tests/common/mod.rs
+++ b/tests/common/mod.rs
@@ -69,6 +69,9 @@ pub const EXT: &str = if cfg!(target_os = "linux") {
""
};
+pub static ENABLE_ZEND_OBSERVER: Lazy<String> =
+ Lazy::new(|| env::var("ENABLE_ZEND_OBSERVER").unwrap_or_else(|_|
"Off".to_owned()));
+
pub static HTTP_CLIENT: Lazy<reqwest::Client> =
Lazy::new(reqwest::Client::new);
pub struct Fixture {
@@ -82,6 +85,13 @@ pub struct Fixture {
pub async fn setup() -> Fixture {
setup_logging();
+ info!(
+ TARGET = &*TARGET,
+ EXT = &*EXT,
+ ENABLE_ZEND_OBSERVER = &*ENABLE_ZEND_OBSERVER,
+ "setup fixture"
+ );
+
Fixture {
http_server_1_handle: tokio::spawn(setup_http_proxy_server(
PROXY_SERVER_1_ADDRESS,
@@ -156,6 +166,7 @@ async fn http_proxy_fpm_handler(
let method = &req.method().to_string();
let path = &req.uri().path().to_string();
let query = &req.uri().query().unwrap_or_default().to_string();
+ info!(method, uri=?req.uri(), "http proxy to fpm");
let stream = TcpStream::connect(&state.fpm_addr).await?;
let client = fastcgi_client::Client::new(stream);
@@ -293,6 +304,11 @@ fn setup_php_fpm(index: usize, fpm_addr: &str) -> Child {
),
"-d",
"skywalking_agent.worker_threads=3",
+ "-d",
+ &format!(
+ "skywalking_agent.enable_zend_observer={}",
+ *ENABLE_ZEND_OBSERVER
+ ),
];
info!(cmd = args.join(" "), "start command");
let child = Command::new(&args[0])
@@ -334,6 +350,11 @@ fn setup_php_swoole(index: usize) -> Child {
),
"-d",
"skywalking.worker_threads=3",
+ "-d",
+ &format!(
+ "skywalking_agent.enable_zend_observer={}",
+ *ENABLE_ZEND_OBSERVER
+ ),
&format!("tests/php/swoole/main.{}.php", index),
];
info!(cmd = args.join(" "), "start command");
diff --git a/tests/data/expected_context.yaml b/tests/data/expected_context.yaml
index 8e07feb..7ff9a3d 100644
--- a/tests/data/expected_context.yaml
+++ b/tests/data/expected_context.yaml
@@ -684,9 +684,9 @@ segmentItems:
}
logs:
- logEvent:
- - { key: SQLSTATE, value: 42S02 }
- - { key: Error Code, value: '1146' }
- - { key: Error, value: "Table 'skywalking.not_exist' doesn't
exist" }
+ - { key: error.kind, value: PDOException }
+ - { key: message, value: not null }
+ - { key: stack, value: not null }
- operationName: GET:/pdo.php
parentSpanId: -1
spanId: 0
diff --git a/tests/e2e.rs b/tests/e2e.rs
index 3ae9e99..00b1f72 100644
--- a/tests/e2e.rs
+++ b/tests/e2e.rs
@@ -215,6 +215,6 @@ async fn request_common(request_builder: RequestBuilder,
actual_content: impl In
let response = request_builder.send().await.unwrap();
let status = response.status();
let content = response.text().await.unwrap();
- info!(content, "response content");
+ info!("response content: {}", content);
assert_eq!((status, content), (StatusCode::OK, actual_content.into()));
}
