diff --git a/.gitignore b/.gitignore index c811615b9e..5ec1b2a58d 100644 --- a/.gitignore +++ b/.gitignore @@ -23,4 +23,5 @@ benchmark/.env benchmark/ci/pr/dashboard-screenshots benchmark/ci/pr/table-report benchmark/ci/pr/image-report +benchmark/ci/pr/meta-report /.env diff --git a/benchmark/ci/pr/dangerfile.js b/benchmark/ci/pr/dangerfile.js index 1eeefd8713..88067d62ba 100644 --- a/benchmark/ci/pr/dangerfile.js +++ b/benchmark/ci/pr/dangerfile.js @@ -3,10 +3,11 @@ const fs = require('fs'); const path = require('path'); +const metaReport = fs.readFileSync(path.join(__dirname,"./meta-report")); const tableReport = fs.readFileSync(path.join(__dirname,"./table-report")); const imgReport = fs.readFileSync(path.join(__dirname,"./image-report")); // the markdown calls seem to be LIFO // so it's easier to just use a single call -markdown(`${tableReport} \n${imgReport}`); +markdown(`${metaReport} \n${tableReport} \n${imgReport}`); diff --git a/benchmark/ci/pr/run-benchmark.sh b/benchmark/ci/pr/run-benchmark.sh index f8f370a333..2bf46c6076 100755 --- a/benchmark/ci/pr/run-benchmark.sh +++ b/benchmark/ci/pr/run-benchmark.sh @@ -11,6 +11,17 @@ PUSHGATEWAY_ADDRESS="http://pushgateway:9091" PROMETHEUS_ADDRESS="http://prometheus:9090" GRAFANA_ADDRESS="http://grafana:3000" +# since we gonna report this values in the meta table +# let's define them explicitly here +PYROBENCH_RAND_SEED="${PYROBENCH_RAND_SEED:-2306912}" +PYROBENCH_PROFILE_WIDTH="${PYROBENCH_PROFILE_WIDTH:-20}" +PYROBENCH_PROFILE_DEPTH="${PYROBENCH_PROFILE_DEPTH:-20}" +PYROBENCH_PROFILE_SYMBOL_LENGTH="${PYROBENCH_PROFILE_SYMBOL_LENGTH:-30}" +PYROBENCH_APPS="${PYROBENCH_APPS:-20}" +PYROBENCH_CLIENTS="${PYROBENCH_CLIENTS:-20}" +PYROBENCH_REQUESTS="${PYROBENCH_REQUESTS:-10000}" + + # For more info, check the cli documentation PYROBENCH_UPLOAD_DEST="${PYROBENCH_UPLOAD_DEST:-/screenshots}" PYROBENCH_UPLOAD_BUCKET="${PYROBENCH_UPLOAD_BUCKET:-}" @@ -58,12 +69,28 @@ function run() { docker-compose -p "$PREFIX" up -d --force-recreate --remove-orphans echo "Generating test load" - docker exec "${PREFIX}_client_1" ./pyrobench loadgen \ + docker exec \ + -e "PYROBENCH_RAND_SEED=$PYROBENCH_RAND_SEED" \ + -e "PYROBENCH_PROFILE_WIDTH=$PYROBENCH_PROFILE_WIDTH"\ + -e "PYROBENCH_PROFILE_DEPTH=$PYROBENCH_PROFILE_DEPTH" \ + -e "PYROBENCH_PROFILE_SYMBOL_LENGTH=$PYROBENCH_PROFILE_SYMBOL_LENGTH" \ + -e "PYROBENCH_APPS=$PYROBENCH_APPS" \ + -e "PYROBENCH_CLIENTS=$PYROBENCH_CLIENTS" \ + -e "PYROBENCH_REQUESTS=$PYROBENCH_REQUESTS" \ + "${PREFIX}_client_1" ./pyrobench loadgen \ --log-level=error \ --server-address="$PYROSCOPE_ADDRESS" \ --pushgateway-address="$PUSHGATEWAY_ADDRESS" \ > /dev/null & - docker exec "${PREFIX}_client_1" ./pyrobench loadgen \ + docker exec \ + -e "PYROBENCH_RAND_SEED=$PYROBENCH_RAND_SEED" \ + -e "PYROBENCH_PROFILE_WIDTH=$PYROBENCH_PROFILE_WIDTH"\ + -e "PYROBENCH_PROFILE_DEPTH=$PYROBENCH_PROFILE_DEPTH" \ + -e "PYROBENCH_PROFILE_SYMBOL_LENGTH=$PYROBENCH_PROFILE_SYMBOL_LENGTH" \ + -e "PYROBENCH_APPS=$PYROBENCH_APPS" \ + -e "PYROBENCH_CLIENTS=$PYROBENCH_CLIENTS" \ + -e "PYROBENCH_REQUESTS=$PYROBENCH_REQUESTS" \ + "${PREFIX}_client_1" ./pyrobench loadgen \ --log-level=error \ --server-address="$PYROSCOPE_MAIN_ADDRESS" \ --pushgateway-address="$PUSHGATEWAY_ADDRESS" \ @@ -77,6 +104,19 @@ function run() { end=$(date +%s%3N) # TODO(eh-am): use docker-compose exec + echo "generating meta report" + docker exec \ + "${PREFIX}_client_1" ./pyrobench report meta \ + --params "BENCH_RUN_FOR=$BENCH_RUN_FOR" \ + --params "PYROBENCH_RAND_SEED=$PYROBENCH_RAND_SEED" \ + --params "PYROBENCH_PROFILE_WIDTH=$PYROBENCH_PROFILE_WIDTH"\ + --params "PYROBENCH_PROFILE_DEPTH=$PYROBENCH_PROFILE_DEPTH" \ + --params "PYROBENCH_PROFILE_SYMBOL_LENGTH=$PYROBENCH_PROFILE_SYMBOL_LENGTH" \ + --params "PYROBENCH_APPS=$PYROBENCH_APPS" \ + --params "PYROBENCH_CLIENTS=$PYROBENCH_CLIENTS" \ + --params "PYROBENCH_REQUESTS=$PYROBENCH_REQUESTS" > "$SCRIPT_DIR/meta-report" + + echo "generating image report" docker exec \ -e "AWS_ACCESS_KEY_ID=$AWS_ACCESS_KEY_ID" \ -e "AWS_SECRET_ACCESS_KEY=$AWS_SECRET_ACCESS_KEY" \ @@ -88,6 +128,7 @@ function run() { --to="$end" \ --grafana-address "$GRAFANA_ADDRESS" > "$SCRIPT_DIR/image-report" + echo "generating table report" docker exec \ -e "AWS_ACCESS_KEY_ID=$AWS_ACCESS_KEY_ID" \ -e "AWS_SECRET_ACCESS_KEY=$AWS_SECRET_ACCESS_KEY" \ diff --git a/benchmark/cmd/command/ci-report.go b/benchmark/cmd/command/ci-report.go index 768d1a8d79..ea0db95ad0 100644 --- a/benchmark/cmd/command/ci-report.go +++ b/benchmark/cmd/command/ci-report.go @@ -52,10 +52,42 @@ func newReport(cfg *config.Report) *cobra.Command { }), } + metaReport := &cobra.Command{ + Use: "meta [flags]", + Short: "generates a markdown report to be used by ci", + RunE: cli.CreateCmdRunFn(&cfg.MetaReport, vpr, func(_ *cobra.Command, args []string) error { + setLogLevel(cfg.MetaReport.LogLevel) + + mr, err := cireport.NewMetaReport([]string{ + "BENCH_RUN_FOR", + "PYROBENCH_RAND_SEED", + "PYROBENCH_PROFILE_WIDTH", + "PYROBENCH_PROFILE_DEPTH", + "PYROBENCH_PROFILE_SYMBOL_LENGTH", + "PYROBENCH_APPS", + "PYROBENCH_CLIENTS", + "PYROBENCH_REQUESTS", + }) + if err != nil { + return err + } + + report, err := mr.Report(cfg.MetaReport.Params) + if err != nil { + return err + } + + fmt.Println(report) + return nil + }), + } + report.AddCommand(tableReport) report.AddCommand(imageReport) + report.AddCommand(metaReport) cli.PopulateFlagSet(&cfg.TableReport, tableReport.Flags(), vpr) cli.PopulateFlagSet(&cfg.ImageReport, imageReport.Flags(), vpr) + cli.PopulateFlagSet(&cfg.MetaReport, metaReport.Flags(), vpr) return report } diff --git a/benchmark/internal/cireport/metareport.go b/benchmark/internal/cireport/metareport.go new file mode 100644 index 0000000000..1fd8615303 --- /dev/null +++ b/benchmark/internal/cireport/metareport.go @@ -0,0 +1,116 @@ +package cireport + +import ( + "bytes" + "fmt" + "strings" + "text/template" + + "github.com/sirupsen/logrus" +) + +type metaReport struct { + allowlist map[string]bool +} + +// NewMetaReport creates a meta report +// the allowlist parameter refers to which keys are valid +func NewMetaReport(allowlist []string) (*metaReport, error) { + if len(allowlist) <= 0 { + return nil, fmt.Errorf("at least one item should be allowed") + } + + a := make(map[string]bool) + for _, v := range allowlist { + a[v] = true + } + + return &metaReport{ + allowlist: a, + }, nil +} + +type meta struct { + Key string + Val string +} + +// Report generates a markdown report +func (mr *metaReport) Report(vars []string) (string, error) { + if len(vars) <= 0 { + return "", fmt.Errorf("at least one item should be reported") + } + + // transform 'A=B' into {key: A, val: B} + m := make([]meta, 0, len(vars)) + for _, v := range vars { + key, val, err := mr.breakOnEqual(v) + logrus.Debugf("breaking string %s on '=' produces key %s value %s\n", v, key, val) + if err != nil { + return "", err + } + + n := meta{key, val} + m = append(m, n) + } + + // validate it's in the allowlist + logrus.Debug("validating there're no values not in the allowlist") + err := mr.validate(m) + if err != nil { + return "", err + } + + logrus.Debug("generating template") + report, err := mr.template(m) + if err != nil { + return "", err + } + + return report, nil +} + +func (mr *metaReport) breakOnEqual(s string) (string, string, error) { + split := strings.Split(s, "=") + if len(split) != 2 { + return "", "", fmt.Errorf("expect value in format A=B") + } + + if split[0] == "" || split[1] == "" { + return "", "", fmt.Errorf("expect non empty key/value") + } + + return split[0], split[1], nil +} + +func (mr *metaReport) validate(m []meta) error { + for _, v := range m { + if _, ok := mr.allowlist[v.Key]; !ok { + return fmt.Errorf("key is not allowed: '%s'", v.Key) + } + } + + return nil +} + +func (mr *metaReport) template(m []meta) (string, error) { + var tpl bytes.Buffer + + data := struct { + Meta []meta + }{ + Meta: m, + } + t, err := template.New("meta-report.gotpl"). + ParseFS(resources, "resources/meta-report.gotpl") + + if err != nil { + return "", err + } + + if err := t.Execute(&tpl, data); err != nil { + return "", err + } + + return tpl.String(), nil +} diff --git a/benchmark/internal/cireport/metareport_test.go b/benchmark/internal/cireport/metareport_test.go new file mode 100644 index 0000000000..032b352854 --- /dev/null +++ b/benchmark/internal/cireport/metareport_test.go @@ -0,0 +1,62 @@ +package cireport_test + +import ( + "github.com/pyroscope-io/pyroscope/benchmark/internal/cireport" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + "github.com/tommy351/goldga" +) + +var _ = Describe("metareport", func() { + It("generates a markdown report correctly", func() { + mr, err := cireport.NewMetaReport([]string{"execution", "seed"}) + Expect(err).NotTo(HaveOccurred()) + + report, err := mr.Report([]string{"execution=5m", "seed=4"}) + Expect(err).ToNot(HaveOccurred()) + Expect(report).To(goldga.Match()) + }) + + Context("error conditions", func() { + It("should fail when there are no elements allowed", func() { + mr, err := cireport.NewMetaReport([]string{}) + + Expect(err).To(HaveOccurred()) + Expect(mr).To(BeNil()) + }) + + It("should fail when no element is asked to be reported", func() { + mr, err := cireport.NewMetaReport([]string{"allowed"}) + Expect(err).NotTo(HaveOccurred()) + + _, err = mr.Report([]string{}) + Expect(err).To(HaveOccurred()) + }) + + It("should fail when an element doesn't pass the allowlist", func() { + mr, err := cireport.NewMetaReport([]string{"allowed"}) + Expect(err).NotTo(HaveOccurred()) + + _, err = mr.Report([]string{"A=B"}) + Expect(err).To(HaveOccurred()) + }) + + It("only accepts variables in the format A=B", func() { + mr, err := cireport.NewMetaReport([]string{"A"}) + Expect(err).NotTo(HaveOccurred()) + + _, err = mr.Report([]string{"A"}) + Expect(err).To(HaveOccurred()) + + _, err = mr.Report([]string{"A="}) + Expect(err).To(HaveOccurred()) + + _, err = mr.Report([]string{"=B"}) + Expect(err).To(HaveOccurred()) + + _, err = mr.Report([]string{"A=B"}) + Expect(err).ToNot(HaveOccurred()) + }) + }) +}) diff --git a/benchmark/internal/cireport/resources/image-report.gotpl b/benchmark/internal/cireport/resources/image-report.gotpl index 308f1eb427..cfb28cb228 100644 --- a/benchmark/internal/cireport/resources/image-report.gotpl +++ b/benchmark/internal/cireport/resources/image-report.gotpl @@ -1,4 +1,4 @@ -# Benchmark Screenshots +## Screenshots {{ $root := . }} {{ range .Panels }} diff --git a/benchmark/internal/cireport/resources/meta-report.gotpl b/benchmark/internal/cireport/resources/meta-report.gotpl new file mode 100644 index 0000000000..17dfe8da93 --- /dev/null +++ b/benchmark/internal/cireport/resources/meta-report.gotpl @@ -0,0 +1,14 @@ +## Parameters + +
+ Details + + +| Name | Value | +|-------------|------------| +{{ range .Meta -}} +| `{{ .Key }}` | `{{ .Val }}` | +{{ end -}} + + +
diff --git a/benchmark/internal/cireport/resources/pr.gotpl b/benchmark/internal/cireport/resources/pr.gotpl index f3413d69ad..5d28600633 100644 --- a/benchmark/internal/cireport/resources/pr.gotpl +++ b/benchmark/internal/cireport/resources/pr.gotpl @@ -1,4 +1,4 @@ -# Benchmark Result +## Result | | {{ .QC.BaseName }} | {{ .QC.TargetName }} | diff | threshold | |-------------|------------------------------|----------------------|---------------------|----------------| {{ range .QC.Queries -}} diff --git a/benchmark/internal/cireport/testdata/metareport.golden b/benchmark/internal/cireport/testdata/metareport.golden new file mode 100644 index 0000000000..8fb1eb68d7 --- /dev/null +++ b/benchmark/internal/cireport/testdata/metareport.golden @@ -0,0 +1,5 @@ +# Generated by goldga. DO NOT EDIT. +[snapshots] +"metareport generates a markdown report correctly" = ''' +(string) (len=165) "## Parameters\n\n
\n Details\n\n\n| Name | Value |\n|-------------|------------|\n| `execution` | `5m` |\n| `seed` | `4` |\n
\n" +''' diff --git a/benchmark/internal/cireport/testdata/tablereport.golden b/benchmark/internal/cireport/testdata/tablereport.golden index 6d52280830..f882a7f6fb 100644 --- a/benchmark/internal/cireport/testdata/tablereport.golden +++ b/benchmark/internal/cireport/testdata/tablereport.golden @@ -1,11 +1,11 @@ # Generated by goldga. DO NOT EDIT. [snapshots] "tablereport diff bigger than threshold should generate a report correctly" = ''' -(string) (len=547) "# Benchmark Result\n| | base name | target name | diff | threshold |\n|-------------|------------------------------|----------------------|---------------------|----------------|\n| my name | 0.00 | 2.00 | :green_square: 2.00 (200.00%) | 5% |\n\n\n\n
\n Details\n\n\n| Name | Description | Query for base name | Query for target name |\n|----------|-------------|----------------------|----------------------|\n| my name | my description | `query_base` | `query_target` |\n\n\n
\n" +(string) (len=538) "## Result\n| | base name | target name | diff | threshold |\n|-------------|------------------------------|----------------------|---------------------|----------------|\n| my name | 0.00 | 2.00 | :green_square: 2.00 (200.00%) | 5% |\n\n\n\n
\n Details\n\n\n| Name | Description | Query for base name | Query for target name |\n|----------|-------------|----------------------|----------------------|\n| my name | my description | `query_base` | `query_target` |\n\n\n
\n" ''' "tablereport diff smaller than threshold should generate a report correctly" = ''' -(string) (len=547) "# Benchmark Result\n| | base name | target name | diff | threshold |\n|-------------|------------------------------|----------------------|---------------------|----------------|\n| my name | 2.00 | 0.00 | :red_square: -2.00 (-200.00%) | 5% |\n\n\n\n
\n Details\n\n\n| Name | Description | Query for base name | Query for target name |\n|----------|-------------|----------------------|----------------------|\n| my name | my description | `query_base` | `query_target` |\n\n\n
\n" +(string) (len=538) "## Result\n| | base name | target name | diff | threshold |\n|-------------|------------------------------|----------------------|---------------------|----------------|\n| my name | 2.00 | 0.00 | :red_square: -2.00 (-200.00%) | 5% |\n\n\n\n
\n Details\n\n\n| Name | Description | Query for base name | Query for target name |\n|----------|-------------|----------------------|----------------------|\n| my name | my description | `query_base` | `query_target` |\n\n\n
\n" ''' "tablereport diff within threshold should generate a report correctly" = ''' -(string) (len=530) "# Benchmark Result\n| | base name | target name | diff | threshold |\n|-------------|------------------------------|----------------------|---------------------|----------------|\n| my name | 1.00 | 1.00 | 0.00 (0.00%) | 5% |\n\n\n\n
\n Details\n\n\n| Name | Description | Query for base name | Query for target name |\n|----------|-------------|----------------------|----------------------|\n| my name | my description | `query_base` | `query_target` |\n\n\n
\n" +(string) (len=521) "## Result\n| | base name | target name | diff | threshold |\n|-------------|------------------------------|----------------------|---------------------|----------------|\n| my name | 1.00 | 1.00 | 0.00 (0.00%) | 5% |\n\n\n\n
\n Details\n\n\n| Name | Description | Query for base name | Query for target name |\n|----------|-------------|----------------------|----------------------|\n| my name | my description | `query_base` | `query_target` |\n\n\n
\n" ''' diff --git a/benchmark/internal/config/config.go b/benchmark/internal/config/config.go index defa0e32bf..2dd4b4ed9d 100644 --- a/benchmark/internal/config/config.go +++ b/benchmark/internal/config/config.go @@ -31,6 +31,7 @@ type PromQuery struct { type Report struct { TableReport ImageReport + MetaReport } type TableReport struct { @@ -51,6 +52,11 @@ type ImageReport struct { To int `def:"0" desc:"timestamp"` } +type MetaReport struct { + LogLevel string `def:"info" desc:"log level: debug|info|warn|error" mapstructure:"log-level"` + Params []string `def:"" desc:"the parameters in format A=B. value must be in the allowlist"` +} + type DashboardScreenshot struct { LogLevel string `def:"info" desc:"log level: debug|info|warn|error" mapstructure:"log-level"` TimeoutSeconds int `def:"300" desc:"timeout in seconds of each call"` diff --git a/pkg/cli/flags.go b/pkg/cli/flags.go index 52a974b2db..ddee0bc9eb 100644 --- a/pkg/cli/flags.go +++ b/pkg/cli/flags.go @@ -28,7 +28,7 @@ func (i *arrayFlags) String() string { if len(*i) == 0 { return "[]" } - return strings.Join(*i, ", ") + return strings.Join(*i, ",") } func (i *arrayFlags) Set(value string) error {