Skip to content

prombench: add support for extra flags on Prometheus instances #956

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
roidelapluie wants to merge 1 commit into
base: master
Choose a base branch
from
Open

prombench: add support for extra flags on Prometheus instances #956

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ require (
github.com/go-kit/log v0.2.1
github.com/google/go-cmp v0.7.0
github.com/google/go-github/v29 v29.0.3
github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510
github.com/nelkinda/health-go v0.0.1
github.com/oklog/run v1.2.0
github.com/prometheus/alertmanager v0.31.1
Expand Down
26 changes: 23 additions & 3 deletions pkg/provider/provider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,17 +127,37 @@ func RetryUntilTrue(name string, retryCount int, fn func() (bool, error)) error
return fmt.Errorf("Request for '%v' hasn't completed after retrying %d times", name, retryCount)
}

// prometheusArgs builds a YAML flow-sequence injection string starting with
// baseArg, followed by any space-separated flags from extraFlags. The returned
// value is intended to be placed inside a double-quoted YAML string: the
// embedded "," separators will break into additional flow-sequence items after
// template rendering, e.g. "--log.level=info","--flag1","--flag2".
func prometheusArgs(baseArg, extraFlags string) string {
parts := []string{baseArg}
for _, f := range strings.Fields(extraFlags) {
parts = append(parts, strings.ReplaceAll(f, `"`, `\"`))
}
return strings.Join(parts, `","`)
}

// applyTemplateVars applies golang templates to deployment files.
func applyTemplateVars(content []byte, deploymentVars map[string]string) ([]byte, error) {
fileContentParsed := bytes.NewBufferString("")
t := template.New("resource").Option("missingkey=error")
t = t.Funcs(template.FuncMap{
// k8s objects can't have dots(.) se we add a custom function to allow normalising the variable values.
// k8s objects can't have dots(.) so we add a custom function to allow normalising the variable values.
"normalise": func(t string) string {
return strings.ReplaceAll(t, ".", "-")
},
"split": func(rangeVars, separator string) []string {
return strings.Split(rangeVars, separator)
// prArgs returns a YAML flow-sequence injection string for the PR prometheus
// instance, including --log.level=info and any PR_EXTRA_FLAGS.
"prArgs": func(vars map[string]string) string {
return prometheusArgs("--log.level=info", vars["PR_EXTRA_FLAGS"])
Copy link
Member

@bwplotka bwplotka Mar 13, 2026

Choose a reason for hiding this comment

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

Kind of confusing we hardcode info here. Why not removing this and allowing this option to set it? (info is default)

a.Flag(LevelFlagName, LevelFlagHelp).
	Default("info").HintOptions(promslog.LevelFlagOptions...).
	SetValue(config.Level)

},
// mainArgs returns a YAML flow-sequence injection string for the main/release
// prometheus instance, including --log.level=info and any MAIN_EXTRA_FLAGS.
"mainArgs": func(vars map[string]string) string {
return prometheusArgs("--log.level=info", vars["MAIN_EXTRA_FLAGS"])
},
})
if err := template.Must(t.Parse(string(content))).Execute(fileContentParsed, deploymentVars); err != nil {
Expand Down
2 changes: 2 additions & 0 deletions prombench/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,8 @@ resource_apply:
-v CLUSTER_NAME:${CLUSTER_NAME} \
-v PR_NUMBER:${PR_NUMBER} -v RELEASE:${RELEASE} -v DOMAIN_NAME:${DOMAIN_NAME} \
-v GITHUB_ORG:${GITHUB_ORG} -v GITHUB_REPO:${GITHUB_REPO} \
$(if $(PR_EXTRA_FLAGS),-v "PR_EXTRA_FLAGS:$(PR_EXTRA_FLAGS)") \
$(if $(MAIN_EXTRA_FLAGS),-v "MAIN_EXTRA_FLAGS:$(MAIN_EXTRA_FLAGS)") \
-f ${PROMBENCH_DIR}/${BENCHMARK_DIRECTORY}/benchmark

# Required because namespace and cluster-role are not part of the created nodes
Expand Down
10 changes: 8 additions & 2 deletions prombench/manifests/cluster-infra/7a_commentmonitor_configmap_noparse.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ data:
* See the details [here](https://github.com/prometheus/test-infra/tree/master/prombench/manifests/prombench#benchmarking-from-the-different-directory), defaults to `manifests/prombench`.
* `--bench.version=<branch | @commit>`
* See the details [here](https://github.com/prometheus/test-infra/tree/master/prombench/manifests/prombench#benchmarking-from-the-custom-test-infra-commitbranch), defaults to `master`.
* `--pr.extra-flags=<space-separated Prometheus flags>` - extra flags passed to the PR Prometheus instance.
* `--main.extra-flags=<space-separated Prometheus flags>` - extra flags passed to the main/release Prometheus instance.

**Examples:**
* `/prombench v3.0.0`
Expand All @@ -37,6 +39,8 @@ data:
flag_args:
bench.directory: BENCHMARK_DIRECTORY
bench.version: BENCHMARK_VERSION
pr.extra-flags: PR_EXTRA_FLAGS
main.extra-flags: MAIN_EXTRA_FLAGS
comment_template: |
⏱️ Welcome (again) to Prometheus Benchmarking Tool. ⏱️

Expand All @@ -59,7 +63,7 @@ data:
- [Parca profiles (e.g. in-use memory)](http://{{ index . "DOMAIN_NAME" }}/profiles?expression_a=memory%3Ainuse_space%3Abytes%3Aspace%3Abytes%7Bpr_number%3D%22{{ index . "PR_NUMBER" }}%22%7D&time_selection_a=relative:minute|15)

**Available Commands:**
* To restart benchmark: `/prombench restart {{ index . "RELEASE" }}{{ if index . "BENCHMARK_VERSION" }} --bench.version={{ index . "BENCHMARK_VERSION" }}{{ end }}{{ if index . "BENCHMARK_DIRECTORY" }} --bench.directory={{ index . "BENCHMARK_DIRECTORY" }}{{ end }}`
* To restart benchmark: `/prombench restart {{ index . "RELEASE" }}{{ if index . "BENCHMARK_VERSION" }} --bench.version={{ index . "BENCHMARK_VERSION" }}{{ end }}{{ if index . "BENCHMARK_DIRECTORY" }} --bench.directory={{ index . "BENCHMARK_DIRECTORY" }}{{ end }}{{ if index . "PR_EXTRA_FLAGS" }} --pr.extra-flags={{ index . "PR_EXTRA_FLAGS" }}{{ end }}{{ if index . "MAIN_EXTRA_FLAGS" }} --main.extra-flags={{ index . "MAIN_EXTRA_FLAGS" }}{{ end }}`
* To stop benchmark: `/prombench cancel`
* To print help: `/prombench help`

Expand All @@ -70,6 +74,8 @@ data:
flag_args:
bench.directory: BENCHMARK_DIRECTORY
bench.version: BENCHMARK_VERSION
pr.extra-flags: PR_EXTRA_FLAGS
main.extra-flags: MAIN_EXTRA_FLAGS
label: prombench
comment_template: |
⏱️ Welcome to Prometheus Benchmarking Tool. ⏱️
Expand All @@ -93,6 +99,6 @@ data:
- [Parca profiles (e.g. in-use memory)](http://{{ index . "DOMAIN_NAME" }}/profiles?expression_a=memory%3Ainuse_space%3Abytes%3Aspace%3Abytes%7Bpr_number%3D%22{{ index . "PR_NUMBER" }}%22%7D&time_selection_a=relative:minute|15)

**Available Commands:**
* To restart benchmark: `/prombench restart {{ index . "RELEASE" }}{{ if index . "BENCHMARK_VERSION" }} --bench.version={{ index . "BENCHMARK_VERSION" }}{{ end }}{{ if index . "BENCHMARK_DIRECTORY" }} --bench.directory={{ index . "BENCHMARK_DIRECTORY" }}{{ end }}`
* To restart benchmark: `/prombench restart {{ index . "RELEASE" }}{{ if index . "BENCHMARK_VERSION" }} --bench.version={{ index . "BENCHMARK_VERSION" }}{{ end }}{{ if index . "BENCHMARK_DIRECTORY" }} --bench.directory={{ index . "BENCHMARK_DIRECTORY" }}{{ end }}{{ if index . "PR_EXTRA_FLAGS" }} --pr.extra-flags={{ index . "PR_EXTRA_FLAGS" }}{{ end }}{{ if index . "MAIN_EXTRA_FLAGS" }} --main.extra-flags={{ index . "MAIN_EXTRA_FLAGS" }}{{ end }}`
* To stop benchmark: `/prombench cancel`
* To print help: `/prombench help`
2 changes: 1 addition & 1 deletion prombench/manifests/prombench/benchmark/5_prometheus-test-pr_deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -814,7 +814,7 @@ spec:
"--web.external-url=http://{{ .DOMAIN_NAME }}/{{ .PR_NUMBER }}/prometheus-pr",
"--storage.tsdb.path=/prometheus",
"--config.file=/etc/prometheus/prometheus.yml",
"--log.level=info"
"{{prArgs .}}"
]
resources:
requests:
Expand Down
2 changes: 1 addition & 1 deletion prombench/manifests/prombench/benchmark/5_prometheus-test-release_deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -789,7 +789,7 @@ spec:
"--web.external-url=http://{{ .DOMAIN_NAME }}/{{ .PR_NUMBER }}/prometheus-release",
"--storage.tsdb.path=/prometheus",
"--config.file=/etc/prometheus/prometheus.yml",
"--log.level=info"
"{{mainArgs .}}"
]
resources:
requests:
Expand Down
18 changes: 12 additions & 6 deletions tools/comment-monitor/internal/command.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"strings"
"text/template"

"github.com/google/shlex"
"gopkg.in/yaml.v2"
)

Expand Down Expand Up @@ -167,7 +168,13 @@ func ParseCommand(cfg *Config, comment string) (_ *Command, ok bool, err *Comman
i = len(comment)
}
cmdLine := comment[:i]
rest := strings.Split(strings.TrimSpace(cmdLine[len(prefix.Prefix):]), " ")
rest, tErr := shlex.Split(strings.TrimSpace(cmdLine[len(prefix.Prefix):]))
if tErr != nil {
return nil, false, &CommandParseError{
error: fmt.Errorf("tokenizing command line failed for %v: %w", cmdLine, tErr),
help: fmt.Sprintf("Incorrect `%v` syntax; %v.\n\n%s", prefix.Prefix, tErr.Error(), prefix.Help),
}
}
if len(rest) == 0 || rest[0] == "" {
return nil, false, &CommandParseError{
error: fmt.Errorf("no matching command found for comment line: %v", cmdLine),
Expand Down Expand Up @@ -261,21 +268,20 @@ func ParseCommand(cfg *Config, comment string) (_ *Command, ok bool, err *Comman
}

func parseFlags(rest []string, cfg *CommandConfig, cmd *Command) error {
// TODO(bwplotka: Naive flag parsing, make it support quoting, spaces etc later.
for _, flag := range rest {
if !strings.HasPrefix(flag, "--") {
return fmt.Errorf("expected flag (starting with --), got %v", flag)
}
parts := strings.Split(flag, "=")
if len(parts) != 2 {
eqIdx := strings.Index(flag, "=")
if eqIdx == -1 {
return fmt.Errorf("expected flag format '--<flag>=<value>', got %v", flag)
}

argName, ok := cfg.FlagArgs[strings.TrimPrefix(parts[0], "--")]
argName, ok := cfg.FlagArgs[strings.TrimPrefix(flag[:eqIdx], "--")]
if !ok {
return fmt.Errorf("flag %v is not supported", flag)
}
cmd.Args[argName] = parts[1]
cmd.Args[argName] = flag[eqIdx+1:]
}
return nil
}
32 changes: 32 additions & 0 deletions tools/comment-monitor/internal/command_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,22 @@ func TestParseCommand(t *testing.T) {
comment: "/prombench v3.0.0 --bench.version=yolo --bench.directory=dir1",
expect: testCommand(eventTypeStart, map[string]string{"RELEASE": "v3.0.0", "BENCHMARK_VERSION": "yolo", "BENCHMARK_DIRECTORY": "dir1"}),
},
{
comment: "/prombench v3.0.0 --pr.extra-flags=--enable-feature=foo",
expect: testCommand(eventTypeStart, map[string]string{"RELEASE": "v3.0.0", "PR_EXTRA_FLAGS": "--enable-feature=foo"}),
},
{
comment: "/prombench v3.0.0 --main.extra-flags=--enable-feature=bar",
expect: testCommand(eventTypeStart, map[string]string{"RELEASE": "v3.0.0", "MAIN_EXTRA_FLAGS": "--enable-feature=bar"}),
},
{
comment: "/prombench restart v3.0.0 --pr.extra-flags=--enable-feature=foo --main.extra-flags=--enable-feature=bar",
expect: testCommand(eventTypeRestart, map[string]string{"RELEASE": "v3.0.0", "PR_EXTRA_FLAGS": "--enable-feature=foo", "MAIN_EXTRA_FLAGS": "--enable-feature=bar"}),
},
{
comment: "/prombench v3.0.0 --pr.extra-flags='--enable-feature=foo --enable-feature=bar'",
expect: testCommand(eventTypeStart, map[string]string{"RELEASE": "v3.0.0", "PR_EXTRA_FLAGS": "--enable-feature=foo --enable-feature=bar"}),
},
// Text at the end is generally accepted, after \n.
{
comment: "/prombench v3.0.0\n",
Expand Down Expand Up @@ -262,6 +278,22 @@ func TestParseCommand_ProdCommentMonitorConfig(t *testing.T) {
comment: "/prombench v3.0.0 --bench.version=mybranch --bench.directory=manifests/prombench",
expect: testCommand(eventTypeStart, map[string]string{"RELEASE": "v3.0.0", "BENCHMARK_VERSION": "mybranch", "BENCHMARK_DIRECTORY": "manifests/prombench"}),
},
{
comment: "/prombench v3.0.0 --pr.extra-flags=--enable-feature=foo",
expect: testCommand(eventTypeStart, map[string]string{"RELEASE": "v3.0.0", "PR_EXTRA_FLAGS": "--enable-feature=foo"}),
},
{
comment: "/prombench v3.0.0 --main.extra-flags=--enable-feature=bar",
expect: testCommand(eventTypeStart, map[string]string{"RELEASE": "v3.0.0", "MAIN_EXTRA_FLAGS": "--enable-feature=bar"}),
},
{
comment: "/prombench restart v3.0.0 --pr.extra-flags=--enable-feature=foo --main.extra-flags=--enable-feature=bar",
expect: testCommand(eventTypeRestart, map[string]string{"RELEASE": "v3.0.0", "PR_EXTRA_FLAGS": "--enable-feature=foo", "MAIN_EXTRA_FLAGS": "--enable-feature=bar"}),
},
{
comment: "/prombench v3.0.0 --pr.extra-flags='--enable-feature=foo --enable-feature=bar'",
expect: testCommand(eventTypeStart, map[string]string{"RELEASE": "v3.0.0", "PR_EXTRA_FLAGS": "--enable-feature=foo --enable-feature=bar"}),
},
// Not matching cases.
{comment: ""},
{comment: "How to start prombench? I think it was something like:\n\n /prombench main\n\nYolo"},
Expand Down
4 changes: 4 additions & 0 deletions tools/comment-monitor/internal/testdata/testconfig.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ prefixes:
flag_args:
bench.directory: BENCHMARK_DIRECTORY
bench.version: BENCHMARK_VERSION
pr.extra-flags: PR_EXTRA_FLAGS
main.extra-flags: MAIN_EXTRA_FLAGS

- name: "" # start is a default (empty command).
event_type: prombench_start
Expand All @@ -35,4 +37,6 @@ prefixes:
flag_args:
bench.directory: BENCHMARK_DIRECTORY
bench.version: BENCHMARK_VERSION
pr.extra-flags: PR_EXTRA_FLAGS
main.extra-flags: MAIN_EXTRA_FLAGS
label: prombench
Loading