Skip to content

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

Open
roidelapluie wants to merge 1 commit intoprometheus:masterfrom
roidelapluie:roidelapluie/support-flags
Open

prombench: add support for extra flags on Prometheus instances#956
roidelapluie wants to merge 1 commit intoprometheus:masterfrom
roidelapluie:roidelapluie/support-flags

Conversation

@roidelapluie
Copy link
Member

Add --pr.extra-flags and --main.extra-flags options to the /prombench command, allowing callers to pass arbitrary flags to the PR and main/release Prometheus instances respectively.

Use github.com/google/shlex for proper shell tokenization of the command line, replacing the naive strings.Split approach. Also fix flag value parsing so that flag values containing = signs are handled correctly.

Add --pr.extra-flags and --main.extra-flags options to the /prombench
command, allowing callers to pass arbitrary flags to the PR and
main/release Prometheus instances respectively.

Use github.com/google/shlex for proper shell tokenization of the command
line, replacing the naive strings.Split approach. Also fix flag value
parsing so that flag values containing = signs are handled correctly.

Signed-off-by: Julien Pivotto <291750+roidelapluie@users.noreply.github.com>
@roidelapluie roidelapluie force-pushed the roidelapluie/support-flags branch from 7b44807 to eaf2494 Compare March 12, 2026 15:27
@roidelapluie roidelapluie requested a review from bwplotka March 12, 2026 16:27
Copy link
Member

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

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

We have already a way for this - bench.version.

But this is bit faster indeed, so LGTM. Just please remove log level setting, we don't need that and would unblock new use case

// 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

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants