Skip to content

Emit a metric in CNS for NC Sync count by status #1650

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
Merged
merged 2 commits into from
Oct 13, 2022
Merged

Emit a metric in CNS for NC Sync count by status #1650

merged 2 commits into from
Oct 13, 2022
+55 −26

Conversation

@rbtr
Copy link
Collaborator

@rbtr rbtr commented Oct 10, 2022

Signed-off-by: Evan Baker rbtr@users.noreply.github.com

Reason for Change:

  • emits a metric for total NC sync (attempt) count, labelled by success/failure
  • improves the sync method to consider the scenario where not all of the NCs that CNS knows about are present in the NMA response an error, and return an error indicating this after processing the NCs that are present in the response.

Issue Fixed:

Requirements:

Notes:

@rbtr rbtr requested a review from a team as a code owner October 10, 2022 19:34
@rbtr rbtr requested review from rsagasthya and removed request for a team October 10, 2022 19:34
@rbtr rbtr force-pushed the fix/cns-nc-metrics branch from e2f29e8 to 345db5d Compare October 10, 2022 19:35
@rbtr rbtr requested review from csfmomo and neaggarwMS October 10, 2022 19:44
@rbtr rbtr self-assigned this Oct 10, 2022
@rbtr rbtr added enhancement cns Related to CNS. swift Related to SWIFT networking. fix Fixes something. labels Oct 10, 2022
@rbtr rbtr enabled auto-merge (squash) October 10, 2022 19:44
neaggarwMS
neaggarwMS requested changes Oct 10, 2022
View reviewed changes
@rbtr
emit a metric for NC sync and an error if all NCs are not present
5d81310 
Signed-off-by: Evan Baker <rbtr@users.noreply.github.com>
@rbtr rbtr force-pushed the fix/cns-nc-metrics branch from 345db5d to 5d81310 Compare October 11, 2022 15:47
csfmomo
csfmomo reviewed Oct 11, 2022
View reviewed changes
neaggarwMS
neaggarwMS requested changes Oct 11, 2022
View reviewed changes
neaggarwMS
neaggarwMS previously approved these changes Oct 12, 2022
View reviewed changes
@rbtr
rename variables in nc sync host version for clarity
8601ceb 
Signed-off-by: Evan Baker <rbtr@users.noreply.github.com>
@rbtr rbtr disabled auto-merge October 13, 2022 00:59
@rbtr rbtr merged commit df46082 into Azure:master Oct 13, 2022
@rbtr rbtr deleted the fix/cns-nc-metrics branch October 13, 2022 01:00
rjdenney pushed a commit to rjdenney/azure-container-networking that referenced this pull request Jan 19, 2023
@rbtr @rjdenney
Emit a metric in CNS for NC Sync count by status (Azure#1650)
7bd799a 
* emit a metric for NC sync and an error if all NCs are not present

Signed-off-by: Evan Baker <rbtr@users.noreply.github.com>

* rename variables in nc sync host version for clarity

Signed-off-by: Evan Baker <rbtr@users.noreply.github.com>

Signed-off-by: Evan Baker <rbtr@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@csfmomo csfmomo csfmomo approved these changes

@neaggarwMS neaggarwMS neaggarwMS approved these changes

@rsagasthya rsagasthya Awaiting requested review from rsagasthya rsagasthya is a code owner automatically assigned from Azure/acn-cns-reviewers

Assignees

@rbtr rbtr

Labels

cns Related to CNS. enhancement fix Fixes something. swift Related to SWIFT networking.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@rbtr @csfmomo @neaggarwMS