roachtest: add code changes to benchmarks to emit openmetrics by sambhav-jain-16 · Pull Request #133035 · cockroachdb/cockroach

sambhav-jain-16 · 2024-10-21T09:01:59Z

#129221 and #132023 added changes to
exporters to emit openmetrics. This PR makes changes to the roachtests
to make use of the changes in the above PRs. This change also made some
changes to some roachtests that use neither of the above approaches

Epic: https://cockroachlabs.atlassian.net/browse/CRDB-41852

Release note: None

cockroach-teamcity · 2024-10-21T09:02:13Z

This change is

sambhav-jain-16 · 2024-10-25T06:53:58Z

pkg/cmd/roachprod-microbench/util/util.go

-
-// InitRoachprod initializes the roachprod providers by calling InitProviders.
-// This function sets up the environment for running roachprod commands.
-func InitRoachprod() {
-	_ = roachprod.InitProviders()
-}
-
-// RoachprodRun runs a command on a roachprod cluster with the given cluster name and logger.
-// It takes a list of command arguments and passes them to the roachprod command execution.
-func RoachprodRun(clusterName string, l *logger.Logger, cmdArray []string) error {
-	// Execute the roachprod command with the provided context, logger, cluster name, and options.
-	return roachprod.Run(
-		context.Background(), l, clusterName, "", "", false,
-		os.Stdout, os.Stderr, cmdArray, install.DefaultRunOptions(),
-	)
-}
-
-// InitLogger initializes and returns a logger based on the provided log file path.
-// If the logger configuration fails, the program prints an error and exits.
-func InitLogger(path string) *logger.Logger {
-	loggerCfg := logger.Config{Stdout: os.Stdout, Stderr: os.Stderr} // Create a logger config with standard output and error.
-	var loggerError error
-	l, loggerError := loggerCfg.NewLogger(path) // Create a new logger based on the configuration.
-	if loggerError != nil {
-		// If there is an error initializing the logger, print the error message and exit the program.
-		_, _ = fmt.Fprintf(os.Stderr, "unable to configure logger: %s\n", loggerError)
-		os.Exit(1)
-	}
-	return l // Return the initialized logger.
-}


This change was done due to a deep cyclic dependency that was happening in pkg/kv/kvserver/split/load_based_splitter_test.go

sambhav-jain-16 · 2024-10-25T06:57:53Z

pkg/workload/histogram/exporter/util.go

-	invalidCharRegex      = regexp.MustCompile(`[^a-zA-Z0-9_]`)
-	invalidFirstCharRegex = regexp.MustCompile(`^[^a-zA-Z_]`)
-	summaryQuantiles      = []float64{50, 95, 99, 100}
+	summaryQuantiles = []float64{50, 95, 99, 100}
 )

-func sanitizeOpenMetricsLabels(input string) string {
-	sanitized := invalidCharRegex.ReplaceAllString(input, "_")
-	sanitized = invalidFirstCharRegex.ReplaceAllString(sanitized, "_")
-	return sanitized
-}
-


Removing this in favour of using the roachprod-microbench/util to maintain consistency with roachtest/clusterstats package

nameisbhaskar

Thanks for the changes. I have comments mostly on the usage of fmt.Sprintf which can be made consistent all across and avoid the use of +. Second point is getting the file names form a single utility function.

pkg/cmd/roachtest/roachtestutil/utils.go

pkg/cmd/roachtest/tests/cdc_bench.go

pkg/cmd/roachtest/tests/decommissionbench.go

pkg/cmd/roachtest/tests/failover.go

pkg/cmd/roachtest/tests/indexes.go

cockroachdb#133035 added changes to the `roachtest` binary to emit openmetrics from benchmarks. This change is intended for nightlies to be able to pass on relevant flags and buckets to export openmetrics Epic: https://cockroachlabs.atlassian.net/browse/CRDB-41852 Release note: None

135239: build/teamcity: add changes to enable openmetrics in nightly roachtests r=nameisbhaskar a=sambhav-jain-16 #133035 added changes to the `roachtest` binary to emit openmetrics from benchmarks. This change is intended for nightlies to be able to pass on relevant flags and buckets to export openmetrics Epic: https://cockroachlabs.atlassian.net/browse/CRDB-41852 Release note: None Co-authored-by: Sambhav Jain <sambhav.jain@cockroachlabs.com>

The roachtests starting failing with validation errors `Error: error creating metrics exporter: file path must end with .json` after this PR was merged cockroachdb#133035 path defined in `GetWorkloadHistogramArgs` was jumbled. Also there was an error in cockroachdb#135239. This change intends to fix all the errors. Epic: none Release note: None

135540: roachtest: fix bench roachtests failing with validation when using json r=srosenberg a=sambhav-jain-16 The roachtests starting failing with validation errors `Error: error creating metrics exporter: file path must end with .json` after this PR was merged #133035 path defined in `GetWorkloadHistogramArgs` was jumbled. Also there was an error in #135239. This change intends to fix all the errors. Epic: none Release note: None Co-authored-by: Sambhav Jain <sambhav.jain@cockroachlabs.com>

pkg/cmd/roachtest/tests/admission_control_elastic_io.go

This change intends to fix 3 failures detected in benchmark runs after cockroachdb#133035 1. admission-control/elastic-io due to one wrong flag. 2. tpcc/headroom/n4cpu16 due to wrong passing of labels in case of openmetrics, removed suite label since it is not required. 3. tpccbench/* erroring out with nil pointer error in case of openmetrics, reverted the changes in this PR. Will fix in a different PR. Epic: none Fixes: cockroachdb#135393

135633: roachtest: fix benchmark failing tests r=srosenberg a=sambhav-jain-16 This change intends to fix 3 failures detected in benchmark runs after #133035 1. `admission-control/elastic-io` due to one wrong flag. 2. `tpcc/headroom/n4cpu16` due to wrong passing of labels in case of openmetrics, removed suite label since it is not required. 3. `tpccbench/*` erroring out with nil pointer error in case of openmetrics, reverted the changes in this PR. Will fix in a different PR. Epic: none Fixes: #135393 Co-authored-by: Sambhav Jain <sambhav.jain@cockroachlabs.com>

cockroachdb#133035 inadvertently removed the space. Fixes cockroachdb#135792. Release note: None

135914: roachtest: fix spacing in disk bandwidth test cmd r=herkolategan,DarrylWong a=aadityasondhi #133035 inadvertently removed the space. Fixes #135792. Release note: None Co-authored-by: Aaditya Sondhi <20070511+aadityasondhi@users.noreply.github.com>

`tpcc/large-schema-benchmark` stopped emitting benchmark file after cockroachdb#133035. It was discovered that after `DisableHistogram` was set true for every case and therefore `runTPCC` was skipping setting up the histogram args. Epic: none Release note: None

136265: changefeedccl: change cdc latency hist bucket sizes r=asg0451 a=rharding6373 Increases the changefeed.admit_latency and changefeed.commit_latency max from 5m to 60m and decreases their min to 5ms. The metrics changefeed.parallel_io_queue_nanos, changefeed.parallel_io_result_queue_nanos, changefeed.sink_batch_hist_nanos, changefeed.flush_hist_nanos, and changefeed.kafka_throttling_hist_nanos have the new limits 5ms to 10m (previously 500ms to 5m). Epic: none Fixes: #134593 Release note (general change): In order to improve the granularity of changefeed pipeline metrics, the changefeed metrics changefeed.admit_latency and changefeed.commit_latency have histogram buckets from 5ms to 60m (previously 500ms to 5m). The changefeed metrics changefeed.parallel_io_queue_nanos, changefeed.parallel_io_result_queue_nanos, changefeed.sink_batch_hist_nanos, changefeed.flush_hist_nanos, and changefeed.kafka_throttling_hist_nanos have histogram buckets from 5ms to 10m (previously 500ms to 5m). 136575: roachtest: tpcc/large-schema-benchmark not emitting histogram r=sambhav-jain-16 a=sambhav-jain-16 `tpcc/large-schema-benchmark` stopped emitting benchmark file after #133035. It was discovered that `DisableHistogram` was set true for every case and therefore `runTPCC` was skipping setting up the histogram args. Epic: none Release note: None Co-authored-by: rharding6373 <harding@cockroachlabs.com> Co-authored-by: Sambhav Jain <sambhav.jain@cockroachlabs.com>

sambhav-jain-16 force-pushed the cluster-stats branch 4 times, most recently from 1e0a443 to ce9e8dd Compare October 22, 2024 10:08

sambhav-jain-16 changed the title ~~Cluster stats~~ roachtest: add code changes to tests to emit openmetrics Oct 22, 2024

sambhav-jain-16 force-pushed the cluster-stats branch 9 times, most recently from 6946c90 to 642455d Compare October 25, 2024 06:51

sambhav-jain-16 changed the title ~~roachtest: add code changes to tests to emit openmetrics~~ roachtest: add code changes to benchmarks to emit openmetrics Oct 25, 2024

sambhav-jain-16 commented Oct 25, 2024

View reviewed changes

sambhav-jain-16 force-pushed the cluster-stats branch from 642455d to 1655d3b Compare October 25, 2024 07:05

sambhav-jain-16 force-pushed the cluster-stats branch 3 times, most recently from 4c739dd to 059e4d5 Compare November 8, 2024 06:42

sambhav-jain-16 marked this pull request as ready for review November 8, 2024 06:42

sambhav-jain-16 requested a review from a team as a code owner November 8, 2024 06:42

sambhav-jain-16 requested review from herkolategan, nameisbhaskar and srosenberg and removed request for a team November 8, 2024 06:42

sambhav-jain-16 force-pushed the cluster-stats branch from 059e4d5 to 0103ff9 Compare November 8, 2024 06:56

nameisbhaskar reviewed Nov 10, 2024

View reviewed changes

craig bot merged commit 4d08082 into cockroachdb:master Nov 15, 2024

sambhav-jain-16 deleted the cluster-stats branch November 15, 2024 06:45

sambhav-jain-16 mentioned this pull request Nov 15, 2024

build/teamcity: add changes to enable openmetrics in nightly roachtests #135239

Merged

sambhav-jain-16 mentioned this pull request Nov 18, 2024

roachtest: fix bench roachtests failing with validation when using json #135540

Merged

aadityasondhi mentioned this pull request Nov 18, 2024

roachtest: admission-control/elastic-io failed #135393

Closed

aadityasondhi reviewed Nov 18, 2024

View reviewed changes

pkg/cmd/roachtest/tests/admission_control_elastic_io.go Show resolved Hide resolved

sambhav-jain-16 mentioned this pull request Nov 18, 2024

roachtest: fix benchmark failing tests #135633

Merged

aadityasondhi added a commit to aadityasondhi/cockroach that referenced this pull request Nov 21, 2024

roachtest: fix spacing in disk bandwidth test cmd

5d85f3f

cockroachdb#133035 inadvertently removed the space. Fixes cockroachdb#135792. Release note: None

aadityasondhi mentioned this pull request Nov 21, 2024

roachtest: fix spacing in disk bandwidth test cmd #135914

Merged

sambhav-jain-16 mentioned this pull request Dec 3, 2024

roachtest: tpcc/large-schema-benchmark not emitting histogram #136575

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

roachtest: add code changes to benchmarks to emit openmetrics#133035

roachtest: add code changes to benchmarks to emit openmetrics#133035
craig[bot] merged 1 commit intocockroachdb:masterfrom
sambhav-jain-16:cluster-stats

sambhav-jain-16 commented Oct 21, 2024 •

edited

Loading

Uh oh!

cockroach-teamcity commented Oct 21, 2024

Uh oh!

sambhav-jain-16 Oct 25, 2024

Uh oh!

sambhav-jain-16 Oct 25, 2024

Uh oh!

nameisbhaskar left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Comments

Conversation

sambhav-jain-16 commented Oct 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cockroach-teamcity commented Oct 21, 2024

Uh oh!

sambhav-jain-16 Oct 25, 2024

Choose a reason for hiding this comment

Uh oh!

sambhav-jain-16 Oct 25, 2024

Choose a reason for hiding this comment

Uh oh!

nameisbhaskar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

sambhav-jain-16 commented Oct 21, 2024 •

edited

Loading