From ab15c0f6fc7e10e3dc07925fda57606095430805 Mon Sep 17 00:00:00 2001 From: mayuresh108 Date: Wed, 4 Nov 2026 14:41:23 +0530 Subject: [PATCH 1/5] PCD-10432 Added caching to ensure a single sweep --- README.md | 22 ++++++++ cache/cache.go | 95 ++++++++++++++++++++++++++++++++++ cache/cache_test.go | 122 ++++++++++++++++++++++++++++++++++++++++++++ main.go | 46 ++++++++++++++--- 4 files changed, 279 insertions(+), 6 deletions(-) create mode 100644 cache/cache.go create mode 100644 cache/cache_test.go diff --git a/README.md b/README.md index 0c6a10d7..be68acac 100644 --- a/README.md +++ b/README.md @@ -108,6 +108,8 @@ Flags: Disable UUID generation for Cinder agents --[no-]multi-cloud Toggle the multiple cloud scraping mode under /probe?cloud= --domain-id=DOMAIN-ID Gather metrics only for the given Domain ID (defaults to all domains) + --[no-]cache Collect metrics in the background and serve every scrape from the last collection (legacy mode only) + --cache-ttl=300s TTL for cached metrics (eg. 10s, 11m, 1h). Collection runs every cache-ttl/2; cached metrics older than cache-ttl are not served --[no-]disable-service.network Disable the network service exporter --[no-]disable-service.compute @@ -174,6 +176,26 @@ curl "https://localhost:9180/probe?cloud=test.cloud&include_services=network,com curl "https://localhost:9180/probe?cloud=test.cloud&exclude_services=load-balancer,dns" ``` +### Cache mechanism + +`--cache` applies only to legacy mode (`/metrics`, single cloud). It has no +effect in multi-cloud mode (`/probe?cloud=`), since each probe already +targets a specific cloud and is not swept in the background. + +Without `--cache`, every scrape of `/metrics` runs a full sweep of the OpenStack +APIs, so two scrapers cause two sweeps. With `--cache`: + +* A background loop sweeps once at start-up and then every `cache-ttl/2`. + Sweeps never overlap; a sweep slower than `cache-ttl/2` is followed + immediately by the next one. +* The stored result is replaced only when a sweep completes, and every scrape + is served from it without contacting OpenStack. +* If the stored result is older than `cache-ttl`, scrapes return no OpenStack + metrics until the next sweep completes. Set `cache-ttl` comfortably above + the slowest sweep time. +* Until the first sweep completes after start-up, scrapes return no OpenStack + metrics. + ### OpenStack configuration The cloud credentials and identity configuration diff --git a/cache/cache.go b/cache/cache.go new file mode 100644 index 00000000..553db34d --- /dev/null +++ b/cache/cache.go @@ -0,0 +1,95 @@ +// Package cache decouples Prometheus scrapes from OpenStack API sweeps. +// +// Without the cache every scrape of /metrics runs a full sweep of every +// enabled OpenStack service, so N scrapers mean N concurrent sweeps. With the +// cache a single background loop runs the sweep and stores the result, and +// every scrape is served from that stored result. +// +// Behaviour (ported from upstream openstack-exporter's --cache): +// - a sweep runs at start-up and then every ttl/2; +// - sweeps never overlap: if a sweep takes longer than ttl/2 the next one +// starts as soon as it finishes; +// - the stored result is replaced only once a sweep has fully completed, so +// a scrape never sees a mix of old and new values; +// - a stored result older than ttl is treated as expired and scrapes get no +// metrics until the next sweep completes. +package cache + +import ( + "sync" + "time" + + "github.com/go-kit/log" + "github.com/go-kit/log/level" + "github.com/prometheus/client_golang/prometheus" + dto "github.com/prometheus/client_model/go" +) + +// Gatherer is a prometheus.Gatherer that serves the last result gathered from +// source instead of gathering on every call. +type Gatherer struct { + source prometheus.Gatherer + ttl time.Duration + now func() time.Time + logger log.Logger + + mu sync.RWMutex + families []*dto.MetricFamily + updated time.Time +} + +// New returns a Gatherer caching the output of source for ttl. +func New(source prometheus.Gatherer, ttl time.Duration, logger log.Logger) *Gatherer { + return &Gatherer{source: source, ttl: ttl, now: time.Now, logger: logger} +} + +// Refresh runs one sweep against source and stores the result. +// +// prometheus.Gatherer may return partial results together with an error; the +// partial results are stored, matching what an uncached scrape would serve. +func (c *Gatherer) Refresh() error { + families, err := c.source.Gather() + if families != nil { + c.mu.Lock() + c.families = families + c.updated = c.now() + c.mu.Unlock() + } + return err +} + +// Gather implements prometheus.Gatherer. It never contacts OpenStack. +func (c *Gatherer) Gather() ([]*dto.MetricFamily, error) { + c.mu.RLock() + defer c.mu.RUnlock() + if c.updated.IsZero() || c.now().Sub(c.updated) > c.ttl { + return nil, nil + } + return c.families, nil +} + +// Run refreshes the cache immediately and then every ttl/2 until stop is +// closed. It blocks, so call it in its own goroutine. +func (c *Gatherer) Run(stop <-chan struct{}) { + level.Info(c.logger).Log("msg", "Cache: starting background collection", "ttl", c.ttl, "interval", c.ttl/2) + ticker := time.NewTicker(c.ttl / 2) + defer ticker.Stop() + + for { + c.refreshAndLog() + select { + case <-ticker.C: + case <-stop: + level.Info(c.logger).Log("msg", "Cache: background collection stopped") + return + } + } +} + +func (c *Gatherer) refreshAndLog() { + start := c.now() + if err := c.Refresh(); err != nil { + level.Error(c.logger).Log("msg", "Cache: collection finished with errors", "err", err) + } + level.Info(c.logger).Log("msg", "Cache: collection finished", "duration", c.now().Sub(start)) +} diff --git a/cache/cache_test.go b/cache/cache_test.go new file mode 100644 index 00000000..5f7772d2 --- /dev/null +++ b/cache/cache_test.go @@ -0,0 +1,122 @@ +package cache + +import ( + "errors" + "sync/atomic" + "testing" + "time" + + "github.com/go-kit/log" + "github.com/prometheus/client_golang/prometheus" + dto "github.com/prometheus/client_model/go" + "github.com/stretchr/testify/assert" +) + +// countingGatherer records how many sweeps were run against it. +type countingGatherer struct { + calls int32 + families []*dto.MetricFamily + err error +} + +func (g *countingGatherer) Gather() ([]*dto.MetricFamily, error) { + atomic.AddInt32(&g.calls, 1) + return g.families, g.err +} + +func family(name string) *dto.MetricFamily { + return &dto.MetricFamily{Name: &name} +} + +func TestGatherBeforeFirstRefreshIsEmpty(t *testing.T) { + src := &countingGatherer{families: []*dto.MetricFamily{family("a")}} + c := New(src, time.Minute, log.NewNopLogger()) + + mfs, err := c.Gather() + assert.NoError(t, err) + assert.Empty(t, mfs) + assert.Equal(t, int32(0), src.calls, "Gather must not sweep") +} + +func TestManyScrapesShareOneSweep(t *testing.T) { + src := &countingGatherer{families: []*dto.MetricFamily{family("a"), family("b")}} + c := New(src, time.Minute, log.NewNopLogger()) + + assert.NoError(t, c.Refresh()) + for i := 0; i < 10; i++ { + mfs, err := c.Gather() + assert.NoError(t, err) + assert.Equal(t, src.families, mfs) + } + assert.Equal(t, int32(1), src.calls) +} + +func TestExpiredCacheIsEmpty(t *testing.T) { + src := &countingGatherer{families: []*dto.MetricFamily{family("a")}} + c := New(src, time.Minute, log.NewNopLogger()) + now := time.Unix(1000, 0) + c.now = func() time.Time { return now } + + assert.NoError(t, c.Refresh()) + + now = now.Add(time.Minute) + mfs, _ := c.Gather() + assert.Len(t, mfs, 1, "exactly ttl old is still valid") + + now = now.Add(time.Second) + mfs, _ = c.Gather() + assert.Empty(t, mfs, "older than ttl is expired") +} + +func TestPartialResultsAreStored(t *testing.T) { + src := &countingGatherer{families: []*dto.MetricFamily{family("a")}, err: errors.New("boom")} + c := New(src, time.Minute, log.NewNopLogger()) + + assert.Error(t, c.Refresh()) + mfs, _ := c.Gather() + assert.Len(t, mfs, 1) +} + +func TestFailedRefreshKeepsPreviousResult(t *testing.T) { + src := &countingGatherer{families: []*dto.MetricFamily{family("a")}} + c := New(src, time.Minute, log.NewNopLogger()) + assert.NoError(t, c.Refresh()) + + src.families, src.err = nil, errors.New("boom") + assert.Error(t, c.Refresh()) + + mfs, _ := c.Gather() + assert.Len(t, mfs, 1) +} + +func TestRunRefreshesUntilStopped(t *testing.T) { + src := &countingGatherer{families: []*dto.MetricFamily{family("a")}} + c := New(src, 20*time.Millisecond, log.NewNopLogger()) + stop := make(chan struct{}) + done := make(chan struct{}) + + go func() { c.Run(stop); close(done) }() + time.Sleep(55 * time.Millisecond) + close(stop) + <-done + + // One sweep at start-up plus roughly one every 10ms. + calls := atomic.LoadInt32(&src.calls) + assert.True(t, calls >= 3, "expected at least 3 sweeps, got %d", calls) +} + +func TestServesRealRegistry(t *testing.T) { + reg := prometheus.NewPedanticRegistry() + g := prometheus.NewGauge(prometheus.GaugeOpts{Name: "openstack_test_gauge", Help: "h"}) + reg.MustRegister(g) + g.Set(1) + + c := New(reg, time.Minute, log.NewNopLogger()) + assert.NoError(t, c.Refresh()) + + g.Set(2) // a scrape between sweeps must still see the swept value + mfs, err := c.Gather() + assert.NoError(t, err) + assert.Len(t, mfs, 1) + assert.Equal(t, 1.0, mfs[0].Metric[0].GetGauge().GetValue()) +} diff --git a/main.go b/main.go index f2f177bf..35f97288 100644 --- a/main.go +++ b/main.go @@ -10,9 +10,11 @@ import ( kingpin "github.com/alecthomas/kingpin/v2" "github.com/go-kit/log" "github.com/go-kit/log/level" + "github.com/openstack-exporter/openstack-exporter/cache" "github.com/openstack-exporter/openstack-exporter/exporters" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promhttp" + dto "github.com/prometheus/client_model/go" "github.com/prometheus/common/promlog" "github.com/prometheus/common/promlog/flag" "github.com/prometheus/common/version" @@ -37,6 +39,8 @@ var ( cloud = kingpin.Arg("cloud", "name or id of the cloud to gather metrics from").String() multiCloud = kingpin.Flag("multi-cloud", "Toggle the multiple cloud scraping mode under /probe?cloud=").Default("false").Bool() domainID = kingpin.Flag("domain-id", "Gather metrics only for the given Domain ID (defaults to all domains)").String() + cacheEnable = kingpin.Flag("cache", "Collect metrics in the background and serve every scrape from the last collection (legacy mode only)").Default("false").Bool() + cacheTTL = kingpin.Flag("cache-ttl", "TTL for cached metrics (eg. 10s, 11m, 1h). Collection runs every cache-ttl/2; cached metrics older than cache-ttl are not served").Default("300s").Duration() ) func main() { @@ -68,6 +72,11 @@ func main() { os.Setenv("OS_CLIENT_CONFIG_FILE", *osClientConfig) } + if *cacheEnable && *cacheTTL <= 0 { + level.Error(logger).Log("err", "--cache-ttl must be greater than 0") + os.Exit(-1) + } + links := []web.LandingLinks{} if *multiCloud { @@ -164,8 +173,12 @@ func probeHandler(services map[string]*bool, logger log.Logger) http.HandlerFunc } } -func metricHandler(services map[string]*bool, logger log.Logger) http.HandlerFunc { - return func(w http.ResponseWriter, r *http.Request) { +// sweepFunc returns a function that runs one full sweep of the enabled +// services' OpenStack APIs and returns the resulting metric families. Both +// the uncached, per-request path and the --cache background loop drive the +// sweep through this same function. +func sweepFunc(services map[string]*bool, logger log.Logger) func() ([]*dto.MetricFamily, error) { + return func() ([]*dto.MetricFamily, error) { level.Info(logger).Log("msg", "Starting openstack exporter version for cloud", "version", version.Info(), "cloud", *cloud) level.Info(logger).Log("msg", "Build context", "build_context", version.BuildContext()) @@ -191,11 +204,32 @@ func metricHandler(services map[string]*bool, logger log.Logger) http.HandlerFun } if enabledExporters == 0 { - level.Error(logger).Log("err", "No exporter has been enabled, exiting") - os.Exit(-1) + return nil, fmt.Errorf("no exporter has been enabled") } - h := promhttp.HandlerFor(registry, promhttp.HandlerOpts{}) - h.ServeHTTP(w, r) + return registry.Gather() + } +} + +func metricHandler(services map[string]*bool, logger log.Logger) http.HandlerFunc { + sweep := sweepFunc(services, logger) + + if *cacheEnable { + // A single background loop runs the sweep; every scrape is served + // from its last result instead of triggering its own sweep. + cached := cache.New(prometheus.GathererFunc(sweep), *cacheTTL, logger) + go cached.Run(make(chan struct{})) + return promhttp.HandlerFor(cached, promhttp.HandlerOpts{}).ServeHTTP + } + + return func(w http.ResponseWriter, r *http.Request) { + families, err := sweep() + if err != nil { + level.Error(logger).Log("err", err) + os.Exit(-1) + } + promhttp.HandlerFor(prometheus.GathererFunc(func() ([]*dto.MetricFamily, error) { + return families, nil + }), promhttp.HandlerOpts{}).ServeHTTP(w, r) } } From 58ed642f83aea21a7c94a972d966f41632052866 Mon Sep 17 00:00:00 2001 From: mayuresh108 Date: Wed, 4 Nov 2026 14:45:09 +0530 Subject: [PATCH 2/5] PCD-10432 Added caching to ensure a single sweep --- README.md | 11 ++++++----- cache/cache.go | 26 ++++++++++++++------------ cache/cache_test.go | 14 +++++++------- main.go | 17 ++++++++++++----- 4 files changed, 39 insertions(+), 29 deletions(-) diff --git a/README.md b/README.md index be68acac..6e066816 100644 --- a/README.md +++ b/README.md @@ -109,7 +109,8 @@ Flags: --[no-]multi-cloud Toggle the multiple cloud scraping mode under /probe?cloud= --domain-id=DOMAIN-ID Gather metrics only for the given Domain ID (defaults to all domains) --[no-]cache Collect metrics in the background and serve every scrape from the last collection (legacy mode only) - --cache-ttl=300s TTL for cached metrics (eg. 10s, 11m, 1h). Collection runs every cache-ttl/2; cached metrics older than cache-ttl are not served + --cache-ttl=300s TTL for cached metrics (eg. 10s, 11m, 1h). Cached metrics older than cache-ttl are not served + --cache-interval=150s How often the background collection runs when --cache is enabled (eg. 30s, 5m). Must be less than --cache-ttl --[no-]disable-service.network Disable the network service exporter --[no-]disable-service.compute @@ -185,14 +186,14 @@ targets a specific cloud and is not swept in the background. Without `--cache`, every scrape of `/metrics` runs a full sweep of the OpenStack APIs, so two scrapers cause two sweeps. With `--cache`: -* A background loop sweeps once at start-up and then every `cache-ttl/2`. - Sweeps never overlap; a sweep slower than `cache-ttl/2` is followed - immediately by the next one. +* A background loop sweeps once at start-up and then every `cache-interval`. + Sweeps never overlap; a sweep slower than `cache-interval` is followed + immediately by the next one. `cache-interval` must be less than `cache-ttl`. * The stored result is replaced only when a sweep completes, and every scrape is served from it without contacting OpenStack. * If the stored result is older than `cache-ttl`, scrapes return no OpenStack metrics until the next sweep completes. Set `cache-ttl` comfortably above - the slowest sweep time. + `cache-interval` plus the slowest sweep time. * Until the first sweep completes after start-up, scrapes return no OpenStack metrics. diff --git a/cache/cache.go b/cache/cache.go index 553db34d..57bc95ae 100644 --- a/cache/cache.go +++ b/cache/cache.go @@ -6,8 +6,8 @@ // every scrape is served from that stored result. // // Behaviour (ported from upstream openstack-exporter's --cache): -// - a sweep runs at start-up and then every ttl/2; -// - sweeps never overlap: if a sweep takes longer than ttl/2 the next one +// - a sweep runs at start-up and then every interval; +// - sweeps never overlap: if a sweep takes longer than interval the next one // starts as soon as it finishes; // - the stored result is replaced only once a sweep has fully completed, so // a scrape never sees a mix of old and new values; @@ -28,19 +28,21 @@ import ( // Gatherer is a prometheus.Gatherer that serves the last result gathered from // source instead of gathering on every call. type Gatherer struct { - source prometheus.Gatherer - ttl time.Duration - now func() time.Time - logger log.Logger + source prometheus.Gatherer + ttl time.Duration + interval time.Duration + now func() time.Time + logger log.Logger mu sync.RWMutex families []*dto.MetricFamily updated time.Time } -// New returns a Gatherer caching the output of source for ttl. -func New(source prometheus.Gatherer, ttl time.Duration, logger log.Logger) *Gatherer { - return &Gatherer{source: source, ttl: ttl, now: time.Now, logger: logger} +// New returns a Gatherer that sweeps source every interval and serves each +// sweep's result for up to ttl. +func New(source prometheus.Gatherer, ttl, interval time.Duration, logger log.Logger) *Gatherer { + return &Gatherer{source: source, ttl: ttl, interval: interval, now: time.Now, logger: logger} } // Refresh runs one sweep against source and stores the result. @@ -68,11 +70,11 @@ func (c *Gatherer) Gather() ([]*dto.MetricFamily, error) { return c.families, nil } -// Run refreshes the cache immediately and then every ttl/2 until stop is +// Run refreshes the cache immediately and then every interval until stop is // closed. It blocks, so call it in its own goroutine. func (c *Gatherer) Run(stop <-chan struct{}) { - level.Info(c.logger).Log("msg", "Cache: starting background collection", "ttl", c.ttl, "interval", c.ttl/2) - ticker := time.NewTicker(c.ttl / 2) + level.Info(c.logger).Log("msg", "Cache: starting background collection", "ttl", c.ttl, "interval", c.interval) + ticker := time.NewTicker(c.interval) defer ticker.Stop() for { diff --git a/cache/cache_test.go b/cache/cache_test.go index 5f7772d2..afbc2775 100644 --- a/cache/cache_test.go +++ b/cache/cache_test.go @@ -30,7 +30,7 @@ func family(name string) *dto.MetricFamily { func TestGatherBeforeFirstRefreshIsEmpty(t *testing.T) { src := &countingGatherer{families: []*dto.MetricFamily{family("a")}} - c := New(src, time.Minute, log.NewNopLogger()) + c := New(src, time.Minute, 30*time.Second, log.NewNopLogger()) mfs, err := c.Gather() assert.NoError(t, err) @@ -40,7 +40,7 @@ func TestGatherBeforeFirstRefreshIsEmpty(t *testing.T) { func TestManyScrapesShareOneSweep(t *testing.T) { src := &countingGatherer{families: []*dto.MetricFamily{family("a"), family("b")}} - c := New(src, time.Minute, log.NewNopLogger()) + c := New(src, time.Minute, 30*time.Second, log.NewNopLogger()) assert.NoError(t, c.Refresh()) for i := 0; i < 10; i++ { @@ -53,7 +53,7 @@ func TestManyScrapesShareOneSweep(t *testing.T) { func TestExpiredCacheIsEmpty(t *testing.T) { src := &countingGatherer{families: []*dto.MetricFamily{family("a")}} - c := New(src, time.Minute, log.NewNopLogger()) + c := New(src, time.Minute, 30*time.Second, log.NewNopLogger()) now := time.Unix(1000, 0) c.now = func() time.Time { return now } @@ -70,7 +70,7 @@ func TestExpiredCacheIsEmpty(t *testing.T) { func TestPartialResultsAreStored(t *testing.T) { src := &countingGatherer{families: []*dto.MetricFamily{family("a")}, err: errors.New("boom")} - c := New(src, time.Minute, log.NewNopLogger()) + c := New(src, time.Minute, 30*time.Second, log.NewNopLogger()) assert.Error(t, c.Refresh()) mfs, _ := c.Gather() @@ -79,7 +79,7 @@ func TestPartialResultsAreStored(t *testing.T) { func TestFailedRefreshKeepsPreviousResult(t *testing.T) { src := &countingGatherer{families: []*dto.MetricFamily{family("a")}} - c := New(src, time.Minute, log.NewNopLogger()) + c := New(src, time.Minute, 30*time.Second, log.NewNopLogger()) assert.NoError(t, c.Refresh()) src.families, src.err = nil, errors.New("boom") @@ -91,7 +91,7 @@ func TestFailedRefreshKeepsPreviousResult(t *testing.T) { func TestRunRefreshesUntilStopped(t *testing.T) { src := &countingGatherer{families: []*dto.MetricFamily{family("a")}} - c := New(src, 20*time.Millisecond, log.NewNopLogger()) + c := New(src, time.Minute, 10*time.Millisecond, log.NewNopLogger()) stop := make(chan struct{}) done := make(chan struct{}) @@ -111,7 +111,7 @@ func TestServesRealRegistry(t *testing.T) { reg.MustRegister(g) g.Set(1) - c := New(reg, time.Minute, log.NewNopLogger()) + c := New(reg, time.Minute, 30*time.Second, log.NewNopLogger()) assert.NoError(t, c.Refresh()) g.Set(2) // a scrape between sweeps must still see the swept value diff --git a/main.go b/main.go index 35f97288..0368847b 100644 --- a/main.go +++ b/main.go @@ -40,7 +40,8 @@ var ( multiCloud = kingpin.Flag("multi-cloud", "Toggle the multiple cloud scraping mode under /probe?cloud=").Default("false").Bool() domainID = kingpin.Flag("domain-id", "Gather metrics only for the given Domain ID (defaults to all domains)").String() cacheEnable = kingpin.Flag("cache", "Collect metrics in the background and serve every scrape from the last collection (legacy mode only)").Default("false").Bool() - cacheTTL = kingpin.Flag("cache-ttl", "TTL for cached metrics (eg. 10s, 11m, 1h). Collection runs every cache-ttl/2; cached metrics older than cache-ttl are not served").Default("300s").Duration() + cacheTTL = kingpin.Flag("cache-ttl", "TTL for cached metrics (eg. 10s, 11m, 1h). Cached metrics older than cache-ttl are not served").Default("300s").Duration() + cacheInterval = kingpin.Flag("cache-interval", "How often the background collection runs when --cache is enabled (eg. 30s, 5m). Must be less than --cache-ttl").Default("150s").Duration() ) func main() { @@ -72,9 +73,15 @@ func main() { os.Setenv("OS_CLIENT_CONFIG_FILE", *osClientConfig) } - if *cacheEnable && *cacheTTL <= 0 { - level.Error(logger).Log("err", "--cache-ttl must be greater than 0") - os.Exit(-1) + if *cacheEnable { + if *cacheTTL <= 0 || *cacheInterval <= 0 { + level.Error(logger).Log("err", "--cache-ttl and --cache-interval must be greater than 0") + os.Exit(-1) + } + if *cacheInterval >= *cacheTTL { + level.Error(logger).Log("err", "--cache-interval must be less than --cache-ttl, otherwise the cache expires between collections", "cache_interval", *cacheInterval, "cache_ttl", *cacheTTL) + os.Exit(-1) + } } links := []web.LandingLinks{} @@ -217,7 +224,7 @@ func metricHandler(services map[string]*bool, logger log.Logger) http.HandlerFun if *cacheEnable { // A single background loop runs the sweep; every scrape is served // from its last result instead of triggering its own sweep. - cached := cache.New(prometheus.GathererFunc(sweep), *cacheTTL, logger) + cached := cache.New(prometheus.GathererFunc(sweep), *cacheTTL, *cacheInterval, logger) go cached.Run(make(chan struct{})) return promhttp.HandlerFor(cached, promhttp.HandlerOpts{}).ServeHTTP } From 5e67e35e49fa868dc5ee3c6ce172271e62eb5bd1 Mon Sep 17 00:00:00 2001 From: Kunal Bauskar Date: Thu, 5 Nov 2026 10:32:35 +0530 Subject: [PATCH 3/5] PCD-10432 Add PF9 container build (Dockerfile.pf9, Makefile.pf9) Pulled from private/kunal/PCD-10199-v170 (PCD-10199) so this branch can be built into an image directly. Kept separate from upstream's promu-based Dockerfile/Makefile so rebases onto upstream stay conflict-free. Co-Authored-By: Claude Sonnet 5 --- Dockerfile.pf9 | 46 ++++++++++++++++++++++++++++++++++ Makefile.pf9 | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 114 insertions(+) create mode 100644 Dockerfile.pf9 create mode 100644 Makefile.pf9 diff --git a/Dockerfile.pf9 b/Dockerfile.pf9 new file mode 100644 index 00000000..bfc89004 --- /dev/null +++ b/Dockerfile.pf9 @@ -0,0 +1,46 @@ +# syntax=docker/dockerfile:1 +# +# Platform9 build of openstack-exporter. +# +# Unlike the upstream Dockerfile (which expects promu to have already produced +# .build/$OS-$ARCH/openstack-exporter), this builds the binary itself so CI +# agents need nothing but docker. The runtime stage is deliberately the same +# base upstream publishes, so a PF9-built image differs from the upstream +# release only by the code in this repo. + +ARG GO_VERSION=1.24 + +FROM golang:${GO_VERSION}-alpine AS builder + +ARG GOOS=linux +ARG GOARCH=amd64 +ARG CGO_ENABLED=0 + +WORKDIR /src + +COPY go.mod go.sum ./ +RUN go mod download + +COPY . . + +RUN mkdir -p /out && \ + GOOS=${GOOS} GOARCH=${GOARCH} CGO_ENABLED=${CGO_ENABLED} \ + go build -trimpath -ldflags="-s -w" -o /out/openstack-exporter . + +FROM quay.io/prometheus/busybox:latest + +ARG VERSION=1.0.0 +ARG BUILD_ID=0 +ARG BRANCH=local +ARG APP_METADATA= + +COPY --from=builder /out/openstack-exporter /bin/openstack-exporter + +ENTRYPOINT ["/bin/openstack-exporter"] +EXPOSE 9180 + +LABEL com.platform9.pf9_version=${VERSION} +LABEL com.platform9.build=${BUILD_ID} +LABEL com.platform9.version="${VERSION}-${BUILD_ID}" +LABEL com.platform9.branch=${BRANCH} +LABEL com.platform9.app_metadata=${APP_METADATA} diff --git a/Makefile.pf9 b/Makefile.pf9 new file mode 100644 index 00000000..96648cee --- /dev/null +++ b/Makefile.pf9 @@ -0,0 +1,68 @@ +# Platform9 container build for openstack-exporter. +# +# Kept separate from the upstream Makefile so rebases against +# openstack-exporter/openstack-exporter stay conflict-free. +# +# make -f Makefile.pf9 all +# +# TeamCity supplies PF9_VERSION and BUILD_NUMBER; the defaults below are only +# for local runs. + +APP_NAME ?= openstack-exporter +DOCKER ?= docker + +GOOS ?= linux +GOARCH ?= amd64 +CGO_ENABLED ?= 0 +GO_VERSION ?= 1.24 + +PF9_VERSION ?= 1.0.0 +BUILD_NUMBER ?= 0 +BRANCH ?= $(or $(TEAMCITY_BUILD_BRANCH),$(shell git symbolic-ref --short HEAD 2>/dev/null || echo local)) +APP_METADATA ?= + +IMAGE_NAME ?= quay.io/platform9/$(APP_NAME) +ECR_IMAGE_NAME ?= public.ecr.aws/platform9/$(APP_NAME) +IMAGE_TAG ?= $(PF9_VERSION)-$(BUILD_NUMBER) +FULL_IMAGE ?= $(IMAGE_NAME):$(IMAGE_TAG) +ECR_FULL_IMAGE ?= $(ECR_IMAGE_NAME):$(IMAGE_TAG) + +DOCKERFILE ?= Dockerfile.pf9 + +.PHONY: all build push print clean + +all: build push print + +build: + @echo "Building Docker image: $(FULL_IMAGE)" + $(DOCKER) build \ + --build-arg GO_VERSION=$(GO_VERSION) \ + --build-arg GOOS=$(GOOS) \ + --build-arg GOARCH=$(GOARCH) \ + --build-arg CGO_ENABLED=$(CGO_ENABLED) \ + --build-arg VERSION=$(PF9_VERSION) \ + --build-arg BUILD_ID=$(BUILD_NUMBER) \ + --build-arg BRANCH=$(BRANCH) \ + --build-arg APP_METADATA=$(APP_METADATA) \ + -f $(DOCKERFILE) -t $(FULL_IMAGE) . + $(DOCKER) tag $(FULL_IMAGE) $(ECR_FULL_IMAGE) + echo $(IMAGE_TAG) > container-tag + +push: + @echo "Pushing Docker image: $(FULL_IMAGE) and $(ECR_FULL_IMAGE)" + $(DOCKER) push $(FULL_IMAGE) + $(DOCKER) push $(ECR_FULL_IMAGE) + +print: + @echo "GOOS=$(GOOS)" + @echo "GOARCH=$(GOARCH)" + @echo "CGO_ENABLED=$(CGO_ENABLED)" + @echo "PF9_VERSION=$(PF9_VERSION)" + @echo "BUILD_NUMBER=$(BUILD_NUMBER)" + @echo "BRANCH=$(BRANCH)" + @echo "FULL_IMAGE=$(FULL_IMAGE)" + @echo "ECR_FULL_IMAGE=$(ECR_FULL_IMAGE)" + +clean: + rm -f container-tag + -$(DOCKER) rmi $(FULL_IMAGE) $(ECR_FULL_IMAGE) 2>/dev/null || true From f2cceab155100554a4249808619701106f30cc95 Mon Sep 17 00:00:00 2001 From: Kunal Bauskar Date: Thu, 5 Nov 2026 11:33:08 +0530 Subject: [PATCH 4/5] PCD-10432 Collect metrics concurrently within each exporter Each exporter's Collect() ran every metric's Fn serially, so a service's wall-clock cost is the sum of its metrics (e.g. neutron: port 192s + subnets 94s + network_ip_availabilities_total 70s, per the sweep-timing evidence on this ticket), not the slowest one of them. Ported upstream's errgroup-based Collect (v2.0.0-alpha, exporters/exporter.go, commit 3588a4a) onto this fork's go-kit logging and collection path, without the rest of that alpha's changes (gophercloud v2, slog, persistent exporters). Metrics only read exporter state (client, Metrics map, config) and write to ch, both safe for concurrent use, and the one shared mutable state (the package-level endpointOpts map) is only written during the serial exporter-setup phase, before any Collect() runs -- confirmed by reading every exporters/*.go metric function and by `go test -race` (20x repeated, GOMAXPROCS=1/4) across the full suite. That same -race run caught a real, pre-existing data race once collection went concurrent: jarcoal/httpmock's Responder.Times() uses an unsynchronized closure counter (confirmed unfixed as of httpmock v1.4.2, upstream's own version -- their CI doesn't run -race either, so they've never hit this). Two metrics sharing one mocked URL (e.g. glance's ListImages and ListImageBytes) now call it concurrently. Fixed by serializing our own test responder registration in exporter_test.go:SetResponseFromFixture with a mutex, rather than patching the vendored library. Co-Authored-By: Claude Sonnet 5 --- exporters/exporter.go | 32 ++++++++++++++++++++++++++------ exporters/exporter_test.go | 13 ++++++++++++- go.mod | 4 ++-- 3 files changed, 40 insertions(+), 9 deletions(-) diff --git a/exporters/exporter.go b/exporters/exporter.go index e53637fa..398f967f 100644 --- a/exporters/exporter.go +++ b/exporters/exporter.go @@ -4,6 +4,7 @@ import ( "crypto/tls" "fmt" "net/http" + "sync/atomic" "time" "github.com/go-kit/log" @@ -12,6 +13,7 @@ import ( "github.com/gophercloud/utils/openstack/clientconfig" "github.com/hashicorp/go-uuid" "github.com/prometheus/client_golang/prometheus" + "golang.org/x/sync/errgroup" ) type Metric struct { @@ -111,10 +113,21 @@ func (exporter *BaseOpenStackExporter) RunCollection(metric *PrometheusMetric, m return nil } +// Collect runs every metric's Fn concurrently instead of one after another, +// so one exporter's wall-clock cost is the slowest single metric rather than +// their sum. Metrics only read exporter state (the OpenStack client, +// Metrics map, config) and write to ch, both safe for concurrent use, so +// this changes how long collection takes, not what it collects. +// +// Ported from upstream's errgroup-based Collect (v2.0.0-alpha, +// exporters/exporter.go) onto this fork's logging and collection path, +// without the rest of that alpha's changes. func (exporter *BaseOpenStackExporter) Collect(ch chan<- prometheus.Metric) { - metricsDown := 0 + var metricsDown int32 metricsCount := len(exporter.Metrics) + var g errgroup.Group + for name, metric := range exporter.Metrics { if metric.Fn == nil { level.Debug(exporter.logger).Log("msg", "No function handler set for metric", "metric", name) @@ -122,14 +135,21 @@ func (exporter *BaseOpenStackExporter) Collect(ch chan<- prometheus.Metric) { continue } - if err := exporter.RunCollection(metric, name, ch, exporter.logger); err != nil { - level.Error(exporter.logger).Log("err", "Failed to collect metric for exporter", "exporter", exporter.Name, "error", err) - metricsDown++ - } + name := name + metric := metric + g.Go(func() error { + if err := exporter.RunCollection(metric, name, ch, exporter.logger); err != nil { + level.Error(exporter.logger).Log("err", "Failed to collect metric for exporter", "exporter", exporter.Name, "error", err) + atomic.AddInt32(&metricsDown, 1) + } + return nil + }) } + _ = g.Wait() + //If all metrics collections fails for a given service, we'll flag it as down. - if metricsDown >= metricsCount { + if int(atomic.LoadInt32(&metricsDown)) >= metricsCount { ch <- prometheus.MustNewConstMetric(exporter.Metrics["up"].Metric, prometheus.GaugeValue, 0) } else { ch <- prometheus.MustNewConstMetric(exporter.Metrics["up"].Metric, prometheus.GaugeValue, 1) diff --git a/exporters/exporter_test.go b/exporters/exporter_test.go index cf478c26..1b5d4030 100644 --- a/exporters/exporter_test.go +++ b/exporters/exporter_test.go @@ -6,6 +6,7 @@ import ( "net/http" "os" "path" + "sync" "testing" "github.com/go-kit/log" @@ -35,7 +36,17 @@ func (suite *BaseOpenStackTestSuite) SetResponseFromFixture(method string, statu } responder := httpmock.ResponderFromResponse(response).Times(2) - httpmock.RegisterResponder(method, url, responder) + + // httpmock's Times() call counter is a plain, unsynchronized closure + // variable (still true as of httpmock v1.4.2), so two metrics now + // collected concurrently (Collect runs them in parallel) can race on it + // if they hit the same mocked URL. Serialize calls into it ourselves. + var mu sync.Mutex + httpmock.RegisterResponder(method, url, func(req *http.Request) (*http.Response, error) { + mu.Lock() + defer mu.Unlock() + return responder(req) + }) } func (suite *BaseOpenStackTestSuite) MakeURL(resource string, port string) string { diff --git a/go.mod b/go.mod index 1e048f26..e41fa5b5 100644 --- a/go.mod +++ b/go.mod @@ -10,10 +10,12 @@ require ( github.com/hashicorp/go-uuid v1.0.3 github.com/jarcoal/httpmock v1.3.1 github.com/prometheus/client_golang v1.16.0 + github.com/prometheus/client_model v0.4.0 github.com/prometheus/common v0.44.0 github.com/prometheus/exporter-toolkit v0.10.0 github.com/stretchr/testify v1.8.4 go4.org/netipx v0.0.0-20230728184502-ec4c8b891b28 + golang.org/x/sync v0.2.0 ) require ( @@ -30,13 +32,11 @@ require ( github.com/mitchellh/go-homedir v1.1.0 // indirect github.com/mwitkow/go-conntrack v0.0.0-20190716064945-2f068394615f // indirect github.com/pmezard/go-difflib v1.0.0 // indirect - github.com/prometheus/client_model v0.4.0 // indirect github.com/prometheus/procfs v0.10.1 // indirect github.com/xhit/go-str2duration/v2 v2.1.0 // indirect golang.org/x/crypto v0.8.0 // indirect golang.org/x/net v0.10.0 // indirect golang.org/x/oauth2 v0.8.0 // indirect - golang.org/x/sync v0.2.0 // indirect golang.org/x/sys v0.8.0 // indirect golang.org/x/text v0.9.0 // indirect google.golang.org/appengine v1.6.7 // indirect From 4a2d468645048ad6a65d7b5b7d646c22b10e9c42 Mon Sep 17 00:00:00 2001 From: Kunal Bauskar Date: Thu, 5 Nov 2026 12:43:28 +0530 Subject: [PATCH 5/5] PCD-10432 Pin busybox base image digest in Dockerfile.pf9 hadolint (DL3007) flags FROM ...:latest as non-reproducible. quay.io/ prometheus/busybox only publishes latest/glibc/uclibc tags (no semver), so pin latest's current digest instead. Co-Authored-By: Claude Sonnet 5 --- Dockerfile.pf9 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile.pf9 b/Dockerfile.pf9 index bfc89004..47628ebb 100644 --- a/Dockerfile.pf9 +++ b/Dockerfile.pf9 @@ -27,7 +27,7 @@ RUN mkdir -p /out && \ GOOS=${GOOS} GOARCH=${GOARCH} CGO_ENABLED=${CGO_ENABLED} \ go build -trimpath -ldflags="-s -w" -o /out/openstack-exporter . -FROM quay.io/prometheus/busybox:latest +FROM quay.io/prometheus/busybox:latest@sha256:7379672d53c28113469275f2eb48adffa19954f39f8ef20c1e2992278f658e6c ARG VERSION=1.0.0 ARG BUILD_ID=0