Response Time Breakdowns by HTTP Status Code

[jeremyandrews]

Summary

This PR implements response time breakdowns grouped by HTTP status code in Goose's CLI and HTML/PDF reports, providing valuable performance insights for load testing analysis.

Closes #528

What's New

When a request returns multiple different HTTP status codes during a load test, Goose now automatically provides response time breakdowns grouped by status code. This feature uses smart omission - breakdowns only appear when multiple status codes exist, preventing unnecessary clutter.

CLI Output Example

GET static asset         |        5.11 |          2 |          38 |          5
  └─ 200 (94.2%)         |        4.98 |          2 |          35 |          5
  └─ 404 (5.8%)          |        7.24 |          5 |          38 |          7

This breakdown reveals that 404 responses took longer on average (7.24ms) than successful 200 responses (4.98ms), helping identify performance patterns related to error handling.

HTML Report Integration

The feature is fully integrated into HTML and PDF reports with professional formatting:

• Clean visual hierarchy with proper indentation
• Tree-like formatting clearly indicates sub-metrics
• Works seamlessly with existing report generation

Technical Implementation

• Zero configuration: Works automatically when status code tracking is enabled
• Smart omission: Only shows breakdowns for requests with multiple status codes
• Memory efficient: Leverages existing data structures
• Backward compatible: No breaking changes to existing functionality
• Comprehensive test coverage: New integration tests ensure reliability

Files Modified

• src/metrics.rs: Core tracking infrastructure
• src/metrics/common.rs: HTML report generation
• src/report.rs: Professional HTML formatting
• tests/status_code_response_times.rs: Comprehensive test coverage
• src/docs/goose-book/src/getting-started/metrics.md: Updated documentation

Benefits

• Performance insights: Identify response time differences between success/error responses
• Error analysis: Spot performance patterns in error handling
• Optimization targeting: Focus efforts on specific response types
• Professional reporting: Clean HTML/PDF output suitable for stakeholders

[github-actions]

📋 Review Summary

This PR introduces a valuable feature for analyzing load test results by breaking down response times by status code. The implementation is well-tested and the documentation is clear.

🔍 General Feedback

• The code is generally well-written and the new feature is a great addition.
• The integration tests are comprehensive and cover various scenarios.
• I've left a few minor suggestions to improve code maintainability and robustness.

Great work on this feature!

[LionsAd]

PR Review: Response Time Breakdowns by HTTP Status Code (#663)

🎯 Core Assessment

This is a valuable feature with a solid implementation that provides real diagnostic benefits. The concerns raised are legitimate but represent different levels of priority and business tradeoffs.

📋 Recommendations by Category

🔧 Technical Improvements (If Time Permits)

Memory Optimization

• Problem: Storing duplicate timing data per status code
• Options:
  • Add config flag to disable feature for high-RPS scenarios
  • Use aggregate stats only (min/max/count) instead of full vectors
  • Document the memory tradeoff in user docs
• Business Reality: Most load tests have reasonable request volumes where this won't matter

API Design Consideration

• Problem: Adding public field to public struct
• Options:
  • Make field pub(crate) with accessor methods
  • Accept as minor version bump (common in ecosystem)
  • Add #[non_exhaustive] for future flexibility
• Business Reality: Many Rust crates evolve public structs this way

🧪 Test Reliability (Quick Wins)

Mock Determinism

• Problem: httpmock behavior isn't guaranteed to hit all mocks
• Solution: Use stateful mocking or explicit ratios
• Effort: Low - just restructure existing test setup

Edge Case Safety

• Problem: Empty vectors in median calculation
• Solution: Add guard clauses or verify util::median handles empty input
• Effort: Very low - add a few safety checks

🎨 Polish (Nice to Have)

HTML Accessibility

• Problem: Hardcoded background color
• Solution: Use CSS variables or classes
• Business Reality: Current approach works fine, this is enhancement

Code Style

• Problem: Clippy warnings
• Solution: Run cargo clippy --fix
• Effort: Literally automatic

(Powered by Claude Code)

[LionsAd]

Here is the clippy diff - it must be my version, which is still overly pedantic, but I also find it easier to read:

diff --git i/src/metrics/common.rs w/src/metrics/common.rs
index 57abe1f..cf494ad 100644
--- i/src/metrics/common.rs
+++ w/src/metrics/common.rs
@@ -116,7 +116,7 @@ pub fn prepare_data(options: ReportOptions, metrics: &GooseMetrics) -> ReportDat
                     };
 
                     raw_response_metrics.push(report::get_response_metric(
-                        &format!("└─ {} ({:.1}%)", status_code, percentage),
+                        &format!("└─ {status_code} ({percentage:.1}%)"),
                         &name,
                         &timing_data.times,
                         timing_data.counter,
diff --git i/tests/error.rs w/tests/error.rs
index 5004f48..6d54a0b 100644
--- i/tests/error.rs
+++ w/tests/error.rs
@@ -277,21 +277,21 @@ async fn test_custom_error() {
     // Test 1: Create a custom error directly
     let custom_error = TransactionError::Custom("Direct custom error".to_string());
     assert_eq!(
-        format!("{}", custom_error),
+        format!("{custom_error}"),
         "TransactionError: custom error (Direct custom error)"
     );
 
     // Test 2: Use From<String> implementation
     let string_error: TransactionError = "String conversion error".to_string().into();
     assert_eq!(
-        format!("{}", string_error),
+        format!("{string_error}"),
         "TransactionError: custom error (String conversion error)"
     );
 
     // Test 3: Use From<String> for Box<TransactionError>
     let boxed_error: Box<TransactionError> = "Boxed string error".to_string().into();
     assert_eq!(
-        format!("{}", boxed_error),
+        format!("{boxed_error}"),
         "TransactionError: custom error (Boxed string error)"
     );
 
@@ -305,7 +305,7 @@ async fn test_custom_error() {
         Ok(_) => panic!("Expected custom error, got success"),
         Err(e) => {
             assert_eq!(
-                format!("{}", e),
+                format!("{e}"),
                 "TransactionError: custom error (This is a custom transaction error)"
             );
         }
diff --git i/tests/status_code_response_times.rs w/tests/status_code_response_times.rs
index ea09fec..788c83b 100644
--- i/tests/status_code_response_times.rs
+++ w/tests/status_code_response_times.rs
@@ -95,7 +95,7 @@ fn validate_status_code_response_times(goose_metrics: &GooseMetrics, mock_endpoi
     // Test the /success endpoint - should only have 200 status codes
     let success_request = goose_metrics
         .requests
-        .get(&format!("GET {}", SUCCESS_PATH))
+        .get(&format!("GET {SUCCESS_PATH}"))
         .expect("Success request should exist");
 
     // Should have status code counts
@@ -129,7 +129,7 @@ fn validate_status_code_response_times(goose_metrics: &GooseMetrics, mock_endpoi
     // Test the /mixed endpoint - should have both 200 and 400 status codes
     let mixed_request = goose_metrics
         .requests
-        .get(&format!("GET {}", MIXED_PATH))
+        .get(&format!("GET {MIXED_PATH}"))
         .expect("Mixed request should exist");
 
     // Should have status code counts for both 200 and 400
@@ -151,7 +151,7 @@ fn validate_status_code_response_times(goose_metrics: &GooseMetrics, mock_endpoi
     // Test the /error endpoint - should only have 400 status codes
     let error_request = goose_metrics
         .requests
-        .get(&format!("GET {}", ERROR_PATH))
+        .get(&format!("GET {ERROR_PATH}"))
         .expect("Error request should exist");
 
     // Should have status code counts

[LionsAd]

RTBM - with some concerns raised by the code review

I leave it to your discretion if you want to implement them or not or want to create follow-up issues.

[jeremyandrews]

Here is the clippy diff - it must be my version, which is still overly pedantic, but I also find it easier to read:

LIkely you need to update your local version of Rust and related tools. https://github.com/rust-lang/rust-clippy/blob/master/CHANGELOG.md The uninlined_format_args lint was enabled by default in our configuration for a while, but then re-disabled. While we could force re-enable it, I'd prefer to wait for it to be a clippy default.