diff --git a/cmd/omes/run_scenario.go b/cmd/omes/run_scenario.go index 644214ce..9b847f5d 100644 --- a/cmd/omes/run_scenario.go +++ b/cmd/omes/run_scenario.go @@ -2,6 +2,7 @@ package main import ( "context" + "errors" "fmt" "os" "strings" @@ -15,6 +16,11 @@ import ( "go.uber.org/zap" ) +const ( + iterationFailurePolicyContinue = "continue" + iterationFailurePolicyFailFast = "fail-fast" +) + func runScenarioCmd() *cobra.Command { var r scenarioRunner cmd := &cobra.Command{ @@ -51,6 +57,7 @@ type scenarioRunConfig struct { maxConcurrent int maxIterationsPerSecond float64 maxIterationAttempts int + iterationFailurePolicy string scenarioOptions []string timeout time.Duration doNotRegisterSearchAttributes bool @@ -75,6 +82,8 @@ func (r *scenarioRunConfig) addCLIFlags(fs *pflag.FlagSet) { fs.Float64Var(&r.maxIterationsPerSecond, "max-iterations-per-second", 0, "Override iterations per second rate limit for the scenario."+ " This is the maximum rate at which we will start new iterations of the scenario.") fs.IntVar(&r.maxIterationAttempts, "max-iteration-attempts", 1, "Maximum attempts per iteration") + fs.StringVar(&r.iterationFailurePolicy, "iteration-failure-policy", iterationFailurePolicyContinue, + "How to handle terminal iteration failures: continue or fail-fast") fs.DurationVar(&r.timeout, "timeout", 0, "If set, the scenario will stop after this amount of"+ " time has elapsed. Any still-running iterations will be cancelled, and omes will exit nonzero.") fs.IntVar(&r.maxConcurrent, "max-concurrent", 0, "Override max-concurrent for the scenario") @@ -109,6 +118,13 @@ func (r *scenarioRunner) validateInput() (*loadgen.Scenario, *loadgen.OptionSet, return nil, nil, loadgen.NewUsageError("--iterations and --duration cannot be combined; " + "use --iterations to run a fixed number of times, or --duration to keep starting " + "iterations for a period") + } else if policy := r.resolvedIterationFailurePolicy(); policy != iterationFailurePolicyContinue && policy != iterationFailurePolicyFailFast { + return nil, nil, loadgen.NewUsageError( + "--iteration-failure-policy must be %q or %q, got %q", + iterationFailurePolicyContinue, + iterationFailurePolicyFailFast, + r.iterationFailurePolicy, + ) } // Parse options @@ -140,6 +156,43 @@ func (r *scenarioRunner) validateInput() (*loadgen.Scenario, *loadgen.OptionSet, return scenario, resolvedOptions, nil } +func (r scenarioRunConfig) resolvedIterationFailurePolicy() string { + if r.iterationFailurePolicy == "" { + return iterationFailurePolicyContinue + } + return r.iterationFailurePolicy +} + +func (r scenarioRunConfig) loadgenConfiguration() loadgen.RunConfiguration { + return loadgen.RunConfiguration{ + Iterations: r.iterations, + Duration: r.duration, + MaxConcurrent: r.maxConcurrent, + MaxIterationsPerSecond: r.maxIterationsPerSecond, + MaxIterationAttempts: r.maxIterationAttempts, + Timeout: r.timeout, + DoNotRegisterSearchAttributes: r.doNotRegisterSearchAttributes, + IgnoreAlreadyStarted: r.ignoreAlreadyStarted, + ContinueOnIterationFailure: r.resolvedIterationFailurePolicy() == iterationFailurePolicyContinue, + } +} + +// iterationFailuresOnly recognizes an IterationFailuresError through ordinary +// single-cause wrapping. It deliberately rejects multi-errors so a degraded +// completion cannot hide another run-level failure joined to it. +func iterationFailuresOnly(err error) (*loadgen.IterationFailuresError, bool) { + for err != nil { + if failures, ok := err.(*loadgen.IterationFailuresError); ok { + return failures, true + } + if _, ok := err.(interface{ Unwrap() []error }); ok { + return nil, false + } + err = errors.Unwrap(err) + } + return nil, false +} + func (r *scenarioRunner) run(ctx context.Context) error { scenario, resolvedOptions, err := r.validateInput() if err != nil { @@ -192,19 +245,10 @@ func (r *scenarioRunner) run(ctx context.Context) error { MetricsHandler: metrics.NewHandler(), Client: client, ClientOptions: r.clientOptions, - Configuration: loadgen.RunConfiguration{ - Iterations: r.iterations, - Duration: r.duration, - MaxConcurrent: r.maxConcurrent, - MaxIterationsPerSecond: r.maxIterationsPerSecond, - MaxIterationAttempts: r.maxIterationAttempts, - Timeout: r.timeout, - DoNotRegisterSearchAttributes: r.doNotRegisterSearchAttributes, - IgnoreAlreadyStarted: r.ignoreAlreadyStarted, - }, - Options: resolvedOptions, - Namespace: r.clientOptions.Namespace, - RootPath: repoDir, + Configuration: r.loadgenConfiguration(), + Options: resolvedOptions, + Namespace: r.clientOptions.Namespace, + RootPath: repoDir, ExportOptions: loadgen.ExportOptions{ ExportHistoriesDir: r.exportHistoriesDir, ExportHistoriesFilter: r.exportHistoriesFilter, @@ -213,7 +257,14 @@ func (r *scenarioRunner) run(ctx context.Context) error { executor := scenario.ExecutorFn() err = executor.Run(ctx, scenarioInfo) if err != nil { - return fmt.Errorf("failed scenario: %w", err) + if r.resolvedIterationFailurePolicy() == iterationFailurePolicyContinue { + if _, ok := iterationFailuresOnly(err); ok { + err = nil + } + } + if err != nil { + return fmt.Errorf("failed scenario: %w", err) + } } err = loadgen.ExportWorkflowHistories(ctx, scenarioInfo) if err != nil { diff --git a/cmd/omes/run_scenario_test.go b/cmd/omes/run_scenario_test.go index f8f5294d..e177defa 100644 --- a/cmd/omes/run_scenario_test.go +++ b/cmd/omes/run_scenario_test.go @@ -2,6 +2,7 @@ package main import ( "errors" + "fmt" "path/filepath" "strings" "testing" @@ -45,6 +46,11 @@ func TestValidateInputClassifiesBadInputAsUsageErrors(t *testing.T) { }, wantMsg: "cannot be combined", }, + { + name: "invalid iteration failure policy", + mutate: func(r *scenarioRunner) { r.iterationFailurePolicy = "sometimes" }, + wantMsg: "--iteration-failure-policy must be", + }, { name: "option without equals", mutate: func(r *scenarioRunner) { r.scenarioOptions = []string{"novalue"} }, @@ -85,6 +91,43 @@ func TestValidateInputClassifiesBadInputAsUsageErrors(t *testing.T) { } } +func TestIterationFailurePolicyConfiguration(t *testing.T) { + tests := []struct { + name string + policy string + want bool + }{ + {name: "zero value defaults to continue", want: true}, + {name: "continue", policy: iterationFailurePolicyContinue, want: true}, + {name: "fail fast", policy: iterationFailurePolicyFailFast, want: false}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + config := scenarioRunConfig{iterationFailurePolicy: test.policy}.loadgenConfiguration() + if config.ContinueOnIterationFailure != test.want { + t.Fatalf("ContinueOnIterationFailure = %v, want %v", config.ContinueOnIterationFailure, test.want) + } + }) + } +} + +func TestIterationFailuresOnly(t *testing.T) { + degraded := &loadgen.IterationFailuresError{Attempted: 2, Succeeded: 1, Failed: 1} + + found, ok := iterationFailuresOnly(fmt.Errorf("scenario wrapper: %w", degraded)) + if !ok || found != degraded { + t.Fatalf("expected wrapped degraded completion to be recognized, got %v, %v", found, ok) + } + + if _, ok := iterationFailuresOnly(errors.Join(degraded, errors.New("cleanup failed"))); ok { + t.Fatal("must not treat a joined run-level error as only iteration failures") + } + if _, ok := iterationFailuresOnly(errors.New("run failed")); ok { + t.Fatal("must not treat an ordinary run error as degraded completion") + } +} + func TestValidateInputAcceptsGoodInput(t *testing.T) { r := newRunner("throughput_stress") r.scenarioOptions = []string{"sleep-time=3s"} diff --git a/docs/authoring-scenarios.md b/docs/authoring-scenarios.md index 47da0817..85474162 100644 --- a/docs/authoring-scenarios.md +++ b/docs/authoring-scenarios.md @@ -143,7 +143,8 @@ scenario's `DefaultConfiguration` field over the older `HasDefaultConfiguration` Scenario configuration arrives through **two separate channels**, and knowing which is which matters: -1. **Built-in run flags** — iterations, duration, concurrency, rate, attempts, timeout. These are +1. **Built-in run flags** — iterations, duration, concurrency, rate, attempts, iteration-failure policy, + timeout. These are framework-level and apply to every scenario, so you neither declare nor read them; see [running.md](./running.md#configuring-the-load) for the list. 2. **Your own options** — `--option key=value` pairs that you **declare** on the scenario. diff --git a/docs/running.md b/docs/running.md index 126f9da9..757ab54a 100644 --- a/docs/running.md +++ b/docs/running.md @@ -70,12 +70,20 @@ These apply to every scenario and override its defaults: | `--max-concurrent` | Max iterations running at once. | | `--max-iterations-per-second` | Rate limit on starting iterations (0 = unlimited). | | `--max-iteration-attempts` | Attempts per iteration (default 1). | +| `--iteration-failure-policy` | `continue` (default) records terminal failures and keeps generating load; `fail-fast` stops on the first terminal failure. | | `--timeout` | Hard stop; cancels in-flight iterations and exits non-zero. | If you set neither `--iterations` nor `--duration`, the scenario's own default applies — and most scenarios declare none, in which case omes's default does. `list-scenarios` states which is the case for each scenario. +Iteration retries and the terminal-failure policy are independent. `--max-iteration-attempts` controls +how many times one logical iteration may execute; after those attempts are exhausted, the default +`continue` policy records the iteration as failed and starts more load. A completed run logs attempted, +succeeded, and failed totals plus success/failure rates and successful iterations per second. When the +load-driver Prometheus endpoint is enabled with `--prom-listen-address`, the same terminal outcomes are +exported as `omes_iterations_total`, labeled by scenario, outcome, and status code. + ### 2. Per-scenario options (`--option key=value`) Scenario-specific knobs are passed as repeated `--option key=value` pairs. Each scenario declares the diff --git a/loadgen/generic_executor.go b/loadgen/generic_executor.go index 6a641596..e57256ec 100644 --- a/loadgen/generic_executor.go +++ b/loadgen/generic_executor.go @@ -9,6 +9,14 @@ import ( "go.temporal.io/sdk/client" "go.uber.org/zap" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" +) + +const ( + iterationOutcomeSucceeded = "succeeded" + iterationOutcomeFailed = "failed" + iterationsMetricName = "omes_iterations_total" ) type GenericExecutor struct { @@ -21,10 +29,12 @@ type genericRun struct { info ScenarioInfo config RunConfiguration logger *zap.SugaredLogger + // Metrics tagged with the scenario name for this run. + metricsHandler client.MetricsHandler // Timer capturing E2E execution of each scenario run iteration. executeTimer client.MetricsTimer // Iteration outcome tallies, used for the end-of-run summary. failed counts - // only terminal failures tolerated via ContinueOnIterationFailure. + // terminal failures that reached the executor's outcome channel. completed atomic.Int64 failed atomic.Int64 } @@ -42,16 +52,61 @@ func (g *GenericExecutor) newRun(info ScenarioInfo) (*genericRun, error) { if err := info.Configuration.Validate(); err != nil { return nil, fmt.Errorf("invalid scenario: %w", err) } + metricsHandler := info.MetricsHandler.WithTags(map[string]string{"scenario": info.ScenarioName}) return &genericRun{ - executor: g, - info: info, - config: info.Configuration, - logger: info.Logger, - executeTimer: info.MetricsHandler.WithTags( - map[string]string{"scenario": info.ScenarioName}).Timer("omes_execute_histogram"), + executor: g, + info: info, + config: info.Configuration, + logger: info.Logger, + metricsHandler: metricsHandler, + executeTimer: metricsHandler.Timer("omes_execute_histogram"), }, nil } +func iterationStatusCode(err error) codes.Code { + if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) { + return status.FromContextError(err).Code() + } + return status.Code(err) +} + +func (g *genericRun) recordIterationOutcome(outcome string, code codes.Code) { + g.metricsHandler.WithTags(map[string]string{ + "outcome": outcome, + "status_code": code.String(), + }).Counter(iterationsMetricName).Inc(1) +} + +func (g *genericRun) logRunSummary(elapsed time.Duration) { + succeeded := g.completed.Load() + failed := g.failed.Load() + attempted := succeeded + failed + + var successRate, failureRate, successfulThroughput float64 + if attempted > 0 { + successRate = float64(succeeded) / float64(attempted) + failureRate = float64(failed) / float64(attempted) + } + if elapsed > 0 { + successfulThroughput = float64(succeeded) / elapsed.Seconds() + } + + fields := []any{ + "elapsed", elapsed, + "attempted", attempted, + "succeeded", succeeded, + "failed", failed, + "success_rate", successRate, + "failure_rate", failureRate, + "successful_iterations_per_second", successfulThroughput, + } + if failed > 0 { + g.logger.Warnw("Run completed with iteration failures", fields...) + } else { + g.logger.Infow("Run completed", fields...) + } +} + // Run a scenario. // Spins up coroutines according to the scenario configuration. // Each coroutine runs the scenario Execute method in a loop until the scenario duration or max @@ -160,17 +215,27 @@ func (g *genericRun) Run(ctx context.Context) error { case iterErr == nil: run.Duration = elapsed g.completed.Add(1) + g.recordIterationOutcome(iterationOutcomeSucceeded, codes.OK) if g.config.OnCompletion != nil { g.config.OnCompletion(ctx, run) } default: + code := iterationStatusCode(iterErr) g.failed.Add(1) + g.recordIterationOutcome(iterationOutcomeFailed, code) + g.logger.Errorw("Iteration failed", + "scenario", g.info.ScenarioName, + "iteration", run.Iteration, + "status_code", code.String(), + "error", iterErr, + ) if g.config.OnIterationFailure != nil { g.config.OnIterationFailure(ctx, run, iterErr) } } - // Notify the waiter after callbacks finish so Run cannot return before they do. + // Record outcome state and invoke callbacks before notifying the waiter, + // so Run cannot return before they finish. select { case <-ctx.Done(): case doneCh <- err: @@ -193,7 +258,6 @@ func (g *genericRun) Run(ctx context.Context) error { g.logger.Error(err) } else { err = fmt.Errorf("iteration %v failed: %w", run.Iteration, err) - g.logger.Error(err) break retryLoop } @@ -218,8 +282,9 @@ func (g *genericRun) Run(ctx context.Context) error { return fmt.Errorf("timed out while waiting for runs to complete: %w", ctx.Err()) } } + elapsed := time.Since(startTime) if runErr != nil { - return fmt.Errorf("run finished with error after %v: %w", time.Since(startTime), runErr) + return fmt.Errorf("run finished with error after %v: %w", elapsed, runErr) } // ContinueOnIterationFailure changed only when the run stops (it ran to // completion instead of aborting on the first failure); the verdict is @@ -227,11 +292,15 @@ func (g *genericRun) Run(ctx context.Context) error { // outcomes were tallied into the snapshot, so the caller can read the // success/failure counts and apply its own policy on top. if failed := g.failed.Load(); failed > 0 { - completed := g.completed.Load() - g.logger.Infof("Run completed in %v: %d iterations succeeded, %d failed", - time.Since(startTime), completed, failed) - return fmt.Errorf("run completed with %d of %d iterations failed", failed, completed+failed) + succeeded := g.completed.Load() + g.logRunSummary(elapsed) + return &IterationFailuresError{ + Attempted: succeeded + failed, + Succeeded: succeeded, + Failed: failed, + Elapsed: elapsed, + } } - g.logger.Infof("Run completed in %v", time.Since(startTime)) + g.logRunSummary(elapsed) return nil } diff --git a/loadgen/generic_executor_test.go b/loadgen/generic_executor_test.go index 5f5ddd07..8e1b1a69 100644 --- a/loadgen/generic_executor_test.go +++ b/loadgen/generic_executor_test.go @@ -3,14 +3,20 @@ package loadgen import ( "context" "errors" + "fmt" "sync" + "sync/atomic" "testing" "testing/synctest" "time" + "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/require" + omesmetrics "github.com/temporalio/omes/metrics" "go.temporal.io/sdk/client" "go.uber.org/zap" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" ) type iterationTracker struct { @@ -41,8 +47,7 @@ func execute(executor *GenericExecutor, runConfig RunConfiguration) error { } func executeContext(ctx context.Context, executor *GenericExecutor, runConfig RunConfiguration) error { - logger := zap.Must(zap.NewDevelopment()) - defer logger.Sync() + logger := zap.NewNop() info := ScenarioInfo{ MetricsHandler: client.MetricsNopHandler, Logger: logger.Sugar(), @@ -309,10 +314,13 @@ func TestRunContinueOnIterationFailure(t *testing.T) { }, ) - // Every iteration runs (tolerated failures don't abort), but the verdict is - // unchanged: the run still fails because iterations failed. - require.Error(t, err) - require.Contains(t, err.Error(), "3 of 6 iterations failed") + // Every iteration runs (tolerated failures don't abort), while library + // callers still receive a structured degraded-run verdict. + var failures *IterationFailuresError + require.ErrorAs(t, err, &failures) + require.Equal(t, int64(6), failures.Attempted) + require.Equal(t, int64(3), failures.Succeeded) + require.Equal(t, int64(3), failures.Failed) mu.Lock() defer mu.Unlock() require.ElementsMatch(t, []int{1, 3, 5}, completed) @@ -352,6 +360,165 @@ func TestRunReportsNonCancellationFailureAfterCancellation(t *testing.T) { }) } +func TestRunContinueOnIterationFailureDuration(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + var attempts atomic.Int64 + err := execute(&GenericExecutor{ + Execute: func(ctx context.Context, run *Run) error { + time.Sleep(10 * time.Millisecond) + attempts.Add(1) + if run.Iteration%2 == 0 { + return errors.New("deliberate fail from test") + } + return nil + }}, + RunConfiguration{ + Duration: 50 * time.Millisecond, + MaxConcurrent: 1, + ContinueOnIterationFailure: true, + }, + ) + + var failures *IterationFailuresError + require.ErrorAs(t, err, &failures) + require.Equal(t, attempts.Load(), failures.Attempted) + require.Positive(t, failures.Succeeded) + require.Positive(t, failures.Failed) + }) +} + +func TestRunCommitsFailureOutcomeBeforeReturning(t *testing.T) { + for _, continueOnFailure := range []bool{false, true} { + name := "fail-fast" + if continueOnFailure { + name = "continue" + } + t.Run(name, func(t *testing.T) { + callbackStarted := make(chan struct{}) + releaseCallback := make(chan struct{}) + var releaseOnce sync.Once + t.Cleanup(func() { releaseOnce.Do(func() { close(releaseCallback) }) }) + + runDone := make(chan error, 1) + go func() { + runDone <- (&GenericExecutor{ + Execute: func(context.Context, *Run) error { + return errors.New("terminal failure") + }, + }).Run(context.Background(), ScenarioInfo{ + MetricsHandler: client.MetricsNopHandler, + Logger: zap.NewNop().Sugar(), + Configuration: RunConfiguration{ + Iterations: 1, + MaxConcurrent: 1, + ContinueOnIterationFailure: continueOnFailure, + OnIterationFailure: func(context.Context, *Run, error) { + close(callbackStarted) + <-releaseCallback + }, + }, + }) + }() + + select { + case <-callbackStarted: + case <-time.After(time.Second): + t.Fatal("iteration failure callback did not start") + } + + select { + case err := <-runDone: + t.Fatalf("run returned before failure bookkeeping completed: %v", err) + case <-time.After(20 * time.Millisecond): + } + + releaseOnce.Do(func() { close(releaseCallback) }) + select { + case err := <-runDone: + if continueOnFailure { + var failures *IterationFailuresError + require.ErrorAs(t, err, &failures) + require.Equal(t, int64(1), failures.Failed) + } else { + require.ErrorContains(t, err, "run finished with error") + } + case <-time.After(time.Second): + t.Fatal("run did not return after failure bookkeeping completed") + } + }) + } +} + +func TestIterationStatusCode(t *testing.T) { + tests := []struct { + name string + err error + want codes.Code + }{ + {name: "wrapped canceled context", err: fmt.Errorf("start failed: %w", context.Canceled), want: codes.Canceled}, + {name: "wrapped deadline context", err: fmt.Errorf("start failed: %w", context.DeadlineExceeded), want: codes.DeadlineExceeded}, + {name: "wrapped grpc status", err: fmt.Errorf("start failed: %w", status.Error(codes.ResourceExhausted, "busy")), want: codes.ResourceExhausted}, + {name: "plain error", err: errors.New("plain"), want: codes.Unknown}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + require.Equal(t, test.want, iterationStatusCode(test.err)) + }) + } +} + +func TestRunRecordsTerminalIterationOutcomes(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + registry := prometheus.NewRegistry() + metrics := &omesmetrics.Metrics{ + Registry: registry, + Cache: make(map[string]any), + } + logger := zap.NewNop() + err := (&GenericExecutor{ + Execute: func(ctx context.Context, run *Run) error { + if run.Iteration%2 == 0 { + return fmt.Errorf("workflow start failed: %w", status.Error(codes.Unavailable, "down")) + } + return nil + }, + }).Run(context.Background(), ScenarioInfo{ + ScenarioName: "metric-test", + MetricsHandler: metrics.NewHandler(), + Logger: logger.Sugar(), + Configuration: RunConfiguration{ + Iterations: 4, + MaxConcurrent: 1, + ContinueOnIterationFailure: true, + }, + }) + + var failures *IterationFailuresError + require.ErrorAs(t, err, &failures) + families, gatherErr := registry.Gather() + require.NoError(t, gatherErr) + + outcomes := make(map[string]float64) + for _, family := range families { + if family.GetName() != iterationsMetricName { + continue + } + for _, metric := range family.Metric { + labels := make(map[string]string) + for _, label := range metric.Label { + labels[label.GetName()] = label.GetValue() + } + require.Equal(t, "metric-test", labels["scenario"]) + outcomes[labels["outcome"]+"/"+labels["status_code"]] = metric.Counter.GetValue() + } + } + + require.Equal(t, float64(2), outcomes["succeeded/OK"]) + require.Equal(t, float64(2), outcomes["failed/Unavailable"]) + }) +} + // TestRunStoppedIterationsAreNotCountedAsFailures pins that iterations abandoned // by a caller stopping the run are left out of the tallies, so a clean stop is // not reported as a burst of failures. diff --git a/loadgen/scenario.go b/loadgen/scenario.go index b02312da..2d3519f7 100644 --- a/loadgen/scenario.go +++ b/loadgen/scenario.go @@ -291,6 +291,22 @@ const DefaultMaxIterationAttempts = 1 const BaseIterationRetryBackoff = 1 * time.Second const MaxIterationRetryBackoff = 60 * time.Second +// IterationFailuresError reports a run that reached its configured end while +// tolerating one or more terminal iteration failures. +// +// Library callers receive a non-nil error so they can apply their own failure +// policy. +type IterationFailuresError struct { + Attempted int64 + Succeeded int64 + Failed int64 + Elapsed time.Duration +} + +func (e *IterationFailuresError) Error() string { + return fmt.Sprintf("run completed with %d of %d iterations failed", e.Failed, e.Attempted) +} + type RunConfiguration struct { // Number of iterations to run of this scenario (mutually exclusive with Duration). Iterations int diff --git a/metrics/metrics.go b/metrics/metrics.go index 3aa972c1..91693943 100644 --- a/metrics/metrics.go +++ b/metrics/metrics.go @@ -5,6 +5,7 @@ import ( "fmt" "maps" "net/http" + "sort" "sync" "time" @@ -76,10 +77,15 @@ func (h *metricsHandler) WithTags(tags map[string]string) client.MetricsHandler } maps.Copy(mergedTags, tags) - var labels, values []string - for l, v := range mergedTags { - labels = append(labels, l) - values = append(values, v) + labels := make([]string, 0, len(mergedTags)) + for label := range mergedTags { + labels = append(labels, label) + } + sort.Strings(labels) + + values := make([]string, 0, len(labels)) + for _, label := range labels { + values = append(values, mergedTags[label]) } return &metricsHandler{ diff --git a/metrics/metrics_test.go b/metrics/metrics_test.go new file mode 100644 index 00000000..99e1f7e9 --- /dev/null +++ b/metrics/metrics_test.go @@ -0,0 +1,23 @@ +package metrics + +import ( + "reflect" + "testing" +) + +func TestMetricsHandlerSortsTags(t *testing.T) { + handler := (&Metrics{}).NewHandler().WithTags(map[string]string{ + "status_code": "Unavailable", + "scenario": "test-scenario", + "outcome": "failed", + }).(*metricsHandler) + + wantLabels := []string{"outcome", "scenario", "status_code"} + wantValues := []string{"failed", "test-scenario", "Unavailable"} + if !reflect.DeepEqual(wantLabels, handler.labels) { + t.Fatalf("labels = %v, want %v", handler.labels, wantLabels) + } + if !reflect.DeepEqual(wantValues, handler.values) { + t.Fatalf("values = %v, want %v", handler.values, wantValues) + } +}