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.

Sign up for GitHub

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

Jump to bottom

DNS timing is wrong for page with redirects. #522

Closed
joshhe opened this issue Nov 9, 2015 · 5 comments
Closed

DNS timing is wrong for page with redirects. #522

joshhe opened this issue Nov 9, 2015 · 5 comments
Labels
KNOWN_BUGS material

Comments

@joshhe
Copy link

joshhe commented Nov 9, 2015

Hi,

I ran the following command.
curl -A 'Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/41.0.2227.0 Safari/537.36' -k -v -L -D - --max-time 240 --output /dev/null --silent --show-error --stderr - --write-out 'Timings: %{time_namelookup} %{time_connect} %{time_appconnect} %{time_pretransfer} %{time_redirect} %{time_starttransfer} %{time_total}\n' 'https://actionpacked.atlassian.net'

and the output is here:
Timings: 0.000 0.000 0.000 0.000 0.570 0.234 0.807
Somehow the dns timing is 0ms, which is not correct. If I run it without the -L, it actually returns back 4ms. So I am guessing the dns timing is for the last redirect page and it doesn't add up the previous dns timing values? In this case, the redirect doesn't get a new domain, so dns doesn't happen on the redirect and therefore it return 0ms.

I am not sure if this is by design, but if it is, it's NOT very helpful for the user. Usually I want to know how long dns took in total from the beginning to the last byte, not just the dns timing of the last redirect.

Let me know what you think.
Thanks!
@bagder
Copy link
Member

bagder commented Nov 11, 2015

I agree that it would be neat. The code doesn't work like that now though. We need to change the assigns to adds...

@joshhe
Copy link
Author

joshhe commented Nov 12, 2015

Do you think it will be an easy fix? I can take a look at the source code and try to fix it.

@bagder
Copy link
Member

bagder commented Nov 13, 2015

I believe it could be as easy as making the 5 assignments starting https://github.com/bagder/curl/blob/master/lib/progress.c#L186 to just use '+=' instead of '='.

@bagder bagder closed this as completed in 00cf68c Apr 8, 2016
@fmrahman
Copy link

fmrahman commented Mar 9, 2017

In which curl version this fix is available.

@bagder
Copy link
Member

bagder commented Mar 9, 2017

None, still listed as a known bug

rylwin added a commit to rylwin/curl that referenced this issue Jun 21, 2017
@rylwin
progress: Track total times following redirects
0b4ab4f 
When a redirect is followed, the progress timers `t_nslookup`,
`t_connect`, `t_appconnect`, `t_pretransfer`, and `t_starttransfer`
track the total times for these activities. Previously, only the time
for the most recent request would be tracked.

As a related change, rename `Curl_pgrsResetTimesSizes` to
`Curl_pgrsResetTransferSizes` now that the function only resets transfer
sizes and no longer modified any of the progress timers.

Added test case 1399.

Fixes curl#522 and Known Bug 1.8
Reported-by: joshhe
rylwin added a commit to rylwin/curl that referenced this issue Jun 21, 2017
@rylwin
progress: Track total times following redirects
cb2360a 
When a redirect is followed, the progress timers `t_nslookup`,
`t_connect`, `t_appconnect`, `t_pretransfer`, and `t_starttransfer`
track the total times for these activities. Previously, only the time
for the most recent request would be tracked.

As a related change, rename `Curl_pgrsResetTimesSizes` to
`Curl_pgrsResetTransferSizes` now that the function only resets transfer
sizes and no longer modified any of the progress timers.

Added test case 1399.

Fixes curl#522 and Known Bug 1.8
Reported-by: joshhe
rylwin added a commit to rylwin/curl that referenced this issue Jun 21, 2017
@rylwin
progress: Track total times following redirects
6d4cac1 
Update the progress timers `t_nslookup`, `t_connect`, `t_appconnect`,
`t_pretransfer`, and `t_starttransfer` to track the total times for
these activities when a redirect is followed. Previously, only the times
for the most recent request would be tracked.

As a related change, rename `Curl_pgrsResetTimesSizes` to
`Curl_pgrsResetTransferSizes` now that the function only resets transfer
sizes and no longer modified any of the progress timers.

Added test case 1399.

Fixes curl#522 and Known Bug 1.8
Reported-by: joshhe
@rylwin rylwin mentioned this issue Jun 21, 2017
Closed
rylwin added a commit to rylwin/curl that referenced this issue Jun 21, 2017
@rylwin
progress: Track total times following redirects
7852e3c 
Update the progress timers `t_nslookup`, `t_connect`, `t_appconnect`,
`t_pretransfer`, and `t_starttransfer` to track the total times for
these activities when a redirect is followed. Previously, only the times
for the most recent request would be tracked.

As a related change, rename `Curl_pgrsResetTimesSizes` to
`Curl_pgrsResetTransferSizes` now that the function only resets transfer
sizes and no longer modified any of the progress timers.

Added test case 1399.

Fixes curl#522 and Known Bug 1.8
Reported-by: joshhe
rylwin added a commit to rylwin/curl that referenced this issue Jun 21, 2017
@rylwin
progress: Track total times following redirects
d7ab12b 
Update the progress timers `t_nslookup`, `t_connect`, `t_appconnect`,
`t_pretransfer`, and `t_starttransfer` to track the total times for
these activities when a redirect is followed. Previously, only the times
for the most recent request would be tracked.

As a related change, rename `Curl_pgrsResetTimesSizes` to
`Curl_pgrsResetTransferSizes` now that the function only resets transfer
sizes and no longer modified any of the progress timers.

Added test case 1399.

Fixes curl#522 and Known Bug 1.8
Reported-by: joshhe
rylwin added a commit to rylwin/curl that referenced this issue Jul 3, 2017
@rylwin
progress: Track total times following redirects
d3e2797 
Update the progress timers `t_nslookup`, `t_connect`, `t_appconnect`,
`t_pretransfer`, and `t_starttransfer` to track the total times for
these activities when a redirect is followed. Previously, only the times
for the most recent request would be tracked.

Related changes:

  - Rename `Curl_pgrsResetTimesSizes` to `Curl_pgrsResetTransferSizes`
    now that the function only resets transfer sizes and no longer
    modifies any of the progress timers.

  - Add a bool to the `Progress` struct that is used to prevent
    double-counting `t_starttransfer` times.

Added test case 1399.

Fixes curl#522 and Known Bug 1.8
Reported-by: joshhe
rylwin added a commit to rylwin/curl that referenced this issue Jul 3, 2017
@rylwin
progress: Track total times following redirects
568e24b 
Update the progress timers `t_nslookup`, `t_connect`, `t_appconnect`,
`t_pretransfer`, and `t_starttransfer` to track the total times for
these activities when a redirect is followed. Previously, only the times
for the most recent request would be tracked.

Related changes:

  - Rename `Curl_pgrsResetTimesSizes` to `Curl_pgrsResetTransferSizes`
    now that the function only resets transfer sizes and no longer
    modifies any of the progress timers.

  - Add a bool to the `Progress` struct that is used to prevent
    double-counting `t_starttransfer` times.

Added test case 1399.

Fixes curl#522 and Known Bug 1.8
Reported-by: joshhe
rylwin added a commit to rylwin/curl that referenced this issue Jul 3, 2017
@rylwin
progress: Track total times following redirects
826ee42 
Update the progress timers `t_nslookup`, `t_connect`, `t_appconnect`,
`t_pretransfer`, and `t_starttransfer` to track the total times for
these activities when a redirect is followed. Previously, only the times
for the most recent request would be tracked.

Related changes:

  - Rename `Curl_pgrsResetTimesSizes` to `Curl_pgrsResetTransferSizes`
    now that the function only resets transfer sizes and no longer
    modifies any of the progress timers.

  - Add a bool to the `Progress` struct that is used to prevent
    double-counting `t_starttransfer` times.

Added test case 1399.

Fixes curl#522 and Known Bug 1.8
Reported-by: joshhe
rylwin added a commit to rylwin/curl that referenced this issue Jul 3, 2017
@rylwin
progress: Track total times following redirects
6872ff8 
Update the progress timers `t_nslookup`, `t_connect`, `t_appconnect`,
`t_pretransfer`, and `t_starttransfer` to track the total times for
these activities when a redirect is followed. Previously, only the times
for the most recent request would be tracked.

Related changes:

  - Rename `Curl_pgrsResetTimesSizes` to `Curl_pgrsResetTransferSizes`
    now that the function only resets transfer sizes and no longer
    modifies any of the progress timers.

  - Add a bool to the `Progress` struct that is used to prevent
    double-counting `t_starttransfer` times.

Added test case 1399.

Fixes curl#522 and Known Bug 1.8
Reported-by: joshhe
bagder pushed a commit that referenced this issue Aug 15, 2017
@rylwin @bagder
progress: Track total times following redirects
43d036e 
Update the progress timers `t_nslookup`, `t_connect`, `t_appconnect`,
`t_pretransfer`, and `t_starttransfer` to track the total times for
these activities when a redirect is followed. Previously, only the times
for the most recent request would be tracked.

Related changes:

  - Rename `Curl_pgrsResetTimesSizes` to `Curl_pgrsResetTransferSizes`
    now that the function only resets transfer sizes and no longer
    modifies any of the progress timers.

  - Add a bool to the `Progress` struct that is used to prevent
    double-counting `t_starttransfer` times.

Added test case 1399.

Fixes #522 and Known Bug 1.8
Closes #1602
Reported-by: joshhe on github
@lock lock bot locked as resolved and limited conversation to collaborators May 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
KNOWN_BUGS material
Milestone
No milestone
Development

No branches or pull requests
3 participants
@bagder @joshhe @fmrahman