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

Add more visibility to IP address resolution when using DNS #556

Merged
merged 26 commits into from May 5, 2022

Conversation

wuhaoyujerry
Copy link
Member

Add logs at the end of the test to show the IP address each thread resolve to when using DNS and the usage count of each IP address.

Sample run result:

❯ go run . load https://fortio.org/
Fortio dev running at 8 queries per second, 10->10 procs, for 5s: https://fortio.org/
17:52:06 I httprunner.go:93> Starting http test for https://fortio.org/ with 4 threads at 8.0 qps and parallel warmup
Starting at 8 qps with 4 thread(s) [gomax 10] for 5s : 10 calls each (total 40)
17:52:11 I periodic.go:723> T001 ended after 5.041781917s : 10 calls. qps=1.9834257341202648
17:52:11 I periodic.go:723> T002 ended after 5.042107167s : 10 calls. qps=1.9832977897512427
17:52:11 I periodic.go:723> T003 ended after 5.043662s : 10 calls. qps=1.9826863893734352
17:52:11 I periodic.go:723> T000 ended after 5.044754833s : 10 calls. qps=1.9822568848312554
Ended after 5.04481125s : 40 calls. qps=7.9289
Sleep times : count 36 avg 0.50980665 +/- 0.008107 min 0.478828666 max 0.522668222 sum 18.3530394
Aggregated Function Time : count 40 avg 0.04440421 +/- 0.007715 min 0.032523166 max 0.075609875 sum 1.77616841
# range, mid point, percentile, count
>= 0.0325232 <= 0.035 , 0.0337616 , 2.50, 1
> 0.035 <= 0.04 , 0.0375 , 27.50, 10
> 0.04 <= 0.045 , 0.0425 , 67.50, 16
> 0.045 <= 0.05 , 0.0475 , 80.00, 5
> 0.05 <= 0.06 , 0.055 , 97.50, 7
> 0.07 <= 0.0756099 , 0.0728049 , 100.00, 1
# target 50% 0.0428125
# target 75% 0.048
# target 90% 0.0557143
# target 99% 0.0733659
# target 99.9% 0.0753855
Error cases : no data
[0] Host https://fortio.org/ resolve to IP address: 104.21.74.52:443 
[1] Host https://fortio.org/ resolve to IP address: 104.21.74.52:443 
[2] Host https://fortio.org/ resolve to IP address: 104.21.74.52:443 
[3] Host https://fortio.org/ resolve to IP address: 104.21.74.52:443 
Sockets used: 4 (for perfect keepalive, would be 4)
Uniform: false, Jitter: false
IP address 104.21.74.52:443 usage count: 4
Code 200 : 40 (100.0 %)
Response Header Sizes : count 40 avg 1130.65 +/- 5.764 min 1120 max 1145 sum 45226
Response Body/Total Sizes : count 40 avg 8536.525 +/- 5.886 min 8525 max 8550 sum 341461
All done 40 calls (plus 4 warmup) 44.404 ms avg, 7.9 qps

@codeclimate
Copy link

codeclimate bot commented Apr 28, 2022

Code Climate has analyzed commit db3d86e and detected 1 issue on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 1

View more on Code Climate.

fhttp/http_client.go Show resolved Hide resolved
fhttp/http_client.go Show resolved Hide resolved
@wuhaoyujerry wuhaoyujerry marked this pull request as ready for review April 28, 2022 04:32
@codecov
Copy link

codecov bot commented Apr 28, 2022

Codecov Report

Merging #556 (db3d86e) into master (7052338) will decrease coverage by 0.0%.
The diff coverage is 93.0%.

@@           Coverage Diff            @@
##           master    #556     +/-   ##
========================================
- Coverage    87.5%   87.5%   -0.0%     
========================================
  Files          26      26             
  Lines        3579    3602     +23     
========================================
+ Hits         3130    3150     +20     
- Misses        300     302      +2     
- Partials      149     150      +1     
Impacted Files Coverage Δ
fhttp/httprunner.go 85.6% <78.6%> (-1.1%) ⬇️
fhttp/http_client.go 84.0% <100.0%> (+0.3%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7052338...db3d86e. Read the comment docs.

dflag/configmap/updater_test.go Outdated Show resolved Hide resolved
dflag/dynstringset.go Outdated Show resolved Hide resolved
fhttp/http_client.go Outdated Show resolved Hide resolved
fhttp/http_client.go Outdated Show resolved Hide resolved
fhttp/http_client.go Outdated Show resolved Hide resolved
fhttp/http_client.go Outdated Show resolved Hide resolved
fhttp/httprunner.go Outdated Show resolved Hide resolved
fhttp/httprunner.go Outdated Show resolved Hide resolved
fhttp/httprunner.go Outdated Show resolved Hide resolved
@wuhaoyujerry
Copy link
Member Author

@ldemailly Do you have any other questions about this PR? If not can we merge this? Thanks.

fhttp/http_client.go Outdated Show resolved Hide resolved
fhttp/http_utils.go Outdated Show resolved Hide resolved
ui/restHandler.go Outdated Show resolved Hide resolved
fhttp/httprunner.go Outdated Show resolved Hide resolved
Copy link
Member

@ldemailly ldemailly left a comment

Choose a reason for hiding this comment

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

nice! just 1 important and 1 optional change

fhttp/http_client.go Outdated Show resolved Hide resolved
// Sort the ip address form largest to smallest based on its usage count
ipList := make([]string, 0, len(total.IPCountMap))
for k := range total.IPCountMap {
ipList = append(ipList, k)
Copy link
Member

Choose a reason for hiding this comment

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

it's somewhat minor but you should be able to get that list in the above loop; like keys - though maybe it's easier/as clear as you have it now; given we don't expect hundreds of ips anyway

Copy link
Member

@ldemailly ldemailly left a comment

Choose a reason for hiding this comment

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

thx

fhttp/http_test.go Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants