Merge pull request #353 from jeremyandrews/fix-panic

fix panic when `--no-task-metrics` is enabled and metrics are printed
tag1consulting · Aug 25, 2021 · cc96672 · cc96672
2 parents 02bba84 + d2b588d
commit cc96672
Show file tree

Hide file tree

Showing 3 changed files with 64 additions and 35 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,7 @@
 
 ## 0.13.3-dev
  - document GooseConfiguration fields that were only documented as gumpdrop parameters (in order to generate new lines in the help output) so now they're also documented in the code
+ - fix panic when `--no-task-metrics` is enabled and metrics are printed; add tests to prevent further regressions
 
 ## 0.13.2 August 19, 2021
  - fix broken links within the documentation; general documentation cleanups

diff --git a/src/metrics.rs b/src/metrics.rs
@@ -873,44 +873,52 @@ pub struct GooseMetrics {
     pub(crate) display_metrics: bool,
 }
 impl GooseMetrics {
-    /// Initialize the task_metrics vector.
+    /// Initialize the task_metrics vector, and determine which hosts are being
+    /// load tested to display when printing metrics.
     pub(crate) fn initialize_task_metrics(
         &mut self,
         task_sets: &[GooseTaskSet],
         config: &GooseConfiguration,
         defaults: &GooseDefaults,
     ) -> Result<(), GooseError> {
         self.tasks = Vec::new();
-        if !config.no_metrics && !config.no_task_metrics {
-            for task_set in task_sets {
-                let mut task_vector = Vec::new();
-                for task in &task_set.tasks {
-                    task_vector.push(GooseTaskMetricAggregate::new(
-                        task_set.task_sets_index,
-                        &task_set.name,
-                        task.tasks_index,
-                        &task.name,
-                    ));
+        for task_set in task_sets {
+            // Don't initialize task metrics if metrics or task_metrics are disabled.
+            if !config.no_metrics {
+                if !config.no_task_metrics {
+                    let mut task_vector = Vec::new();
+                    for task in &task_set.tasks {
+                        task_vector.push(GooseTaskMetricAggregate::new(
+                            task_set.task_sets_index,
+                            &task_set.name,
+                            task.tasks_index,
+                            &task.name,
+                        ));
+                    }
+                    self.tasks.push(task_vector);
+                }
+
+                // The host is not needed on the Worker, metrics are only printed on
+                // the Manager.
+                if !config.worker {
+                    // Determine the base_url for this task based on which of the following
+                    // are configured so metrics can be printed.
+                    self.hosts.insert(
+                        get_base_url(
+                            // Determine if --host was configured.
+                            if !config.host.is_empty() {
+                                Some(config.host.to_string())
+                            } else {
+                                None
+                            },
+                            // Determine if the task_set defines a host.
+                            task_set.host.clone(),
+                            // Determine if there is a default host.
+                            defaults.host.clone(),
+                        )?
+                        .to_string(),
+                    );
                 }
-                self.tasks.push(task_vector);
-
-                // Determine the base_url for this task based on which of the following
-                // are configured.
-                self.hosts.insert(
-                    get_base_url(
-                        // Determine if --host was configured.
-                        if !config.host.is_empty() {
-                            Some(config.host.to_string())
-                        } else {
-                            None
-                        },
-                        // Determine if the task_set defines a host.
-                        task_set.host.clone(),
-                        // Determine if there is a default host.
-                        defaults.host.clone(),
-                    )?
-                    .to_string(),
-                );
             }
         }
 

diff --git a/tests/defaults.rs b/tests/defaults.rs
@@ -66,7 +66,7 @@ fn setup_mock_server_endpoints(server: &MockServer) -> Vec<Mock> {
 
 // Helper to confirm all variations generate appropriate results.
 fn validate_test(
-    goose_metrics: GooseMetrics,
+    goose_metrics: &GooseMetrics,
     mock_endpoints: &[Mock],
     requests_files: &[String],
     debug_files: &[String],
@@ -204,7 +204,15 @@ fn test_defaults() {
         .execute()
         .unwrap();
 
-    validate_test(goose_metrics, &mock_endpoints, &[request_log], &[debug_log]);
+    validate_test(
+        &goose_metrics,
+        &mock_endpoints,
+        &[request_log],
+        &[debug_log],
+    );
+
+    // Confirm Goose doesn't panic when printing metrics.
+    goose_metrics.print();
 }
 
 #[test]
@@ -332,7 +340,10 @@ fn test_defaults_gaggle() {
         let file = debug_log.to_string() + &i.to_string();
         debug_logs.push(file);
     }
-    validate_test(goose_metrics, &mock_endpoints, &request_logs, &debug_logs);
+    validate_test(&goose_metrics, &mock_endpoints, &request_logs, &debug_logs);
+
+    // Confirm Goose doesn't panic when printing metrics.
+    goose_metrics.print();
 }
 
 #[test]
@@ -387,11 +398,14 @@ fn test_no_defaults() {
         .unwrap();
 
     validate_test(
-        goose_metrics,
+        &goose_metrics,
         &mock_endpoints,
         &[requests_file],
         &[debug_file],
     );
+
+    // Confirm Goose doesn't panic when printing metrics.
+    goose_metrics.print();
 }
 
 #[test]
@@ -501,11 +515,14 @@ fn test_no_defaults_gaggle() {
         debug_files.push(file);
     }
     validate_test(
-        goose_metrics,
+        &goose_metrics,
         &mock_endpoints,
         &requests_files,
         &debug_files,
     );
+
+    // Confirm Goose doesn't panic when printing metrics.
+    goose_metrics.print();
 }
 
 #[test]
@@ -548,4 +565,7 @@ fn test_defaults_no_metrics() {
     assert!(goose_metrics.tasks.is_empty());
     assert!(goose_metrics.users == USERS);
     assert!(goose_metrics.duration == RUN_TIME);
+
+    // Confirm Goose doesn't panic when printing empty metrics.
+    goose_metrics.print();
 }