PrashantBhanage opened a new pull request, #13678:
URL: https://github.com/apache/cloudstack/pull/13678
### Description
This PR fixes unbounded growth of Prometheus `/metrics` scrape duration
against the CloudStack management server (root cause tracked in #13586,
improvements requested in #13667).
**Before:** the exporter's `HttpServer` used the JDK default single-threaded
executor, so a slow scrape serialized/queued every other scrape behind it.
Additionally, `updateMetrics()` had no guard against concurrent or rapid
recomputation, so scrape frequency could multiply backend load
indefinitely. Only a full management server restart reset the growth.
**After:** three targeted, contained changes, all inside
`plugins/integrations/prometheus/`:
1. **Bounded HTTP executor** — `PrometheusExporterServerImpl#start()` now
sets an explicit `Executors.newFixedThreadPool(2)` as the HttpServer's
executor instead of relying on the JDK default, and shuts it down
cleanly in `stop()`.
2. **TTL / in-flight guard** — `PrometheusExporterImpl#updateMetrics()` is
now `synchronized` and checks a `lastMetricsUpdateTime` timestamp
against a new dynamic global setting,
`prometheus.exporter.metrics.min.refresh.interval` (default 5 seconds).
Scrapes arriving within that window reuse the previously computed
metrics instead of triggering a fresh, expensive recomputation.
3. **Timing instrumentation** — `updateMetrics()` now logs its wall-clock
execution time in milliseconds at `info` level on every run (including
on the exception path), so operators/CI can identify which
sub-collector is slow and confirm the fix closes the growth.
This PR deliberately does **not** touch `AlertManagerImpl` or the
capacity-calculation executor in `server/` — that code already creates a
fresh thread pool per invocation and reads `capacity.calculate.workers`
dynamically on `main`, and rewriting it was flagged in the issue comments
as unnecessary scope creep that introduced its own bugs (stale config,
swallowed shutdown exceptions, thread-pool leaks).
Fixes: #13667
Ref: #13586
### Types of changes
- [x] Bug fix (non-breaking change which fixes an issue)
### Feature/Enhancement Scale or Bug Severity
#### Bug Severity
- [x] Major
### Screenshots (if appropriate):
N/A — backend-only change, no UI impact.
### How Has This Been Tested?
- Unit tests added to `PrometheusExporterImplTest`:
- `testUpdateMetricsTTLGuardSkipsSecondCall` — two rapid calls to
`updateMetrics()` within the TTL window result in only one
recomputation.
- `testUpdateMetricsTTLGuardAllowsAfterInterval` — after the configured
interval elapses, a subsequent call triggers a fresh recomputation.
- `mvn -pl plugins/integrations/prometheus test` → 6/6 tests pass (4
existing + 2 new).
- `mvn -pl plugins/integrations/prometheus -am install` → build success.
- Manually verified repeated rapid scrapes against `/metrics` reuse cached
output within the TTL window, and that logged `updateMetrics()` duration
stays flat instead of climbing across successive scrapes.
#### How did you try to break this feature and the system with this change?
- Fired concurrent scrape requests against `/metrics` to confirm the
bounded executor and `synchronized` guard prevent overlapping
recomputation instead of deadlocking or throwing.
- Verified `stop()` shuts down the new executor cleanly with no resource
leak warnings.
- Confirmed the new `prometheus.exporter.metrics.min.refresh.interval`
setting is dynamically adjustable at runtime without a restart, and
that a value of `0` effectively disables the guard (falls back to
original per-scrape recomputation behavior) for anyone who wants that.
- Confirmed no other module (`AlertManagerImpl`, `CapacityManagerImpl`,
etc.) was touched, keeping this change fully isolated to the exporter
plugin.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
