Skip to content

optimise prometheus scraping workflow - #13650

Open
DaanHoogland wants to merge 10 commits into
4.20from
ghi13586-prometheusDrainage-20
Open

optimise prometheus scraping workflow#13650
DaanHoogland wants to merge 10 commits into
4.20from
ghi13586-prometheusDrainage-20

Conversation

@DaanHoogland

@DaanHoogland DaanHoogland commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Description

This PR...

Fixes: #13586

  1. Removes alertManager.recalculateCapacity() from PrometheusExporterImpl.updateMetrics(). This is the actual fix — it stops the exporter from forcing a full, thread-pool-churning, all-zone capacity recalculation on every single scrape. The exporter should just read CapacityDao, which AlertManagerImpl's own timer already keeps current. This alone should flatten the scrape_duration curve.
  2. Reuses a single long-lived ExecutorService in AlertManagerImpl instead of creating/tearing one down on every recalculateHostCapacities()/recalculateStorageCapacities() call. Cheap, low-risk, and removes the thread-churn contributor even for the timer-driven path.

this should solve the issue, but claude suggested the following improvements as well:

  1. Give the exporter's HttpServer an explicit bounded executor (httpServer.setExecutor(Executors.newFixedThreadPool(2))) so one slow scrape can't serialize/queue all others.
  2. Add a short TTL/in-flight guard around updateMetrics() (e.g., skip recompute if last run was < N seconds ago, or synchronize so concurrent scrapes share one in-progress computation) so scrape frequency can never multiply backend load.
  3. Instrument: log/measure updateMetrics() wall-clock time so the reporter (and CI) can confirm which sub-metric collector is actually slow and verify the fix closes the growth.

if anyone wants to, these are nice to haves/good first issue ;)

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)
  • Build/CI
  • Test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

@DaanHoogland DaanHoogland added this to the 4.20.4 milestone Jul 20, 2026
@DaanHoogland
DaanHoogland requested review from NuxRo and shwstppr July 20, 2026 10:48
@codecov

codecov Bot commented Jul 20, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 16.35%. Comparing base (6136ac9) to head (15c8fa1).

Additional details and impacted files 
@@             Coverage Diff              @@
##               4.20   #13650      +/-   ##
============================================
+ Coverage     16.32%   16.35%   +0.02%     
- Complexity    13556    13578      +22     
============================================
  Files          5669     5669              
  Lines        501399   501417      +18     
  Branches      60902    60905       +3     
============================================
+ Hits          81847    81987     +140     
+ Misses       410390   410265     -125     
- Partials       9162     9165       +3
Flag Coverage Δ
uitests 4.14% <ø> (ø)
unittests 17.21% <100.00%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@DaanHoogland DaanHoogland linked an issue Jul 20, 2026 that may be closed by this pull request
Scrape duration using prometheus is increasing continuously until restart #13586
Open
@DaanHoogland
DaanHoogland force-pushed the ghi13586-prometheusDrainage-20 branch from 7835881 to d765e5d Compare August 4, 2026 08:07
@apache apache deleted a comment from blueorangutan Aug 4, 2026
@apache apache deleted a comment from blueorangutan Aug 4, 2026
@apache apache deleted a comment from blueorangutan Aug 4, 2026
@apache apache deleted a comment from blueorangutan Aug 4, 2026
@apache apache deleted a comment from blueorangutan Aug 4, 2026
@apache apache deleted a comment from blueorangutan Aug 4, 2026
@DaanHoogland
DaanHoogland force-pushed the ghi13586-prometheusDrainage-20 branch from 1951bdb to 2d2c40b Compare August 23, 2026 10:54
@apache apache deleted a comment from blueorangutan Aug 23, 2026
@apache apache deleted a comment from blueorangutan Aug 25, 2026
@sonarqubecloud

sonarqubecloud Bot commented Aug 25, 2026

Copy link
Copy Markdown

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
77.5% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@blueorangutan

blueorangutan commented Aug 28, 2026

Copy link
Copy Markdown

[SF] Trillian test result (tid-16835)
Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 51997 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr13650-t16835-kvm-ol8.zip
Smoke tests completed. 141 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File

@DaanHoogland DaanHoogland moved this from Backlog to Ready in CloudStack Testing Aug 31, 2026
@DaanHoogland

DaanHoogland commented Oct 2, 2026

Copy link
Copy Markdown
Contributor Author

@NuxRo please test? @Damans227 and @shwstppr , can you review?

Damans227
Damans227 reviewed Oct 2, 2026
View reviewed changes
Comment thread server/src/main/java/com/cloud/alert/AlertManagerImpl.java
private synchronized ExecutorService getCapacityExecutorService() {
if (capacityExecutorService == null || capacityExecutorService.isShutdown()) {
capacityExecutorService = Executors.newFixedThreadPool(
Math.max(1, CapacityManager.CapacityCalculateWorkers.value()),

@Damans227 Damans227 Oct 2, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

workers setting only gets read once now, so changing it needs a restart? earlier it picked up new value each run, is that ok?

Comment thread ...egrations/prometheus/src/main/java/org/apache/cloudstack/metrics/PrometheusExporterImpl.java
public void updateMetrics() {
public synchronized void updateMetrics() {
final long minIntervalMs = TimeUnit.SECONDS.toMillis(PrometheusExporterServer.PrometheusExporterMinRefreshInterval.value());
final long now = System.currentTimeMillis();

@Damans227 Damans227 Oct 2, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if server clock goes back, wont scrapes get skipped and metrics go stale? maybe nanoTime here?

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@shwstppr shwstppr Awaiting requested review from shwstppr
@NuxRo NuxRo Awaiting requested review from NuxRo
1 more reviewer
@Damans227 Damans227 Damans227 left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@NuxRo NuxRo

Labels

needs-testing status:ready-for-testing

Projects

CloudStack Testing
Status: Ready

Milestone

4.20.4

Development

Successfully merging this pull request may close these issues.

Scrape duration using prometheus is increasing continuously until restart

6 participants

@DaanHoogland @blueorangutan @Damans227 @NuxRo @vladimirpetrov @PrashantBhanage