Add label propagation metrics #2076

songrx1997 · 2023-04-13T21:55:00Z

k8s-ci-robot · 2023-04-13T21:55:11Z

Hi @songrx1997. 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.

swetharepakula · 2023-04-13T22:09:45Z

/ok-to-test

swetharepakula · 2023-04-13T22:17:51Z

pkg/neg/metrics/label_propagation_metrics.go

+)
+
+const (
+	ratioWithAnnotation       = "ratio_endpoints_with_annotation"


instead of ratio, lets say percentage

swetharepakula · 2023-04-13T22:20:19Z

pkg/neg/metrics/label_propagation_metrics.go

+		prometheus.CounterOpts{
+			Subsystem: negControllerSubsystem,
+			Name:      totalLabelError,
+			Help:      "The total number of errors of label propagation",


nit: the total number of errors occurred for label propagation

swetharepakula · 2023-04-13T22:20:32Z

pkg/neg/metrics/label_propagation_metrics.go

+		prometheus.CounterOpts{
+			Subsystem: negControllerSubsystem,
+			Name:      labelTruncationErrorCount,
+			Help:      "The number of label truncation",


the number of labels truncated

swetharepakula · 2023-04-13T22:21:25Z

pkg/neg/metrics/label_propagation_metrics.go

+	LabelTruncationErrorCount = prometheus.NewCounter(
+		prometheus.CounterOpts{
+			Subsystem: negControllerSubsystem,
+			Name:      labelTruncationErrorCount,
+			Help:      "The number of label truncation",
+		},
+	)
+
+	LabelFailureCount = prometheus.NewCounter(
+		prometheus.CounterOpts{
+			Subsystem: negControllerSubsystem,
+			Name:      labelFailureCount,
+			Help:      "The number of label truncation failures",
+		},
+	)
+)


can these two metrics be combined and use a label to track truncation failures, truncated, and all labels added?

swetharepakula · 2023-04-13T22:37:59Z

pkg/neg/metrics/label_propagation_metrics.go

+	EndpointsWithAnnotations = prometheus.NewGauge(
+		prometheus.GaugeOpts{
+			Subsystem: negControllerSubsystem,
+			Name:      endpointWithAnnotation,
+			Help:      "The number of endpoints with annotations",
+		},
+	)


this one lets name number_of_endpoints and then you would have a label that say with annotations.

This way we track total and number with annotations with a single metric

swetharepakula · 2023-04-13T22:38:42Z

pkg/neg/metrics/label_propagation_metrics.go

+	RatioWithAnnotation = prometheus.NewGauge(
+		prometheus.GaugeOpts{
+			Subsystem: negControllerSubsystem,
+			Name:      ratioWithAnnotation,
+			Help:      "The ratio between the number of endpoints with annotation and the total number",
+		},
+	)


we don't need to have this metric specifically. with the one above, we can calculate the ratio after the fact. For metrics, generally we try to stay away from doing any calculations when emitting/publishing them.

swetharepakula · 2023-04-13T22:41:07Z

pkg/neg/metrics/label_propagation_metrics.go

+	AverageLabelNumber = prometheus.NewGauge(
+		prometheus.GaugeOpts{
+			Subsystem: negControllerSubsystem,
+			Name:      averageLabelNumber,
+			Help:      "The average number of labels per endpoint",
+		},
+	)


This one should be a distribution and we just emit the number of labels per endpoint. That way prometheus will do the calculation.

swetharepakula · 2023-04-13T22:41:21Z

pkg/neg/metrics/label_propagation_metrics.go

+	AverageAnnotationSize = prometheus.NewGauge(
+		prometheus.GaugeOpts{
+			Subsystem: negControllerSubsystem,
+			Name:      averageAnnotationSize,
+			Help:      "The average size of endpoint annotations",
+		},
+	)


same comment as above, change this to a distribution

swetharepakula · 2023-04-13T22:42:56Z

pkg/neg/metrics/neg_metrics_collector.go

+// LabelPropagationStat contains stats related to label propagation.
+type LabelPropagationStats struct {
+	EndpointCount          int
+	EndpointWithAnnotation int
+	PodLabelCount          int
+	AnnotationSize         int
+	TotalLabelError        int
+	LabelTruncationCount   int
+	LabelFailureCount      int
+}
+
+type LabelPropagationMetrics struct {
+	TotalLabelError        int
+	LabelTruncationCount   int
+	LabelFailureCount      int
+	EndpointWithAnnotation int
+	RatioWithAnnotation    float64
+	AverageLabelNum        float64
+	AverageAnnotationSize  float64
+}


the labels should go in the same file as the metrics that will use them

pkg/neg/metrics/neg_metrics_collector.go

swetharepakula · 2023-04-14T00:00:23Z

pkg/neg/metrics/label_propagation_metrics.go

+	LabelNumber = prometheus.NewHistogram(
+		prometheus.HistogramOpts{
+			Subsystem: negControllerSubsystem,
+			Name:      labelNumber,
+			Help:      "The number of labels per endpoint",
+		},
+	)
+
+	AnnotationSize = prometheus.NewHistogram(
+		prometheus.HistogramOpts{
+			Subsystem: negControllerSubsystem,
+			Name:      annotationSize,
+			Help:      "The size of endpoint annotations per endpoint",
+		},
+	)


we should add custom buckets. Otherwise the max bucket is 10 and anything over is not recorded.

For the annotation we should indicate that this is in bytes

swetharepakula

/lgtm
/approve

k8s-ci-robot · 2023-04-14T17:50:45Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: songrx1997, 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 assigned swetharepakula Apr 13, 2023

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 Apr 13, 2023

k8s-ci-robot requested review from aojea and code-elinka April 13, 2023 21:55

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Apr 13, 2023

songrx1997 force-pushed the label-propagation-metrics branch from 3e1ba5e to 0c140d0 Compare April 13, 2023 21:58

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 Apr 13, 2023

swetharepakula reviewed Apr 13, 2023

View reviewed changes

Add label propagation metrics and structs

6ca59bf

songrx1997 force-pushed the label-propagation-metrics branch 2 times, most recently from ee00d32 to 497126d Compare April 13, 2023 23:56

swetharepakula reviewed Apr 14, 2023

View reviewed changes

songrx1997 force-pushed the label-propagation-metrics branch 2 times, most recently from bef626e to 088e9b6 Compare April 14, 2023 01:09

Add functions for calculating label propagation metrics

213c56c

songrx1997 force-pushed the label-propagation-metrics branch from 088e9b6 to 213c56c Compare April 14, 2023 04:44

swetharepakula reviewed Apr 14, 2023

View reviewed changes

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

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 14, 2023

k8s-ci-robot merged commit 9d12b6e into kubernetes:master Apr 14, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add label propagation metrics #2076

Add label propagation metrics #2076

songrx1997 commented Apr 13, 2023

k8s-ci-robot commented Apr 13, 2023

swetharepakula commented Apr 13, 2023

swetharepakula Apr 13, 2023

swetharepakula Apr 13, 2023

swetharepakula Apr 13, 2023

swetharepakula Apr 13, 2023

swetharepakula Apr 13, 2023

swetharepakula Apr 13, 2023

swetharepakula Apr 13, 2023

swetharepakula Apr 13, 2023

swetharepakula Apr 13, 2023

swetharepakula Apr 14, 2023

swetharepakula left a comment

k8s-ci-robot commented Apr 14, 2023

Add label propagation metrics #2076

Add label propagation metrics #2076

Conversation

songrx1997 commented Apr 13, 2023

k8s-ci-robot commented Apr 13, 2023

swetharepakula commented Apr 13, 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

swetharepakula left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Apr 14, 2023