Repository: istio/fortio
Pull request #177 (2018-03-04 23:33:52)

fnet cleanup part 1
- report mode is now tested in Webtest
- use common code, now in fnet/ for listeners. add unit tests
- unified signature of various methods
- servers start are now all async
- fixes for localhost and port 0
- setup PPROF safely/explicitly and test in Webtest (that it's present in "server" and absent in "report" mode)

~based/stacked on top of #173 - might merge there but maybe it's easier to review separately~
now master based

codecov[bot]: # [Codecov](https://codecov.io/gh/istio/fortio/pull/177?src=pr&el=h1) Report
> Merging [#177](https://codecov.io/gh/istio/fortio/pull/177?src=pr&el=desc) into [misc4](https://codecov.io/gh/istio/fortio/commit/36e71c9fb8bd289ca72db3f511bb91a63cf23231?src=pr&el=desc) will **decrease** coverage by `1%`.
> The diff coverage is `60%`.

[![Impacted file tree graph](https://codecov.io/gh/istio/fortio/pull/177/graphs/tree.svg?height=150&token=6HLno2jpvU&width=650&src=pr)](https://codecov.io/gh/istio/fortio/pull/177?src=pr&el=tree)

```diff
@@          Coverage Diff          @@
##           misc4   #177    +/-   ##
=====================================
- Coverage     88%    88%   -<1%     
=====================================
  Files          7      7            
  Lines       1567   1571     +4     
=====================================
- Hits        1376   1372     -4     
- Misses       126    134     +8     
  Partials      65     65
```


| [Impacted Files](https://codecov.io/gh/istio/fortio/pull/177?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [fnet/network.go](https://codecov.io/gh/istio/fortio/pull/177/diff?src=pr&el=tree#diff-Zm5ldC9uZXR3b3JrLmdv) | `29% <0%> (-71%)` | :arrow_down: |
| [fhttp/http.go](https://codecov.io/gh/istio/fortio/pull/177/diff?src=pr&el=tree#diff-Zmh0dHAvaHR0cC5nbw==) | `82% <87%> (+1%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/istio/fortio/pull/177?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/istio/fortio/pull/177?src=pr&el=footer). Last update [36e71c9...68e93cd](https://codecov.io/gh/istio/fortio/pull/177?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/README.md b/README.md
index 212b620..862e5df 100644
--- a/README.md
+++ b/README.md
@@ -11,7 +11,7 @@
 
 The name fortio comes from greek φορτίο which is load/burden.
 
-Fortio is a reusable, embeddable go library as well as a command line tool and server process, the server includes a simple web UI and graphical representation of the results (both a single latency graph and a multiple results comparative min, max, avg and percentiles graphs).
+Fortio is a fast, small, reusable, embeddable go library as well as a command line tool and server process, the server includes a simple web UI and graphical representation of the results (both a single latency graph and a multiple results comparative min, max, avg and percentiles graphs).
 
 ## Installation
 
diff --git a/codecov.yml b/codecov.yml
index 5226ceb..41e6de0 100644
--- a/codecov.yml
+++ b/codecov.yml
@@ -1,6 +1,6 @@
 coverage:
   range: 60..99
-  precision: 0         # how many decimal places to display in the UI: 0 <= value <= 4
-  round: up       # how coverage is rounded: down/up/nearest
-  ignore:          # files and folders that will be removed during processing
+  precision: 1         # how many decimal places to display in the UI: 0 <= value <= 4
+  round: nearest       # how coverage is rounded: down/up/nearest
+  ignore:              # files and folders that will be removed during processing
     - "**.pb.go"
diff --git a/echosrv/echo.go b/echosrv/echo.go
index 3f3068c..a23c096 100644
--- a/echosrv/echo.go
+++ b/echosrv/echo.go
@@ -33,4 +33,5 @@ var (
 func main() {
 	flag.Parse()
 	fhttp.Serve(*port, *debugPath)
+	select {}
 }
diff --git a/fgrpc/grpcrunner.go b/fgrpc/grpcrunner.go
index 5e248f1..3309dc0 100644
--- a/fgrpc/grpcrunner.go
+++ b/fgrpc/grpcrunner.go
@@ -166,6 +166,7 @@ func RunGRPCTest(o *GRPCRunnerOptions) (*GRPCRunnerResults, error) {
 
 // grpcDestination parses dest and returns dest:port based on dest type
 // being a hostname, IP address or hostname/ip:port pair.
+// TODO: change/fix this (NormalizePort and more)
 func grpcDestination(dest string) string {
 	// strip any unintentional http/https scheme prefixes from destination
 	if strings.HasPrefix(dest, prefixHTTP) {
diff --git a/fhttp/http.go b/fhttp/http.go
index f316437..9df609b 100644
--- a/fhttp/http.go
+++ b/fhttp/http.go
@@ -395,21 +395,12 @@ func NewFastClient(o *HTTPOptions) Fetcher {
 		bc.port = url.Scheme // ie http which turns into 80 later
 		log.LogVf("No port specified, using %s", bc.port)
 	}
-	addrs, err := net.LookupIP(bc.hostname)
-	if err != nil {
-		log.Errf("Unable to lookup '%s' : %v", bc.host, err)
-		return nil
-	}
-	if len(addrs) > 1 && log.LogDebug() {
-		log.Debugf("Using only the first of the addresses for %s : %v", bc.host, addrs)
-	}
-	log.Debugf("Will go to %s", addrs[0])
-	bc.dest.IP = addrs[0]
-	bc.dest.Port, err = net.LookupPort("tcp", bc.port)
-	if err != nil {
-		log.Errf("Unable to resolve port '%s' : %v", bc.port, err)
+	addr := fnet.Resolve(bc.hostname, bc.port)
+	if addr == nil {
+		// Error already logged
 		return nil
 	}
+	bc.dest = *addr
 	// Create the bytes for the request:
 	host := bc.host
 	if o.hostOverride != "" {
@@ -1198,35 +1189,43 @@ func closingServer(listener net.Listener) error {
 	return err
 }
 
-// DynamicHTTPServer listens on an available port, sets up an http or https
-// (when secure is true) server on it and returns the listening port and
-// mux to which one can attach handlers to.
-// TODO: merge/refactor/share with Serve()
-func DynamicHTTPServer(secure bool) (int, *http.ServeMux) {
+// HTTPServer creates an http server named name on address/port port.
+// Port can include binding address and/or be port 0.
+func HTTPServer(name string, port string) (*http.ServeMux, *net.TCPAddr) {
 	m := http.NewServeMux()
 	s := &http.Server{
 		Handler: m,
 	}
-	listener, err := net.Listen("tcp", ":0") // nolint: gas
-	if err != nil {
-		log.Fatalf("Unable to listen to dynamic port: %v", err)
-	}
-	port := listener.Addr().(*net.TCPAddr).Port
-	log.Infof("Using port: %d", port)
+	listener, addr := fnet.Listen(name, port)
 	go func() {
-		var err error
-		if secure {
-			log.Errf("Secure setup not yet supported. Will just close incoming connections for now")
-			//err = http.ServeTLS(listener, nil, "", "") // go 1.9
-			err = closingServer(listener)
-		} else {
-			err = s.Serve(listener)
+		err := s.Serve(listener)
+		if err != nil {
+			log.Fatalf("Unable to serve %s on %s: %v", name, addr.String(), err)
 		}
+	}()
+	return m, addr
+}
+
+// DynamicHTTPServer listens on an available port, sets up an http or a closing
+// server simulating an https server (when closing is true) server on it and
+// returns the listening port and mux to which one can attach handlers to.
+// Note: in a future version of istio, the closing will be actually be secure
+// on/off and create an https server instead of a closing server.
+func DynamicHTTPServer(closing bool) (*http.ServeMux, *net.TCPAddr) {
+	if !closing {
+		return HTTPServer("dynamic", "0")
+	}
+	// Note: we actually use the fact it's not supported as an error server for tests - need to change that
+	log.Errf("Secure setup not yet supported. Will just close incoming connections for now")
+	listener, addr := fnet.Listen("closing server", "0")
+	//err = http.ServeTLS(listener, nil, "", "") // go 1.9
+	go func() {
+		err := closingServer(listener)
 		if err != nil {
-			log.Fatalf("Unable to serve with secure=%v on %d: %v", secure, port, err)
+			log.Fatalf("Unable to serve closing server on %s: %v", addr.String(), err)
 		}
 	}()
-	return port, m
+	return nil, addr
 }
 
 /*
@@ -1348,26 +1347,15 @@ func DebugHandler(w http.ResponseWriter, r *http.Request) {
 }
 
 // Serve starts a debug / echo http server on the given port.
-// Returns the addr where the listening socket is bound.
+// Returns the mux and addr where the listening socket is bound.
 // The .Port can be retrieved from it when requesting the 0 port as
 // input for dynamic http server.
-func Serve(port, debugPath string) *net.TCPAddr {
+func Serve(port, debugPath string) (*http.ServeMux, *net.TCPAddr) {
 	startTime = time.Now()
-	nPort := fnet.NormalizePort(port)
-	listener, err := net.Listen("tcp", nPort)
-	if err != nil {
-		log.Fatalf("Error occurred while listening %v: %v", nPort, err)
+	mux, addr := HTTPServer("echo", port)
+	if debugPath != "" {
+		mux.HandleFunc(debugPath, DebugHandler)
 	}
-	addr := listener.Addr().(*net.TCPAddr)
-	fmt.Printf("Fortio %s echo server listening on %s\n", version.Short(), addr.String())
-	go func() {
-		if debugPath != "" {
-			http.HandleFunc(debugPath, DebugHandler)
-		}
-		http.HandleFunc("/", EchoHandler)
-		if err := http.Serve(listener, nil); err != nil {
-			fmt.Println("Error starting server", err)
-		}
-	}()
-	return addr
+	mux.HandleFunc("/", EchoHandler)
+	return mux, addr
 }
diff --git a/fnet/network.go b/fnet/network.go
index 5d101e8..a3be284 100644
--- a/fnet/network.go
+++ b/fnet/network.go
@@ -15,14 +15,86 @@
 package fnet // import "istio.io/fortio/fnet"
 
 import (
+	"fmt"
+	"net"
 	"strings"
+
+	"istio.io/fortio/log"
+	"istio.io/fortio/version"
 )
 
 // NormalizePort parses port and returns host:port if port is in the form
-// of host:port or :port if port is in the form of port.
+// of host:port already or :port if port is only a port (doesn't contain :).
 func NormalizePort(port string) string {
 	if strings.ContainsAny(port, ":") {
 		return port
 	}
 	return ":" + port
 }
+
+// Listen returns a listener for the port. Port can be a port or a
+// bind address and a port (e.g. "8080" or "[::1]:8080"...). If the
+// port component is 0 a free port will be returned by the system.
+// This logs fatal on error and is meant for servers that must start.
+// For library use, we could extract into 2 functions, one returning
+// error,... if needed.
+func Listen(name string, port string) (net.Listener, *net.TCPAddr) {
+	nPort := NormalizePort(port)
+	listener, err := net.Listen("tcp", nPort)
+	if err != nil {
+		log.Fatalf("Can't listen to %v: %v", nPort, err)
+	}
+	addr := listener.Addr().(*net.TCPAddr)
+	if len(name) > 0 {
+		fmt.Printf("Fortio %s %s server listening on %s\n", version.Short(), name, addr.String())
+	}
+	return listener, addr
+}
+
+// ResolveDestination returns the TCP address of the "host:port" suitable for net.Dial.
+// nil in case of errors.
+func ResolveDestination(dest string) *net.TCPAddr {
+	i := strings.LastIndex(dest, ":")
+	if i < 0 {
+		log.Errf("Destination '%s' is not host:port format", dest)
+		return nil
+	}
+	host := dest[0:i]
+	port := dest[i+1:]
+	return Resolve(host, port)
+}
+
+// Resolve returns the TCP address of the host,port suitable for net.Dial.
+// nil in case of errors.
+func Resolve(host string, port string) *net.TCPAddr {
+	addrs, err := net.LookupIP(host)
+	if err != nil {
+		log.Errf("Unable to lookup '%s' : %v", host, err)
+		return nil
+	}
+	if len(addrs) > 1 && log.LogDebug() {
+		log.Debugf("Using only the first of the addresses for %s : %v", host, addrs)
+	}
+	log.Debugf("Will go to %s", addrs[0])
+	dest := &net.TCPAddr{}
+	dest.IP = addrs[0]
+	dest.Port, err = net.LookupPort("tcp", port)
+	if err != nil {
+		log.Errf("Unable to resolve port '%s' : %v", port, err)
+		return nil
+	}
+	return dest
+}
+
+/*
+// Proxy starts a tcp proxy.
+func Proxy(port string, dest *net.TCPAddr) {
+	l, a := Listen("proxy", port)
+	d, err := net.DialTCP("tcp", nil, dest)
+	if err != nil {
+		log.Errf("Unable to connect to %v : %v", dest, err)
+		return nil
+	}
+
+}
+*/
diff --git a/fortio_main.go b/fortio_main.go
index 3afdb03..f92e41b 100644
--- a/fortio_main.go
+++ b/fortio_main.go
@@ -152,30 +152,38 @@ func main() {
 			os.Exit(1)
 		}
 	}
-
+	isServer := false
 	switch command {
 	case "curl":
 		fortioLoad(true, nil)
 	case "load":
 		fortioLoad(*curlFlag, percList)
 	case "redirect":
+		isServer = true
 		ui.RedirectToHTTPS(*redirectFlag)
 	case "report":
+		isServer = true
 		if *redirectFlag != "disabled" {
-			go ui.RedirectToHTTPS(*redirectFlag)
+			ui.RedirectToHTTPS(*redirectFlag)
 		}
 		ui.Report(baseURL, *echoPortFlag, *staticDirFlag, *dataDirFlag)
 	case "server":
+		isServer = true
+		go pingServer(*grpcPortFlag)
 		if *redirectFlag != "disabled" {
-			go ui.RedirectToHTTPS(*redirectFlag)
+			ui.RedirectToHTTPS(*redirectFlag)
 		}
-		go ui.Serve(baseURL, *echoPortFlag, *echoDbgPathFlag, *uiPathFlag, *staticDirFlag, *dataDirFlag)
-		pingServer(*grpcPortFlag)
+		ui.Serve(baseURL, *echoPortFlag, *echoDbgPathFlag, *uiPathFlag, *staticDirFlag, *dataDirFlag)
 	case "grpcping":
 		grpcClient()
 	default:
 		usage("Error: unknown command ", command)
 	}
+	if isServer {
+		// To get a start time log/timestamp in the logs
+		log.Infof("All fortio %s servers started!", version.Long())
+		select {}
+	}
 }
 
 func fetchURL(o *fhttp.HTTPOptions) {
diff --git a/pingsrv.go b/pingsrv.go
index 91490d1..dd326b2 100644
--- a/pingsrv.go
+++ b/pingsrv.go
@@ -22,7 +22,6 @@ package main
 import (
 	"flag"
 	"fmt"
-	"net"
 	"os"
 	"time"
 
@@ -36,7 +35,6 @@ import (
 	"istio.io/fortio/fnet"
 	"istio.io/fortio/log"
 	"istio.io/fortio/stats"
-	"istio.io/fortio/version"
 )
 
 // To get most debugging/tracing:
@@ -59,18 +57,13 @@ func (s *pingSrv) Ping(c context.Context, in *fgrpc.PingMessage) (*fgrpc.PingMes
 }
 
 func pingServer(port string) {
-	port = fnet.NormalizePort(port)
-	socket, err := net.Listen("tcp", port)
-	if err != nil {
-		log.Fatalf("failed to listen: %v", err)
-	}
+	socket, _ := fnet.Listen("grpc ping", port)
 	grpcServer := grpc.NewServer()
 	reflection.Register(grpcServer)
 	healthServer := health.NewServer()
 	healthServer.SetServingStatus("ping", grpc_health_v1.HealthCheckResponse_SERVING)
 	grpc_health_v1.RegisterHealthServer(grpcServer, healthServer)
 	fgrpc.RegisterPingServerServer(grpcServer, &pingSrv{})
-	fmt.Printf("Fortio %s grpc ping server listening on port %v\n", version.Short(), port)
 	if err := grpcServer.Serve(socket); err != nil {
 		log.Fatalf("failed to start grpc server: %v", err)
 	}
diff --git a/ui/uihandler.go b/ui/uihandler.go
index 0bec1ca..c2b38b6 100644
--- a/ui/uihandler.go
+++ b/ui/uihandler.go
@@ -29,6 +29,7 @@ import (
 	"io"
 	"io/ioutil"
 	"net/http"
+	"net/http/pprof"
 	"net/url"
 	"os"
 	"path"
@@ -40,7 +41,6 @@ import (
 
 	"istio.io/fortio/fgrpc"
 	"istio.io/fortio/fhttp"
-	"istio.io/fortio/fnet"
 	"istio.io/fortio/log"
 	"istio.io/fortio/periodic"
 	"istio.io/fortio/stats"
@@ -846,17 +846,34 @@ func downloadOne(w http.ResponseWriter, client *fhttp.Client, name string, u str
 	w.Write([]byte("<td class='checkmark'>✓")) // nolint: gas, errcheck
 }
 
+// LogAndCall wrapps an HTTP handler to log the request first.
+func LogAndCall(msg string, hf http.HandlerFunc) http.HandlerFunc {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		LogRequest(r, msg)
+		hf(w, r)
+	})
+}
+
+// SetupPPROF add pprof to the mux (mirror the init() of http pprof).
+func SetupPPROF(mux *http.ServeMux) {
+	mux.HandleFunc("/debug/pprof/", LogAndCall("pprof:index", pprof.Index))
+	mux.HandleFunc("/debug/pprof/cmdline", LogAndCall("pprof:cmdline", pprof.Cmdline))
+	mux.HandleFunc("/debug/pprof/profile", LogAndCall("pprof:profile", pprof.Profile))
+	mux.HandleFunc("/debug/pprof/symbol", LogAndCall("pprof:symbol", pprof.Symbol))
+	mux.HandleFunc("/debug/pprof/trace", LogAndCall("pprof:trace", pprof.Trace))
+}
+
 // Serve starts the fhttp.Serve() plus the UI server on the given port
 // and paths (empty disables the feature). uiPath should end with /
 // (be a 'directory' path)
 func Serve(baseurl, port, debugpath, uipath, staticRsrcDir string, datadir string) {
 	baseURL = baseurl
 	startTime = time.Now()
-	hostPort := fnet.NormalizePort(port)
+	mux, addr := fhttp.Serve(port, debugpath)
 	if uipath == "" {
-		fhttp.Serve(hostPort, debugpath)
 		return
 	}
+	SetupPPROF(mux)
 	uiPath = uipath
 	dataDir = datadir
 	if uiPath[len(uiPath)-1] != '/' {
@@ -864,9 +881,9 @@ func Serve(baseurl, port, debugpath, uipath, staticRsrcDir string, datadir strin
 		uiPath += "/"
 	}
 	debugPath = ".." + debugpath // TODO: calculate actual path if not same number of directories
-	http.HandleFunc(uiPath, Handler)
+	mux.HandleFunc(uiPath, Handler)
 	fetchPath = uiPath + fetchURI
-	http.HandleFunc(fetchPath, FetcherHandler)
+	mux.HandleFunc(fetchPath, FetcherHandler)
 	fhttp.CheckConnectionClosedHeader = true // needed for proxy to avoid errors
 
 	logoPath = version.Short() + "/static/img/logo.svg"
@@ -881,8 +898,8 @@ func Serve(baseurl, port, debugpath, uipath, staticRsrcDir string, datadir strin
 	if staticRsrcDir != "" {
 		fs := http.FileServer(http.Dir(staticRsrcDir))
 		prefix := uiPath + version.Short()
-		http.Handle(prefix+"/static/", LogAndAddCacheControl(http.StripPrefix(prefix, fs)))
-		http.Handle(faviconPath, LogAndAddCacheControl(fs))
+		mux.Handle(prefix+"/static/", LogAndAddCacheControl(http.StripPrefix(prefix, fs)))
+		mux.Handle(faviconPath, LogAndAddCacheControl(fs))
 		var err error
 		mainTemplate, err = template.ParseFiles(path.Join(staticRsrcDir, "templates/main.html"))
 		if err != nil {
@@ -892,38 +909,37 @@ func Serve(baseurl, port, debugpath, uipath, staticRsrcDir string, datadir strin
 		if err != nil {
 			log.Critf("Unable to parse browse template: %v", err)
 		} else {
-			http.HandleFunc(uiPath+"browse", BrowseHandler)
+			mux.HandleFunc(uiPath+"browse", BrowseHandler)
 		}
 		syncTemplate, err = template.ParseFiles(path.Join(staticRsrcDir, "templates/sync.html"))
 		if err != nil {
 			log.Critf("Unable to parse sync template: %v", err)
 		} else {
-			http.HandleFunc(uiPath+"sync", SyncHandler)
+			mux.HandleFunc(uiPath+"sync", SyncHandler)
 		}
 	}
 	if dataDir != "" {
 		fs := http.FileServer(http.Dir(dataDir))
-		http.Handle(uiPath+"data/", LogAndFilterDataRequest(http.StripPrefix(uiPath+"data", fs)))
+		mux.Handle(uiPath+"data/", LogAndFilterDataRequest(http.StripPrefix(uiPath+"data", fs)))
 	}
-	addr := fhttp.Serve(hostPort, debugpath)
-	setHostAndPort(hostPort, addr)
-	uiMsg := fmt.Sprintf("UI starting - visit:\nhttp://%s%s", urlHostPort, uiPath)
+	setHostAndPort(port, addr)
+	uiMsg := fmt.Sprintf("UI started - visit:\nhttp://%s%s", urlHostPort, uiPath)
 	if !strings.Contains(port, ":") {
 		uiMsg += "   (or any host/ip reachable on this server)"
 	}
 	fmt.Printf(uiMsg + "\n")
-
 }
 
 // Report starts the browsing only UI server on the given port.
 // Similar to Serve with only the read only part.
 func Report(baseurl, port, staticRsrcDir string, datadir string) {
-	http.DefaultServeMux = http.NewServeMux() // drop the pprof default handlers
+	// drop the pprof default handlers [shouldn't be needed with custom mux but better safe than sorry]
+	http.DefaultServeMux = http.NewServeMux()
 	baseURL = baseurl
 	extraBrowseLabel = ", report only limited UI"
-	hostPort := fnet.NormalizePort(port)
-	setHostAndPort(hostPort, nil)
-	uiMsg := fmt.Sprintf("Browse only UI starting - visit:\nhttp://%s/", urlHostPort)
+	mux, addr := fhttp.HTTPServer("report", port)
+	setHostAndPort(port, addr)
+	uiMsg := fmt.Sprintf("Browse only UI started - visit:\nhttp://%s/", urlHostPort)
 	if !strings.Contains(port, ":") {
 		uiMsg += "   (or any host/ip reachable on this server)"
 	}
@@ -935,20 +951,17 @@ func Report(baseurl, port, staticRsrcDir string, datadir string) {
 	staticRsrcDir = getResourcesDir(staticRsrcDir)
 	fs := http.FileServer(http.Dir(staticRsrcDir))
 	prefix := uiPath + version.Short()
-	http.Handle(prefix+"/static/", LogAndAddCacheControl(http.StripPrefix(prefix, fs)))
-	http.Handle(faviconPath, LogAndAddCacheControl(fs))
+	mux.Handle(prefix+"/static/", LogAndAddCacheControl(http.StripPrefix(prefix, fs)))
+	mux.Handle(faviconPath, LogAndAddCacheControl(fs))
 	var err error
 	browseTemplate, err = template.ParseFiles(path.Join(staticRsrcDir, "templates/browse.html"))
 	if err != nil {
 		log.Critf("Unable to parse browse template: %v", err)
 	} else {
-		http.HandleFunc(uiPath, BrowseHandler)
+		mux.HandleFunc(uiPath, BrowseHandler)
 	}
 	fsd := http.FileServer(http.Dir(dataDir))
-	http.Handle(uiPath+"data/", LogAndFilterDataRequest(http.StripPrefix(uiPath+"data", fsd)))
-	if err := http.ListenAndServe(hostPort, nil); err != nil {
-		log.Critf("Error starting report server: %v", err)
-	}
+	mux.Handle(uiPath+"data/", LogAndFilterDataRequest(http.StripPrefix(uiPath+"data", fsd)))
 }
 
 // -- Redirection to https feature --
@@ -963,21 +976,12 @@ func RedirectToHTTPSHandler(w http.ResponseWriter, r *http.Request) {
 // RedirectToHTTPS Sets up a redirector to https on the given port.
 // (Do not create a loop, make sure this is addressed from an ingress)
 func RedirectToHTTPS(port string) {
-	m := http.NewServeMux()
+	m, _ := fhttp.HTTPServer("https redirector", port)
 	m.HandleFunc("/", RedirectToHTTPSHandler)
-	s := &http.Server{
-		Addr:    fnet.NormalizePort(port),
-		Handler: m,
-	}
-	fmt.Printf("Https redirector running on %v\n", s.Addr)
-	if err := s.ListenAndServe(); err != nil {
-		log.Critf("Error starting redirect server: %v", err)
-	}
-	fmt.Printf("Not reached, https redirector exiting - was on %v\n", s.Addr)
 }
 
 // setHostAndPort takes hostport in the form of hostname:port, ip:port or :port,
-// sets the urlHostPort variable and returns hostport unmodified.
+// sets the urlHostPort variable.
 func setHostAndPort(inputPort string, addr *net.TCPAddr) {
 	urlHostPort = inputPort
 	portStr := inputPort
@@ -985,6 +989,9 @@ func setHostAndPort(inputPort string, addr *net.TCPAddr) {
 		urlHostPort = addr.String()
 		portStr = fmt.Sprintf(":%d", addr.Port)
 	}
+	if !strings.Contains(inputPort, ":") {
+		inputPort = ":" + inputPort
+	}
 	if strings.HasPrefix(inputPort, ":") {
 		urlHostPort = "localhost" + portStr
 	}


Test patch:
diff --git a/Webtest.sh b/Webtest.sh
index c860c48..70a0b81 100755
--- a/Webtest.sh
+++ b/Webtest.sh
@@ -5,10 +5,11 @@ make docker-internal TAG=webtest || exit 1
 FORTIO_UI_PREFIX=/newprefix/ # test the non default prefix (not /fortio/)
 FILE_LIMIT=20 # must be low to detect leaks
 LOGLEVEL=info # change to debug to debug
-DOCKERID=$(docker run -d --ulimit nofile=$FILE_LIMIT --name fortio_server istio/fortio:webtest server -ui-path $FORTIO_UI_PREFIX -loglevel $LOGLEVEL)
+DOCKERNAME=fortio_server
+DOCKERID=$(docker run -d --ulimit nofile=$FILE_LIMIT --name $DOCKERNAME istio/fortio:webtest server -ui-path $FORTIO_UI_PREFIX -loglevel $LOGLEVEL)
 function cleanup {
   docker stop $DOCKERID
-  docker rm fortio_server
+  docker rm $DOCKERNAME
 }
 trap cleanup EXIT
 set -e
@@ -16,7 +17,7 @@ set -o pipefail
 docker ps
 BASE_URL="http://localhost:8080"
 BASE_FORTIO="$BASE_URL$FORTIO_UI_PREFIX"
-CURL="docker exec fortio_server /usr/local/bin/fortio load -curl -loglevel $LOGLEVEL"
+CURL="docker exec $DOCKERNAME /usr/local/bin/fortio curl -loglevel $LOGLEVEL"
 # Check https works (certs are in the image) - also tests autoswitch to std client for https
 $CURL https://istio.io/robots.txt
 # Check that browse doesn't 404s
@@ -28,7 +29,7 @@ $CURL "${BASE_FORTIO}fetch/localhost:8080$FORTIO_UI_PREFIX?url=http://localhost:
 # Check we can connect, and run a grpc QPS test against ourselves through fetch
 $CURL "${BASE_FORTIO}fetch/localhost:8080$FORTIO_UI_PREFIX?url=localhost:8079&load=Start&qps=-1&json=on&n=100&runner=grpc" | grep '"1": 100'
 # Check we get the logo (need to remove the CR from raw headers)
-VERSION=$(docker exec fortio_server /usr/local/bin/fortio version -s)
+VERSION=$(docker exec $DOCKERNAME /usr/local/bin/fortio version -s)
 LOGO_TYPE=$($CURL "${BASE_FORTIO}${VERSION}/static/img/logo.svg" | grep -i Content-Type: | tr -d '\r'| awk '{print $2}')
 if [ "$LOGO_TYPE" != "image/svg+xml" ]; then
   echo "Unexpected content type for the logo: $LOGO_TYPE"
@@ -43,11 +44,28 @@ fi
 # Check the main page
 $CURL $BASE_FORTIO
 # Do a small http load using std client
-docker exec fortio_server /usr/local/bin/fortio load -stdclient -qps 1 -t 2s -c 1 https://www.google.com/
+docker exec $DOCKERNAME /usr/local/bin/fortio load -stdclient -qps 1 -t 2s -c 1 https://www.google.com/
 # and with normal and with custom headers
-docker exec fortio_server /usr/local/bin/fortio load -H Foo:Bar -H Blah:Blah -qps 1 -t 2s -c 2 http://www.google.com/
+docker exec $DOCKERNAME /usr/local/bin/fortio load -H Foo:Bar -H Blah:Blah -qps 1 -t 2s -c 2 http://www.google.com/
 # Do a grpcping
-docker exec fortio_server /usr/local/bin/fortio grpcping localhost
+docker exec $DOCKERNAME /usr/local/bin/fortio grpcping localhost
 # Do a grpcping to a scheme-prefixed destination
-docker exec fortio_server /usr/local/bin/fortio grpcping https://fortio.istio.io:443
-# TODO: check report mode and pprof
+docker exec $DOCKERNAME /usr/local/bin/fortio grpcping https://fortio.istio.io:443
+# pprof should be there, no 404/error
+PPROF_URL="$BASE_URL/debug/pprof/heap?debug=1"
+$CURL $PPROF_URL | grep -i TotalAlloc # should find this in memory profile
+# switch to report mode
+docker stop $DOCKERID
+docker rm $DOCKERNAME
+DOCKERNAME=fortio_report
+DOCKERID=$(docker run -d --ulimit nofile=$FILE_LIMIT --name $DOCKERNAME istio/fortio:webtest report -loglevel $LOGLEVEL)
+docker ps
+CURL="docker exec $DOCKERNAME /usr/local/bin/fortio curl -loglevel $LOGLEVEL"
+if $CURL $PPROF_URL ; then
+  echo "pprof should 404 on report mode!"
+  exit 1
+else
+  echo "expected pprof failure to access in report mode - good !"
+fi
+# base url should serve report only UI in report mode
+$CURL $BASE_URL | grep "report only limited UI"
diff --git a/fgrpc/grpcrunner_test.go b/fgrpc/grpcrunner_test.go
index d6c16f7..0a902eb 100644
--- a/fgrpc/grpcrunner_test.go
+++ b/fgrpc/grpcrunner_test.go
@@ -24,9 +24,9 @@ import (
 	"net"
 	"testing"
 
+	"istio.io/fortio/fnet"
 	"istio.io/fortio/log"
 	"istio.io/fortio/periodic"
-	"istio.io/fortio/version"
 
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/health"
@@ -36,22 +36,17 @@ import (
 // DynamicGRPCHealthServer starts and returns the port where a GRPC Health
 // server is running. It runs until error or program exit (separate go routine)
 func DynamicGRPCHealthServer() int {
-	socket, err := net.Listen("tcp", ":0")
-	if err != nil {
-		log.Fatalf("failed to listen: %v", err)
-	}
-	addr := socket.Addr()
+	listener, addr := fnet.Listen("grpc health", ":0")
 	grpcServer := grpc.NewServer()
 	healthServer := health.NewServer()
 	healthServer.SetServingStatus("ping", grpc_health_v1.HealthCheckResponse_SERVING)
 	grpc_health_v1.RegisterHealthServer(grpcServer, healthServer)
-	fmt.Printf("Fortio %s grpc health server listening on port %v\n", version.Short(), addr)
 	go func(socket net.Listener) {
 		if e := grpcServer.Serve(socket); e != nil {
 			log.Fatalf("failed to start grpc server: %v", e)
 		}
-	}(socket)
-	return addr.(*net.TCPAddr).Port
+	}(listener)
+	return addr.Port
 }
 
 func TestGRPCRunner(t *testing.T) {
diff --git a/fhttp/http_test.go b/fhttp/http_test.go
index 883bbff..79c001f 100644
--- a/fhttp/http_test.go
+++ b/fhttp/http_test.go
@@ -86,8 +86,8 @@ func TestNewHTTPRequest(t *testing.T) {
 
 func TestMultiInitAndEscape(t *testing.T) {
 	// one escaped already 2 not
-	o := NewHTTPOptions("localhost:8080/?delay=1s:10%,0.5s:15%25,0.25s:5%")
-	expected := "http://localhost:8080/?delay=1s:10%25,0.5s:15%25,0.25s:5%25"
+	o := NewHTTPOptions("localhost:8080/?delay=10ms:10%,0.5s:15%25,0.25s:5%")
+	expected := "http://localhost:8080/?delay=10ms:10%25,0.5s:15%25,0.25s:5%25"
 	if o.URL != expected {
 		t.Errorf("Got initially '%s', expected '%s'", o.URL, expected)
 	}
@@ -493,11 +493,11 @@ func TestGenerateSize(t *testing.T) {
 // Many of the earlier http tests are through httprunner but new tests should go here
 
 func TestEchoBack(t *testing.T) {
-	p, m := DynamicHTTPServer(false)
+	m, a := DynamicHTTPServer(false)
 	m.HandleFunc("/", EchoHandler)
 	v := url.Values{}
 	v.Add("foo", "bar")
-	url := fmt.Sprintf("http://localhost:%d/", p)
+	url := fmt.Sprintf("http://localhost:%d/?delay=2s", a.Port) // trigger max delay
 	resp, err := http.PostForm(url, v)
 	if err != nil {
 		t.Fatalf("post form err %v", err)
@@ -527,9 +527,9 @@ func delayedChunkedSize(w http.ResponseWriter, r *http.Request) {
 }
 
 func TestNoFirstChunkSizeInitially(t *testing.T) {
-	p, m := DynamicHTTPServer(false)
+	m, a := DynamicHTTPServer(false)
 	m.HandleFunc("/", delayedChunkedSize)
-	url := fmt.Sprintf("http://localhost:%d/delayedChunkedSize", p)
+	url := fmt.Sprintf("http://localhost:%d/delayedChunkedSize", a.Port)
 	o := HTTPOptions{URL: url}
 	client := NewClient(&o)
 	code, data, header := client.Fetch() // used to panic/bug #127
@@ -562,10 +562,10 @@ func TestInvalidRequest(t *testing.T) {
 }
 
 func TestPayloadSizeSmall(t *testing.T) {
-	p, m := DynamicHTTPServer(false)
+	m, a := DynamicHTTPServer(false)
 	m.HandleFunc("/", EchoHandler)
 	for _, size := range []int{768, 0, 1} {
-		url := fmt.Sprintf("http://localhost:%d/with-size?size=%d", p, size)
+		url := fmt.Sprintf("http://localhost:%d/with-size?size=%d", a.Port, size)
 		o := HTTPOptions{URL: url}
 		client := NewClient(&o)
 		code, data, header := client.Fetch() // used to panic/bug #127
@@ -580,11 +580,11 @@ func TestPayloadSizeSmall(t *testing.T) {
 }
 
 func TestPayloadSizeLarge(t *testing.T) {
-	p, m := DynamicHTTPServer(false)
+	m, a := DynamicHTTPServer(false)
 	m.HandleFunc("/", EchoHandler)
 	//basic client 128k buffer can't do 200k, also errors out on non 200 codes so doing this other bg
 	size := 200000
-	url := fmt.Sprintf("http://localhost:%d/with-size?size=%d&status=888", p, size)
+	url := fmt.Sprintf("http://localhost:%d/with-size?size=%d&status=888", a.Port, size)
 	o := HTTPOptions{URL: url, DisableFastClient: true}
 	client := NewClient(&o)
 	code, data, header := client.Fetch() // used to panic/bug #127
@@ -598,9 +598,9 @@ func TestPayloadSizeLarge(t *testing.T) {
 }
 
 func TestDebugHandlerSortedHeaders(t *testing.T) {
-	p, m := DynamicHTTPServer(false)
+	m, a := DynamicHTTPServer(false)
 	m.HandleFunc("/debug", DebugHandler)
-	url := fmt.Sprintf("http://localhost:%d/debug", p)
+	url := fmt.Sprintf("http://localhost:%d/debug", a.Port)
 	o := HTTPOptions{URL: url, DisableFastClient: true}
 	o.AddAndValidateExtraHeader("BBB: bbb")
 	o.AddAndValidateExtraHeader("CCC: ccc")
@@ -624,7 +624,7 @@ func TestDebugHandlerSortedHeaders(t *testing.T) {
 		"Ccc: ccc\n"+
 		"User-Agent: %s\n"+
 		"Zzz: zzz\n\n"+
-		"body:\n\n\n", p, userAgent)
+		"body:\n\n\n", a.Port, userAgent)
 	if body != expected {
 		t.Errorf("Get body: %s not as expected: %s", body, expected)
 	}
diff --git a/fhttp/httprunner_test.go b/fhttp/httprunner_test.go
index d8a5e97..471c788 100644
--- a/fhttp/httprunner_test.go
+++ b/fhttp/httprunner_test.go
@@ -31,9 +31,9 @@ import (
 )
 
 func TestHTTPRunner(t *testing.T) {
-	port, mux := DynamicHTTPServer(false)
+	mux, addr := DynamicHTTPServer(false)
 	mux.HandleFunc("/foo/", EchoHandler)
-	baseURL := fmt.Sprintf("http://localhost:%d/", port)
+	baseURL := fmt.Sprintf("http://localhost:%d/", addr.Port)
 
 	opts := HTTPRunnerOptions{}
 	opts.QPS = 100
@@ -44,7 +44,7 @@ func TestHTTPRunner(t *testing.T) {
 		t.Error("Expecting an error but didn't get it when not using full url")
 	}
 	opts.DisableFastClient = false
-	opts.URL = baseURL + "foo/bar?delay=2s&status=200:100"
+	opts.URL = baseURL + "foo/bar?delay=20ms&status=200:100"
 	opts.Profiler = "test.profile"
 	res, err := RunHTTPTest(&opts)
 	if err != nil {
@@ -79,9 +79,9 @@ func TestHTTPRunner(t *testing.T) {
 func testHTTPNotLeaking(t *testing.T, opts *HTTPRunnerOptions) {
 	ngBefore1 := runtime.NumGoroutine()
 	t.Logf("Number go routine before test %d", ngBefore1)
-	port, mux := DynamicHTTPServer(false)
+	mux, addr := DynamicHTTPServer(false)
 	mux.HandleFunc("/echo100", EchoHandler)
-	url := fmt.Sprintf("http://localhost:%d/echo100", port)
+	url := fmt.Sprintf("http://localhost:%d/echo100", addr.Port)
 	numCalls := 100
 	opts.NumThreads = numCalls / 2 // make 2 calls per thread
 	opts.Exactly = int64(numCalls)
@@ -134,9 +134,9 @@ func TestHttpNotLeakingStdClient(t *testing.T) {
 }
 
 func TestHTTPRunnerClientRace(t *testing.T) {
-	port, mux := DynamicHTTPServer(false)
+	mux, addr := DynamicHTTPServer(false)
 	mux.HandleFunc("/echo1/", EchoHandler)
-	URL := fmt.Sprintf("http://localhost:%d/echo1/", port)
+	URL := fmt.Sprintf("http://localhost:%d/echo1/", addr.Port)
 
 	opts := HTTPRunnerOptions{}
 	opts.Init(URL)
@@ -156,9 +156,9 @@ func TestHTTPRunnerClientRace(t *testing.T) {
 }
 
 func TestClosingAndSocketCount(t *testing.T) {
-	port, mux := DynamicHTTPServer(false)
+	mux, addr := DynamicHTTPServer(false)
 	mux.HandleFunc("/echo42/", EchoHandler)
-	URL := fmt.Sprintf("http://localhost:%d/echo42/?close=1", port)
+	URL := fmt.Sprintf("http://localhost:%d/echo42/?close=1", addr.Port)
 	opts := HTTPRunnerOptions{}
 	opts.Init(URL)
 	opts.QPS = 10
@@ -185,8 +185,8 @@ func TestClosingAndSocketCount(t *testing.T) {
 func TestHTTPRunnerBadServer(t *testing.T) {
 	// Using http to an https server (or the current 'close all' dummy https server)
 	// should fail:
-	port, _ := DynamicHTTPServer(true)
-	baseURL := fmt.Sprintf("http://localhost:%d/", port)
+	_, addr := DynamicHTTPServer(true)
+	baseURL := fmt.Sprintf("http://localhost:%d/", addr.Port)
 
 	opts := HTTPRunnerOptions{}
 	opts.QPS = 10
@@ -202,7 +202,7 @@ func TestHTTPRunnerBadServer(t *testing.T) {
 // the error test for / url above fail:
 
 func TestServe(t *testing.T) {
-	addr := Serve("0", "/debugx1")
+	_, addr := Serve("0", "/debugx1")
 	port := addr.Port
 	log.Infof("On addr %s found port: %d", addr, port)
 	url := fmt.Sprintf("http://localhost:%d/debugx1?env=dump", port)
diff --git a/fnet/network_test.go b/fnet/network_test.go
index 9550872..cf7b56d 100644
--- a/fnet/network_test.go
+++ b/fnet/network_test.go
@@ -12,10 +12,13 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package fnet // import "istio.io/fortio/fnet"
+package fnet
 
 import (
+	"strconv"
 	"testing"
+
+	"istio.io/fortio/log"
 )
 
 func TestNormalizePort(t *testing.T) {
@@ -53,3 +56,72 @@ func TestNormalizePort(t *testing.T) {
 		}
 	}
 }
+
+func TestListen(t *testing.T) {
+	l, a := Listen("test listen", "0")
+	if l == nil || a == nil {
+		t.Fatalf("Unexpected nil in Listen() %v %v", l, a)
+	}
+	if a.Port == 0 {
+		t.Errorf("Unexpected 0 port after listen %+v", a)
+	}
+	_ = l.Close() // nolint: gas
+}
+
+func TestListenFailure(t *testing.T) {
+	reached := false
+	defer func(rp *bool) {
+		if r := recover(); r == nil {
+			t.Error("expected a panic from listen, didn't get one")
+		}
+		if !*rp {
+			t.Error("didn't reach expected statement")
+		}
+	}(&reached)
+	_, a1 := Listen("test listen1", "0")
+	if a1.Port == 0 {
+		t.Errorf("Unexpected 0 port after listen %+v", a1)
+	}
+	reached = true // last reached statement
+	_, _ = Listen("this should fail", strconv.Itoa(a1.Port))
+	t.Error("should not reach this")
+}
+
+func TestResolveDestination(t *testing.T) {
+	tests := []struct {
+		name        string
+		destination string
+		want        string
+	}{
+		// Error cases:
+		{"missing :", "foo", ""},
+		{"using ip:bogussvc", "8.8.8.8:doesnotexisthopefully", ""},
+		{"using bogus hostname", "doesnotexist.istio.io:443", ""},
+		// Good cases:
+		{"using ip:portname", "8.8.8.8:http", "8.8.8.8:80"},
+		{"using ip:port", "8.8.8.8:12345", "8.8.8.8:12345"},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := ResolveDestination(tt.destination)
+			gotStr := ""
+			if got != nil {
+				gotStr = got.String()
+			}
+			if gotStr != tt.want {
+				t.Errorf("ResolveDestination(%s) = %v, want %s", tt.destination, got, tt.want)
+			}
+		})
+	}
+}
+
+func TestResolveDestinationMultipleIps(t *testing.T) {
+	addr := ResolveDestination("www.google.com:443")
+	t.Logf("Found google addr %+v", addr)
+	if addr == nil {
+		t.Error("got nil address for google")
+	}
+}
+func init() {
+	log.SetLogLevel(log.Debug)
+}