From 7c7fbe9b3cf8b271de8acc6e41b1f315d332cb6a Mon Sep 17 00:00:00 2001 From: Alec Smrekar Date: Fri, 19 Jan 2024 19:08:58 +0100 Subject: [PATCH] Issue 222: Allow naming of tasks without naming requests --- src/goose.rs | 100 +++++++++++++++++++++++++++++++++++-------------- src/lib.rs | 21 ++++++----- src/logger.rs | 2 +- src/metrics.rs | 33 +++++++++------- src/user.rs | 28 +++++++------- 5 files changed, 117 insertions(+), 67 deletions(-) diff --git a/src/goose.rs b/src/goose.rs index 5f951d5f..8f351e69 100644 --- a/src/goose.rs +++ b/src/goose.rs @@ -555,7 +555,11 @@ impl Scenario { /// } /// ``` pub fn register_transaction(mut self, mut transaction: Transaction) -> Self { - trace!("{} register_transaction: {}", self.name, transaction.name); + trace!( + "{} register_transaction: {}", + self.name, + transaction.name.name_for_transaction() + ); transaction.transactions_index = self.transactions.len(); self.transactions.push(transaction); self @@ -830,7 +834,7 @@ pub struct GooseUser { /// An optional index into [`Scenario`]`.transaction`, indicating which transaction this is. pub(crate) transaction_index: Option, /// Current transaction name, if set. - pub(crate) transaction_name: Option, + pub(crate) transaction_name: Option, /// Client used to make requests, managing sessions and cookies. pub client: Client, /// The base URL to prepend to all relative paths. @@ -1562,15 +1566,15 @@ impl GooseUser { .map_or_else(|| "", |v| v.as_ref()), transaction_name: self .transaction_name - .as_ref() - .map_or_else(|| "", |v| v.as_ref()), + .clone() + .unwrap_or(TransactionName::default_value()), }; // Record information about the request. let mut request_metric = GooseRequestMetric::new( raw_request, transaction_detail, - request_name, + request_name.as_str(), self.started.elapsed().as_millis(), self.weighted_users_index, ); @@ -1628,7 +1632,7 @@ impl GooseUser { warn!("{:?}: {}", &path, e); request_metric.success = false; request_metric.set_status_code(None); - request_metric.error = clean_reqwest_error(e, request_name); + request_metric.error = clean_reqwest_error(e, request_name.as_str()); } }; @@ -1763,7 +1767,10 @@ impl GooseUser { && request_metric.response_time > self.request_cadence.user_cadence { let transaction_name = if let Some(transaction_name) = &self.transaction_name { - format!(", transaction name: \"{}\"", transaction_name) + format!( + ", transaction name: \"{}\"", + transaction_name.name_for_transaction() + ) } else { "".to_string() }; @@ -1826,18 +1833,19 @@ impl GooseUser { /// If `request_name` is set, unwrap and use this. Otherwise, if the Transaction has a name /// set use it. Otherwise use the path. - fn get_request_name<'a>(&'a self, request: &'a GooseRequest) -> &'a str { + fn get_request_name(&self, request: &GooseRequest) -> String { match request.name { // If a request.name is set, unwrap and return it. - Some(rn) => rn, + Some(rn) => rn.to_string(), None => { // Otherwise determine if the current Transaction is named, and if so return it. if let Some(transaction_name) = &self.transaction_name { - transaction_name - } else { - // Otherwise return a copy of the the path. - request.path + let request_name = transaction_name.name_for_request(); + if !request_name.is_empty() { + return request_name; + } } + request.path.to_string() } } } @@ -2722,6 +2730,30 @@ pub type TransactionFunction = Arc< + Sync, >; +#[derive(Clone, Deserialize, Serialize, Hash, PartialEq, Eq, Debug)] +pub enum TransactionName { + InheritNameByRequests(String), + TransactionOnly(String), +} + +impl TransactionName { + pub fn name_for_transaction(&self) -> String { + match self { + TransactionName::InheritNameByRequests(v) => v.to_string(), + TransactionName::TransactionOnly(v) => v.to_string(), + } + } + pub fn name_for_request(&self) -> String { + match self { + TransactionName::InheritNameByRequests(v) => v.to_string(), + TransactionName::TransactionOnly(_) => "".to_string(), + } + } + pub fn default_value() -> Self { + Self::InheritNameByRequests("".to_string()) + } +} + /// An individual transaction within a [`Scenario`](./struct.Scenario.html). #[derive(Clone)] pub struct Transaction { @@ -2729,7 +2761,7 @@ pub struct Transaction { /// transaction this is. pub transactions_index: usize, /// An optional name for the transaction, used when displaying metrics. - pub name: String, + pub name: TransactionName, /// An integer value that controls the frequency that this transaction will be run. pub weight: usize, /// An integer value that controls when this transaction runs compared to other transactions in the same @@ -2747,7 +2779,7 @@ impl Transaction { trace!("new transaction"); Transaction { transactions_index: usize::max_value(), - name: "".to_string(), + name: TransactionName::default_value(), weight: 1, sequence: 0, on_start: false, @@ -2774,8 +2806,18 @@ impl Transaction { /// } /// ``` pub fn set_name(mut self, name: &str) -> Self { - trace!("[{}] set_name: {}", self.transactions_index, self.name); - self.name = name.to_string(); + trace!("[{}] set_name: {}", self.transactions_index, name); + self.name = TransactionName::InheritNameByRequests(name.to_string()); + self + } + + pub fn set_name_transaction_only(mut self, name: &str) -> Self { + trace!( + "[{}] set_name (for transaction only): {}", + self.transactions_index, + name + ); + self.name = TransactionName::TransactionOnly(name.to_string()); self } @@ -2804,7 +2846,7 @@ impl Transaction { pub fn set_on_start(mut self) -> Self { trace!( "{} [{}] set_on_start transaction", - self.name, + self.name.name_for_transaction(), self.transactions_index ); self.on_start = true; @@ -2836,7 +2878,7 @@ impl Transaction { pub fn set_on_stop(mut self) -> Self { trace!( "{} [{}] set_on_stop transaction", - self.name, + self.name.name_for_transaction(), self.transactions_index ); self.on_stop = true; @@ -2867,7 +2909,7 @@ impl Transaction { pub fn set_weight(mut self, weight: usize) -> Result { trace!( "{} [{}] set_weight: {}", - self.name, + self.name.name_for_transaction(), self.transactions_index, weight ); @@ -2955,14 +2997,14 @@ impl Transaction { pub fn set_sequence(mut self, sequence: usize) -> Self { trace!( "{} [{}] set_sequence: {}", - self.name, + self.name.name_for_transaction(), self.transactions_index, sequence ); if sequence < 1 { info!( "setting sequence to 0 for transaction {} is unnecessary, sequence disabled", - self.name + self.name.name_for_transaction() ); } self.sequence = sequence; @@ -3114,7 +3156,7 @@ mod tests { // Initialize scenario. let mut transaction = transaction!(test_function_a); assert_eq!(transaction.transactions_index, usize::max_value()); - assert_eq!(transaction.name, "".to_string()); + assert_eq!(transaction.name.name_for_transaction(), "".to_string()); assert_eq!(transaction.weight, 1); assert_eq!(transaction.sequence, 0); assert!(!transaction.on_start); @@ -3122,7 +3164,7 @@ mod tests { // Name can be set, without affecting other fields. transaction = transaction.set_name("foo"); - assert_eq!(transaction.name, "foo".to_string()); + assert_eq!(transaction.name.name_for_transaction(), "foo".to_string()); assert_eq!(transaction.weight, 1); assert_eq!(transaction.sequence, 0); assert!(!transaction.on_start); @@ -3130,12 +3172,12 @@ mod tests { // Name can be set multiple times. transaction = transaction.set_name("bar"); - assert_eq!(transaction.name, "bar".to_string()); + assert_eq!(transaction.name.name_for_transaction(), "bar".to_string()); // On start flag can be set, without affecting other fields. transaction = transaction.set_on_start(); assert!(transaction.on_start); - assert_eq!(transaction.name, "bar".to_string()); + assert_eq!(transaction.name.name_for_transaction(), "bar".to_string()); assert_eq!(transaction.weight, 1); assert_eq!(transaction.sequence, 0); assert!(!transaction.on_stop); @@ -3149,7 +3191,7 @@ mod tests { transaction = transaction.set_on_stop(); assert!(transaction.on_stop); assert!(transaction.on_start); - assert_eq!(transaction.name, "bar".to_string()); + assert_eq!(transaction.name.name_for_transaction(), "bar".to_string()); assert_eq!(transaction.weight, 1); assert_eq!(transaction.sequence, 0); @@ -3162,7 +3204,7 @@ mod tests { assert_eq!(transaction.weight, 2); assert!(transaction.on_stop); assert!(transaction.on_start); - assert_eq!(transaction.name, "bar".to_string()); + assert_eq!(transaction.name.name_for_transaction(), "bar".to_string()); assert_eq!(transaction.sequence, 0); // Weight field can be changed multiple times. @@ -3175,7 +3217,7 @@ mod tests { assert_eq!(transaction.weight, 3); assert!(transaction.on_stop); assert!(transaction.on_start); - assert_eq!(transaction.name, "bar".to_string()); + assert_eq!(transaction.name.name_for_transaction(), "bar".to_string()); // Sequence field can be changed multiple times. transaction = transaction.set_sequence(8); diff --git a/src/lib.rs b/src/lib.rs index 437655da..0f20b42b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -65,7 +65,7 @@ use tokio::fs::File; use crate::config::{GooseConfiguration, GooseDefaults}; use crate::controller::{ControllerProtocol, ControllerRequest}; -use crate::goose::{GooseUser, GooseUserCommand, Scenario, Transaction}; +use crate::goose::{GooseUser, GooseUserCommand, Scenario, Transaction, TransactionName}; use crate::graph::GraphData; use crate::logger::{GooseLoggerJoinHandle, GooseLoggerTx}; use crate::metrics::{GooseMetric, GooseMetrics}; @@ -86,7 +86,7 @@ lazy_static! { } /// Internal representation of a weighted transaction list. -type WeightedTransactions = Vec<(usize, String)>; +type WeightedTransactions = Vec<(usize, TransactionName)>; /// Internal representation of unsequenced transactions. type UnsequencedTransactions = Vec; @@ -884,7 +884,8 @@ impl GooseAttack { for transaction in scenario.transactions { println!( " o {} (weight: {})", - transaction.name, transaction.weight + transaction.name.name_for_transaction(), + transaction.weight ); } } @@ -2007,19 +2008,19 @@ fn allocate_transactions( for transaction in scheduled_sequenced_on_start_transactions.iter() { on_start_transactions.extend(vec![( *transaction, - scenario.transactions[*transaction].name.to_string(), + scenario.transactions[*transaction].name.clone(), )]) } for transaction in scheduled_sequenced_transactions.iter() { transactions.extend(vec![( *transaction, - scenario.transactions[*transaction].name.to_string(), + scenario.transactions[*transaction].name.clone(), )]) } for transaction in scheduled_sequenced_on_stop_transactions.iter() { on_stop_transactions.extend(vec![( *transaction, - scenario.transactions[*transaction].name.to_string(), + scenario.transactions[*transaction].name.clone(), )]) } @@ -2027,19 +2028,19 @@ fn allocate_transactions( for transaction in scheduled_unsequenced_on_start_transactions.iter() { on_start_transactions.extend(vec![( *transaction, - scenario.transactions[*transaction].name.to_string(), + scenario.transactions[*transaction].name.clone(), )]) } for transaction in scheduled_unsequenced_transactions.iter() { transactions.extend(vec![( *transaction, - scenario.transactions[*transaction].name.to_string(), + scenario.transactions[*transaction].name.clone(), )]) } for transaction in scheduled_unsequenced_on_stop_transactions.iter() { on_stop_transactions.extend(vec![( *transaction, - scenario.transactions[*transaction].name.to_string(), + scenario.transactions[*transaction].name.clone(), )]) } @@ -2061,7 +2062,7 @@ fn weight_unsequenced_transactions( trace!( "{}: {} has weight of {} (reduced with gcd to {})", transaction.transactions_index, - transaction.name, + transaction.name.name_for_transaction(), transaction.weight, weight ); diff --git a/src/logger.rs b/src/logger.rs index 60e5d61a..9adbd4f6 100644 --- a/src/logger.rs +++ b/src/logger.rs @@ -386,7 +386,7 @@ impl GooseLogger for GooseConfiguration { message.scenario_index, message.scenario_name, message.transaction_index, - message.transaction_name, + message.transaction_name.name_for_request(), format!("{:?}", message.raw), message.name, message.final_url, diff --git a/src/metrics.rs b/src/metrics.rs index 86cd5e6a..13ae9261 100644 --- a/src/metrics.rs +++ b/src/metrics.rs @@ -9,7 +9,7 @@ //! [`GooseErrorMetrics`] are displayed in tables. use crate::config::GooseDefaults; -use crate::goose::{get_base_url, GooseMethod, Scenario}; +use crate::goose::{get_base_url, GooseMethod, Scenario, TransactionName}; use crate::logger::GooseLog; use crate::report; use crate::test_plan::{TestPlanHistory, TestPlanStepAction}; @@ -324,7 +324,7 @@ pub struct TransactionDetail<'a> { /// An optional index into [`Scenario`]`.transaction`, indicating which transaction this is. pub transaction_index: &'a str, /// An optional name for the transaction. - pub transaction_name: &'a str, + pub transaction_name: TransactionName, } /// How many milliseconds the load test has been running. @@ -347,7 +347,7 @@ pub struct GooseRequestMetric { /// Stored as string, `""` is no transaction, while `0` is the first `Scenario.transaction`. pub transaction_index: String, /// The optional transaction name. - pub transaction_name: String, + pub transaction_name: TransactionName, /// The raw request that the GooseClient made. pub raw: GooseRawRequest, /// The optional name of the request. @@ -389,7 +389,7 @@ impl GooseRequestMetric { scenario_index: transaction_detail.scenario_index, scenario_name: transaction_detail.scenario_name.to_string(), transaction_index: transaction_detail.transaction_index.to_string(), - transaction_name: transaction_detail.transaction_name.to_string(), + transaction_name: transaction_detail.transaction_name, raw, name: name.to_string(), final_url: "".to_string(), @@ -738,7 +738,7 @@ pub struct TransactionMetricAggregate { /// indicating which transaction this is. pub transaction_index: usize, /// An optional name for the transaction. - pub transaction_name: String, + pub transaction_name: TransactionName, /// Per-run-time counters, tracking how often transactions take a given time to complete. pub times: BTreeMap, /// The shortest run-time for this transaction. @@ -760,13 +760,13 @@ impl TransactionMetricAggregate { scenario_index: usize, scenario_name: &str, transaction_index: usize, - transaction_name: &str, + transaction_name: TransactionName, ) -> Self { TransactionMetricAggregate { scenario_index, scenario_name: scenario_name.to_string(), transaction_index, - transaction_name: transaction_name.to_string(), + transaction_name, times: BTreeMap::new(), min_time: 0, max_time: 0, @@ -1090,7 +1090,7 @@ impl GooseMetrics { scenario.scenarios_index, &scenario.name, transaction.transactions_index, - &transaction.name, + transaction.name.clone(), )); } self.transactions.push(transaction_vector); @@ -1357,7 +1357,7 @@ impl GooseMetrics { &format!( " {}: {}", transaction.transaction_index + 1, - transaction.transaction_name + transaction.transaction_name.name_for_transaction() ), 24 ), @@ -1381,7 +1381,7 @@ impl GooseMetrics { &format!( " {}: {}", transaction.transaction_index + 1, - transaction.transaction_name + transaction.transaction_name.name_for_transaction() ), 24 ), @@ -1534,7 +1534,7 @@ impl GooseMetrics { &format!( " {}: {}", transaction.transaction_index + 1, - transaction.transaction_name + transaction.transaction_name.name_for_transaction() ), 24 ), @@ -3351,7 +3351,7 @@ impl GooseAttack { transaction_metrics.push(report::TransactionMetric { is_scenario: false, transaction: format!("{}.{}", scenario_counter, transaction_counter), - name: transaction.transaction_name.to_string(), + name: transaction.transaction_name.name_for_transaction(), number_of_requests: total_run_count, number_of_failures: transaction.fail_count, response_time_average: format!("{:.2}", average), @@ -3844,7 +3844,9 @@ mod test { scenario_index: 0, scenario_name: "LoadTestUser", transaction_index: 5.to_string().as_str(), - transaction_name: "front page", + transaction_name: TransactionName::InheritNameByRequests( + "front page".to_string().to_string(), + ), }, "/", 0, @@ -3854,7 +3856,10 @@ mod test { assert_eq!(request_metric.scenario_index, 0); assert_eq!(request_metric.scenario_name, "LoadTestUser"); assert_eq!(request_metric.transaction_index, "5"); - assert_eq!(request_metric.transaction_name, "front page"); + assert_eq!( + request_metric.transaction_name.name_for_transaction(), + "front page".to_string() + ); assert_eq!(request_metric.raw.url, PATH.to_string()); assert_eq!(request_metric.name, "/".to_string()); assert_eq!(request_metric.response_time, 0); diff --git a/src/user.rs b/src/user.rs index 2218dc0c..9cae9c09 100644 --- a/src/user.rs +++ b/src/user.rs @@ -1,7 +1,7 @@ use rand::Rng; use std::time::{self, Duration}; -use crate::goose::{GooseUser, GooseUserCommand, Scenario, TransactionFunction}; +use crate::goose::{GooseUser, GooseUserCommand, Scenario, TransactionFunction, TransactionName}; use crate::logger::GooseLog; use crate::metrics::{GooseMetric, ScenarioMetric, TransactionMetric}; @@ -26,7 +26,9 @@ pub(crate) async fn user_main( let function = &thread_scenario.transactions[*thread_transaction_index].function; debug!( "[user {}]: launching on_start {} transaction from {}", - thread_number, thread_transaction_name, thread_scenario.name + thread_number, + thread_transaction_name.name_for_transaction(), + thread_scenario.name ); // Invoke the transaction function. let _todo = invoke_transaction_function( @@ -54,7 +56,9 @@ pub(crate) async fn user_main( let function = &thread_scenario.transactions[*thread_transaction_index].function; debug!( "[user {}]: launching {} transaction from {}", - thread_number, thread_transaction_name, thread_scenario.name + thread_number, + thread_transaction_name.name_for_transaction(), + thread_scenario.name ); // Invoke the transaction function. let _todo = invoke_transaction_function( @@ -154,7 +158,9 @@ pub(crate) async fn user_main( let function = &thread_scenario.transactions[*thread_transaction_index].function; debug!( "[user: {}]: launching on_stop {} transaction from {}", - thread_number, thread_transaction_name, thread_scenario.name + thread_number, + thread_transaction_name.name_for_transaction(), + thread_scenario.name ); // Invoke the transaction function. let _todo = invoke_transaction_function( @@ -226,24 +232,20 @@ async fn invoke_transaction_function( function: &TransactionFunction, thread_user: &mut GooseUser, thread_transaction_index: usize, - thread_transaction_name: &str, + thread_transaction_name: &TransactionName, ) -> Result<(), flume::SendError>> { let started = time::Instant::now(); let mut raw_transaction = TransactionMetric::new( thread_user.started.elapsed().as_millis(), thread_user.scenarios_index, thread_transaction_index, - thread_transaction_name.to_string(), + thread_transaction_name.name_for_transaction(), thread_user.weighted_users_index, ); // Store details about the currently running transaction for logging purposes. - if !thread_transaction_name.is_empty() { - thread_user - .transaction_name - .replace(thread_transaction_name.to_string()); - } else { - thread_user.transaction_name.take(); - } + thread_user + .transaction_name + .replace(thread_transaction_name.clone()); // As the index is optional, `""` is none, whereas `"0"` is the first index. thread_user.transaction_index = Some(thread_transaction_index.to_string());