You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by wu...@apache.org on 2023/08/26 15:09:58 UTC
[skywalking-php] branch master updated: Refactor mysqli plugin, support procedural api. (#94)
This is an automated email from the ASF dual-hosted git repository.
wusheng 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 b26e94e Refactor mysqli plugin, support procedural api. (#94)
b26e94e is described below
commit b26e94e272bfbfbd3c3ec36fbe9f465d747fab1f
Author: jmjoy <jm...@apache.org>
AuthorDate: Sat Aug 26 23:09:53 2023 +0800
Refactor mysqli plugin, support procedural api. (#94)
---
src/plugin/plugin_memcache.rs | 2 +-
src/plugin/plugin_mysqli.rs | 232 +++++++++++++++++++++++++--------------
src/plugin/style.rs | 3 +-
tests/data/expected_context.yaml | 39 +++++++
tests/e2e.rs | 32 +++++-
tests/php/fpm/mysqli.php | 13 +++
6 files changed, 231 insertions(+), 90 deletions(-)
diff --git a/src/plugin/plugin_memcache.rs b/src/plugin/plugin_memcache.rs
index 84ab071..cba62bc 100644
--- a/src/plugin/plugin_memcache.rs
+++ b/src/plugin/plugin_memcache.rs
@@ -249,7 +249,7 @@ fn create_exit_span(
) -> anyhow::Result<Span> {
RequestContext::try_with_global_ctx(request_id, |ctx| {
let mut span = ctx.create_exit_span(
- &style.generate_peer_name(class_name, function_name),
+ &style.generate_operation_name(class_name, function_name),
remote_peer,
);
diff --git a/src/plugin/plugin_mysqli.rs b/src/plugin/plugin_mysqli.rs
index 8251686..cc2ad0f 100644
--- a/src/plugin/plugin_mysqli.rs
+++ b/src/plugin/plugin_mysqli.rs
@@ -13,25 +13,23 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-use super::{log_exception, Plugin};
+use super::{log_exception, style::ApiStyle, Plugin};
use crate::{
component::COMPONENT_PHP_MYSQLI_ID,
context::RequestContext,
- execute::{get_this_mut, AfterExecuteHook, BeforeExecuteHook},
+ execute::{AfterExecuteHook, BeforeExecuteHook},
+};
+use phper::{
+ alloc::ToRefOwned,
+ functions::call,
+ objects::ZObj,
+ values::{ExecuteData, ZVal},
};
-use anyhow::Context;
-use dashmap::DashMap;
-use once_cell::sync::Lazy;
-use phper::{objects::ZObj, sys};
use skywalking::{
proto::v3::SpanLayer,
trace::span::{HandleSpanObject, Span},
};
-use tracing::debug;
-
-static MYSQL_MAP: Lazy<DashMap<u32, MySQLInfo>> = Lazy::new(Default::default);
-
-static DTOR_MAP: Lazy<DashMap<u32, sys::zend_object_dtor_obj_t>> = Lazy::new(Default::default);
+use tracing::{debug, error};
#[derive(Default, Clone)]
pub struct MySQLImprovedPlugin;
@@ -44,16 +42,42 @@ impl Plugin for MySQLImprovedPlugin {
#[inline]
fn function_name_prefix(&self) -> Option<&'static str> {
- None
+ Some("mysqli_")
}
fn hook(
&self, class_name: Option<&str>, function_name: &str,
) -> Option<(Box<BeforeExecuteHook>, Box<AfterExecuteHook>)> {
match (class_name, function_name) {
- (Some("mysqli"), "__construct") => Some(self.hook_mysqli_construct()),
- (Some("mysqli"), f) if ["query"].contains(&f) => {
- Some(self.hook_mysqli_methods(function_name))
+ (Some("mysqli"), "__construct" | "real_connect") => {
+ Some(self.hook_mysqli_connect(class_name, function_name, ApiStyle::OO))
+ }
+ (None, "mysqli_connect" | "mysqli_real_connect") => {
+ Some(self.hook_mysqli_connect(class_name, function_name, ApiStyle::Procedural))
+ }
+ (Some("mysqli"), f)
+ if [
+ "query",
+ "execute_query",
+ "multi_query",
+ "real_query",
+ "prepare",
+ ]
+ .contains(&f) =>
+ {
+ Some(self.hook_mysqli_methods(class_name, function_name, ApiStyle::OO))
+ }
+ (None, f)
+ if [
+ "mysqli_query",
+ "mysqli_execute_query",
+ "mysqli_multi_query",
+ "mysqli_real_query",
+ "mysqli_prepare",
+ ]
+ .contains(&f) =>
+ {
+ Some(self.hook_mysqli_methods(class_name, function_name, ApiStyle::Procedural))
}
_ => None,
}
@@ -61,45 +85,53 @@ impl Plugin for MySQLImprovedPlugin {
}
impl MySQLImprovedPlugin {
- fn hook_mysqli_construct(&self) -> (Box<BeforeExecuteHook>, Box<AfterExecuteHook>) {
+ fn hook_mysqli_connect(
+ &self, class_name: Option<&str>, function_name: &str, style: ApiStyle,
+ ) -> (Box<BeforeExecuteHook>, Box<AfterExecuteHook>) {
+ let class_name = class_name.map(ToOwned::to_owned);
+ let function_name = function_name.to_owned();
(
- Box::new(|request_id, execute_data| {
- let this = get_this_mut(execute_data)?;
- let handle = this.handle();
- hack_dtor(this, Some(mysqli_dtor));
-
- let mut info: MySQLInfo = MySQLInfo {
- hostname: "127.0.0.1".to_string(),
- port: 3306,
- };
-
- let num_args = execute_data.num_args();
- if num_args >= 1 {
- // host only
- let hostname = execute_data.get_parameter(0);
- let hostname = hostname
- .as_z_str()
- .context("hostname isn't str")?
- .to_str()?;
- debug!(hostname, "mysqli hostname");
-
- info.hostname = hostname.to_owned();
- }
- if num_args >= 5 {
- let port = execute_data.get_parameter(4);
- let port = port.as_long().context("port isn't str")?;
- debug!(port, "mysqli port");
- info.port = port
- }
-
- let span = create_mysqli_exit_span(request_id, "mysqli", "__construct", &info)?;
+ Box::new(move |request_id, execute_data| {
+ // Sometimes the connection is failed. Therefore, first assemble the peer from
+ // the parameters to prevent assembly failure in the after hook.
+ let peer = get_peer_by_parameters(execute_data, style);
- MYSQL_MAP.insert(handle, info);
+ let span = create_mysqli_exit_span(
+ request_id,
+ class_name.as_deref(),
+ &function_name,
+ &peer,
+ style,
+ )?;
Ok(Box::new(span))
}),
- Box::new(move |_, span, _, _| {
+ Box::new(move |_, span, execute_data, return_value| {
let mut span = span.downcast::<Span>().unwrap();
+
+ // Reset the peer here, it should be more precise.
+ if let Some(b) = return_value.as_bool() {
+ if !b {
+ span.span_object_mut().is_error = true;
+ }
+ }
+ if let Some(this) = return_value.as_mut_z_obj() {
+ if let Some(peer) = get_peer_by_this(this) {
+ span.span_object_mut().peer = peer;
+ }
+ } else {
+ match style.get_this_mut(execute_data) {
+ Ok(this) => {
+ if let Some(peer) = get_peer_by_this(this) {
+ span.span_object_mut().peer = peer;
+ }
+ }
+ Err(err) => {
+ error!(?err, "reset peer failed");
+ }
+ }
+ }
+
log_exception(&mut *span);
Ok(())
}),
@@ -107,19 +139,25 @@ impl MySQLImprovedPlugin {
}
fn hook_mysqli_methods(
- &self, function_name: &str,
+ &self, class_name: Option<&str>, function_name: &str, style: ApiStyle,
) -> (Box<BeforeExecuteHook>, Box<AfterExecuteHook>) {
+ let class_name = class_name.map(ToOwned::to_owned);
let function_name = function_name.to_owned();
(
Box::new(move |request_id, execute_data| {
- let this = get_this_mut(execute_data)?;
+ let this = style.get_this_mut(execute_data)?;
let handle = this.handle();
- debug!(handle, function_name, "call mysqli method");
+ debug!(handle, class_name, function_name, "call mysqli method");
- let mut span = with_info(handle, |info| {
- create_mysqli_exit_span(request_id, "mysqli", &function_name, info)
- })?;
+ let peer = &get_peer_by_this(this).unwrap_or_default();
+ let mut span = create_mysqli_exit_span(
+ request_id,
+ class_name.as_deref(),
+ &function_name,
+ peer,
+ style,
+ )?;
if execute_data.num_args() >= 1 {
if let Some(statement) = execute_data.get_parameter(0).as_z_str() {
@@ -129,8 +167,13 @@ impl MySQLImprovedPlugin {
Ok(Box::new(span) as _)
}),
- Box::new(move |_, span, _, _| {
+ Box::new(move |_, span, _, return_value| {
let mut span = span.downcast::<Span>().unwrap();
+ if let Some(b) = return_value.as_bool() {
+ if !b {
+ span.span_object_mut().is_error = true;
+ }
+ }
log_exception(&mut *span);
Ok(())
}),
@@ -139,12 +182,13 @@ impl MySQLImprovedPlugin {
}
fn create_mysqli_exit_span(
- request_id: Option<i64>, class_name: &str, function_name: &str, info: &MySQLInfo,
+ request_id: Option<i64>, class_name: Option<&str>, function_name: &str, peer: &str,
+ style: ApiStyle,
) -> anyhow::Result<Span> {
RequestContext::try_with_global_ctx(request_id, |ctx| {
let mut span = ctx.create_exit_span(
- &format!("{}->{}", class_name, function_name),
- &format!("{}:{}", info.hostname, info.port),
+ &style.generate_operation_name(class_name, function_name),
+ peer,
);
let span_object = span.span_object_mut();
@@ -156,38 +200,60 @@ fn create_mysqli_exit_span(
})
}
-fn with_info<T>(handle: u32, f: impl FnOnce(&MySQLInfo) -> anyhow::Result<T>) -> anyhow::Result<T> {
- MYSQL_MAP
- .get(&handle)
- .map(|r| f(r.value()))
- .context("info not exists")?
-}
+fn get_peer_by_this(this: &mut ZObj) -> Option<String> {
+ let handle = this.handle();
+
+ debug!(handle, "start to call mysqli_get_host_info");
-struct MySQLInfo {
- hostname: String,
- port: i64,
+ let host_info = match call("mysqli_get_host_info", [ZVal::from(this.to_ref_owned())]) {
+ Ok(host_info) => host_info,
+ Err(err) => {
+ error!(handle, ?err, "call mysqli_get_host_info failed");
+ return None;
+ }
+ };
+
+ host_info
+ .as_z_str()
+ .and_then(|info| info.to_str().ok())
+ .and_then(|info| info.split(' ').next())
+ .map(ToOwned::to_owned)
+ .map(|mut info| {
+ if !info.contains(':') {
+ info.push_str(":3306");
+ }
+ info
+ })
}
-fn hack_dtor(this: &mut ZObj, new_dtor: sys::zend_object_dtor_obj_t) {
- let handle = this.handle();
+fn get_peer_by_parameters(execute_data: &mut ExecuteData, style: ApiStyle) -> String {
+ let mut peer = "".to_owned();
- unsafe {
- let ori_dtor = (*(*this.as_mut_ptr()).handlers).dtor_obj;
- DTOR_MAP.insert(handle, ori_dtor);
- (*((*this.as_mut_ptr()).handlers as *mut sys::zend_object_handlers)).dtor_obj = new_dtor;
+ if style.validate_num_args(execute_data, 1).is_ok() {
+ peer.push_str(
+ style
+ .get_mut_parameter(execute_data, 0)
+ .as_z_str()
+ .and_then(|s| s.to_str().ok())
+ .unwrap_or_default(),
+ );
}
-}
-unsafe extern "C" fn mysqli_dtor(object: *mut sys::zend_object) {
- debug!("call mysqli dtor");
- dtor(object);
-}
+ if !peer.is_empty() {
+ let port = style.get_mut_parameter(execute_data, 4);
-unsafe extern "C" fn dtor(object: *mut sys::zend_object) {
- let handle = ZObj::from_ptr(object).handle();
+ #[allow(clippy::manual_map)]
+ let port = if let Some(port) = port.as_z_str() {
+ port.to_str().ok().map(ToOwned::to_owned)
+ } else if let Some(port) = port.as_long() {
+ Some(port.to_string())
+ } else {
+ None
+ };
- MYSQL_MAP.remove(&handle);
- if let Some((_, Some(dtor))) = DTOR_MAP.remove(&handle) {
- dtor(object);
+ peer.push(':');
+ peer.push_str(port.as_deref().unwrap_or("3306"));
}
+
+ peer
}
diff --git a/src/plugin/style.rs b/src/plugin/style.rs
index d06b828..4d99bf6 100644
--- a/src/plugin/style.rs
+++ b/src/plugin/style.rs
@@ -48,7 +48,6 @@ impl ApiStyle {
execute_data.get_mut_parameter(index)
}
- #[allow(dead_code)]
pub fn validate_num_args(
self, execute_data: &mut ExecuteData, num: usize,
) -> anyhow::Result<()> {
@@ -59,7 +58,7 @@ impl ApiStyle {
validate_num_args(execute_data, num)
}
- pub fn generate_peer_name(self, class_name: Option<&str>, function_name: &str) -> String {
+ pub fn generate_operation_name(self, class_name: Option<&str>, function_name: &str) -> String {
match self {
ApiStyle::OO => format!("{}->{}", class_name.unwrap_or_default(), function_name),
ApiStyle::Procedural => function_name.to_owned(),
diff --git a/tests/data/expected_context.yaml b/tests/data/expected_context.yaml
index 3729f39..1a421b5 100644
--- a/tests/data/expected_context.yaml
+++ b/tests/data/expected_context.yaml
@@ -834,6 +834,45 @@ segmentItems:
key: db.statement,
value: "SELECT * FROM `mysql`.`user` WHERE `User` = 'root'",
}
+ - operationName: mysqli_connect
+ parentSpanId: 0
+ spanId: 5
+ spanLayer: Database
+ startTime: gt 0
+ endTime: gt 0
+ componentId: 8004
+ isError: false
+ spanType: Exit
+ peer: 127.0.0.1:3306
+ skipAnalysis: false
+ tags:
+ - { key: db.type, value: mysql }
+ - operationName: mysqli_query
+ parentSpanId: 0
+ spanId: 6
+ spanLayer: Database
+ startTime: gt 0
+ endTime: gt 0
+ componentId: 8004
+ isError: false
+ spanType: Exit
+ peer: 127.0.0.1:3306
+ skipAnalysis: false
+ tags:
+ - { key: db.type, value: mysql }
+ - operationName: mysqli_real_connect
+ parentSpanId: 0
+ spanId: 7
+ spanLayer: Database
+ startTime: gt 0
+ endTime: gt 0
+ componentId: 8004
+ isError: true
+ spanType: Exit
+ peer: 127.0.0.1:3306
+ skipAnalysis: false
+ tags:
+ - { key: db.type, value: mysql }
- operationName: GET:/mysqli.php
parentSpanId: -1
spanId: 0
diff --git a/tests/e2e.rs b/tests/e2e.rs
index 9c8854b..937134b 100644
--- a/tests/e2e.rs
+++ b/tests/e2e.rs
@@ -21,11 +21,17 @@ use crate::common::{
};
use reqwest::{header::CONTENT_TYPE, RequestBuilder, StatusCode};
use std::{
+ future::Future,
panic::{catch_unwind, resume_unwind},
time::Duration,
};
-use tokio::{fs::File, runtime::Handle, task, time::sleep};
-use tracing::info;
+use tokio::{
+ fs::{self, File},
+ runtime::Handle,
+ task,
+ time::sleep,
+};
+use tracing::{error, info};
#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
async fn e2e() {
@@ -233,7 +239,7 @@ async fn request_swoole_2_memcache() {
}
async fn request_collector_validate() {
- request_common(
+ request(
HTTP_CLIENT
.post(format!("http://{}/dataValidate", COLLECTOR_HTTP_ADDRESS))
.header(CONTENT_TYPE, "text/yaml")
@@ -243,14 +249,32 @@ async fn request_collector_validate() {
.unwrap(),
),
"success",
+ |content| async move {
+ let result_file = "/tmp/skywalking-agent-collector-validate-result.txt";
+ if let Err(err) = fs::write(result_file, content).await {
+ error!(?err, "write to {} failed", result_file);
+ }
+ },
)
.await;
}
async fn request_common(request_builder: RequestBuilder, actual_content: impl Into<String>) {
+ request(request_builder, actual_content, |content| async move {
+ info!("response content: {}", content);
+ })
+ .await
+}
+
+async fn request<F>(
+ request_builder: RequestBuilder, actual_content: impl Into<String>,
+ handler: impl FnOnce(String) -> F,
+) where
+ F: Future<Output = ()>,
+{
let response = request_builder.send().await.unwrap();
let status = response.status();
let content = response.text().await.unwrap();
- info!("response content: {}", content);
+ handler(content.clone()).await;
assert_eq!((status, content), (StatusCode::OK, actual_content.into()));
}
diff --git a/tests/php/fpm/mysqli.php b/tests/php/fpm/mysqli.php
index de90a7f..e435a76 100644
--- a/tests/php/fpm/mysqli.php
+++ b/tests/php/fpm/mysqli.php
@@ -32,4 +32,17 @@ require_once dirname(__DIR__) . "/vendor/autoload.php";
Assert::same(count($rs), 2);
}
+{
+ $mysqli = mysqli_connect("127.0.0.1", "root", "password", "skywalking", 3306);
+ $result = mysqli_query($mysqli, "SELECT * FROM `mysql`.`user` WHERE `User` = 'root'");
+ $rs = $result->fetch_all();
+ Assert::same(count($rs), 2);
+}
+
+{
+ mysqli_report(MYSQLI_REPORT_OFF);
+ $mysqli = mysqli_init();
+ @mysqli_real_connect($mysqli, "127.0.0.1", "root", "password_incorrect", "skywalking", 3306);
+}
+
echo "ok";