prometheus / client_golang

Prometheus instrumentation library for Go applications
https://pkg.go.dev/github.com/prometheus/client_golang
Apache License 2.0
5.39k stars 1.18k forks source link

testutil: Better registry state assertion framework/API #1639

Open bwplotka opened 1 week ago

bwplotka commented 1 week ago

Context

With our breaking (for a good reason) testutil (Gather|Scrape)AndCompare function change a good discussions e.g. here emerged where we chatted about what broke and how and what those downstream users (e.g. Mimir, Kubernetes and others) needs in terms of registry testing capabilities.

Historically, testutil was meant to be an experimental, internal utils for testing, mostly for client_golang use. Then Prometheus started to use it then everywhere else. Funny enough, we even did some tech talks with @kakkoyun around why it's useful. Now here we are -- many downstream project use it, primarily for:

Notably, when chatting with @dashpole on Kubernetes changes there are two cases currently impossible with client_golang functions:

One could implement everything on their own using ToFloat64 etc, but maybe it's time to redesign testutil, perhaps as another package or even module?

Let's discuss some ideas here, help wanted to propose some design.

Ideally we verify the design on some downstream user e.g. https://github.com/kubernetes/component-base before releasing (:

Yolo Ideas

I guess we could create a redesigned package with a stable API and ways to:

For example (yolo naming and structure):

err := testutilv2.For(registry).
   MetricsMatching(`kubernetes_some_counter_total{network="a"}`, testutilv2.Equals(2)).
   MetricsMatching(`kubernetes_some_counter_total{network=~"b|c"}`, testutilv2.LessThan(10)),
   MetricsMatching(`kubernetes_some_counter_total{network="yolo"}`, testutilv2.NotExists()).
   WaitExpect(5 * time.Second)

err := testutilv2.For(registry).MatchTextExpectation(`HELP kubernetes_some_counter_total "sdfsdfsfsfs"`, <opts>).Expect()
bwplotka commented 6 days ago

This is now higher priority given the revert in https://github.com/prometheus/client_golang/releases/tag/v1.20.5

dashpole commented 6 days ago

One idea: If I can filter a registry/gatherer by metric names, then I would have been more easily assert that a metric is not present (after the breaking change)

bwplotka commented 6 days ago

Nice! similar to https://github.com/prometheus/client_golang/pull/1327/files

machine424 commented 6 days ago

Thanks @bwplotka! The first PR totally slipped past me (not that I would have necessarily spotted anything ;)). I'll keep an eye on this second attempt and try to help, since there’s room for improvement as you explained.