Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

responseTime should not include tail time #530

Closed
kevinmstephens opened this issue Nov 9, 2016 · 1 comment
Closed

responseTime should not include tail time #530

kevinmstephens opened this issue Nov 9, 2016 · 1 comment
Labels
Milestone

Comments

@kevinmstephens
Copy link
Contributor

@kevinmstephens kevinmstephens commented Nov 9, 2016

responseTime should represent the difference between request received and response sent.

Currently, when a tail is used and log event is tail then responseTime includes the time taken for the tail to complete.

https://github.com/hapijs/good/blob/master/lib/utils.js#L71

Recommend changing to
this.responseTime = request.info.responded || Date.now() - request.info.received;

@arb arb added this to the 7.1.0 milestone Nov 9, 2016
@arb arb added the feature label Nov 9, 2016
@arb

This comment has been minimized.

Copy link
Contributor

@arb arb commented Nov 9, 2016

I'll take a PR.

kevinmstephens added a commit to kevinmstephens/good that referenced this issue Nov 9, 2016
@kevinmstephens kevinmstephens changed the title responseTime shoudl not include tail time responseTime should not include tail time Nov 9, 2016
kevinmstephens added a commit to kevinmstephens/good that referenced this issue Nov 10, 2016
kevinmstephens added a commit to kevinmstephens/good that referenced this issue Nov 10, 2016
@arb arb closed this in dfe7912 Nov 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.