Skip to content

Add multiple service deployer to benchmarks#3304

Merged
kcreddy merged 20 commits intoelastic:mainfrom
kcreddy:benchmarks-multi-deployer
Mar 26, 2026
Merged

Add multiple service deployer to benchmarks#3304
kcreddy merged 20 commits intoelastic:mainfrom
kcreddy:benchmarks-multi-deployer

Conversation

@kcreddy
Copy link
Copy Markdown

@kcreddy kcreddy commented Feb 24, 2026

Support multiple service deployers in system benchmarks

Allow system benchmarks to use different service deployers (docker, k8s,
tf) configured per scenario via the `deployer` field in the benchmark
YAML, mirroring how system tests handle deployer selection.

Changes:
- Wire scenario `deployer` field to service deployer factory with
  validation for allowed deployer names (docker, k8s, tf).
- Use CreateTestRunIDWithPrefix for benchmark RunIDs, consistent with
  system tests, avoiding S3 bucket name length issues.
- Mask secret variables (secret: true in manifest) in benchmark reports.
- Add CrowdStrike test package exercising docker and tf deployers.
- Add unit tests for deployer name validation in scenario config.

Closes #3266

@kcreddy kcreddy self-assigned this Feb 24, 2026
@kcreddy kcreddy added the enhancement New feature or request label Feb 24, 2026
- Mask secrets in reports.
- Add test integration
@kcreddy kcreddy added the Team:Ecosystem Label for the Packages Ecosystem team label Feb 24, 2026
@kcreddy kcreddy marked this pull request as ready for review February 24, 2026 13:42
@kcreddy
Copy link
Copy Markdown
Author

kcreddy commented Feb 24, 2026

The CI error Additional property deployer is not allowed in the step Integration test: benchmarks is because of the addition of new property in this PR; it is unrecognised in previous elastic-package version.

Here's the full error:

Error: checking package failed: linting package failed: found 2 validation errors:
--
1. file "/opt/buildkite-agent/builds/bk-agent-prod-gcp-1771951948282219822/elastic/elastic-package/test/packages/benchmarks/system_benchmark_crowdstrike/_dev/benchmark/system/alert-benchmark.yml" is invalid: field (root): Additional property deployer is not allowed
2. file "/opt/buildkite-agent/builds/bk-agent-prod-gcp-1771951948282219822/elastic/elastic-package/test/packages/benchmarks/system_benchmark_crowdstrike/_dev/benchmark/system/fdr-benchmark.yml" is invalid: field (root): Additional property deployer is not allowed

Error: checking package failed: linting package failed: found 2 validation errors:
--
1. file "/opt/buildkite-agent/builds/bk-agent-prod-gcp-1771951948282219822/elastic/elastic-package/test/packages/benchmarks/system_benchmark_crowdstrike/_dev/benchmark/system/alert-benchmark.yml" is invalid: field (root): Additional property deployer is not allowed
2. file "/opt/buildkite-agent/builds/bk-agent-prod-gcp-1771951948282219822/elastic/elastic-package/test/packages/benchmarks/system_benchmark_crowdstrike/_dev/benchmark/system/fdr-benchmark.yml" is invalid: field (root): Additional property deployer is not allowed

The other CI errors are unrelated to this change. They are failing to bring up the Elastic Stack due to Client Timeout. Re-triggered the step in hopes of getting it fixed.

@kcreddy kcreddy requested a review from mrodm February 25, 2026 04:51
@mrodm mrodm requested a review from a team February 25, 2026 09:19
@mrodm
Copy link
Copy Markdown
Contributor

mrodm commented Feb 25, 2026

The CI error Additional property deployer is not allowed in the step Integration test: benchmarks is because of the addition of new property in this PR; it is unrecognised in previous elastic-package version.

@kcreddy This errors is because this new field requires to be added in the package spec for the benchmark system configuration here in this file:
https://github.com/elastic/package-spec/blob/dde18ea1f8f6481bcc756063df442b0e56272e50/spec/integration/_dev/benchmark/system.scenario.spec.yml

Could you create a PR to include that? That would imply that this would need a new package-spec release to include this field here.

@kcreddy
Copy link
Copy Markdown
Author

kcreddy commented Feb 27, 2026

alert-benchmark.yml" is invalid: field (root): Additional property deployer is not allowed

@mrodm, added here - elastic/package-spec#1099 into 3.6.0 spec improvements.
Does this mean we can't merge this PR until that PR is merged and package spec 3.6.0 is released?

@mrodm
Copy link
Copy Markdown
Contributor

mrodm commented Mar 2, 2026

Does this mean we can't merge this PR until that PR is merged and package spec 3.6.0 is released?

In order to run CI successfully , yes, I think so. It could be tested and worked locally until that happens, but this PR to add support for multi-deployer in system benchmarks will require a new package-spec release to include your change.

kcreddy added 13 commits March 19, 2026 12:59
Adds a benchmark test package that exercises both the `docker` and `tf`
service deployers in the system benchmark runner within a single package:

- `httpjson` data stream: `deployer: docker` — runs a mock HTTP server
  (stream image) serving generated JSON events, collected via httpjson input.
- `logfile` data stream: `deployer: tf` — uses the Terraform `local`
  provider to write a sample log file, collected via filestream input.
  No cloud credentials required, making it safe to run in standard CI.
@kcreddy
Copy link
Copy Markdown
Author

kcreddy commented Mar 25, 2026

@mrodm, the CI is failing on tests unrelated to this change: Integration test (false positive): pipeline_error_system_tests

Can you please review the PR?

@kcreddy kcreddy requested a review from mrodm March 25, 2026 19:20

func newReport(benchName, corporaFile string, s *scenario, sum *metricsSummary) *report {
func newReport(benchName, corporaFile string, s *scenario, sum *metricsSummary, secretVarNames map[string]bool) *report {
var report report
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be interesting or worthy to add the deployer used to run the benchmark in the report too ?
It could be kept as it is the report if it is not needed.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 6210301

  ╭─────────────────────────────────────────────────────────────────────────────────────╮
  │ parameters                                                                          │
  ├──────────────────────────────────────────┬──────────────────────────────────────────┤
  │ package version                          │                                   3.12.0 │
+ │ deployer                                 │                                   docker │
  │ input                                    │                                      cel │
  │ vars.client_id                           │                                     xxxx │
  │ vars.client_secret                       │                                     xxxx │
  │ vars.token_url                           │ http://svc-crowdstrike:8080/oauth2/token │
  │ vars.url                                 │              http://svc-crowdstrike:8080 │
  │ data_stream.name                         │                                    alert │
  │ data_stream.vars.enable_request_tracer   │                                     true │
  │ data_stream.vars.preserve_original_event │                                     true │
  │ warmup time period                       │                                       2s │
  │ benchmark time period                    │                                       0s │
  │ wait for data timeout                    │                                    10m0s │
  │ corpora.generator.total_events           │                                     1000 │
  │ corpora.generator.template.path          │        ./alert-benchmark/template.ndjson │
  │ corpora.generator.template.raw           │                                          │
  │ corpora.generator.template.type          │                                   gotext │
  │ corpora.generator.config.path            │             ./alert-benchmark/config.yml │
  │ corpora.generator.config.raw             │                                    map[] │
  │ corpora.generator.fields.path            │             ./alert-benchmark/fields.yml │
  │ corpora.generator.fields.raw             │                                    map[] │
  │ corpora.input_service.name               │                              crowdstrike │
  │ corpora.input_service.signal             │                                          │
  ╰──────────────────────────────────────────┴──────────────────────────────────────────╯

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works in those packages that define multiple deployers, but in the ones that just use the only deployer defined, that field is shown as empty:

https://buildkite.com/elastic/elastic-package/builds/7576#019d29de-e5d8-49c8-9f5f-94b89e96ce1f/L1092

╭─────────────────────────────────────────────────────────────────╮
│ parameters                                                      │
├─────────────────────────────────┬───────────────────────────────┤
│ package version                 │                   999.999.999 │
│ deployer                        │                               │
│ input                           │                    filestream │

@kcreddy kcreddy requested a review from mrodm March 26, 2026 11:17
Copy link
Copy Markdown
Contributor

@mrodm mrodm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a minor change to show the deployer value in those packages that use the only deployer defined in the package.


func newReport(benchName, corporaFile string, s *scenario, sum *metricsSummary) *report {
func newReport(benchName, corporaFile string, s *scenario, sum *metricsSummary, secretVarNames map[string]bool) *report {
var report report
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works in those packages that define multiple deployers, but in the ones that just use the only deployer defined, that field is shown as empty:

https://buildkite.com/elastic/elastic-package/builds/7576#019d29de-e5d8-49c8-9f5f-94b89e96ce1f/L1092

╭─────────────────────────────────────────────────────────────────╮
│ parameters                                                      │
├─────────────────────────────────┬───────────────────────────────┤
│ package version                 │                   999.999.999 │
│ deployer                        │                               │
│ input                           │                    filestream │

Resolves empty deployer field in benchmark reports for packages with a single deployer that don't explicitly specify it in the scenario config.
@kcreddy
Copy link
Copy Markdown
Author

kcreddy commented Mar 26, 2026

Just a minor change to show the deployer value in those packages that use the only deployer defined in the package.

Fixed in cab532b

I tested out ti_abusech package's malware-benchmark

 ╭────────────────────────────────────────────────────────────────────────────────────────╮
  │ parameters                                                                             │
  ├──────────────────────────────────────────┬─────────────────────────────────────────────┤
  │ package version                          │                                       3.6.0 │
+ │ deployer                                 │                                      docker │
  │ input                                    │                                         cel │
  │ vars.auth_key                            │                                        xxxx │
  │ vars.enable_request_tracer               │                                        true │
  │ data_stream.name                         │                                     malware │

@mrodm
Copy link
Copy Markdown
Contributor

mrodm commented Mar 26, 2026

/test

@elasticmachine
Copy link
Copy Markdown
Collaborator

elasticmachine commented Mar 26, 2026

💛 Build succeeded, but was flaky

Failed CI Steps

History

cc @kcreddy

Copy link
Copy Markdown
Contributor

@mrodm mrodm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@kcreddy kcreddy merged commit 583f4d7 into elastic:main Mar 26, 2026
4 checks passed
@kcreddy kcreddy deleted the benchmarks-multi-deployer branch March 26, 2026 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request Team:Ecosystem Label for the Packages Ecosystem team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[system benchmarks] Add support for multiple service deployers

3 participants