Add metrics for endpoint and endpoint slice state #1919

sawsa307 · 2023-01-26T21:37:30Z

Added metrics to collect the state of each endpoint and endpoint slice. This metrics is only for L7 endpoints.

k8s-ci-robot · 2023-01-26T21:37:39Z

Hi @sawsa307. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

sawsa307 · 2023-01-26T21:37:39Z

/assign @swetharepakula

bowei · 2023-02-09T18:05:37Z

/ok-to-test

swetharepakula · 2023-04-29T00:13:18Z

pkg/neg/metrics/neg_metrics_collector.go

+	for state, count := range epsStateCount {
+		label := string(state)
+		if state != negtypes.Total {
+			label = fmt.Sprintf("Contains%sEndpoint", string(label))


I don't think we need to do this. We can use the labels as they are currently.

swetharepakula · 2023-05-01T21:14:36Z

pkg/neg/metrics/endpoint_metrics.go

+
+	syncerEndpointSliceStateLabels = []string{
+		"endpoint_slice_state", // state of endpoint slice
+		"calculation_type",     // type of endpoint calculation


we don't need this.

swetharepakula · 2023-05-01T22:15:25Z

pkg/neg/syncers/utils.go

-			// pod is used for label propagation
-			_, getPodErr := getEndpointPod(endpointAddress, podLister)
+				klog.Errorf("Detected unexpected error when getting zone: %v", getZoneErr)
+				if returnErr == nil {


this isn't a good pattern to have of storing the error we want to return. I also don't think we gain much from trying to save this error because we will immediately have another sync right after with degraded mode that will give us these metrics

Updated. Now we return the error immediately.

swetharepakula · 2023-05-01T22:41:33Z

pkg/neg/syncers/utils.go

+				localEPCount[negtypes.PodLabelMismatch] += validatePodStat.podLabelMismatch
+			}
+			if getPodErr != nil || validateErr != nil || getZoneErr != nil || checkIPErr != nil || otherError != nil {
+				localEPCount[negtypes.InvalidField] += 1


what is otherError being used for? Initially I intended for InvalidField when the number of error classifications were less, but we are having more detail, I don't think we need to track by InvalidField

It is used as a catch-all. Since we no longer need to track invalidField, it is removed.

swetharepakula · 2023-05-01T22:43:04Z

pkg/neg/syncers/utils.go

+				klog.Errorf("Endpoint %q in Endpoints %s/%s correponds to an invalid pod: %v, skipping", endpointAddress.Addresses, ed.Meta.Namespace, ed.Meta.Name, validateErr)
+				localEPCount[negtypes.PodTerminal] += validatePodStat.podTerminal
+				localEPCount[negtypes.NodeNotFound] += validatePodStat.nodeNotFound
+				localEPCount[negtypes.NodeTypeAssertionFailed] += validatePodStat.nodeTypeAssertionFailed


lets count this as an other error

Updated. Thanks!

swetharepakula · 2023-05-01T22:43:18Z

pkg/neg/syncers/utils.go

+				klog.Errorf("Endpoint %q in Endpoints %s/%s receives error when getting pod: %v, skipping", endpointAddress.Addresses, ed.Meta.Namespace, ed.Meta.Name, getPodErr)
+				localEPCount[negtypes.PodMissing] += getPodStat.podMissing
+				localEPCount[negtypes.PodNotFound] += getPodStat.podNotFound
+				localEPCount[negtypes.PodTypeAssertionFailed] += getPodStat.podTypeAssertionFailed


count this as other error

Updated. Thanks!

swetharepakula · 2023-05-01T22:43:44Z

pkg/neg/syncers/utils.go

+				localEPCount[negtypes.PodMissing] += getPodStat.podMissing
+				localEPCount[negtypes.PodNotFound] += getPodStat.podNotFound


these two are similar. I would consolidate these into a single classification

Updated. Thanks!

swetharepakula · 2023-05-02T21:50:37Z

pkg/neg/syncers/utils.go

 type getPodStat struct {
-	podMissing             int
-	podNotFound            int
-	podTypeAssertionFailed int
+	podInvalid int
+	otherError int


Instead of using a stat struct, would it be easier to return a map with the counts of the errors? Then you can use the same merge function to merge it into the local count. You can do something similar with all the other function. This will reduce some code as you won't have to enumerate through all the errors after calling a helper function. It will also reduce the need in knowing what errors are returned by a helper function.

swetharepakula · 2023-05-02T22:45:31Z

/retest

swetharepakula

/lgtm
/approve

sawsa307 · 2023-05-03T00:03:47Z

/retest

sawsa307 · 2023-05-03T01:30:24Z

/retest

Added metrics to collect the state of each endpoint and endpoint slice. These metrics are only for L7 endpoints.

sawsa307 · 2023-05-03T02:42:24Z

pkg/neg/metrics/neg_metrics_collector.go

@@ -145,7 +155,7 @@ func (sm *SyncerMetrics) DeleteSyncer(key negtypes.NegSyncerKey) {
 	defer sm.mu.Unlock()
 	delete(sm.syncerStatusMap, key)
 	delete(sm.syncerEndpointStateMap, key)
-	delete(sm.syncerEPSStateMap, key)
+	delete(sm.syncerEndpointSliceStateMap, key)


Resolve conflict.

swetharepakula

/lgtm

k8s-ci-robot · 2023-05-03T17:03:50Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sawsa307, swetharepakula

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [swetharepakula]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot requested review from aojea and prameshj January 26, 2023 21:37

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jan 26, 2023

k8s-ci-robot assigned swetharepakula Jan 26, 2023

k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Jan 26, 2023

sawsa307 force-pushed the neg-ep-state-metric branch 3 times, most recently from dd692aa to 11f5710 Compare February 2, 2023 19:35

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 4, 2023

sawsa307 marked this pull request as draft February 4, 2023 05:51

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 4, 2023

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Feb 9, 2023

sawsa307 force-pushed the neg-ep-state-metric branch from 11f5710 to 767e13b Compare March 1, 2023 00:40

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Mar 1, 2023

sawsa307 force-pushed the neg-ep-state-metric branch from 767e13b to a74b700 Compare March 1, 2023 22:07

sawsa307 marked this pull request as ready for review March 1, 2023 22:07

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 1, 2023

k8s-ci-robot requested a review from freehan March 1, 2023 22:07

sawsa307 force-pushed the neg-ep-state-metric branch 2 times, most recently from 0ac3c6b to f769999 Compare March 5, 2023 11:07

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 9, 2023

sawsa307 force-pushed the neg-ep-state-metric branch from f769999 to e206616 Compare March 21, 2023 22:07

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 21, 2023

sawsa307 force-pushed the neg-ep-state-metric branch 4 times, most recently from 9f133fd to e36d9fe Compare May 1, 2023 19:12

swetharepakula reviewed May 1, 2023

View reviewed changes

sawsa307 force-pushed the neg-ep-state-metric branch from e36d9fe to 8c2b996 Compare May 1, 2023 23:35

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels May 1, 2023

swetharepakula reviewed May 2, 2023

View reviewed changes

sawsa307 force-pushed the neg-ep-state-metric branch 3 times, most recently from 5bc8d83 to 65abd66 Compare May 2, 2023 22:30

swetharepakula reviewed May 2, 2023

View reviewed changes

k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels May 2, 2023

sawsa307 added 4 commits May 3, 2023 01:30

add metric collector and syncerKey to L7 calculator

59adcf2

rename endpoint state

cc795d5

Refactor helper functions to return count of invalid endpoints

052146e

Add metrics for endpoint and endpoint slice state

de9e231

Added metrics to collect the state of each endpoint and endpoint slice. These metrics are only for L7 endpoints.

sawsa307 force-pushed the neg-ep-state-metric branch from 65abd66 to de9e231 Compare May 3, 2023 02:29

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 3, 2023

sawsa307 commented May 3, 2023

View reviewed changes

swetharepakula reviewed May 3, 2023

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 3, 2023

k8s-ci-robot merged commit 0a8854d into kubernetes:master May 3, 2023

sawsa307 deleted the neg-ep-state-metric branch September 2, 2023 20:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add metrics for endpoint and endpoint slice state #1919

Add metrics for endpoint and endpoint slice state #1919

sawsa307 commented Jan 26, 2023 •

edited

Loading

k8s-ci-robot commented Jan 26, 2023

sawsa307 commented Jan 26, 2023

bowei commented Feb 9, 2023

swetharepakula Apr 29, 2023

sawsa307 May 1, 2023

swetharepakula May 1, 2023

sawsa307 May 1, 2023

swetharepakula May 1, 2023

sawsa307 May 1, 2023

swetharepakula May 1, 2023

sawsa307 May 1, 2023

swetharepakula May 1, 2023

sawsa307 May 1, 2023

swetharepakula May 1, 2023

sawsa307 May 1, 2023

swetharepakula May 1, 2023

sawsa307 May 1, 2023

swetharepakula May 2, 2023

swetharepakula commented May 2, 2023

swetharepakula left a comment

sawsa307 commented May 3, 2023

sawsa307 commented May 3, 2023

sawsa307 May 3, 2023 •

edited

Loading

swetharepakula left a comment

k8s-ci-robot commented May 3, 2023

		localEPCount[negtypes.PodMissing] += getPodStat.podMissing
		localEPCount[negtypes.PodNotFound] += getPodStat.podNotFound

Add metrics for endpoint and endpoint slice state #1919

Add metrics for endpoint and endpoint slice state #1919

Conversation

sawsa307 commented Jan 26, 2023 • edited Loading

k8s-ci-robot commented Jan 26, 2023

sawsa307 commented Jan 26, 2023

bowei commented Feb 9, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

swetharepakula commented May 2, 2023

swetharepakula left a comment

Choose a reason for hiding this comment

sawsa307 commented May 3, 2023

sawsa307 commented May 3, 2023

sawsa307 May 3, 2023 • edited Loading

Choose a reason for hiding this comment

swetharepakula left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented May 3, 2023

sawsa307 commented Jan 26, 2023 •

edited

Loading

sawsa307 May 3, 2023 •

edited

Loading