Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

shutdown manager: use filter on downstream_cx_active stats url #6523

Merged
merged 6 commits into from
Jun 20, 2024

Conversation

therealak12
Copy link
Contributor

The PR adds a query param to the stats URL used in the shutdown-manager when waiting for the active connections to drain.

In clusters with a high number of routes (such as ours in Snapp with over 3000 routes), the /stats/prometheus response size is above 80MBs. This makes rendering and fetching the URL take a few seconds.

The introduced filter makes rendering faster and the response time shorter. Also, it reduces the time required for parsing and searching the response.

An example of the response when utilizing the filter:

image

Comparison of total request time before and after the filter:

image

Add a filter to prometheus stats url so that
only the desired stats are returned. This improves
the performance especially in clusters with high
number of routes

Signed-off-by: Ahmad Karimi <[email protected]>
@therealak12 therealak12 requested a review from a team as a code owner June 20, 2024 11:29
@therealak12 therealak12 requested review from tsaarni and skriss and removed request for a team June 20, 2024 11:29
@sunjayBhatia sunjayBhatia requested review from a team, davinci26 and clayton-gonsalves and removed request for a team June 20, 2024 11:29
Signed-off-by: Ahmad Karimi <[email protected]>
@sunjayBhatia sunjayBhatia added the release-note/small A small change that needs one line of explanation in the release notes. label Jun 20, 2024
@sunjayBhatia sunjayBhatia changed the title use filter on downstream_cx_active stats url shutdown manager: use filter on downstream_cx_active stats url Jun 20, 2024
Signed-off-by: Ahmad Karimi <[email protected]>
Copy link
Member

@sunjayBhatia sunjayBhatia left a comment

Choose a reason for hiding this comment

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

looks great! just a couple nits

Signed-off-by: Ahmad Karimi <[email protected]>
Signed-off-by: Ahmad Karimi <[email protected]>
Signed-off-by: Ahmad Karimi <[email protected]>
Copy link

codecov bot commented Jun 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.64%. Comparing base (9f25ff6) to head (394af7a).

Current head 394af7a differs from pull request most recent head f0a7e54

Please upload reports for the commit f0a7e54 to get more accurate results.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #6523   +/-   ##
=======================================
  Coverage   81.64%   81.64%           
=======================================
  Files         133      133           
  Lines       15873    15873           
=======================================
  Hits        12959    12959           
  Misses       2620     2620           
  Partials      294      294           
Files Coverage Δ
cmd/contour/shutdownmanager.go 45.45% <ø> (ø)

@sunjayBhatia sunjayBhatia merged commit db4092e into projectcontour:main Jun 20, 2024
24 checks passed
@therealak12 therealak12 deleted the prometheus-filter branch June 20, 2024 19:39
SamMHD pushed a commit to SamMHD/contour that referenced this pull request Sep 8, 2024
…ctcontour#6523)

Add a filter to prometheus stats url so that
only the desired stats are returned. This improves
the performance especially in clusters with high
number of routes

Signed-off-by: Ahmad Karimi <[email protected]>
Signed-off-by: Saman Mahdanian <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/small A small change that needs one line of explanation in the release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants