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.

Sign up for GitHub

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

Jump to bottom

ARO-13380 - metrics: cwp status #4002

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

ARO-13380 - metrics: cwp status #4002

wants to merge 1 commit into from

Conversation

LiniSusan
Copy link
Collaborator

@LiniSusan LiniSusan commented Dec 11, 2024 •
edited
Loading

Which issue this PR addresses: Metrics for CWP status

Fixes
https://issues.redhat.com/browse/ARO-13380

What this PR does / why we need it:

Develop metrics to create a monitoring alert for the SRE team when the CWP configuration is incorrect or any expected no_proxy url/ip is missing.

Test plan for issue:

Check all the necessary no_proxies that needs to be verified at the time of enabling cluster wide proxy.

Is there any documentation that needs to be updated for this PR?

Necessary documents are currently being developed as part of the epic.

How do you know this will function as expected in production?

Metrics will be available in Geneva for cwp status.

@LiniSusan LiniSusan force-pushed the lkurien/ARO-13380-cwpstatus branch 11 times, most recently from f48d836 to 0303459 Compare December 30, 2024 06:40
@LiniSusan LiniSusan marked this pull request as ready for review December 30, 2024 08:51
@LiniSusan LiniSusan force-pushed the lkurien/ARO-13380-cwpstatus branch 3 times, most recently from b61ef23 to 0147f68 Compare January 30, 2025 11:40
@github-actions github-actions bot removed the needs-rebase branch needs a rebase label Jan 30, 2025
@LiniSusan LiniSusan force-pushed the lkurien/ARO-13380-cwpstatus branch from 0147f68 to f1991df Compare January 30, 2025 14:49
kimorris27
kimorris27 requested changes Jan 31, 2025
View reviewed changes
Copy link
Contributor

@kimorris27 kimorris27 left a comment

Choose a reason for hiding this comment

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

Mostly LGTM, but I made a few suggestions.

pkg/monitor/cluster/clusterwideproxystatus_test.go Show resolved Hide resolved
pkg/monitor/cluster/clusterwideproxystatus.go Outdated Show resolved Hide resolved
pkg/monitor/cluster/clusterwideproxystatus.go Outdated Show resolved Hide resolved
pkg/monitor/cluster/clusterwideproxystatus.go Show resolved Hide resolved
pkg/monitor/cluster/clusterwideproxystatus.go Outdated Show resolved Hide resolved
pkg/monitor/cluster/clusterwideproxystatus.go Outdated Show resolved Hide resolved
pkg/monitor/cluster/clusterwideproxystatus.go Show resolved Hide resolved
@LiniSusan LiniSusan force-pushed the lkurien/ARO-13380-cwpstatus branch 4 times, most recently from 2e5bc14 to a47855f Compare February 4, 2025 11:21
@LiniSusan LiniSusan force-pushed the lkurien/ARO-13380-cwpstatus branch 3 times, most recently from 3c262e1 to 3bdc3ef Compare February 5, 2025 12:05
@LiniSusan LiniSusan force-pushed the lkurien/ARO-13380-cwpstatus branch 2 times, most recently from 50bb179 to f2aa526 Compare February 6, 2025 09:41
@github-actions GitHub Actions
Copy link

github-actions bot commented Feb 7, 2025

Please rebase pull request.

@github-actions github-actions bot added the needs-rebase branch needs a rebase label Feb 7, 2025
@LiniSusan LiniSusan force-pushed the lkurien/ARO-13380-cwpstatus branch from f2aa526 to db1bc54 Compare February 10, 2025 04:30
@github-actions github-actions bot removed the needs-rebase branch needs a rebase label Feb 10, 2025
@LiniSusan LiniSusan force-pushed the lkurien/ARO-13380-cwpstatus branch from db1bc54 to e60de01 Compare February 10, 2025 05:07
@LiniSusan LiniSusan requested a review from kimorris27 February 10, 2025 07:14
kimorris27
kimorris27 reviewed Feb 10, 2025
View reviewed changes
Copy link
Contributor

@kimorris27 kimorris27 left a comment

Choose a reason for hiding this comment

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

LGTM, but I'll refer to Ankur for final review and approval since I don't have as much context on this work, and we will want to be sure we get this monitoring functionality right.

@LiniSusan LiniSusan force-pushed the lkurien/ARO-13380-cwpstatus branch from e60de01 to 2a0877e Compare February 12, 2025 06:32
sankur-codes
sankur-codes approved these changes Feb 14, 2025
View reviewed changes
Copy link
Collaborator

@sankur-codes sankur-codes left a comment

Choose a reason for hiding this comment

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

LGTM

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

@sankur-codes sankur-codes sankur-codes approved these changes

@kimorris27 kimorris27 kimorris27 approved these changes

@jewzaam jewzaam Awaiting requested review from jewzaam jewzaam is a code owner

@bennerv bennerv Awaiting requested review from bennerv bennerv is a code owner

@hawkowl hawkowl Awaiting requested review from hawkowl hawkowl is a code owner

@rogbas rogbas Awaiting requested review from rogbas rogbas is a code owner

@petrkotas petrkotas Awaiting requested review from petrkotas petrkotas is a code owner

@jharrington22 jharrington22 Awaiting requested review from jharrington22 jharrington22 is a code owner

@cblecker cblecker Awaiting requested review from cblecker cblecker is a code owner

@cadenmarchese cadenmarchese Awaiting requested review from cadenmarchese cadenmarchese is a code owner

@UlrichSchlueter UlrichSchlueter Awaiting requested review from UlrichSchlueter UlrichSchlueter is a code owner

@SudoBrendan SudoBrendan Awaiting requested review from SudoBrendan SudoBrendan is a code owner

@yjst2012 yjst2012 Awaiting requested review from yjst2012 yjst2012 is a code owner

@jaitaiwan jaitaiwan Awaiting requested review from jaitaiwan jaitaiwan is a code owner

@anshulvermapatel anshulvermapatel Awaiting requested review from anshulvermapatel anshulvermapatel is a code owner

@hlipsig hlipsig Awaiting requested review from hlipsig hlipsig is a code owner

@tiguelu tiguelu Awaiting requested review from tiguelu tiguelu is a code owner

@mociarain mociarain Awaiting requested review from mociarain mociarain is a code owner

@tsatam tsatam Awaiting requested review from tsatam tsatam is a code owner

@bitoku bitoku Awaiting requested review from bitoku bitoku is a code owner

@fahlmant fahlmant Awaiting requested review from fahlmant fahlmant is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@LiniSusan @rhamitarora @sankur-codes @kimorris27