Skip to content
This repository has been archived by the owner on May 3, 2024. It is now read-only.

feat(metrics): response time summary #1026

Merged
merged 6 commits into from
Jun 21, 2023
Merged

Conversation

JAdshead
Copy link
Contributor

Description

This adds metrics for tracking response time.
Includes a fix for configureRequestLog which was silently throwing. Required to enable this feature.

# HELP oneapp_route_response_time_seconds Time taken by one-app to send response
# TYPE oneapp_route_response_time_seconds summary
oneapp_route_response_time_seconds{quantile="0.01"} 0.00050399655
oneapp_route_response_time_seconds{quantile="0.05"} 0.0005324120999999999
oneapp_route_response_time_seconds{quantile="0.5"} 0.0006485346015810277
oneapp_route_response_time_seconds{quantile="0.9"} 1.5966553650915032
oneapp_route_response_time_seconds{quantile="0.95"} 1.651541715142857
oneapp_route_response_time_seconds{quantile="0.99"} 1.6829195575302083
oneapp_route_response_time_seconds{quantile="0.999"} 2.4841202639223816
oneapp_route_response_time_seconds_sum 1149.460227097001
oneapp_route_response_time_seconds_count 3495

Motivation and Context

Provide metrics to summarize the total time it takes for the one-app server to send response back to client.

How Has This Been Tested?

Locally and unit tests

Types of Changes

  • [] Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation (adding or updating documentation)
  • Dependency update
  • Security update

Checklist:

  • My change requires a change to the documentation and I have updated the documentation accordingly.
  • These changes should be applied to a maintenance branch.
  • This change requires cross browser checks.
  • Performance tests should be ran against the server prior to merging.
  • This change impacts caching for client browsers.
  • This change impacts HTTP headers.
  • This change adds additional environment variable requirements for One App users.
  • I have added the Apache 2.0 license header to any new files created.

What is the Impact to Developers Using One App?

@github-actions
Copy link
Contributor

github-actions bot commented Jun 16, 2023

Size Change: 0 B

Total Size: 687 kB

ℹ️ View Unchanged
Filename Size
./build/app/app.js 164 kB
./build/app/app~vendors.js 389 kB
./build/app/runtime.js 7.07 kB
./build/app/service-worker-client.js 7.26 kB
./build/app/vendors.js 120 kB

compressed-size-action

@@ -1578,9 +1578,6 @@ describe('Tests that require Docker setup', () => {
date: [
expect.any(String),
],
'expect-ct': [
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JAdshead JAdshead requested a review from a team June 20, 2023 17:44
@10xLaCroixDrinker 10xLaCroixDrinker enabled auto-merge (squash) June 21, 2023 15:30
@10xLaCroixDrinker 10xLaCroixDrinker merged commit ad92ba2 into main Jun 21, 2023
@10xLaCroixDrinker 10xLaCroixDrinker deleted the feat/request-time-metrics branch June 21, 2023 15:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants