From 78c14b8b0da0f5e85d008f0adf1b203f2d6137e6 Mon Sep 17 00:00:00 2001 From: Chris Berkhout Date: Thu, 12 Oct 2023 16:16:36 +0200 Subject: [PATCH 1/4] Remove redundant setting of configPath. --- internal/benchrunner/runners/system/scenario.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/benchrunner/runners/system/scenario.go b/internal/benchrunner/runners/system/scenario.go index b4df352734..415d439852 100644 --- a/internal/benchrunner/runners/system/scenario.go +++ b/internal/benchrunner/runners/system/scenario.go @@ -97,7 +97,6 @@ func readConfig(path, scenario string, ctxt servicedeployer.ServiceContext) (*sc cfg, err := yaml.NewConfig(data, ucfg.PathSep(".")) if err != nil { if errors.Is(err, os.ErrNotExist) { - configPath = filepath.Join(path, devPath, fmt.Sprintf("%s.yaml", scenario)) cfg, err = yaml.NewConfigWithFile(configPath) } if err != nil { From e23935d45a8001d2893fb4dad123ffd9744fd52c Mon Sep 17 00:00:00 2001 From: Chris Berkhout Date: Thu, 12 Oct 2023 16:15:53 +0200 Subject: [PATCH 2/4] Add --path option. --- cmd/benchmark.go | 7 +++++ .../benchrunner/runners/system/options.go | 7 +++++ internal/benchrunner/runners/system/runner.go | 31 ++++++++++++++++--- .../benchrunner/runners/system/scenario.go | 9 ++++-- internal/cobraext/flags.go | 3 ++ 5 files changed, 50 insertions(+), 7 deletions(-) diff --git a/cmd/benchmark.go b/cmd/benchmark.go index b916eff74a..4a270a91f2 100644 --- a/cmd/benchmark.go +++ b/cmd/benchmark.go @@ -527,6 +527,7 @@ func getSystemCommand() *cobra.Command { RunE: systemCommandAction, } + cmd.Flags().StringP(cobraext.BenchPathFlagName, "", "", cobraext.BenchPathFlagDescription) cmd.Flags().StringP(cobraext.BenchNameFlagName, "", "", cobraext.BenchNameFlagDescription) cmd.Flags().BoolP(cobraext.BenchReindexToMetricstoreFlagName, "", false, cobraext.BenchReindexToMetricstoreFlagDescription) cmd.Flags().DurationP(cobraext.BenchMetricsIntervalFlagName, "", time.Second, cobraext.BenchMetricsIntervalFlagDescription) @@ -544,6 +545,11 @@ func systemCommandAction(cmd *cobra.Command, args []string) error { return cobraext.FlagParsingError(err, cobraext.VariantFlagName) } + benchPath, err := cmd.Flags().GetString(cobraext.BenchPathFlagName) + if err != nil { + return cobraext.FlagParsingError(err, cobraext.BenchPathFlagName) + } + benchName, err := cmd.Flags().GetString(cobraext.BenchNameFlagName) if err != nil { return cobraext.FlagParsingError(err, cobraext.BenchNameFlagName) @@ -595,6 +601,7 @@ func systemCommandAction(cmd *cobra.Command, args []string) error { withOpts := []system.OptionFunc{ system.WithVariant(variant), + system.WithBenchmarkPath(benchPath), system.WithBenchmarkName(benchName), system.WithDeferCleanup(deferCleanup), system.WithMetricsInterval(metricsInterval), diff --git a/internal/benchrunner/runners/system/options.go b/internal/benchrunner/runners/system/options.go index e719a027c7..7fe2d34d54 100644 --- a/internal/benchrunner/runners/system/options.go +++ b/internal/benchrunner/runners/system/options.go @@ -20,6 +20,7 @@ type Options struct { MetricsInterval time.Duration ReindexData bool ESMetricsAPI *elasticsearch.API + BenchPath string BenchName string PackageRootPath string Variant string @@ -54,6 +55,12 @@ func WithPackageRootPath(path string) OptionFunc { } } +func WithBenchmarkPath(path string) OptionFunc { + return func(opts *Options) { + opts.BenchPath = path + } +} + func WithBenchmarkName(name string) OptionFunc { return func(opts *Options) { opts.BenchName = name diff --git a/internal/benchrunner/runners/system/runner.go b/internal/benchrunner/runners/system/runner.go index 44119e9fe3..de8c16e09c 100644 --- a/internal/benchrunner/runners/system/runner.go +++ b/internal/benchrunner/runners/system/runner.go @@ -40,7 +40,7 @@ const ( // ServiceLogsAgentDir is folder path where log files produced by the service // are stored on the Agent container's filesystem. ServiceLogsAgentDir = "/tmp/service_logs" - devDeployDir = "_dev/benchmark/system/deploy" + defaultDevDeployDir = "_dev/benchmark/system/deploy" // BenchType defining system benchmark BenchType benchrunner.Type = "system" @@ -145,7 +145,7 @@ func (r *runner) setUp() error { } r.ctxt.OutputDir = outputDir - scenario, err := readConfig(r.options.PackageRootPath, r.options.BenchName, r.ctxt) + scenario, err := readConfig(r.options.PackageRootPath, r.options.BenchPath, r.options.BenchName, r.ctxt) if err != nil { return err } @@ -237,6 +237,12 @@ func (r *runner) run() (report reporters.Reportable, err error) { // Setup service. logger.Debug("setting up service...") + var devDeployDir string + if r.options.BenchPath != "" { + devDeployDir = filepath.Clean(filepath.Join(r.options.BenchPath, "deploy")) + } else { + devDeployDir = defaultDevDeployDir + } opts := servicedeployer.FactoryOptions{ PackageRootPath: r.options.PackageRootPath, DevDeployDir: devDeployDir, @@ -484,7 +490,12 @@ func (r *runner) getGeneratorConfig() (*config.Config, error) { ) if r.scenario.Corpora.Generator.Config.Path != "" { - configPath := filepath.Clean(filepath.Join(devPath, r.scenario.Corpora.Generator.Config.Path)) + var configPath string + if r.options.BenchPath != "" { + configPath = filepath.Clean(filepath.Join(r.options.BenchPath, r.scenario.Corpora.Generator.Config.Path)) + } else { + configPath = filepath.Clean(filepath.Join(devPath, r.scenario.Corpora.Generator.Config.Path)) + } configPath = os.ExpandEnv(configPath) if _, err := os.Stat(configPath); err != nil { return nil, fmt.Errorf("can't find config file %s: %w", configPath, err) @@ -515,7 +526,12 @@ func (r *runner) getGeneratorFields() (fields.Fields, error) { ) if r.scenario.Corpora.Generator.Fields.Path != "" { - fieldsPath := filepath.Clean(filepath.Join(devPath, r.scenario.Corpora.Generator.Fields.Path)) + var fieldsPath string + if r.options.BenchPath != "" { + fieldsPath = filepath.Clean(filepath.Join(r.options.BenchPath, r.scenario.Corpora.Generator.Config.Path)) + } else { + fieldsPath = filepath.Clean(filepath.Join(devPath, r.scenario.Corpora.Generator.Fields.Path)) + } fieldsPath = os.ExpandEnv(fieldsPath) if _, err := os.Stat(fieldsPath); err != nil { return nil, fmt.Errorf("can't find fields file %s: %w", fieldsPath, err) @@ -547,7 +563,12 @@ func (r *runner) getGeneratorTemplate() ([]byte, error) { ) if r.scenario.Corpora.Generator.Template.Path != "" { - tplPath := filepath.Clean(filepath.Join(devPath, r.scenario.Corpora.Generator.Template.Path)) + var tplPath string + if r.options.BenchPath != "" { + tplPath = filepath.Clean(filepath.Join(r.options.BenchPath, r.scenario.Corpora.Generator.Template.Path)) + } else { + tplPath = filepath.Clean(filepath.Join(devPath, r.scenario.Corpora.Generator.Template.Path)) + } tplPath = os.ExpandEnv(tplPath) if _, err := os.Stat(tplPath); err != nil { return nil, fmt.Errorf("can't find template file %s: %w", tplPath, err) diff --git a/internal/benchrunner/runners/system/scenario.go b/internal/benchrunner/runners/system/scenario.go index 415d439852..9a4e3a1571 100644 --- a/internal/benchrunner/runners/system/scenario.go +++ b/internal/benchrunner/runners/system/scenario.go @@ -79,8 +79,13 @@ func defaultConfig() *scenario { } } -func readConfig(path, scenario string, ctxt servicedeployer.ServiceContext) (*scenario, error) { - configPath := filepath.Join(path, devPath, fmt.Sprintf("%s.yml", scenario)) +func readConfig(path, benchPath string, scenario string, ctxt servicedeployer.ServiceContext) (*scenario, error) { + var configPath string + if benchPath != "" { + configPath = filepath.Join(benchPath, fmt.Sprintf("%s.yml", scenario)) + } else { + configPath = filepath.Join(path, devPath, fmt.Sprintf("%s.yml", scenario)) + } data, err := os.ReadFile(configPath) if err != nil { if errors.Is(err, os.ErrNotExist) { diff --git a/internal/cobraext/flags.go b/internal/cobraext/flags.go index 3223487cde..4cd1580e08 100644 --- a/internal/cobraext/flags.go +++ b/internal/cobraext/flags.go @@ -29,6 +29,9 @@ const ( AllowSnapshotFlagName = "allow-snapshot" AllowSnapshotDescription = "allow to export dashboards from a Elastic stack SNAPSHOT version" + BenchPathFlagName = "path" + BenchPathFlagDescription = "path of the benchmark scenario to run" + BenchNameFlagName = "benchmark" BenchNameFlagDescription = "name of the benchmark scenario to run" From aee2b29a206f336214acf6c0f6674e3ca4b54d3d Mon Sep 17 00:00:00 2001 From: Chris Berkhout Date: Tue, 19 Dec 2023 11:55:26 +0100 Subject: [PATCH 3/4] Remove duplication in default vs custom benchmark scenario path handling. --- cmd/benchmark.go | 2 +- internal/benchrunner/runners/system/runner.go | 31 +++---------------- .../benchrunner/runners/system/scenario.go | 11 ++----- 3 files changed, 8 insertions(+), 36 deletions(-) diff --git a/cmd/benchmark.go b/cmd/benchmark.go index 4a270a91f2..4364ebaab9 100644 --- a/cmd/benchmark.go +++ b/cmd/benchmark.go @@ -527,7 +527,7 @@ func getSystemCommand() *cobra.Command { RunE: systemCommandAction, } - cmd.Flags().StringP(cobraext.BenchPathFlagName, "", "", cobraext.BenchPathFlagDescription) + cmd.Flags().StringP(cobraext.BenchPathFlagName, "", "_dev/benchmark/system", cobraext.BenchPathFlagDescription) cmd.Flags().StringP(cobraext.BenchNameFlagName, "", "", cobraext.BenchNameFlagDescription) cmd.Flags().BoolP(cobraext.BenchReindexToMetricstoreFlagName, "", false, cobraext.BenchReindexToMetricstoreFlagDescription) cmd.Flags().DurationP(cobraext.BenchMetricsIntervalFlagName, "", time.Second, cobraext.BenchMetricsIntervalFlagDescription) diff --git a/internal/benchrunner/runners/system/runner.go b/internal/benchrunner/runners/system/runner.go index de8c16e09c..6fd411d6d1 100644 --- a/internal/benchrunner/runners/system/runner.go +++ b/internal/benchrunner/runners/system/runner.go @@ -40,7 +40,6 @@ const ( // ServiceLogsAgentDir is folder path where log files produced by the service // are stored on the Agent container's filesystem. ServiceLogsAgentDir = "/tmp/service_logs" - defaultDevDeployDir = "_dev/benchmark/system/deploy" // BenchType defining system benchmark BenchType benchrunner.Type = "system" @@ -145,7 +144,7 @@ func (r *runner) setUp() error { } r.ctxt.OutputDir = outputDir - scenario, err := readConfig(r.options.PackageRootPath, r.options.BenchPath, r.options.BenchName, r.ctxt) + scenario, err := readConfig(r.options.BenchPath, r.options.BenchName, r.ctxt) if err != nil { return err } @@ -237,12 +236,7 @@ func (r *runner) run() (report reporters.Reportable, err error) { // Setup service. logger.Debug("setting up service...") - var devDeployDir string - if r.options.BenchPath != "" { - devDeployDir = filepath.Clean(filepath.Join(r.options.BenchPath, "deploy")) - } else { - devDeployDir = defaultDevDeployDir - } + devDeployDir := filepath.Clean(filepath.Join(r.options.BenchPath, "deploy")) opts := servicedeployer.FactoryOptions{ PackageRootPath: r.options.PackageRootPath, DevDeployDir: devDeployDir, @@ -490,12 +484,7 @@ func (r *runner) getGeneratorConfig() (*config.Config, error) { ) if r.scenario.Corpora.Generator.Config.Path != "" { - var configPath string - if r.options.BenchPath != "" { - configPath = filepath.Clean(filepath.Join(r.options.BenchPath, r.scenario.Corpora.Generator.Config.Path)) - } else { - configPath = filepath.Clean(filepath.Join(devPath, r.scenario.Corpora.Generator.Config.Path)) - } + configPath := filepath.Clean(filepath.Join(r.options.BenchPath, r.scenario.Corpora.Generator.Config.Path)) configPath = os.ExpandEnv(configPath) if _, err := os.Stat(configPath); err != nil { return nil, fmt.Errorf("can't find config file %s: %w", configPath, err) @@ -526,12 +515,7 @@ func (r *runner) getGeneratorFields() (fields.Fields, error) { ) if r.scenario.Corpora.Generator.Fields.Path != "" { - var fieldsPath string - if r.options.BenchPath != "" { - fieldsPath = filepath.Clean(filepath.Join(r.options.BenchPath, r.scenario.Corpora.Generator.Config.Path)) - } else { - fieldsPath = filepath.Clean(filepath.Join(devPath, r.scenario.Corpora.Generator.Fields.Path)) - } + fieldsPath := filepath.Clean(filepath.Join(r.options.BenchPath, r.scenario.Corpora.Generator.Config.Path)) fieldsPath = os.ExpandEnv(fieldsPath) if _, err := os.Stat(fieldsPath); err != nil { return nil, fmt.Errorf("can't find fields file %s: %w", fieldsPath, err) @@ -563,12 +547,7 @@ func (r *runner) getGeneratorTemplate() ([]byte, error) { ) if r.scenario.Corpora.Generator.Template.Path != "" { - var tplPath string - if r.options.BenchPath != "" { - tplPath = filepath.Clean(filepath.Join(r.options.BenchPath, r.scenario.Corpora.Generator.Template.Path)) - } else { - tplPath = filepath.Clean(filepath.Join(devPath, r.scenario.Corpora.Generator.Template.Path)) - } + tplPath := filepath.Clean(filepath.Join(r.options.BenchPath, r.scenario.Corpora.Generator.Template.Path)) tplPath = os.ExpandEnv(tplPath) if _, err := os.Stat(tplPath); err != nil { return nil, fmt.Errorf("can't find template file %s: %w", tplPath, err) diff --git a/internal/benchrunner/runners/system/scenario.go b/internal/benchrunner/runners/system/scenario.go index 9a4e3a1571..00a7909f0a 100644 --- a/internal/benchrunner/runners/system/scenario.go +++ b/internal/benchrunner/runners/system/scenario.go @@ -18,8 +18,6 @@ import ( "github.com/elastic/elastic-package/internal/servicedeployer" ) -const devPath = "_dev/benchmark/system" - type scenario struct { Package string `config:"package" json:"package"` Description string `config:"description" json:"description"` @@ -79,13 +77,8 @@ func defaultConfig() *scenario { } } -func readConfig(path, benchPath string, scenario string, ctxt servicedeployer.ServiceContext) (*scenario, error) { - var configPath string - if benchPath != "" { - configPath = filepath.Join(benchPath, fmt.Sprintf("%s.yml", scenario)) - } else { - configPath = filepath.Join(path, devPath, fmt.Sprintf("%s.yml", scenario)) - } +func readConfig(benchPath string, scenario string, ctxt servicedeployer.ServiceContext) (*scenario, error) { + configPath := filepath.Clean(filepath.Join(benchPath, fmt.Sprintf("%s.yml", scenario))) data, err := os.ReadFile(configPath) if err != nil { if errors.Is(err, os.ErrNotExist) { From 9ce91119f29c00cef3a67ba9e0b0963d170cb54b Mon Sep 17 00:00:00 2001 From: Chris Berkhout Date: Tue, 19 Dec 2023 19:15:05 +0100 Subject: [PATCH 4/4] Fix missing fields data. --- internal/benchrunner/runners/system/runner.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/benchrunner/runners/system/runner.go b/internal/benchrunner/runners/system/runner.go index 6fd411d6d1..34e7d014e1 100644 --- a/internal/benchrunner/runners/system/runner.go +++ b/internal/benchrunner/runners/system/runner.go @@ -515,7 +515,7 @@ func (r *runner) getGeneratorFields() (fields.Fields, error) { ) if r.scenario.Corpora.Generator.Fields.Path != "" { - fieldsPath := filepath.Clean(filepath.Join(r.options.BenchPath, r.scenario.Corpora.Generator.Config.Path)) + fieldsPath := filepath.Clean(filepath.Join(r.options.BenchPath, r.scenario.Corpora.Generator.Fields.Path)) fieldsPath = os.ExpandEnv(fieldsPath) if _, err := os.Stat(fieldsPath); err != nil { return nil, fmt.Errorf("can't find fields file %s: %w", fieldsPath, err)