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

add azr metrics in heartbeats #2735

Merged
merged 10 commits into from
Jun 5, 2024
Merged

Conversation

ZetaoZhuang
Copy link
Contributor

@ZetaoZhuang ZetaoZhuang commented May 8, 2024

Reason for Change:

for Direct CNS mode, we want to add IsAZRSupported and HomeAZ as custom metrics in heartbeats, that way we know if a node has azr enabled or not.

Issue Fixed:

Requirements:

Notes:

@ZetaoZhuang ZetaoZhuang requested a review from a team as a code owner May 8, 2024 06:51
@ZetaoZhuang ZetaoZhuang requested a review from csfmomo May 8, 2024 06:51
@timraymond
Copy link
Member

It looks like this suffers from some import cycles, so it won't compile.

@ZetaoZhuang ZetaoZhuang force-pushed the add_azr_metrics_in_heartbeat branch from b100cdf to 4a96e92 Compare May 9, 2024 06:31
Copy link
Contributor

@smittal22 smittal22 left a comment

Choose a reason for hiding this comment

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

Left a few comments. Can sync offline as well.

cns/internal/heartbeat.go Outdated Show resolved Hide resolved
cns/internal/heartbeat.go Outdated Show resolved Hide resolved
cns/internal/heartbeat.go Outdated Show resolved Hide resolved
cns/service/main.go Outdated Show resolved Hide resolved
smittal22
smittal22 previously approved these changes May 30, 2024
Copy link
Contributor

@smittal22 smittal22 left a comment

Choose a reason for hiding this comment

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

LGTM

timraymond
timraymond previously approved these changes May 31, 2024
@smittal22 smittal22 enabled auto-merge May 31, 2024 16:00
Copy link
Contributor

@ramiro-gamarra ramiro-gamarra left a comment

Choose a reason for hiding this comment

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

Please see comments for feedback that needs to be addressed.

@ZetaoZhuang ZetaoZhuang dismissed stale reviews from smittal22 and timraymond via fa7dbed June 4, 2024 19:25
@ZetaoZhuang ZetaoZhuang requested a review from a team as a code owner June 4, 2024 19:33
@ZetaoZhuang ZetaoZhuang force-pushed the add_azr_metrics_in_heartbeat branch from 9eb54c3 to 135948b Compare June 4, 2024 19:40
@ZetaoZhuang ZetaoZhuang force-pushed the add_azr_metrics_in_heartbeat branch from f12c33a to f1c5107 Compare June 4, 2024 23:52
@ZetaoZhuang
Copy link
Contributor Author

/azp run Azure Container Networking PR

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@smittal22 smittal22 added this pull request to the merge queue Jun 5, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 5, 2024
@ZetaoZhuang ZetaoZhuang added this pull request to the merge queue Jun 5, 2024
Merged via the queue into master with commit 7b5b879 Jun 5, 2024
11 checks passed
@ZetaoZhuang ZetaoZhuang deleted the add_azr_metrics_in_heartbeat branch June 5, 2024 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants