From 2c6bc944416cd2b6c0707dbd52212b13568a3c37 Mon Sep 17 00:00:00 2001 From: jmjoy Date: Thu, 27 Oct 2022 20:17:55 +0800 Subject: [PATCH 1/9] Prepare to add redis plugin. --- src/plugin/mod.rs | 1 + src/plugin/plugin_redis.rs | 14 ++++++++++++++ 2 files changed, 15 insertions(+) create mode 100644 src/plugin/plugin_redis.rs diff --git a/src/plugin/mod.rs b/src/plugin/mod.rs index 5b16556..916f44d 100644 --- a/src/plugin/mod.rs +++ b/src/plugin/mod.rs @@ -19,6 +19,7 @@ mod plugin_mysqli; mod plugin_pdo; mod plugin_predis; mod plugin_swoole; +mod plugin_redis; use crate::execute::{AfterExecuteHook, BeforeExecuteHook}; use once_cell::sync::Lazy; diff --git a/src/plugin/plugin_redis.rs b/src/plugin/plugin_redis.rs new file mode 100644 index 0000000..63f7276 --- /dev/null +++ b/src/plugin/plugin_redis.rs @@ -0,0 +1,14 @@ +// Licensed to the Apache Software Foundation (ASF) under one or more +// contributor license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright ownership. +// The ASF licenses this file to You under the Apache License, Version 2.0 +// (the "License"); you may not use this file except in compliance with +// the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. From d85a5fce613bac2ac3e4af4916f68668bba73aba Mon Sep 17 00:00:00 2001 From: jmjoy Date: Fri, 4 Nov 2022 10:25:48 +0800 Subject: [PATCH 2/9] Adding redis plugin. --- src/component.rs | 1 + src/plugin/mod.rs | 3 +- src/plugin/plugin_redis.rs | 173 +++++++++++++++++++++++++++++++++++++ 3 files changed, 176 insertions(+), 1 deletion(-) diff --git a/src/component.rs b/src/component.rs index cb6ab19..b6aaf08 100644 --- a/src/component.rs +++ b/src/component.rs @@ -23,3 +23,4 @@ pub const COMPONENT_PHP_PDO_ID: i32 = 8003; pub const COMPONENT_PHP_MYSQLI_ID: i32 = 8004; pub const COMPONENT_PHP_PREDIS_ID: i32 = 8006; pub const COMPONENT_PHP_MEMCACHED_ID: i32 = 20; +pub const COMPONENT_PHP_REDIS_ID: i32 = 7; diff --git a/src/plugin/mod.rs b/src/plugin/mod.rs index 916f44d..070185c 100644 --- a/src/plugin/mod.rs +++ b/src/plugin/mod.rs @@ -18,8 +18,8 @@ mod plugin_memcached; mod plugin_mysqli; mod plugin_pdo; mod plugin_predis; -mod plugin_swoole; mod plugin_redis; +mod plugin_swoole; use crate::execute::{AfterExecuteHook, BeforeExecuteHook}; use once_cell::sync::Lazy; @@ -34,6 +34,7 @@ static PLUGINS: Lazy>> = Lazy::new(|| { Box::new(plugin_swoole::SwooleHttpResponsePlugin::default()), Box::new(plugin_predis::PredisPlugin::default()), Box::new(plugin_memcached::MemcachedPlugin::default()), + Box::new(plugin_redis::RedisPlugin::default()), ] }); diff --git a/src/plugin/plugin_redis.rs b/src/plugin/plugin_redis.rs index 63f7276..28ff004 100644 --- a/src/plugin/plugin_redis.rs +++ b/src/plugin/plugin_redis.rs @@ -12,3 +12,176 @@ // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // See the License for the specific language governing permissions and // limitations under the License. + +use super::Plugin; +use crate::execute::{get_this_mut, AfterExecuteHook, BeforeExecuteHook, Noop}; +use anyhow::Context; +use dashmap::DashMap; +use once_cell::sync::Lazy; +use phper::{objects::ZObj, sys, values::ExecuteData}; +use tracing::{debug, warn}; + +static PEER_MAP: Lazy> = Lazy::new(Default::default); +static FREE_MAP: Lazy> = Lazy::new(Default::default); + +#[derive(Default, Clone)] +pub struct RedisPlugin; + +impl Plugin for RedisPlugin { + #[inline] + fn class_names(&self) -> Option<&'static [&'static str]> { + Some(&["Redis"]) + } + + #[inline] + fn function_name_prefix(&self) -> Option<&'static str> { + None + } + + fn hook( + &self, class_name: Option<&str>, function_name: &str, + ) -> Option<(Box, Box)> { + debug!(function_name, "REDIS COMMAND"); + match (class_name, function_name) { + (Some("Redis"), "__construct") => Some(self.hook_redis_construct()), + (Some("Redis"), f) if ["connect", "open", "pconnect", "popen"].contains(&f) => { + Some(self.hook_redis_connect()) + } + (Some("Redis"), f) if ["get", "mget"].contains(&f) => { + Some(self.hook_redis_methods(function_name)) + } + _ => None, + } + } +} + +impl RedisPlugin { + /// TODO Support first optional argument as config for phpredis 6.0+. + /// + fn hook_redis_construct(&self) -> (Box, Box) { + ( + Box::new(|_, execute_data| { + let this = get_this_mut(execute_data)?; + hack_free(this, Some(redis_dtor)); + + Ok(Box::new(())) + }), + Noop::noop(), + ) + } + + fn hook_redis_connect(&self) -> (Box, Box) { + ( + Box::new(|_, execute_data| { + if execute_data.num_args() < 2 { + debug!("argument count less than 2, skipped."); + return Ok(Box::new(())); + } + + let host = { + let mut f = || { + Ok::<_, anyhow::Error>( + execute_data + .get_parameter(0) + .as_z_str() + .context("isn't string")? + .to_str()? + .to_owned(), + ) + }; + match f() { + Ok(host) => host, + Err(err) => { + warn!(?err, "parse first argument to host failed, skipped."); + return Ok(Box::new(())); + } + } + }; + let port = { + let mut f = || { + Ok::<_, anyhow::Error>( + execute_data + .get_parameter(1) + .as_long() + .context("isn't long")?, + ) + }; + match f() { + Ok(port) => port, + Err(err) => { + warn!(?err, "parse second argument to port failed, skipped."); + return Ok(Box::new(())); + } + } + }; + + let this = get_this_mut(execute_data)?; + let addr = format!("{}:{}", host, port); + debug!(addr, "Get redis peer"); + PEER_MAP.insert(this.handle(), Peer { addr }); + + Ok(Box::new(())) + }), + Noop::noop(), + ) + } + + fn hook_redis_methods( + &self, function_name: &str, + ) -> (Box, Box) { + let function_name = function_name.to_owned(); + ( + Box::new(move |request_id, execute_data| { + let handle = get_this_mut(execute_data)?.handle(); + + debug!(handle, function_name, "call redis method"); + + let command = generate_command(&function_name, execute_data)?; + + debug!(handle, function_name, command, "call redis command"); + + Ok(Box::new(())) + }), + Noop::noop(), + ) + } +} + +struct Peer { + addr: String, +} + +fn hack_free(this: &mut ZObj, new_free: sys::zend_object_free_obj_t) { + let handle = this.handle(); + + unsafe { + let ori_free = (*(*this.as_mut_ptr()).handlers).free_obj; + FREE_MAP.insert(handle, ori_free); + (*((*this.as_mut_ptr()).handlers as *mut sys::zend_object_handlers)).free_obj = new_free; + } +} + +unsafe extern "C" fn redis_dtor(object: *mut sys::zend_object) { + debug!("call Redis free"); + + let handle = ZObj::from_ptr(object).handle(); + + PEER_MAP.remove(&handle); + if let Some((_, Some(free))) = FREE_MAP.remove(&handle) { + free(object); + } +} + +fn generate_command(function_name: &str, execute_data: &mut ExecuteData) -> anyhow::Result { + let num_args = execute_data.num_args(); + let mut args = Vec::with_capacity(num_args + 1); + args.push(function_name.to_owned()); + + for i in 0..num_args { + let mut arg = execute_data.get_parameter(i).clone(); + arg.convert_to_string(); + args.push(arg.as_z_str().unwrap().to_str()?.to_string()); + } + + Ok(args.join(" ")) +} From 1b4073c88833493130ab4ce07912f5698c2bb266 Mon Sep 17 00:00:00 2001 From: jmjoy Date: Sun, 6 Nov 2022 17:21:53 +0800 Subject: [PATCH 3/9] Done. --- Cargo.lock | 6 +- Cargo.toml | 1 + src/plugin/plugin_redis.rs | 251 ++++++++++++++++++++++++++++--- src/util.rs | 70 ++++++++- tests/data/expected_context.yaml | 135 ++++++++++++++++- tests/e2e.rs | 15 ++ tests/php/fpm/redis.fail.php | 30 ++++ tests/php/fpm/redis.succ.php | 32 ++++ 8 files changed, 515 insertions(+), 25 deletions(-) create mode 100644 tests/php/fpm/redis.fail.php create mode 100644 tests/php/fpm/redis.succ.php diff --git a/Cargo.lock b/Cargo.lock index 1b08c5b..3000865 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1782,10 +1782,11 @@ dependencies = [ [[package]] name = "serde_json" -version = "1.0.85" +version = "1.0.87" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e55a28e3aaef9d5ce0506d0a14dbba8054ddc7e499ef522dd8b26859ec9d4a44" +checksum = "6ce777b7b150d76b9cf60d28b55f5847135a003f7d7350c6be7a773508ce7d45" dependencies = [ + "indexmap", "itoa", "ryu", "serde", @@ -1885,6 +1886,7 @@ dependencies = [ "phper", "prost", "reqwest", + "serde_json", "skywalking", "systemstat", "tempfile", diff --git a/Cargo.toml b/Cargo.toml index 79875cb..788c3e3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -46,6 +46,7 @@ libc = "0.2.132" once_cell = "1.14.0" phper = "0.5.1" prost = "0.11.0" +serde_json = { version = "1.0.87", features = ["preserve_order"] } skywalking = "0.4.0" systemstat = "0.2.0" tempfile = "3.3.0" diff --git a/src/plugin/plugin_redis.rs b/src/plugin/plugin_redis.rs index 28ff004..a71eaac 100644 --- a/src/plugin/plugin_redis.rs +++ b/src/plugin/plugin_redis.rs @@ -13,17 +13,174 @@ // See the License for the specific language governing permissions and // limitations under the License. +use std::{any::Any, collections::HashSet}; + use super::Plugin; -use crate::execute::{get_this_mut, AfterExecuteHook, BeforeExecuteHook, Noop}; +use crate::{ + component::COMPONENT_PHP_REDIS_ID, + context::RequestContext, + execute::{get_this_mut, AfterExecuteHook, BeforeExecuteHook, Noop}, + util::json_encode_values, +}; use anyhow::Context; use dashmap::DashMap; use once_cell::sync::Lazy; -use phper::{objects::ZObj, sys, values::ExecuteData}; +use phper::{ + eg, + objects::ZObj, + sys, + values::{ExecuteData, ZVal}, +}; +use skywalking::{skywalking_proto::v3::SpanLayer, trace::span::Span}; use tracing::{debug, warn}; static PEER_MAP: Lazy> = Lazy::new(Default::default); + static FREE_MAP: Lazy> = Lazy::new(Default::default); +static REDIS_COMMANDS: Lazy> = Lazy::new(|| { + [ + "ping", + "echo", + "append", + "bitCount", + "bitOp", + "decr", + "decrBy", + "get", + "getBit", + "getRange", + "getSet", + "incr", + "incrBy", + "incrByFloat", + "mGet", + "getMultiple", + "mSet", + "mSetNX", + "set", + "setBit", + "setEx", + "pSetEx", + "setNx", + "setRange", + "strLen", + "del", + "delete", + "unlink", + "dump", + "exists", + "expire", + "setTimeout", + "pexpire", + "expireAt", + "pexpireAt", + "keys", + "getKeys", + "scan", + "migrate", + "move", + "object", + "persist", + "randomKey", + "rename", + "renameKey", + "renameNx", + "type", + "sort", + "ttl", + "pttl", + "restore", + "hDel", + "hExists", + "hGet", + "hGetAll", + "hIncrBy", + "hIncrByFloat", + "hKeys", + "hLen", + "hMGet", + "hMSet", + "hSet", + "hSetNx", + "hVals", + "hScan", + "hStrLen", + "blPop", + "brPop", + "bRPopLPush", + "lIndex", + "lGet", + "lInsert", + "lLen", + "lSize", + "lPop", + "lPush", + "lPushx", + "lRange", + "lGetRange", + "lRem", + "lRemove", + "lSet", + "lTrim", + "listTrim", + "rPop", + "rPopLPush", + "rPush", + "rPushX", + "sAdd", + "sCard", + "sSize", + "sDiff", + "sDiffStore", + "sInter", + "sInterStore", + "sIsMember", + "sContains", + "sMembers", + "sGetMembers", + "sMove", + "sPop", + "sRandMember", + "sRem", + "sRemove", + "sUnion", + "sUnionStore", + "sScan", + "bzPop", + "zAdd", + "zCard", + "zSize", + "zCount", + "zIncrBy", + "zinterstore", + "zInter", + "zPop", + "zRange", + "zRangeByScore", + "zRevRangeByScore", + "zRangeByLex", + "zRank", + "zRevRank", + "zRem", + "zDelete", + "zRemove", + "zRemRangeByRank", + "zDeleteRangeByRank", + "zRemRangeByScore", + "zDeleteRangeByScore", + "zRemoveRangeByScore", + "zRevRange", + "zScore", + "zunionstore", + "zUnion", + "zScan", + ] + .into_iter() + .map(str::to_ascii_lowercase) + .collect() +}); + #[derive(Default, Clone)] pub struct RedisPlugin; @@ -41,14 +198,15 @@ impl Plugin for RedisPlugin { fn hook( &self, class_name: Option<&str>, function_name: &str, ) -> Option<(Box, Box)> { - debug!(function_name, "REDIS COMMAND"); match (class_name, function_name) { (Some("Redis"), "__construct") => Some(self.hook_redis_construct()), - (Some("Redis"), f) if ["connect", "open", "pconnect", "popen"].contains(&f) => { - Some(self.hook_redis_connect()) + (Some(class_name @ "Redis"), f) + if ["connect", "open", "pconnect", "popen"].contains(&f) => + { + Some(self.hook_redis_connect(class_name, function_name)) } - (Some("Redis"), f) if ["get", "mget"].contains(&f) => { - Some(self.hook_redis_methods(function_name)) + (Some(class_name @ "Redis"), f) if REDIS_COMMANDS.contains(&f.to_ascii_lowercase()) => { + Some(self.hook_redis_methods(class_name, function_name)) } _ => None, } @@ -70,9 +228,13 @@ impl RedisPlugin { ) } - fn hook_redis_connect(&self) -> (Box, Box) { + fn hook_redis_connect( + &self, class_name: &str, function_name: &str, + ) -> (Box, Box) { + let class_name = class_name.to_owned(); + let function_name = function_name.to_owned(); ( - Box::new(|_, execute_data| { + Box::new(move |request_id, execute_data| { if execute_data.num_args() < 2 { debug!("argument count less than 2, skipped."); return Ok(Box::new(())); @@ -118,31 +280,50 @@ impl RedisPlugin { let this = get_this_mut(execute_data)?; let addr = format!("{}:{}", host, port); debug!(addr, "Get redis peer"); - PEER_MAP.insert(this.handle(), Peer { addr }); + PEER_MAP.insert(this.handle(), Peer { addr: addr.clone() }); - Ok(Box::new(())) + let span = RequestContext::try_with_global_ctx(request_id, |ctx| { + Ok(ctx.create_exit_span(&format!("{}->{}", class_name, function_name), &addr)) + })?; + + Ok(Box::new(span)) }), - Noop::noop(), + Box::new(after_hook), ) } fn hook_redis_methods( - &self, function_name: &str, + &self, class_name: &str, function_name: &str, ) -> (Box, Box) { + let class_name = class_name.to_owned(); let function_name = function_name.to_owned(); ( Box::new(move |request_id, execute_data| { let handle = get_this_mut(execute_data)?.handle(); - debug!(handle, function_name, "call redis method"); + let peer = PEER_MAP + .get(&handle) + .map(|r| r.value().addr.clone()) + .unwrap_or_default(); let command = generate_command(&function_name, execute_data)?; debug!(handle, function_name, command, "call redis command"); - Ok(Box::new(())) + let mut span = RequestContext::try_with_global_ctx(request_id, |ctx| { + Ok(ctx.create_exit_span(&format!("{}->{}", class_name, function_name), &peer)) + })?; + + span.with_span_object_mut(|span| { + span.set_span_layer(SpanLayer::Cache); + span.component_id = COMPONENT_PHP_REDIS_ID; + span.add_tag("db.type", "redis"); + span.add_tag("redis.command", command); + }); + + Ok(Box::new(span)) }), - Noop::noop(), + Box::new(after_hook), ) } } @@ -175,13 +356,41 @@ unsafe extern "C" fn redis_dtor(object: *mut sys::zend_object) { fn generate_command(function_name: &str, execute_data: &mut ExecuteData) -> anyhow::Result { let num_args = execute_data.num_args(); let mut args = Vec::with_capacity(num_args + 1); - args.push(function_name.to_owned()); + args.push(ZVal::from(function_name)); for i in 0..num_args { - let mut arg = execute_data.get_parameter(i).clone(); - arg.convert_to_string(); - args.push(arg.as_z_str().unwrap().to_str()?.to_string()); + let arg = execute_data.get_parameter(i).clone(); + args.push(arg); + } + + Ok(json_encode_values(&args)?) +} + +fn after_hook( + _request_id: Option, span: Box, _execute_data: &mut ExecuteData, + _return_value: &mut ZVal, +) -> anyhow::Result<()> { + let mut span = span.downcast::().unwrap(); + + let ex = unsafe { ZObj::try_from_mut_ptr(eg!(exception)) }; + if let Some(ex) = ex { + span.with_span_object_mut(|span| { + span.is_error = true; + + let mut logs = Vec::new(); + if let Ok(class_name) = ex.get_class().get_name().to_str() { + logs.push(("Exception Class", class_name.to_owned())); + } + if let Some(message) = ex.get_property("message").as_z_str() { + if let Ok(message) = message.to_str() { + logs.push(("Exception Message", message.to_owned())); + } + } + if !logs.is_empty() { + span.add_log(logs); + } + }); } - Ok(args.join(" ")) + Ok(()) } diff --git a/src/util.rs b/src/util.rs index 460c39f..20d9e44 100644 --- a/src/util.rs +++ b/src/util.rs @@ -16,7 +16,8 @@ use anyhow::bail; use chrono::Local; use once_cell::sync::Lazy; -use phper::{sys, values::ZVal}; +use phper::{arrays::IterKey, sys, values::ZVal}; +use serde_json::{json, Number, Value}; use std::{ ffi::CStr, panic::{catch_unwind, UnwindSafe}, @@ -112,3 +113,70 @@ pub fn catch_unwind_anyhow anyhow::Result + UnwindSafe, R>( pub fn get_sapi_module_name() -> &'static CStr { unsafe { CStr::from_ptr(sys::sapi_module.name) } } + +pub fn json_encode_values(values: &[ZVal]) -> serde_json::Result { + fn add(json_value: &mut Value, key: Option, item: Value) { + match key { + Some(key) => { + json_value.as_object_mut().unwrap().insert(key, item); + } + None => { + json_value.as_array_mut().unwrap().push(item); + } + } + } + + fn handle(json_value: &mut Value, key: Option, val: &ZVal) { + let type_info = val.get_type_info(); + + if type_info.is_null() { + add(json_value, key, Value::Null); + } else if type_info.is_true() { + add(json_value, key, Value::Bool(true)); + } else if type_info.is_false() { + add(json_value, key, Value::Bool(false)); + } else if type_info.is_long() { + let i = val.as_long().unwrap(); + add(json_value, key, Value::Number(i.into())); + } else if type_info.is_double() { + let d = val.as_double().unwrap(); + let n = match Number::from_f64(d) { + Some(n) => Value::Number(n), + None => Value::String("".to_owned()), + }; + add(json_value, key, n); + } else if type_info.is_string() { + let s = val + .as_z_str() + .unwrap() + .to_str() + .map(ToOwned::to_owned) + .unwrap_or_default(); + add(json_value, key, Value::String(s)); + } else if type_info.is_array() { + let arr = val.as_z_arr().unwrap(); + let is_arr = arr.iter().all(|(key, _)| matches!(key, IterKey::Index(_))); + let mut new_json_value = if is_arr { json!([]) } else { json!({}) }; + for (key, new_val) in arr.iter() { + if is_arr { + handle(&mut new_json_value, None, new_val); + } else { + let key = match key { + IterKey::Index(i) => i.to_string(), + IterKey::ZStr(s) => s.to_str().map(ToOwned::to_owned).unwrap_or_default(), + }; + handle(&mut new_json_value, Some(key), new_val); + } + } + add(json_value, key, new_json_value); + } else if type_info.is_object() { + add(json_value, key, Value::String("".to_owned())); + } + } + + let mut json_value = json!([]); + for val in values { + handle(&mut json_value, None, val); + } + serde_json::to_string(&json_value) +} diff --git a/tests/data/expected_context.yaml b/tests/data/expected_context.yaml index f5ca94a..900e490 100644 --- a/tests/data/expected_context.yaml +++ b/tests/data/expected_context.yaml @@ -15,7 +15,7 @@ segmentItems: - serviceName: skywalking-agent-test-1 - segmentSize: 10 + segmentSize: 12 segments: - segmentId: "not null" spans: @@ -681,6 +681,139 @@ segmentItems: - { key: url, value: /memcached.php } - { key: http.method, value: GET } - { key: http.status_code, value: "200" } + - segmentId: "not null" + spans: + - { + operationName: Redis->connect, + parentSpanId: 0, + spanId: 1, + spanLayer: Http, + startTime: gt 0, + endTime: gt 0, + componentId: 11000, + isError: false, + spanType: Exit, + peer: "127.0.0.1:6379", + skipAnalysis: false, + } + - operationName: Redis->mset + parentSpanId: 0 + spanId: 2 + spanLayer: Cache + startTime: gt 0 + endTime: gt 0 + componentId: 7 + isError: false + spanType: Exit + peer: 127.0.0.1:6379 + skipAnalysis: false + tags: + - key: db.type + value: redis + - key: redis.command + value: | + ["mset",{"key0":"value0","key1":"value1"}] + - operationName: Redis->get + parentSpanId: 0 + spanId: 3 + spanLayer: Cache + startTime: gt 0 + endTime: gt 0 + componentId: 7 + isError: false + spanType: Exit + peer: 127.0.0.1:6379 + skipAnalysis: false + tags: + - key: db.type + value: redis + - key: redis.command + value: | + ["get","key0"] + - operationName: Redis->get + parentSpanId: 0 + spanId: 4 + spanLayer: Cache + startTime: gt 0 + endTime: gt 0 + componentId: 7 + isError: false + spanType: Exit + peer: 127.0.0.1:6379 + skipAnalysis: false + tags: + - key: db.type + value: redis + - key: redis.command + value: | + ["get","key1"] + - operationName: GET:/redis.succ.php + parentSpanId: -1 + spanId: 0 + spanLayer: Http + startTime: gt 0 + endTime: gt 0 + componentId: 8001 + isError: false + spanType: Entry + peer: "" + skipAnalysis: false + tags: + - { key: url, value: /redis.succ.php } + - { key: http.method, value: GET } + - { key: http.status_code, value: "200" } + - segmentId: "not null" + spans: + - operationName: Redis->connect + parentSpanId: 0 + spanId: 1 + spanLayer: Http + startTime: gt 0 + endTime: gt 0 + componentId: 11000 + isError: false + spanType: Exit + peer: "127.0.0.1:6379" + skipAnalysis: false + - operationName: Redis->set + parentSpanId: 0 + spanId: 2 + spanLayer: Cache + startTime: gt 0 + endTime: gt 0 + componentId: 7 + isError: true + spanType: Exit + peer: 127.0.0.1:6379 + skipAnalysis: false + tags: + - key: db.type + value: redis + - key: redis.command + value: | + ["set","foo","bar"] + logs: + - logEvent: + - { key: Exception Class, value: RedisException } + - { + key: Exception Message, + value: NOAUTH Authentication required., + } + - operationName: GET:/redis.fail.php + parentSpanId: -1 + spanId: 0 + spanLayer: Http + startTime: gt 0 + endTime: gt 0 + componentId: 8001 + isError: false + spanType: Entry + peer: "" + skipAnalysis: false + tags: + - { key: url, value: /redis.fail.php } + - { key: http.method, value: GET } + - { key: http.status_code, value: "200" } - serviceName: skywalking-agent-test-2 segmentSize: 1 segments: diff --git a/tests/e2e.rs b/tests/e2e.rs index a15671b..bc3f876 100644 --- a/tests/e2e.rs +++ b/tests/e2e.rs @@ -52,6 +52,7 @@ async fn run_e2e() { request_fpm_predis().await; request_fpm_mysqli().await; request_fpm_memcached().await; + request_fpm_redis().await; request_swoole_curl().await; sleep(Duration::from_secs(3)).await; request_collector_validate().await; @@ -97,6 +98,20 @@ async fn request_fpm_memcached() { .await; } +async fn request_fpm_redis() { + request_common( + HTTP_CLIENT.get(format!("http://{}/redis.succ.php", PROXY_SERVER_1_ADDRESS)), + "ok", + ) + .await; + + request_common( + HTTP_CLIENT.get(format!("http://{}/redis.fail.php", PROXY_SERVER_1_ADDRESS)), + "ok", + ) + .await; +} + async fn request_swoole_curl() { request_common( HTTP_CLIENT.get(format!("http://{}/curl", SWOOLE_SERVER_1_ADDRESS)), diff --git a/tests/php/fpm/redis.fail.php b/tests/php/fpm/redis.fail.php new file mode 100644 index 0000000..be6610f --- /dev/null +++ b/tests/php/fpm/redis.fail.php @@ -0,0 +1,30 @@ +connect("127.0.0.1", 6379); + $client->set('foo', 'bar'); +} catch (RedisException $e) { +} + +echo "ok"; diff --git a/tests/php/fpm/redis.succ.php b/tests/php/fpm/redis.succ.php new file mode 100644 index 0000000..1abc743 --- /dev/null +++ b/tests/php/fpm/redis.succ.php @@ -0,0 +1,32 @@ +connect("127.0.0.1", 6379); + $client->auth('password'); + $client->mSet(['key0' => 'value0', 'key1' => 'value1']); + Assert::same($client->get('key0'), 'value0'); + Assert::same($client->get('key1'), 'value1'); +} + +echo "ok"; From 54ac4a1226411be5b749d1aa59f7efe2d8e159db Mon Sep 17 00:00:00 2001 From: jmjoy Date: Sun, 6 Nov 2022 23:26:55 +0800 Subject: [PATCH 4/9] Cllippy. --- src/plugin/plugin_redis.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/plugin/plugin_redis.rs b/src/plugin/plugin_redis.rs index a71eaac..fa28602 100644 --- a/src/plugin/plugin_redis.rs +++ b/src/plugin/plugin_redis.rs @@ -261,12 +261,10 @@ impl RedisPlugin { }; let port = { let mut f = || { - Ok::<_, anyhow::Error>( - execute_data - .get_parameter(1) - .as_long() - .context("isn't long")?, - ) + execute_data + .get_parameter(1) + .as_long() + .context("isn't long") }; match f() { Ok(port) => port, From 56279566b0ce5e8b86cdfe6d315c946ef973a73d Mon Sep 17 00:00:00 2001 From: jmjoy Date: Mon, 7 Nov 2022 00:09:33 +0800 Subject: [PATCH 5/9] CI: Add redis extension. --- .github/workflows/rust.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 0430487..b407ec1 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -85,8 +85,9 @@ jobs: tools: php-config, composer:v2 extensions: > bcmath, calendar, ctype, dom, exif, gettext, iconv, intl, json, mbstring, - mysqli, mysqlnd, opcache, pdo, pdo_mysql, phar, posix, readline, memcached, - swoole-${{ matrix.version.swoole }}, xml, xmlreader, xmlwriter, yaml, zip + mysqli, mysqlnd, opcache, pdo, pdo_mysql, phar, posix, readline, redis, + memcached, swoole-${{ matrix.version.swoole }}, xml, xmlreader, xmlwriter, + yaml, zip - name: Setup php-fpm for Linux if: matrix.os == 'ubuntu-20.04' From 2d022e18a352ec166bf54d295d04493caf3d1b49 Mon Sep 17 00:00:00 2001 From: jmjoy Date: Mon, 7 Nov 2022 09:51:22 +0800 Subject: [PATCH 6/9] Update documnets. --- README.md | 2 +- docs/en/setup/service-agent/php-agent/Supported-list.md | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index cc757df..0d39aae 100644 --- a/README.md +++ b/README.md @@ -29,7 +29,7 @@ SkyWalking PHP Agent requires SkyWalking 8.4+ and PHP 7.2+ * [x] [PDO](https://www.php.net/manual/en/book.pdo.php) * [x] [MySQL Improved](https://www.php.net/manual/en/book.mysqli.php) * [x] [Memcached](https://www.php.net/manual/en/book.memcached.php) - * [ ] [phpredis](https://github.com/phpredis/phpredis) + * [x] [phpredis](https://github.com/phpredis/phpredis) * [ ] [php-amqp](https://github.com/php-amqp/php-amqp) * [ ] [php-rdkafka](https://github.com/arnaud-lb/php-rdkafka) * [x] [predis](https://github.com/predis/predis) diff --git a/docs/en/setup/service-agent/php-agent/Supported-list.md b/docs/en/setup/service-agent/php-agent/Supported-list.md index d4227e7..4e11b15 100644 --- a/docs/en/setup/service-agent/php-agent/Supported-list.md +++ b/docs/en/setup/service-agent/php-agent/Supported-list.md @@ -13,6 +13,7 @@ The following plugins provide the distributed tracing capability. * [PDO](https://www.php.net/manual/en/book.pdo.php) * [MySQL Improved](https://www.php.net/manual/en/book.mysqli.php) * [Memcached](https://www.php.net/manual/en/book.memcached.php) +* [phpredis](https://github.com/phpredis/phpredis) ## Support PHP library From 83499a45e2082c928199bb412e82f68e6f277d4f Mon Sep 17 00:00:00 2001 From: jmjoy Date: Wed, 9 Nov 2022 14:42:03 +0800 Subject: [PATCH 7/9] Fix redis connect span. --- src/plugin/plugin_redis.rs | 8 +++++++- tests/data/expected_context.yaml | 34 ++++++++++++++++++-------------- 2 files changed, 26 insertions(+), 16 deletions(-) diff --git a/src/plugin/plugin_redis.rs b/src/plugin/plugin_redis.rs index fa28602..38e8cdb 100644 --- a/src/plugin/plugin_redis.rs +++ b/src/plugin/plugin_redis.rs @@ -280,10 +280,16 @@ impl RedisPlugin { debug!(addr, "Get redis peer"); PEER_MAP.insert(this.handle(), Peer { addr: addr.clone() }); - let span = RequestContext::try_with_global_ctx(request_id, |ctx| { + let mut span = RequestContext::try_with_global_ctx(request_id, |ctx| { Ok(ctx.create_exit_span(&format!("{}->{}", class_name, function_name), &addr)) })?; + span.with_span_object_mut(|span| { + span.set_span_layer(SpanLayer::Cache); + span.component_id = COMPONENT_PHP_REDIS_ID; + span.add_tag("db.type", "redis"); + }); + Ok(Box::new(span)) }), Box::new(after_hook), diff --git a/tests/data/expected_context.yaml b/tests/data/expected_context.yaml index 900e490..2aab703 100644 --- a/tests/data/expected_context.yaml +++ b/tests/data/expected_context.yaml @@ -683,19 +683,20 @@ segmentItems: - { key: http.status_code, value: "200" } - segmentId: "not null" spans: - - { - operationName: Redis->connect, - parentSpanId: 0, - spanId: 1, - spanLayer: Http, - startTime: gt 0, - endTime: gt 0, - componentId: 11000, - isError: false, - spanType: Exit, - peer: "127.0.0.1:6379", - skipAnalysis: false, - } + - operationName: Redis->connect + parentSpanId: 0 + spanId: 1 + spanLayer: Cache + startTime: gt 0 + endTime: gt 0 + componentId: 7 + isError: false + spanType: Exit + peer: "127.0.0.1:6379" + skipAnalysis: false + tags: + - key: db.type + value: redis - operationName: Redis->mset parentSpanId: 0 spanId: 2 @@ -767,14 +768,17 @@ segmentItems: - operationName: Redis->connect parentSpanId: 0 spanId: 1 - spanLayer: Http + spanLayer: Cache startTime: gt 0 endTime: gt 0 - componentId: 11000 + componentId: 7 isError: false spanType: Exit peer: "127.0.0.1:6379" skipAnalysis: false + tags: + - key: db.type + value: redis - operationName: Redis->set parentSpanId: 0 spanId: 2 From f9db3494fde02f3ea3dfa87c560907ac2b62417f Mon Sep 17 00:00:00 2001 From: jmjoy Date: Wed, 9 Nov 2022 20:08:05 +0800 Subject: [PATCH 8/9] Modify tags ref to virtual cache. --- src/lib.rs | 1 + src/plugin/plugin_redis.rs | 210 ++++++++++++++----------------- src/tag.rs | 25 ++++ src/util.rs | 2 + tests/data/expected_context.yaml | 46 ++++--- 5 files changed, 147 insertions(+), 137 deletions(-) create mode 100644 src/tag.rs diff --git a/src/lib.rs b/src/lib.rs index dd97960..6569149 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -25,6 +25,7 @@ mod execute; mod module; mod plugin; mod request; +mod tag; mod util; mod worker; diff --git a/src/plugin/plugin_redis.rs b/src/plugin/plugin_redis.rs index 38e8cdb..e376d07 100644 --- a/src/plugin/plugin_redis.rs +++ b/src/plugin/plugin_redis.rs @@ -20,7 +20,7 @@ use crate::{ component::COMPONENT_PHP_REDIS_ID, context::RequestContext, execute::{get_this_mut, AfterExecuteHook, BeforeExecuteHook, Noop}, - util::json_encode_values, + tag::{TAG_CACHE_CMD, TAG_CACHE_KEY, TAG_CACHE_OP, TAG_CACHE_TYPE}, }; use anyhow::Context; use dashmap::DashMap; @@ -38,149 +38,120 @@ static PEER_MAP: Lazy> = Lazy::new(Default::default); static FREE_MAP: Lazy> = Lazy::new(Default::default); -static REDIS_COMMANDS: Lazy> = Lazy::new(|| { +static REDIS_READ_COMMANDS: Lazy> = Lazy::new(|| { [ - "ping", - "echo", - "append", - "bitCount", - "bitOp", - "decr", - "decrBy", + "blPop", + "brPop", "get", "getBit", - "getRange", - "getSet", - "incr", - "incrBy", - "incrByFloat", - "mGet", - "getMultiple", - "mSet", - "mSetNX", - "set", - "setBit", - "setEx", - "pSetEx", - "setNx", - "setRange", - "strLen", - "del", - "delete", - "unlink", - "dump", - "exists", - "expire", - "setTimeout", - "pexpire", - "expireAt", - "pexpireAt", - "keys", "getKeys", - "scan", - "migrate", - "move", - "object", - "persist", - "randomKey", - "rename", - "renameKey", - "renameNx", - "type", - "sort", - "ttl", - "pttl", - "restore", - "hDel", + "getMultiple", + "getRange", "hExists", "hGet", "hGetAll", - "hIncrBy", - "hIncrByFloat", "hKeys", "hLen", "hMGet", - "hMSet", - "hSet", - "hSetNx", - "hVals", "hScan", "hStrLen", - "blPop", - "brPop", - "bRPopLPush", - "lIndex", + "hVals", + "keys", "lGet", - "lInsert", + "lGetRange", "lLen", + "lRange", "lSize", - "lPop", + "mGet", + "sContains", + "sGetMembers", + "sIsMember", + "sMembers", + "sScan", + "sSize", + "strLen", + "zCount", + "zRange", + "zRangeByLex", + "zRangeByScore", + "zScan", + "zSize", + ] + .into_iter() + .map(str::to_ascii_lowercase) + .collect() +}); + +static REDIS_WRITE_COMMANDS: Lazy> = Lazy::new(|| { + [ + "append", + "bRPopLPush", + "decr", + "decrBy", + "del", + "delete", + "hDel", + "hIncrBy", + "hIncrByFloat", + "hMSet", + "hSet", + "hSetNx", + "incr", + "incrBy", + "incrByFloat", + "lInsert", "lPush", "lPushx", - "lRange", - "lGetRange", "lRem", "lRemove", "lSet", "lTrim", "listTrim", - "rPop", + "mSet", + "mSetNX", + "pSetEx", "rPopLPush", "rPush", "rPushX", + "randomKey", "sAdd", - "sCard", - "sSize", - "sDiff", - "sDiffStore", "sInter", "sInterStore", - "sIsMember", - "sContains", - "sMembers", - "sGetMembers", "sMove", - "sPop", "sRandMember", "sRem", "sRemove", - "sUnion", - "sUnionStore", - "sScan", - "bzPop", + "set", + "setBit", + "setEx", + "setNx", + "setRange", + "setTimeout", + "sort", + "unlink", "zAdd", - "zCard", - "zSize", - "zCount", + "zDelete", + "zDeleteRangeByRank", + "zDeleteRangeByScore", "zIncrBy", - "zinterstore", - "zInter", - "zPop", - "zRange", - "zRangeByScore", - "zRevRangeByScore", - "zRangeByLex", - "zRank", - "zRevRank", "zRem", - "zDelete", - "zRemove", "zRemRangeByRank", - "zDeleteRangeByRank", "zRemRangeByScore", - "zDeleteRangeByScore", + "zRemove", "zRemoveRangeByScore", - "zRevRange", - "zScore", - "zunionstore", - "zUnion", - "zScan", ] .into_iter() .map(str::to_ascii_lowercase) .collect() }); +static REDIS_ALL_COMMANDS: Lazy> = Lazy::new(|| { + let mut commands = HashSet::new(); + commands.extend(REDIS_READ_COMMANDS.iter().map(Clone::clone)); + commands.extend(REDIS_WRITE_COMMANDS.iter().map(Clone::clone)); + commands +}); + #[derive(Default, Clone)] pub struct RedisPlugin; @@ -205,7 +176,9 @@ impl Plugin for RedisPlugin { { Some(self.hook_redis_connect(class_name, function_name)) } - (Some(class_name @ "Redis"), f) if REDIS_COMMANDS.contains(&f.to_ascii_lowercase()) => { + (Some(class_name @ "Redis"), f) + if REDIS_ALL_COMMANDS.contains(&f.to_ascii_lowercase()) => + { Some(self.hook_redis_methods(class_name, function_name)) } _ => None, @@ -287,7 +260,7 @@ impl RedisPlugin { span.with_span_object_mut(|span| { span.set_span_layer(SpanLayer::Cache); span.component_id = COMPONENT_PHP_REDIS_ID; - span.add_tag("db.type", "redis"); + span.add_tag(TAG_CACHE_TYPE, "redis"); }); Ok(Box::new(span)) @@ -310,9 +283,17 @@ impl RedisPlugin { .map(|r| r.value().addr.clone()) .unwrap_or_default(); - let command = generate_command(&function_name, execute_data)?; + let key = execute_data + .get_parameter(0) + .as_z_str() + .and_then(|s| s.to_str().ok()); + let op = if REDIS_READ_COMMANDS.contains(&function_name.to_ascii_lowercase()) { + "read" + } else { + "write" + }; - debug!(handle, function_name, command, "call redis command"); + debug!(handle, function_name, 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)) @@ -321,8 +302,12 @@ impl RedisPlugin { span.with_span_object_mut(|span| { span.set_span_layer(SpanLayer::Cache); span.component_id = COMPONENT_PHP_REDIS_ID; - span.add_tag("db.type", "redis"); - span.add_tag("redis.command", command); + span.add_tag(TAG_CACHE_TYPE, "redis"); + span.add_tag(TAG_CACHE_CMD, function_name); + span.add_tag(TAG_CACHE_OP, op); + if let Some(key) = key { + span.add_tag(TAG_CACHE_KEY, key) + } }); Ok(Box::new(span)) @@ -357,19 +342,6 @@ unsafe extern "C" fn redis_dtor(object: *mut sys::zend_object) { } } -fn generate_command(function_name: &str, execute_data: &mut ExecuteData) -> anyhow::Result { - let num_args = execute_data.num_args(); - let mut args = Vec::with_capacity(num_args + 1); - args.push(ZVal::from(function_name)); - - for i in 0..num_args { - let arg = execute_data.get_parameter(i).clone(); - args.push(arg); - } - - Ok(json_encode_values(&args)?) -} - fn after_hook( _request_id: Option, span: Box, _execute_data: &mut ExecuteData, _return_value: &mut ZVal, diff --git a/src/tag.rs b/src/tag.rs new file mode 100644 index 0000000..69ca800 --- /dev/null +++ b/src/tag.rs @@ -0,0 +1,25 @@ +// Licensed to the Apache Software Foundation (ASF) under one or more +// contributor license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright ownership. +// The ASF licenses this file to You under the Apache License, Version 2.0 +// (the "License"); you may not use this file except in compliance with +// the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License.. + +//! Tags +//! +//! Virtual Cache +//! +//! https://skywalking.apache.org/docs/main/next/en/setup/service-agent/virtual-cache/ + +pub const TAG_CACHE_TYPE: &str = "cache.type"; +pub const TAG_CACHE_OP: &str = "cache.op"; +pub const TAG_CACHE_CMD: &str = "cache.cmd"; +pub const TAG_CACHE_KEY: &str = "cache.key"; diff --git a/src/util.rs b/src/util.rs index 20d9e44..1643780 100644 --- a/src/util.rs +++ b/src/util.rs @@ -114,6 +114,8 @@ pub fn get_sapi_module_name() -> &'static CStr { unsafe { CStr::from_ptr(sys::sapi_module.name) } } +/// Use for later scene. +#[allow(dead_code)] pub fn json_encode_values(values: &[ZVal]) -> serde_json::Result { fn add(json_value: &mut Value, key: Option, item: Value) { match key { diff --git a/tests/data/expected_context.yaml b/tests/data/expected_context.yaml index 2aab703..9d1dae4 100644 --- a/tests/data/expected_context.yaml +++ b/tests/data/expected_context.yaml @@ -695,7 +695,7 @@ segmentItems: peer: "127.0.0.1:6379" skipAnalysis: false tags: - - key: db.type + - key: cache.type value: redis - operationName: Redis->mset parentSpanId: 0 @@ -709,11 +709,12 @@ segmentItems: peer: 127.0.0.1:6379 skipAnalysis: false tags: - - key: db.type + - key: cache.type value: redis - - key: redis.command - value: | - ["mset",{"key0":"value0","key1":"value1"}] + - key: cache.cmd + value: mset + - key: cache.op + value: write - operationName: Redis->get parentSpanId: 0 spanId: 3 @@ -726,11 +727,14 @@ segmentItems: peer: 127.0.0.1:6379 skipAnalysis: false tags: - - key: db.type + - key: cache.type value: redis - - key: redis.command - value: | - ["get","key0"] + - key: cache.cmd + value: get + - key: cache.key + value: key0 + - key: cache.op + value: read - operationName: Redis->get parentSpanId: 0 spanId: 4 @@ -743,11 +747,14 @@ segmentItems: peer: 127.0.0.1:6379 skipAnalysis: false tags: - - key: db.type + - key: cache.type value: redis - - key: redis.command - value: | - ["get","key1"] + - key: cache.cmd + value: get + - key: cache.key + value: key1 + - key: cache.op + value: read - operationName: GET:/redis.succ.php parentSpanId: -1 spanId: 0 @@ -777,7 +784,7 @@ segmentItems: peer: "127.0.0.1:6379" skipAnalysis: false tags: - - key: db.type + - key: cache.type value: redis - operationName: Redis->set parentSpanId: 0 @@ -791,11 +798,14 @@ segmentItems: peer: 127.0.0.1:6379 skipAnalysis: false tags: - - key: db.type + - key: cache.type value: redis - - key: redis.command - value: | - ["set","foo","bar"] + - key: cache.cmd + value: set + - key: cache.key + value: foo + - key: cache.op + value: write logs: - logEvent: - { key: Exception Class, value: RedisException } From 97222b936ddf9c608ece065d2157d117490be509 Mon Sep 17 00:00:00 2001 From: jmjoy Date: Thu, 10 Nov 2022 10:08:20 +0800 Subject: [PATCH 9/9] Fix CI. --- tests/data/expected_context.yaml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/data/expected_context.yaml b/tests/data/expected_context.yaml index 9d1dae4..f366211 100644 --- a/tests/data/expected_context.yaml +++ b/tests/data/expected_context.yaml @@ -731,10 +731,10 @@ segmentItems: value: redis - key: cache.cmd value: get - - key: cache.key - value: key0 - key: cache.op value: read + - key: cache.key + value: key0 - operationName: Redis->get parentSpanId: 0 spanId: 4 @@ -751,10 +751,10 @@ segmentItems: value: redis - key: cache.cmd value: get - - key: cache.key - value: key1 - key: cache.op value: read + - key: cache.key + value: key1 - operationName: GET:/redis.succ.php parentSpanId: -1 spanId: 0 @@ -802,10 +802,10 @@ segmentItems: value: redis - key: cache.cmd value: set - - key: cache.key - value: foo - key: cache.op value: write + - key: cache.key + value: foo logs: - logEvent: - { key: Exception Class, value: RedisException }