Repository: fortio/fortio

Issue:
Strange bucketing with low `-r` value
My test:
```
fortio load -uniform=true -qps 100 -t 1s -c 100 -payload-size 0 -httpccch -json /tmp/results/fortio.json -access-log-file /dev/stdout  -r 0.000001 echo?delay=0.5s
```

My server waits 500ms and returns. The p50 and p99 should both be ~500ms.

Depending on the `-r` value, I get different results for p50:

* unset: 500ms
* 0.0001: 500ms
* 0.00001: 500ms
* 0.000001: 100ms

Here is the result on the last one:

```json
{
  "Count": 100,
  "Min": 0.501253173,
  "Max": 0.502924856,
  "Sum": 50.19715515699999,
  "Avg": 0.50197155157,
  "StdDev": 0.00035887444724468163,
  "Data": [
    {
      "Start": 0.09999999999999999,
      "End": 0.502924856,
      "Percent": 100,
      "Count": 100
    }
  ],
  "Percentiles": [
    {
      "Percentile": 50,
      "Value": 0.29942745397979803
    },
    {
      "Percentile": 75,
      "Value": 0.401176154989899
    },
    {
      "Percentile": 90,
      "Value": 0.4622253755959596
    },
    {
      "Percentile": 99,
      "Value": 0.49885490795959603
    },
    {
      "Percentile": 99.9,
      "Value": 0.5025178611959596
    }
  ]
}
```

Hints from issue comments:
@ldemailly: in that json all the data is in a single bucket which means there isn’t much to figure out pXX

I suppose that in addition to complaining maybe it should use min/max instead of bucket start and end (bucket end is already max)?@howardjohn: With the other `-r` values the bucket looks like this btw:

```
    "Data": [
      {
        "Start": 0.500304758,
        "End": 0.501782939,
        "Percent": 100,
        "Count": 100
      }
```

Which seems more expected. I am not sure how we end up with a single bucket for  0.09999999999999999 to 0.502924856@ldemailly: indeed that’s better. is it reproducible? (the one that doesn’t start at min)@howardjohn: yep, looks like this will trigger it:

```
fortio load -qps 1 -t 1s -c 1 -json /tmp/results/fortio.json -r 0.000001 httpbingo.org/delay/0.5
```@ldemailly: I'm writing a unit test and it's reproducible, the issue is that  1/1Mth is supposed to be for latencies of ~ 1/100_000 or nanoseconds and you have 500ms so it's... very wrong setting ... but yet it shouldnt bug out (maybe I can reject user errors like that because you wouldn't get a useful histogram with that value even if your system was very very fast)

@ldemailly: btw if you know all/most of your data is above some minimum duration,  use  -offset duration

also use  "http://localhost:8080/?delay=0.5s" instead of httpbingo.org :)

eg
```
fortio server &
fortio load -qps 10  -json - -offset 501ms -r .0001 "localhost:8080/?delay=0.5s"
```
shows
```json
    "Data": [
      {
        "Start": 0.500643167,
        "End": 0.5007,
        "Percent": 2.5,
        "Count": 1
      },
      {
        "Start": 0.5007,
        "End": 0.5008,
        "Percent": 7.5,
        "Count": 2
      },
      {
        "Start": 0.5008,
        "End": 0.5009,
        "Percent": 15,
        "Count": 3
      },
      {
        "Start": 0.5009,
        "End": 0.501,
        "Percent": 30,
        "Count": 6
      },
      {
        "Start": 0.501,
        "End": 0.5011,
        "Percent": 32.5,
        "Count": 1
      },
      {
        "Start": 0.5012,
        "End": 0.5014,
        "Percent": 35,
        "Count": 1
      },
      {
        "Start": 0.5014,
        "End": 0.5016,
        "Percent": 75,
        "Count": 16
      },
      {
        "Start": 0.5016,
        "End": 0.5018,
        "Percent": 97.5,
        "Count": 9
      },
      {
        "Start": 0.502,
        "End": 0.502402458,
        "Percent": 100,
        "Count": 1
      }
    ],
```
and thus more useful pXX

@ldemailly: Thanks for the bug report, indeed a bug, please review the fix #1081 
(yet also consider that the value you pass defeat the histogram, I am wondering if when all the data is in 1 bucket I should just warning or error)

Solution patch:
diff --git a/README.md b/README.md
index 401443b..a630534 100644
--- a/README.md
+++ b/README.md
@@ -1,4 +1,4 @@
-<!-- 1.71.0 -->
+<!-- 1.71.1 -->
 # Fortio
 
 [![Awesome Go](https://fortio.org/mentioned-badge.svg)](https://github.com/avelino/awesome-go#networking)
@@ -59,13 +59,13 @@ You can install from source:
 The [releases](https://github.com/fortio/fortio/releases) page has binaries for many OS/architecture combinations (see assets):
 
 ```shell
-curl -L https://github.com/fortio/fortio/releases/download/v1.71.0/fortio-linux_amd64-1.71.0.tgz \
+curl -L https://github.com/fortio/fortio/releases/download/v1.71.1/fortio-linux_amd64-1.71.1.tgz \
  | sudo tar -C / -xvzpf -
 # or the debian package
-wget https://github.com/fortio/fortio/releases/download/v1.71.0/fortio_1.71.0_amd64.deb
-dpkg -i fortio_1.71.0_amd64.deb
+wget https://github.com/fortio/fortio/releases/download/v1.71.1/fortio_1.71.1_amd64.deb
+dpkg -i fortio_1.71.1_amd64.deb
 # or the rpm
-rpm -i https://github.com/fortio/fortio/releases/download/v1.71.0/fortio-1.71.0-1.x86_64.rpm
+rpm -i https://github.com/fortio/fortio/releases/download/v1.71.1/fortio-1.71.1-1.x86_64.rpm
 # and more, see assets in release page
 ```
 
@@ -75,7 +75,7 @@ On macOS you can also install Fortio using [Homebrew](https://brew.sh/):
 brew install fortio
 ```
 
-On Windows, download https://github.com/fortio/fortio/releases/download/v1.71.0/fortio_win_1.71.0.zip and extract `fortio.exe` to any location, then using the Windows Command Prompt:
+On Windows, download https://github.com/fortio/fortio/releases/download/v1.71.1/fortio_win_1.71.1.zip and extract `fortio.exe` to any location, then using the Windows Command Prompt:
 ```
 fortio.exe server
 ```
@@ -124,7 +124,7 @@ Most important flags for HTTP load generation:
 | `-n numcalls` | Run for exactly this number of calls instead of duration. Default (0) is to use duration (-t). |
 | `-payload str` or `-payload-file fname` | Switch to using POST with the given payload (see also `-payload-size` for random payload)|
 | `-uniform` | Spread the calls in time across threads for a more uniform call distribution. Works even better in conjunction with `-nocatchup`. |
-| `-r resolution` | Resolution of the histogram lowest buckets in seconds (default 0.001 i.e, 1ms), use 1/10th of your expected typical latency |
+| `-r resolution` | Resolution of the histogram lowest buckets in seconds (default 0.001 i.e, 1ms), use 1/10th of your expected typical latency (and -offset if needed)|
 | `-H "header: value"` | Can be specified multiple times to add headers (including Host:) |
 | `-a`     |  Automatically save JSON result with filename based on labels and timestamp |
 | `-json filename` | Filename or `-` for stdout to output JSON result (relative to `-data-dir` by default, should end with .json if you want `fortio report` to show them; using `-a` is typically a better option)|
@@ -141,7 +141,7 @@ Full list of command line flags (`fortio help`):
 <!-- use release/updateFlags.sh to update this section -->
 <pre>
 <!-- USAGE_START -->
-Φορτίο 1.71.0 usage:
+Φορτίο 1.71.1 usage:
         fortio command [flags] target
 where command is one of: load (load testing), server (starts ui, rest api,
  http-echo, redirect, proxies, tcp-echo, udp-echo and grpc ping servers),
diff --git a/stats/stats.go b/stats/stats.go
index 5e05af4..bd9a639 100644
--- a/stats/stats.go
+++ b/stats/stats.go
@@ -379,7 +379,6 @@ func (h *Histogram) Export() *HistogramData {
 			prev = cur
 		} else {
 			// Last Entry
-			b.Start = multiplier*float64(prev) + offset
 			b.End = h.Max
 		}
 		b.Count = int64(h.Hdata[i])



Test patch:
diff --git a/stats/stats_test.go b/stats/stats_test.go
index 696699f..5630dbf 100644
--- a/stats/stats_test.go
+++ b/stats/stats_test.go
@@ -183,6 +183,15 @@ func TestHistogram(t *testing.T) {
 	}
 }
 
+const (
+	roundingPrecision = 100000
+)
+
+// roundFloat rounds a float64 to 5 decimal places.
+func roundFloat(f float64) float64 {
+	return float64(int64(f*roundingPrecision+0.5)) / roundingPrecision
+}
+
 func TestPercentiles1(t *testing.T) {
 	h := NewHistogram(0, 10)
 	h.Record(10)
@@ -218,7 +227,56 @@ func TestPercentiles1(t *testing.T) {
 		{e.CalcPercentile(101), 30, "p101"},
 	}
 	for _, tst := range tests {
-		actualRounded := float64(int64(tst.actual*100000+0.5)) / 100000.
+		actualRounded := roundFloat(tst.actual)
+		if actualRounded != tst.expected {
+			t.Errorf("%s: got %g (%g), not as expected %g", tst.msg, actualRounded, tst.actual, tst.expected)
+		}
+	}
+}
+
+func TestPercentilesIssue1080(t *testing.T) {
+	h := NewHistogram(0, 0.000001)
+	h.Record(500.1)
+	h.Record(500.2)
+	h.Record(500.3)
+	h.Record(500.4)
+	minValue := 500.1
+	maxValue := 500.4
+	h.Print(os.Stdout, "TestPercentilesIssue1080", []float64{50})
+	e := h.Export()
+	b, err := json.MarshalIndent(e.CalcPercentiles([]float64{50}), "", "  ")
+	if err != nil {
+		t.Fatalf("Unable to create Json: %v", err)
+	}
+	fmt.Println("TestPercentilesIssue1080 json:")
+	os.Stdout.Write(b)
+	fmt.Println()
+	for i := 0; i <= 100; i += 10 {
+		v := e.CalcPercentile(float64(i))
+		fmt.Printf("%d%% at %g\n", i, v)
+		if v < minValue || v > maxValue {
+			t.Errorf("percentile %d: got %g, not in range [%g, %g]", i, v, minValue, maxValue)
+		}
+	}
+	tests := []struct {
+		actual   float64
+		expected float64
+		msg      string
+	}{
+		{h.Avg(), 500.25, "avg"},
+		{e.CalcPercentile(-1), 500.1, "p-1"}, // not valid but should return min
+		{e.CalcPercentile(0), 500.1, "p0"},
+		{e.CalcPercentile(0.1), 500.1, "p0.1"},
+		{e.CalcPercentile(1), 500.1, "p1"},
+		{e.CalcPercentile(50), 500.2, "p50"},
+		{e.CalcPercentile(100), 500.4, "p100"},
+		{e.CalcPercentile(101), 500.4, "p101"},
+	}
+	for _, tst := range tests {
+		if tst.actual < minValue || tst.actual > maxValue {
+			t.Errorf("%s: got %g, not in range [%g, %g]", tst.msg, tst.actual, minValue, maxValue)
+		}
+		actualRounded := roundFloat(tst.actual)
 		if actualRounded != tst.expected {
 			t.Errorf("%s: got %g (%g), not as expected %g", tst.msg, actualRounded, tst.actual, tst.expected)
 		}