Repository: istio/fortio
Pull request #156 (2018-02-24 14:17:17)

Change the long version to space separated + port 0 fixes
```
$ docker run istio/fortio:webtest version
0.7.2-pre 2018-02-24 14:14 48d91e285fa8d1c6baf7e8bf741ea8a36bbe2fdf
```

also fixes UI URL printout for port 0 (#110)

```
ldemailly-macbookpro:fortio ldemailly$ fortio server --http-port :0
Https redirector running on :8081
Fortio 0.7.2-pre grpc ping server listening on port :8079
Fortio 0.7.2-pre echo server listening on [::]:57063
UI starting - visit:
http://localhost:57063/fortio/
^C
ldemailly-macbookpro:fortio ldemailly$ fortio server --http-port [::1]:0
Https redirector running on :8081
Fortio 0.7.2-pre grpc ping server listening on port :8079
Fortio 0.7.2-pre echo server listening on [::1]:57068
UI starting - visit:
http://[::1]:57068/fortio/
^C
ldemailly-macbookpro:fortio ldemailly$ fortio server --http-port
192.168.1.90:blp5
Https redirector running on :8081
Fortio 0.7.2-pre grpc ping server listening on port :8079
Fortio 0.7.2-pre echo server listening on 192.168.1.90:48129
UI starting - visit:
http://192.168.1.90:48129/fortio/
```

And also: extra test for #83 (all stats bucket boundaries)

codecov[bot]: # [Codecov](https://codecov.io/gh/istio/fortio/pull/156?src=pr&el=h1) Report
> Merging [#156](https://codecov.io/gh/istio/fortio/pull/156?src=pr&el=desc) into [master](https://codecov.io/gh/istio/fortio/commit/48d91e285fa8d1c6baf7e8bf741ea8a36bbe2fdf?src=pr&el=desc) will **increase** coverage by `0.4%`.
> The diff coverage is `n/a`.

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

```diff
@@           Coverage Diff            @@
##           master    #156     +/-   ##
========================================
+ Coverage      88%   88.5%   +0.4%     
========================================
  Files           7       7             
  Lines        1505    1482     -23     
========================================
- Hits         1325    1311     -14     
+ Misses        120     114      -6     
+ Partials       60      57      -3
```


| [Impacted Files](https://codecov.io/gh/istio/fortio/pull/156?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [fgrpc/grpcrunner.go](https://codecov.io/gh/istio/fortio/pull/156/diff?src=pr&el=tree#diff-ZmdycGMvZ3JwY3J1bm5lci5nbw==) | `77.8% <0%> (+5.1%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/istio/fortio/pull/156?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/156?src=pr&el=footer). Last update [48d91e2...7d8fb7a](https://codecov.io/gh/istio/fortio/pull/156?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

codecov[bot]: # [Codecov](https://codecov.io/gh/istio/fortio/pull/156?src=pr&el=h1) Report
> Merging [#156](https://codecov.io/gh/istio/fortio/pull/156?src=pr&el=desc) into [master](https://codecov.io/gh/istio/fortio/commit/48d91e285fa8d1c6baf7e8bf741ea8a36bbe2fdf?src=pr&el=desc) will **increase** coverage by `0.4%`.
> The diff coverage is `n/a`.

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

```diff
@@           Coverage Diff            @@
##           master    #156     +/-   ##
========================================
+ Coverage      88%   88.5%   +0.4%     
========================================
  Files           7       7             
  Lines        1505    1482     -23     
========================================
- Hits         1325    1311     -14     
+ Misses        120     114      -6     
+ Partials       60      57      -3
```


| [Impacted Files](https://codecov.io/gh/istio/fortio/pull/156?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [fgrpc/grpcrunner.go](https://codecov.io/gh/istio/fortio/pull/156/diff?src=pr&el=tree#diff-ZmdycGMvZ3JwY3J1bm5lci5nbw==) | `77.8% <0%> (+5.1%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/istio/fortio/pull/156?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/156?src=pr&el=footer). Last update [48d91e2...20a5c29](https://codecov.io/gh/istio/fortio/pull/156?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

ldemailly: ps: it's not fixing the report case

ldemailly: timing out on review- will merge this now; feel free to comment though


Patch:
diff --git a/Dockerfile b/Dockerfile
index c4e1576..fe859af 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -4,13 +4,13 @@ WORKDIR /go/src/istio.io
 COPY . fortio
 # Submodule handling
 RUN make -C fortio submodule
-# NOTE: changes to this file should be propagated to release/Dockerfile.in too
-# (wtb docker include)
-# Demonstrate moving the static directory outside of the go source tree and
+# Putting spaces in linker replaced variables is hard but does work.
+RUN echo "$(date +'%Y-%m-%d %H:%M') $(cd fortio; git rev-parse HEAD)" > /build-info.txt
+# Sets up the static directory outside of the go source tree and
 # the default data directory to a /var/lib/... volume
 RUN CGO_ENABLED=0 GOOS=linux go build -a -ldflags \
   "-s -X istio.io/fortio/ui.resourcesDir=/usr/local/lib/fortio -X main.defaultDataDir=/var/lib/istio/fortio \
-  -X istio.io/fortio/version.buildInfo=$(date +%y%m%d_%H%M_)$(cd fortio; git rev-parse HEAD) \
+  -X \"istio.io/fortio/version.buildInfo=$(cat /build-info.txt)\" \
   -X istio.io/fortio/version.tag=$(cd fortio; git describe --tags) \
   -X istio.io/fortio/version.gitstatus=$(cd fortio; git status --porcelain | wc -l)" \
   -o fortio.bin istio.io/fortio
@@ -18,6 +18,7 @@ RUN CGO_ENABLED=0 GOOS=linux go build -a -ldflags \
 RUN CGO_ENABLED=0 GOOS=windows go build -a -o fortio.exe istio.io/fortio
 # Minimal image with just the binary and certs
 FROM scratch as release
+# NOTE: the list of files here, if updated, must be changed in release/Dockerfile.in too
 COPY --from=build /etc/ssl/certs/ca-certificates.crt /etc/ssl/certs/
 COPY --from=build /go/src/istio.io/fortio/ui/static /usr/local/lib/fortio/static
 COPY --from=build /go/src/istio.io/fortio/ui/templates /usr/local/lib/fortio/templates
diff --git a/Gopkg.lock b/Gopkg.lock
index 8359d5c..8e6eff8 100644
--- a/Gopkg.lock
+++ b/Gopkg.lock
@@ -91,6 +91,6 @@
 [solve-meta]
   analyzer-name = "dep"
   analyzer-version = 1
-  inputs-digest = "b87fa2e8b347c2db684f38a5bdaa0874d1eff8c7210f75aafc4cce9b8cd4ea08"
+  inputs-digest = "609bfbdf38285567deed0838cd25fb06e5c090268c34b50f2da0873547106580"
   solver-name = "gps-cdcl"
   solver-version = 1
diff --git a/Makefile b/Makefile
index d925766..bad3ce7 100755
--- a/Makefile
+++ b/Makefile
@@ -16,6 +16,7 @@ DOCKER_TAG = $(DOCKER_PREFIX)$(IMAGE):$(TAG)
 
 # go test ./... and others run in vendor/ and cause problems (!)
 PACKAGES:=$(shell find . -type d -print | egrep -v "/(\.|vendor|static|templates|release|docs)")
+#PACKAGES:=$(shell go list ./... | grep -v vendor)
 
 # Marker for whether vendor submodule is here or not already
 GRPC_DIR:=./vendor/google.golang.org/grpc
@@ -41,7 +42,7 @@ LINT_PACKAGES:=$(PACKAGES)
 # TODO: do something about cyclomatic complexity
 # Note CGO_ENABLED=0 is needed to avoid errors as gcc isn't part of the
 # build image
-lint: submodule
+lint: submodule vendor.check
 	docker run -v $(shell pwd):/go/src/istio.io/fortio $(BUILD_IMAGE) bash -c \
 		"cd fortio && time go install $(LINT_PACKAGES) \
 		&& time make local-lint LINT_PACKAGES=\"$(LINT_PACKAGES)\""
@@ -69,6 +70,33 @@ pull:
 	git pull
 	$(MAKE) submodule-sync
 
+# https://github.com/istio/istio/wiki/Vendor-FAQ#how-do-i-add--change-a-dependency
+# PS: for fortio no dependencies should be added, only grpc updated.
+depend.status:
+	@echo "No error means your Gopkg.* are in sync and ok with vendor/"
+	dep status
+	cp Gopkg.* vendor/
+
+depend.update.full: depend.cleanlock depend.update
+
+depend.cleanlock:
+	-rm Gopkg.lock
+
+depend.update:
+	@echo "Running dep ensure with DEPARGS=$(DEPARGS)"
+	time dep ensure $(DEPARGS)
+	cp Gopkg.* vendor/
+	@echo "now check the diff in vendor/ and make a PR"
+
+vendor.check:
+	@echo "Checking that Gopkg.* are in sync with vendor/ submodule:"
+	@echo "if this fails, 'make pull' and/or seek on-call help"
+	diff Gopkg.toml vendor/
+	diff Gopkg.lock vendor/
+
+.PHONY: depend.status depend.cleanlock depend.update depend.update.full vendor.check
+
+
 # Docker: Pushes the combo image and the smaller image(s)
 all: test install lint docker-version docker-push-internal
 	@for img in $(IMAGES); do \
diff --git a/codecov.yml b/codecov.yml
index c70268c..5226ceb 100644
--- a/codecov.yml
+++ b/codecov.yml
@@ -1,6 +1,6 @@
 coverage:
   range: 60..99
-  precision: 1         # how many decimal places to display in the UI: 0 <= value <= 4
-  round: nearest       # how coverage is rounded: down/up/nearest
+  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
     - "**.pb.go"
diff --git a/fhttp/http.go b/fhttp/http.go
index c927baf..988c85a 100644
--- a/fhttp/http.go
+++ b/fhttp/http.go
@@ -1130,6 +1130,7 @@ func closingServer(listener net.Listener) error {
 // 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) {
 	m := http.NewServeMux()
 	s := &http.Server{
@@ -1276,16 +1277,18 @@ func DebugHandler(w http.ResponseWriter, r *http.Request) {
 }
 
 // Serve starts a debug / echo http server on the given port.
-// Returns the port that opened for listing socket
-func Serve(port, debugPath string) int {
+// Returns the 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 {
 	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)
 	}
-	portValue := listener.Addr().(*net.TCPAddr).Port
-	fmt.Printf("Fortio %s echo server listening on port %d\n", version.Short(), portValue)
+	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)
@@ -1295,5 +1298,5 @@ func Serve(port, debugPath string) int {
 			fmt.Println("Error starting server", err)
 		}
 	}()
-	return portValue
+	return addr
 }
diff --git a/ui/uihandler.go b/ui/uihandler.go
index 3342697..e8b76e8 100644
--- a/ui/uihandler.go
+++ b/ui/uihandler.go
@@ -17,6 +17,7 @@ package ui // import "istio.io/fortio/ui"
 
 import (
 	"bytes"
+	"net"
 	// md5 is mandated, not our choice
 	"crypto/md5" // nolint: gas
 	"encoding/base64"
@@ -845,9 +846,9 @@ func downloadOne(w http.ResponseWriter, client *fhttp.Client, name string, u str
 func Serve(baseurl, port, debugpath, uipath, staticRsrcDir string, datadir string) {
 	baseURL = baseurl
 	startTime = time.Now()
-	hostPort := setHostAndPort(fnet.NormalizePort(port))
+	hostPort := fnet.NormalizePort(port)
 	if uipath == "" {
-		fhttp.Serve(hostPort, debugpath) // doesn't return until exit
+		fhttp.Serve(hostPort, debugpath)
 		return
 	}
 	uiPath = uipath
@@ -858,11 +859,6 @@ func Serve(baseurl, port, debugpath, uipath, staticRsrcDir string, datadir strin
 	}
 	debugPath = ".." + debugpath // TODO: calculate actual path if not same number of directories
 	http.HandleFunc(uiPath, Handler)
-	uiMsg := fmt.Sprintf("UI starting - visit:\nhttp://%s%s", urlHostPort, uiPath)
-	if !strings.Contains(port, ":") {
-		uiMsg += "   (or any host/ip reachable on this server)"
-	}
-	fmt.Printf(uiMsg + "\n")
 	fetchPath = uiPath + fetchURI
 	http.HandleFunc(fetchPath, FetcherHandler)
 	fhttp.CheckConnectionClosedHeader = true // needed for proxy to avoid errors
@@ -903,7 +899,14 @@ func Serve(baseurl, port, debugpath, uipath, staticRsrcDir string, datadir strin
 		fs := http.FileServer(http.Dir(dataDir))
 		http.Handle(uiPath+"data/", LogAndFilterDataRequest(http.StripPrefix(uiPath+"data", fs)))
 	}
-	fhttp.Serve(hostPort, debugpath)
+	addr := fhttp.Serve(hostPort, debugpath)
+	setHostAndPort(hostPort, addr)
+	uiMsg := fmt.Sprintf("UI starting - 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.
@@ -912,7 +915,8 @@ func Report(baseurl, port, staticRsrcDir string, datadir string) {
 	http.DefaultServeMux = http.NewServeMux() // drop the pprof default handlers
 	baseURL = baseurl
 	extraBrowseLabel = ", report only limited UI"
-	hostPort := setHostAndPort(fnet.NormalizePort(port))
+	hostPort := fnet.NormalizePort(port)
+	setHostAndPort(hostPort, nil)
 	uiMsg := fmt.Sprintf("Browse only UI starting - visit:\nhttp://%s/", urlHostPort)
 	if !strings.Contains(port, ":") {
 		uiMsg += "   (or any host/ip reachable on this server)"
@@ -968,11 +972,14 @@ func RedirectToHTTPS(port string) {
 
 // setHostAndPort takes hostport in the form of hostname:port, ip:port or :port,
 // sets the urlHostPort variable and returns hostport unmodified.
-func setHostAndPort(hostport string) string {
-	if strings.HasPrefix(hostport, ":") {
-		urlHostPort = "localhost" + hostport
-		return hostport
+func setHostAndPort(inputPort string, addr *net.TCPAddr) {
+	urlHostPort = inputPort
+	portStr := inputPort
+	if addr != nil {
+		urlHostPort = addr.String()
+		portStr = fmt.Sprintf(":%d", addr.Port)
+	}
+	if strings.HasPrefix(inputPort, ":") {
+		urlHostPort = "localhost" + portStr
 	}
-	urlHostPort = hostport
-	return hostport
 }
diff --git a/vendor b/vendor
index 868e377..bff66ee 160000
--- a/vendor
+++ b/vendor
@@ -1,1 +1,1 @@
-Subproject commit 868e377b7e84ee33278d978602634b7e1eb65145
+Subproject commit bff66ee24fe5342cdabea3425c07a4288f54bbf4
diff --git a/version/version.go b/version/version.go
index 672c7a7..a8e9fca 100644
--- a/version/version.go
+++ b/version/version.go
@@ -30,7 +30,7 @@ const (
 
 var (
 	// The following are set by Dockerfile during link time:
-	tag       = "pre"
+	tag       = "n/a"
 	buildInfo = "unknown"
 	// Number of lines in git status --porcelain; 0 means clean
 	gitstatus = "0" // buildInfo default is unknown so no need to add -dirty
@@ -39,29 +39,34 @@ var (
 	longVersion = ""
 )
 
-// Major returns the numerical major version number (first digit of Version()).
+// Major returns the numerical major version number (first digit of version.Short()).
 func Major() int {
 	return major
 }
 
-// Minor returns the numerical minor version number (second digit of Version()).
+// Minor returns the numerical minor version number (second digit of version.Short()).
 func Minor() int {
 	return minor
 }
 
-// Patch returns the numerical patch level (third digit of Version()).
+// Patch returns the numerical patch level (third digit of version.Short()).
 func Patch() int {
 	return patch
 }
 
-// Short returns the 3 digit short version string Major.Minor.Patch[-build]
+// Short returns the 3 digit short version string Major.Minor.Patch[-pre]
 // version.Short() is the overall project version (used to version json
-// output too).
+// output too). "-pre" is added when the version doesn't match exactly
+// a git tag or the build isn't from a clean source tree. (only standard
+// dockerfile based build of a clean, tagged source tree should print "X.Y.Z"
+// as short version).
 func Short() string {
 	return version
 }
 
 // Long returns the full version and build information.
+// Format is "X.Y.X[-pre] YYYY-MM-DD HH:MM SHA[-dirty]" date and time is
+// the build date (UTC), sha is the git sha of the source tree.
 func Long() string {
 	return longVersion
 }
@@ -83,5 +88,5 @@ func init() {
 		buildInfo += "-dirty"
 		log.Debugf("gitstatus is %q, marking buildinfo as dirty: %v", gitstatus, buildInfo)
 	}
-	longVersion = version + "-" + buildInfo
+	longVersion = version + " " + buildInfo
 }


Test patch:
diff --git a/fhttp/httprunner_test.go b/fhttp/httprunner_test.go
index 970fa4c..3193f90 100644
--- a/fhttp/httprunner_test.go
+++ b/fhttp/httprunner_test.go
@@ -114,8 +114,9 @@ func TestHTTPRunnerBadServer(t *testing.T) {
 // the error test for / url above fail:
 
 func TestServe(t *testing.T) {
-	port := Serve("0", "/debugx1")
-	log.Infof("Using port: %d", port)
+	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)
 	if port == 0 {
 		t.Errorf("outport: %d must be different", port)
diff --git a/stats/stats_test.go b/stats/stats_test.go
index be6ef38..64f9b81 100644
--- a/stats/stats_test.go
+++ b/stats/stats_test.go
@@ -765,6 +765,35 @@ func TestBucketLookUp(t *testing.T) {
 	}
 }
 
+func TestAllBucketBoundaries(t *testing.T) {
+	h := NewHistogram(0, 1)
+
+	for i, value := range histogramBucketValues {
+		v := float64(value)
+		h.Reset()
+		h.Record(-1)
+		h.RecordN(v-0.01, 50)
+		h.RecordN(v, 700)       // so first interval gets a count of 750 + 1 for first
+		h.RecordN(v+0.01, 1003) // second interval gets a count of 1003 + 1 for last
+		h.Record(1e6)
+		hData := h.Export()
+		var firstInterval int64
+		if i == 0 {
+			firstInterval = 1 // fist interval is [min, 0[
+		}
+		var lastInterval int64
+		if i == len(histogramBucketValues)-1 {
+			lastInterval = 1
+		}
+		if hData.Data[1-firstInterval].End != v || hData.Data[1-firstInterval].Count != 750+firstInterval {
+			t.Errorf("= Boundary, got %+v unexpectedly for %d %g", hData, value, v)
+		}
+		if hData.Data[2-firstInterval].Start != v || hData.Data[2-firstInterval].Count != 1003+lastInterval {
+			t.Errorf("> Boundary, got %+v unexpectedly for %d %g", hData, value, v)
+		}
+	}
+}
+
 // TODO: add test with data 1.0 1.0001 1.999 2.0 2.5
 // should get 3 buckets 0-1 with count 1
 // 1-2 with count 3