Repository: fortio/fortio
Pull request #556 (2022-04-28 00:54:34)

Add more visibility to IP address resolution when using DNS
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[bot]: Code Climate has analyzed commit a96fb274 and detected **3 issues** on this pull request.

Here's the issue category breakdown:

| Category | Count |
|:---:|:---:|
| Complexity | 1 |
| Style | 2 |


View more on [Code Climate](https://codeclimate.com/github/fortio/fortio/pull/556).

codecov[bot]: # [Codecov](https://codecov.io/gh/fortio/fortio/pull/556?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fortio) Report
> Merging [#556](https://codecov.io/gh/fortio/fortio/pull/556?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fortio) (f976f40) into [master](https://codecov.io/gh/fortio/fortio/commit/7052338f704d0b836d290250ee50b828c10c68f3?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fortio) (7052338) will **increase** coverage by `0.0%`.
> The diff coverage is `88.2%`.

```diff
@@          Coverage Diff           @@
##           master    #556   +/-   ##
======================================
  Coverage    87.5%   87.5%           
======================================
  Files          26      26           
  Lines        3579    3595   +16     
======================================
+ Hits         3130    3144   +14     
- Misses        300     301    +1     
- Partials      149     150    +1     
```


| [Impacted Files](https://codecov.io/gh/fortio/fortio/pull/556?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fortio) | Coverage Δ | |
|---|---|---|
| [fhttp/http\_client.go](https://codecov.io/gh/fortio/fortio/pull/556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fortio#diff-Zmh0dHAvaHR0cF9jbGllbnQuZ28=) | `83.6% <80.0%> (-0.1%)` | :arrow_down: |
| [dflag/dynstringset.go](https://codecov.io/gh/fortio/fortio/pull/556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fortio#diff-ZGZsYWcvZHluc3RyaW5nc2V0Lmdv) | `95.8% <100.0%> (ø)` | |
| [fhttp/httprunner.go](https://codecov.io/gh/fortio/fortio/pull/556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fortio#diff-Zmh0dHAvaHR0cHJ1bm5lci5nbw==) | `87.3% <100.0%> (+0.6%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/fortio/fortio/pull/556?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fortio).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fortio)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/fortio/fortio/pull/556?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fortio). Last update [7052338...f976f40](https://codecov.io/gh/fortio/fortio/pull/556?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fortio). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fortio).

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

Patch:
diff --git a/fhttp/http_client.go b/fhttp/http_client.go
index 76a76e1..d49c411 100644
--- a/fhttp/http_client.go
+++ b/fhttp/http_client.go
@@ -46,6 +46,8 @@ type Fetcher interface {
 	// Close() cleans up connections and state - must be paired with NewClient calls.
 	// returns how many sockets have been used (Fastclient only)
 	Close() int
+	// GetIPAddress() get the ip address that DNS resolves to
+	GetIPAddress() string
 }
 
 const (
@@ -308,6 +310,7 @@ type Client struct {
 	bodyContainsUUID     bool // if body contains the "{uuid}" pattern (lowercase)
 	logErrors            bool
 	id                   int
+	socketCount          int
 }
 
 // Close cleans up any resources used by NewStdClient.
@@ -324,7 +327,7 @@ func (c *Client) Close() int {
 	if c.transport != nil {
 		c.transport.CloseIdleConnections()
 	}
-	return 0 // TODO: find a way to track std client socket usage.
+	return c.socketCount
 }
 
 // ChangeURL only for standard client, allows fetching a different URL.
@@ -396,6 +399,11 @@ func (c *Client) Fetch() (int, []byte, int) {
 	return code, data, 0
 }
 
+// GetIPAddress get the ip address that DNS resolves to when using stdClient.
+func (c *Client) GetIPAddress() string {
+	return c.req.RemoteAddr
+}
+
 // NewClient creates either a standard or fast client (depending on
 // the DisableFastClient flag).
 func NewClient(o *HTTPOptions) (Fetcher, error) {
@@ -415,6 +423,23 @@ func NewStdClient(o *HTTPOptions) (*Client, error) {
 	if req == nil {
 		return nil, err
 	}
+
+	client := Client{
+		url:                  o.URL,
+		path:                 req.URL.Path,
+		pathContainsUUID:     strings.Contains(req.URL.Path, uuidToken),
+		rawQuery:             req.URL.RawQuery,
+		rawQueryContainsUUID: strings.Contains(req.URL.RawQuery, uuidToken),
+		body:                 o.PayloadString(),
+		bodyContainsUUID:     strings.Contains(o.PayloadString(), uuidToken),
+		req:                  req,
+		client: &http.Client{
+			Timeout: o.HTTPReqTimeOut,
+		},
+		id:        o.ID,
+		logErrors: o.LogErrors,
+	}
+
 	tr := http.Transport{
 		MaxIdleConns:        o.NumConnections,
 		MaxIdleConnsPerHost: o.NumConnections,
@@ -426,35 +451,33 @@ func NewStdClient(o *HTTPOptions) (*Client, error) {
 			if o.Resolve != "" {
 				addr = o.Resolve + addr[strings.LastIndex(addr, ":"):]
 			}
-			return (&net.Dialer{
+			conn, err := (&net.Dialer{
 				Timeout: o.HTTPReqTimeOut,
 			}).DialContext(ctx, network, addr)
+
+			if conn != nil {
+				newRemoteAddress := conn.RemoteAddr().String()
+				if req.RemoteAddr != "" {
+					log.Infof("Standard client IP address changed from %s to %s", req.RemoteAddr, newRemoteAddress)
+				}
+				req.RemoteAddr = newRemoteAddress
+				client.socketCount++
+			}
+
+			return conn, err
 		},
 		TLSHandshakeTimeout: o.HTTPReqTimeOut,
 	}
+
+	client.client.Transport = &tr
+	client.transport = &tr
+
 	if o.https {
 		tr.TLSClientConfig, err = o.TLSOptions.TLSClientConfig()
 		if err != nil {
 			return nil, err
 		}
 	}
-	client := Client{
-		url:                  o.URL,
-		path:                 req.URL.Path,
-		pathContainsUUID:     strings.Contains(req.URL.Path, uuidToken),
-		rawQuery:             req.URL.RawQuery,
-		rawQueryContainsUUID: strings.Contains(req.URL.RawQuery, uuidToken),
-		body:                 o.PayloadString(),
-		bodyContainsUUID:     strings.Contains(o.PayloadString(), uuidToken),
-		req:                  req,
-		client: &http.Client{
-			Timeout:   o.HTTPReqTimeOut,
-			Transport: &tr,
-		},
-		transport: &tr,
-		id:        o.ID,
-		logErrors: o.LogErrors,
-	}
 	if !o.FollowRedirects {
 		// Lets us see the raw response instead of auto following redirects.
 		client.client.CheckRedirect = func(req *http.Request, via []*http.Request) error {
@@ -511,6 +534,11 @@ type FastClient struct {
 	tlsConfig    *tls.Config
 }
 
+// GetIPAddress get the ip address that DNS resolves to when using fast client.
+func (c *FastClient) GetIPAddress() string {
+	return c.dest.String()
+}
+
 // Close cleans up any resources used by FastClient.
 func (c *FastClient) Close() int {
 	log.Debugf("Closing %p %s socket count %d", c, c.url, c.socketCount)
diff --git a/fhttp/httprunner.go b/fhttp/httprunner.go
index bf83f4f..1dbb246 100644
--- a/fhttp/httprunner.go
+++ b/fhttp/httprunner.go
@@ -35,8 +35,9 @@ import (
 // Also is the internal type used per thread/goroutine.
 type HTTPRunnerResults struct {
 	periodic.RunnerResults
-	client   Fetcher
-	RetCodes map[int]int64
+	client     Fetcher
+	RetCodes   map[int]int64
+	IPCountMap map[string]int // TODO: Move it to a shared results struct where all runner should have this field
 	// internal type/data
 	sizes       *stats.Histogram
 	headerSizes *stats.Histogram
@@ -98,6 +99,7 @@ func RunHTTPTest(o *HTTPRunnerOptions) (*HTTPRunnerResults, error) {
 	total := HTTPRunnerResults{
 		HTTPOptions: o.HTTPOptions,
 		RetCodes:    make(map[int]int64),
+		IPCountMap:  make(map[string]int),
 		sizes:       stats.NewHistogram(0, 100),
 		headerSizes: stats.NewHistogram(0, 5),
 		AbortOn:     o.AbortOn,
@@ -182,6 +184,11 @@ func RunHTTPTest(o *HTTPRunnerOptions) (*HTTPRunnerResults, error) {
 	// unused ones. We also must cleanup all the created clients.
 	keys := []int{}
 	for i := 0; i < numThreads; i++ {
+		// Get the report on the IP address each thread use to send traffic
+		ip := httpstate[i].client.GetIPAddress()
+		log.Infof("[%d] %s resolve to IP address: %s\n", i, o.URL, ip)
+		total.IPCountMap[ip]++
+
 		total.SocketCount += httpstate[i].client.Close()
 		// Q: is there some copying each time stats[i] is used?
 		for k := range httpstate[i].RetCodes {
@@ -193,14 +200,27 @@ func RunHTTPTest(o *HTTPRunnerOptions) (*HTTPRunnerResults, error) {
 		total.sizes.Transfer(httpstate[i].sizes)
 		total.headerSizes.Transfer(httpstate[i].headerSizes)
 	}
+
+	// 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)
+	}
+
+	sort.Slice(ipList, func(i, j int) bool {
+		return total.IPCountMap[ipList[i]] > total.IPCountMap[ipList[j]]
+	})
+
 	// Cleanup state:
 	r.Options().ReleaseRunners()
 	sort.Ints(keys)
 	totalCount := float64(total.DurationHistogram.Count)
-	if !o.DisableFastClient {
-		_, _ = fmt.Fprintf(out, "Sockets used: %d (for perfect keepalive, would be %d)\n", total.SocketCount, r.Options().NumThreads)
-	}
+	_, _ = fmt.Fprintf(out, "Sockets used: %d (for perfect keepalive, would be %d)\n", total.SocketCount, r.Options().NumThreads)
 	_, _ = fmt.Fprintf(out, "Uniform: %t, Jitter: %t\n", total.Uniform, total.Jitter)
+	_, _ = fmt.Fprintf(out, "IP addresses distribution:\n")
+	for _, v := range ipList {
+		_, _ = fmt.Fprintf(out, "%s: %d\n", v, total.IPCountMap[v])
+	}
 	for _, k := range keys {
 		_, _ = fmt.Fprintf(out, "Code %3d : %d (%.1f %%)\n", k, total.RetCodes[k], 100.*float64(total.RetCodes[k])/totalCount)
 	}
@@ -216,7 +236,7 @@ func RunHTTPTest(o *HTTPRunnerOptions) (*HTTPRunnerResults, error) {
 	return &total, nil
 }
 
-// A errgroup is a collection of goroutines working on subtasks that are part of
+// An errgroup is a collection of goroutines working on subtasks that are part of
 // the same overall task.
 type errgroup struct {
 	wg sync.WaitGroup


Test patch:
diff --git a/fhttp/http_test.go b/fhttp/http_test.go
index 030563c..e861353 100644
--- a/fhttp/http_test.go
+++ b/fhttp/http_test.go
@@ -676,7 +676,7 @@ func TestSmallBufferAndNoKeepAlive(t *testing.T) {
 	cli.Close()
 }
 
-func TestBadUrl(t *testing.T) {
+func TestBadUrlFastClient(t *testing.T) {
 	opts := NewHTTPOptions("not a valid url")
 	cli, err := NewFastClient(opts)
 	if cli != nil || err == nil {
@@ -691,6 +691,22 @@ func TestBadUrl(t *testing.T) {
 	}
 }
 
+func TestBadURLStdClient(t *testing.T) {
+	opts := NewHTTPOptions("not a valid url")
+	cli, err := NewStdClient(opts)
+	if cli != nil || err == nil {
+		t.Errorf("config1: got a client %v despite bogus url %s", cli, opts.URL)
+		cli.Close()
+	}
+
+	opts.URL = "http://doesnotexist.fortio.org"
+	cli, _ = NewStdClient(opts)
+	code, _, _ := cli.Fetch()
+	if code != -1 {
+		t.Errorf("config2: client can send request despite bogus url %s", opts.URL)
+	}
+}
+
 func TestDefaultPort(t *testing.T) {
 	// TODO: change back to fortio demo server once setup
 	url := "http://istio.io/" // shall imply port 80
diff --git a/fhttp/httprunner_test.go b/fhttp/httprunner_test.go
index 677d507..da3631a 100644
--- a/fhttp/httprunner_test.go
+++ b/fhttp/httprunner_test.go
@@ -60,6 +60,10 @@ func TestHTTPRunner(t *testing.T) {
 	if res.SocketCount != res.RunnerResults.NumThreads {
 		t.Errorf("%d socket used, expected same as thread# %d", res.SocketCount, res.RunnerResults.NumThreads)
 	}
+	count := getIPUsageCount(res.IPCountMap)
+	if count != res.RunnerResults.NumThreads {
+		t.Errorf("Total IP usage count %d, expected same as thread %d", count, res.RunnerResults.NumThreads)
+	}
 	// Test raw client, should get warning about non init timeout:
 	rawOpts := HTTPOptions{
 		URL: opts.URL,
@@ -121,11 +125,8 @@ func testHTTPNotLeaking(t *testing.T, opts *HTTPRunnerOptions) {
 	if ngAfter > ngBefore2+8 {
 		t.Errorf("Goroutines after test %d, expected it to stay near %d", ngAfter, ngBefore2)
 	}
-	if !opts.DisableFastClient {
-		// only fast client so far has a socket count
-		if res.SocketCount != res.RunnerResults.NumThreads {
-			t.Errorf("%d socket used, expected same as thread# %d", res.SocketCount, res.RunnerResults.NumThreads)
-		}
+	if res.SocketCount != res.RunnerResults.NumThreads {
+		t.Errorf("%d socket used, expected same as thread# %d", res.SocketCount, res.RunnerResults.NumThreads)
 	}
 }
 
@@ -190,17 +191,16 @@ func TestHTTPRunnerClientRace(t *testing.T) {
 	}
 }
 
-func TestClosingAndSocketCount(t *testing.T) {
+func testClosingAndSocketCount(t *testing.T, o *HTTPRunnerOptions) {
 	mux, addr := DynamicHTTPServer(false)
 	mux.HandleFunc("/echo42/", EchoHandler)
 	URL := fmt.Sprintf("http://localhost:%d/echo42/?close=true", addr.Port)
-	opts := HTTPRunnerOptions{}
-	opts.Init(URL)
-	opts.QPS = 10
+	o.Init(URL)
+	o.QPS = 10
 	numReq := int64(50) // can't do too many without running out of fds on mac
-	opts.Exactly = numReq
-	opts.NumThreads = 5
-	res, err := RunHTTPTest(&opts)
+	o.Exactly = numReq
+	o.NumThreads = 5
+	res, err := RunHTTPTest(o)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -217,6 +217,14 @@ func TestClosingAndSocketCount(t *testing.T) {
 	}
 }
 
+func TestClosingAndSocketCountFastClient(t *testing.T) {
+	testClosingAndSocketCount(t, &HTTPRunnerOptions{})
+}
+
+func TestClosingAndSocketCountStdClient(t *testing.T) {
+	testClosingAndSocketCount(t, &HTTPRunnerOptions{HTTPOptions: HTTPOptions{DisableFastClient: true}})
+}
+
 func TestHTTPRunnerBadServer(t *testing.T) {
 	// Using http to an https server (or the current 'close all' dummy https server)
 	// should fail:
@@ -462,3 +470,11 @@ func TestAbortOn(t *testing.T) {
 		t.Errorf("Abort2 not working, did %d requests expecting ideally 1 and <= %d", count, o.NumThreads)
 	}
 }
+
+func getIPUsageCount(ipCountMap map[string]int) (count int) {
+	for _, v := range ipCountMap {
+		count += v
+	}
+
+	return count
+}