improve histogram plot and plot error propagation (#308)

Author: Tantalor93

pkg/reporter/plot.go

@@ -3,6 +3,7 @@ package reporter
 import (
 	"fmt"
 	"image/color"
+	"math"
 	"os"
 	"sort"
 	"time"
@@ -11,6 +12,7 @@ import (
 	"github.com/montanaflynn/stats"
 	"github.com/tantalor93/dnspyre/v3/pkg/dnsbench"
 	"go-hep.org/x/hep/hplot"
+	"gonum.org/v1/gonum/stat"
 	"gonum.org/v1/plot"
 	"gonum.org/v1/plot/plotter"
 	"gonum.org/v1/plot/plotutil"
@@ -30,14 +32,14 @@ func plotHistogramLatency(file string, times []dnsbench.Datapoint) {
 	p := plot.New()
 	p.Title.Text = "Latencies distribution"
 
-	hist, err := plotter.NewHist(values, 16)
+	hist, err := plotter.NewHist(values, numBins(values))
 	if err != nil {
 		panic(err)
 	}
 	p.X.Label.Text = "Latencies (ms)"
-	p.X.Tick.Marker = hplot.Ticks{N: 3, Format: "%.0f"}
+	p.X.Tick.Marker = hplot.Ticks{N: 5, Format: "%.0f"}
 	p.Y.Label.Text = "Number of requests"
-	p.Y.Tick.Marker = hplot.Ticks{N: 3, Format: "%.0f"}
+	p.Y.Tick.Marker = hplot.Ticks{N: 5, Format: "%.0f"}
 	hist.FillColor = color.RGBA{R: 175, G: 238, B: 238, A: 255}
 	p.Add(hist)
 
@@ -46,6 +48,32 @@ func plotHistogramLatency(file string, times []dnsbench.Datapoint) {
 	}
 }
 
+// numBins calculates number of bins for histogram.
+func numBins(values plotter.Values) int {
+	n := float64(len(values))
+
+	// small dataset
+	if n < 100 {
+		sqrt := math.Sqrt(n)
+		return int(math.Min(15, sqrt))
+	}
+
+	// medium dataset - use Rice's rule
+	if n < 1000 {
+		rice := 2 * math.Cbrt(n)
+		return int(math.Min(30, rice))
+	}
+
+	// large dataset - use Doane's rule
+	// Calculate skewness
+	skewness := stat.Skew(values, nil)
+
+	// Calculate standard error of skewness
+	sigmaG := math.Sqrt(6 * (n - 2) / ((n + 1) * (n + 3)))
+	doane := 1 + math.Log2(n) + math.Log2(1+math.Abs(skewness)/sigmaG)
+	return int(math.Min(50, doane))
+}
+
 func plotBoxPlotLatency(file, server string, times []dnsbench.Datapoint) {
 	if len(times) == 0 {
 		// nothing to plot

pkg/reporter/plot_test.go

@@ -8,6 +8,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"github.com/tantalor93/dnspyre/v3/pkg/dnsbench"
+	"gonum.org/v1/plot/plotter"
 )
 
 var testStart = time.Now()
@@ -130,3 +131,46 @@ func Test_plotErrorRate(t *testing.T) {
 
 	assert.Equal(t, expected, actual, "generated error rate plot does not equal to expected 'test-errorrate-lineplot.png")
 }
+
+func Test_numBins(t *testing.T) {
+	tests := []struct {
+		name   string
+		values plotter.Values
+		want   int
+	}{
+		{
+			name:   "small dataset",
+			values: dataset(25),
+			want:   5,
+		},
+		{
+			name:   "medium dataset",
+			values: dataset(500),
+			want:   15,
+		},
+		{
+			name:   "large dataset",
+			values: dataset(2000),
+			want:   11,
+		},
+		{
+			name:   "single item dataset",
+			values: dataset(1),
+			want:   1,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			assert.Equal(t, tt.want, numBins(tt.values))
+		})
+	}
+}
+
+// dataset generates uniformorly distributed dataset.
+func dataset(len int) plotter.Values {
+	values := make(plotter.Values, len)
+	for i := range values {
+		values[i] = float64(i)
+	}
+	return values
+}

pkg/reporter/report.go

@@ -56,10 +56,14 @@ func PrintReport(b *dnsbench.Benchmark, stats []*dnsbench.ResultStats, benchStar
 	}
 
 	if len(b.PlotDir) != 0 {
+		if err := directoryExists(b.PlotDir); err != nil {
+			return fmt.Errorf("unable to plot results: %w", err)
+		}
+
 		now := time.Now().Format(time.RFC3339)
 		dir := fmt.Sprintf("%s/graphs-%s", b.PlotDir, now)
 		if err := os.Mkdir(dir, os.ModePerm); err != nil {
-			panic(err)
+			return fmt.Errorf("unable to plot results: %w", err)
 		}
 		plotHistogramLatency(fileName(b, dir, "latency-histogram"), totals.Timings)
 		plotBoxPlotLatency(fileName(b, dir, "latency-boxplot"), b.Server, totals.Timings)
@@ -108,6 +112,19 @@ func PrintReport(b *dnsbench.Benchmark, stats []*dnsbench.ResultStats, benchStar
 	return printer(b).print(params)
 }
 
+func directoryExists(plotDir string) error {
+	stat, err := os.Stat(plotDir)
+	if err != nil {
+		if os.IsNotExist(err) {
+			return fmt.Errorf("'%s' path does not point to an existing directory", plotDir)
+		}
+		return err
+	} else if !stat.IsDir() {
+		return fmt.Errorf("'%s' is not a path to a directory", plotDir)
+	}
+	return nil
+}
+
 func printer(b *dnsbench.Benchmark) reportPrinter {
 	switch {
 	case b.JSON:

pkg/reporter/report_test.go

@@ -6,6 +6,8 @@ import (
 	"io"
 	"net"
 	"os"
+	"path/filepath"
+	"strings"
 	"testing"
 	"time"
 
@@ -101,6 +103,55 @@ func Test_PrintReport_errors(t *testing.T) {
 	assert.Equal(t, readResource("errorReport"), buffer.String())
 }
 
+func Test_PrintReport_plot(t *testing.T) {
+	dir := t.TempDir()
+
+	buffer := bytes.Buffer{}
+	b, rs := testReportData(&buffer)
+	b.PlotDir = dir
+	b.PlotFormat = dnsbench.DefaultPlotFormat
+
+	err := reporter.PrintReport(&b, []*dnsbench.ResultStats{&rs}, time.Now(), time.Second)
+
+	require.NoError(t, err)
+
+	testDir, err := os.ReadDir(dir)
+
+	require.NoError(t, err)
+	require.Len(t, testDir, 1)
+
+	graphsDir := testDir[0].Name()
+	assert.True(t, strings.HasPrefix(graphsDir, "graphs-"))
+
+	graphsDirContent, err := os.ReadDir(filepath.Join(dir, graphsDir))
+	require.NoError(t, err)
+
+	var graphFiles []string
+	for _, v := range graphsDirContent {
+		graphFiles = append(graphFiles, v.Name())
+	}
+
+	assert.ElementsMatch(t, graphFiles,
+		[]string{
+			"errorrate-lineplot.svg", "latency-boxplot.svg", "latency-histogram.svg", "latency-lineplot.svg",
+			"responses-barchart.svg", "throughput-lineplot.svg",
+		},
+	)
+}
+
+func Test_PrintReport_plot_error(t *testing.T) {
+	dir := t.TempDir()
+
+	buffer := bytes.Buffer{}
+	b, rs := testReportData(&buffer)
+	b.PlotDir = dir + "/non-existing-directory"
+	b.PlotFormat = dnsbench.DefaultPlotFormat
+
+	err := reporter.PrintReport(&b, []*dnsbench.ResultStats{&rs}, time.Now(), time.Second)
+
+	require.Error(t, err)
+}
+
 func testReportData(testOutputWriter io.Writer) (dnsbench.Benchmark, dnsbench.ResultStats) {
 	b := dnsbench.Benchmark{
 		HistPre: 1,