Skip to content

Commit 11abaa6

Browse files
dannykoppingdannykopping
committed
Review comments, expanding tests
Signed-off-by: Danny Kopping <danny@coder.com>
1 parent 50ccb7c commit 11abaa6

File tree

3 files changed

+80
-30
lines changed

3 files changed

+80
-30
lines changed

‎cli/server.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -894,7 +894,12 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
894894
}
895895
defer closeAgentsFunc()
896896

897-
if err = prometheusmetrics.Experiments(logger, options.PrometheusRegistry, options.DeploymentValues.Experiments.Value(), codersdk.ExperimentsAll); err != nil {
897+
var active codersdk.Experiments
898+
for _, exp := range options.DeploymentValues.Experiments.Value() {
899+
active = append(active, codersdk.Experiment(exp))
900+
}
901+
902+
if err = prometheusmetrics.Experiments(options.PrometheusRegistry, active); err != nil {
898903
return xerrors.Errorf("register experiments metric: %w", err)
899904
}
900905
}

‎coderd/prometheusmetrics/prometheusmetrics.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -517,7 +517,7 @@ func AgentStats(ctx context.Context, logger slog.Logger, registerer prometheus.R
517517
}
518518

519519
// Experiments registers a metric which indicates whether each experiment is enabled or not.
520-
func Experiments(_ slog.Logger, registerer prometheus.Registerer, exps []string, all codersdk.Experiments) error {
520+
func Experiments(registerer prometheus.Registerer, active codersdk.Experiments) error {
521521
experimentsGauge := prometheus.NewGaugeVec(prometheus.GaugeOpts{
522522
Namespace: "coderd",
523523
Name: "experiments",
@@ -527,10 +527,10 @@ func Experiments(_ slog.Logger, registerer prometheus.Registerer, exps []string,
527527
return err
528528
}
529529

530-
for _, exp := range all {
530+
for _, exp := range codersdk.ExperimentsAll {
531531
var val float64
532-
for _, enabled := range exps {
533-
if string(exp) == enabled {
532+
for _, enabled := range active {
533+
if exp == enabled {
534534
val = 1
535535
break
536536
}

‎coderd/prometheusmetrics/prometheusmetrics_test.go

Lines changed: 70 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -503,37 +503,82 @@ func TestAgentStats(t *testing.T) {
503503
func TestExperimentsMetric(t *testing.T) {
504504
t.Parallel()
505505

506-
log := slogtest.Make(t, nil).Leveled(slog.LevelDebug)
507-
reg := prometheus.NewRegistry()
506+
tests := []struct {
507+
name string
508+
experiments codersdk.Experiments
509+
expected map[codersdk.Experiment]float64
510+
}{
511+
{
512+
name: "Enabled experiment is exported in metrics",
513+
experiments: codersdk.Experiments{codersdk.ExperimentSharedPorts},
514+
expected: map[codersdk.Experiment]float64{
515+
codersdk.ExperimentSharedPorts: 1,
516+
},
517+
},
518+
{
519+
name: "Disabled experiment is exported in metrics",
520+
experiments: codersdk.Experiments{},
521+
expected: map[codersdk.Experiment]float64{
522+
codersdk.ExperimentSharedPorts: 0,
523+
},
524+
},
525+
{
526+
name: "Unknown experiment is not exported in metrics",
527+
experiments: codersdk.Experiments{codersdk.Experiment("bob")},
528+
expected: map[codersdk.Experiment]float64{},
529+
},
530+
}
508531

509-
const (
510-
a codersdk.Experiment = "a"
511-
b codersdk.Experiment = "b"
512-
c codersdk.Experiment = "c"
513-
)
514-
allExps := codersdk.Experiments{a, b, c}
515-
require.NoError(t, prometheusmetrics.Experiments(log, reg, []string{string(b), string(c)}, allExps))
532+
for _, tc := range tests {
533+
tc := tc
516534

517-
expectation := map[codersdk.Experiment]float64{
518-
a: 0,
519-
b: 1,
520-
c: 1,
521-
}
535+
t.Run(tc.name, func(t *testing.T) {
536+
t.Parallel()
537+
reg := prometheus.NewRegistry()
522538

523-
out, err := reg.Gather()
524-
require.NoError(t, err)
525-
require.Lenf(t, out, 1, "unexpected number of registered metrics")
539+
require.NoError(t, prometheusmetrics.Experiments(reg, tc.experiments))
540+
541+
out, err := reg.Gather()
542+
require.NoError(t, err)
543+
require.Lenf(t, out, 1, "unexpected number of registered metrics")
544+
545+
seen := make(map[codersdk.Experiment]float64)
546+
547+
for _, metric := range out[0].GetMetric() {
548+
require.Equal(t, "coderd_experiments", out[0].GetName())
549+
550+
labels := metric.GetLabel()
551+
require.Lenf(t, labels, 1, "unexpected number of labels")
526552

527-
for _, metric := range out[0].GetMetric() {
528-
require.Equal(t, "coderd_experiments", out[0].GetName())
553+
experiment := codersdk.Experiment(labels[0].GetValue())
554+
value := metric.GetGauge().GetValue()
529555

530-
labels := metric.GetLabel()
531-
require.Lenf(t, labels, 1, "unexpected number of labels")
556+
seen[experiment] = value
532557

533-
experiment := labels[0].GetValue()
534-
expected, found := expectation[codersdk.Experiment(experiment)]
535-
require.Truef(t, found, "did not find experiment %q in expectations", experiment)
536-
require.EqualValues(t, expected, metric.GetGauge().GetValue())
558+
expectedValue := 0
559+
560+
// Find experiment we expect to be enabled.
561+
for _, exp := range tc.experiments {
562+
if experiment == exp {
563+
expectedValue = 1
564+
break
565+
}
566+
}
567+
568+
require.EqualValuesf(t, expectedValue, value, "expected %d value for experiment %q", expectedValue, experiment)
569+
}
570+
571+
// We don't want to define the state of all experiments because codersdk.ExperimentAll will change at some
572+
// point and break these tests; so we only validate the experiments we know about.
573+
for exp, val := range seen {
574+
expectedVal, found := tc.expected[exp]
575+
if !found {
576+
t.Logf("ignoring experiment %q; it is not listed in expectations", exp)
577+
continue
578+
}
579+
require.Equalf(t, expectedVal, val, "experiment %q did not match expected value %v", exp, expectedVal)
580+
}
581+
})
537582
}
538583
}
539584

0 commit comments

Comments
 (0)