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

NEG Metrics #332

Merged
merged 1 commit into from
Jun 12, 2018
Merged

NEG Metrics #332

merged 1 commit into from
Jun 12, 2018

Conversation

freehan
Copy link
Contributor

@freehan freehan commented Jun 11, 2018 •
edited
Loading

promethues dump:

# HELP neg_controller_neg_sync_count Number of execution for a NEG syncer.
# TYPE neg_controller_neg_sync_count counter
neg_controller_neg_sync_count{key="k8s1-84a1883c1680fcf8-default-hostname-9376-d05afea4",result="success",type="attach"} 3
# HELP neg_controller_neg_sync_duration_seconds Sync latency of a NEG syncer
# TYPE neg_controller_neg_sync_duration_seconds histogram
neg_controller_neg_sync_duration_seconds_bucket{key="k8s1-84a1883c1680fcf8-default-hostname-9376-d05afea4",result="success",type="attach",le="0.005"} 0
neg_controller_neg_sync_duration_seconds_bucket{key="k8s1-84a1883c1680fcf8-default-hostname-9376-d05afea4",result="success",type="attach",le="0.01"} 0
neg_controller_neg_sync_duration_seconds_bucket{key="k8s1-84a1883c1680fcf8-default-hostname-9376-d05afea4",result="success",type="attach",le="0.025"} 0
neg_controller_neg_sync_duration_seconds_bucket{key="k8s1-84a1883c1680fcf8-default-hostname-9376-d05afea4",result="success",type="attach",le="0.05"} 0
neg_controller_neg_sync_duration_seconds_bucket{key="k8s1-84a1883c1680fcf8-default-hostname-9376-d05afea4",result="success",type="attach",le="0.1"} 0
neg_controller_neg_sync_duration_seconds_bucket{key="k8s1-84a1883c1680fcf8-default-hostname-9376-d05afea4",result="success",type="attach",le="0.25"} 0
neg_controller_neg_sync_duration_seconds_bucket{key="k8s1-84a1883c1680fcf8-default-hostname-9376-d05afea4",result="success",type="attach",le="0.5"} 2
neg_controller_neg_sync_duration_seconds_bucket{key="k8s1-84a1883c1680fcf8-default-hostname-9376-d05afea4",result="success",type="attach",le="1"} 2
neg_controller_neg_sync_duration_seconds_bucket{key="k8s1-84a1883c1680fcf8-default-hostname-9376-d05afea4",result="success",type="attach",le="2.5"} 3
neg_controller_neg_sync_duration_seconds_bucket{key="k8s1-84a1883c1680fcf8-default-hostname-9376-d05afea4",result="success",type="attach",le="5"} 3
neg_controller_neg_sync_duration_seconds_bucket{key="k8s1-84a1883c1680fcf8-default-hostname-9376-d05afea4",result="success",type="attach",le="10"} 3
neg_controller_neg_sync_duration_seconds_bucket{key="k8s1-84a1883c1680fcf8-default-hostname-9376-d05afea4",result="success",type="attach",le="+Inf"} 3
neg_controller_neg_sync_duration_seconds_sum{key="k8s1-84a1883c1680fcf8-default-hostname-9376-d05afea4",result="success",type="attach"} 3.252480561
neg_controller_neg_sync_duration_seconds_count{key="k8s1-84a1883c1680fcf8-default-hostname-9376-d05afea4",result="success",type="attach"} 3
# HELP neg_controller_sync_timestamp The timestamp of the last execution of NEG controller sync loop.
# TYPE neg_controller_sync_timestamp gauge
neg_controller_sync_timestamp 1.5288422052249308e+18

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 11, 2018
rramkumar1
rramkumar1 reviewed Jun 12, 2018
View reviewed changes
pkg/neg/metrics.go Outdated
var registerMetrics sync.Once

// Register the metrics that are to be monitored.
func Register() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you call this RegisterMetrics()

Copy link
Contributor

Choose a reason for hiding this comment

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

Also it does not need to be exported.

pkg/neg/metrics.go Outdated
prometheus.CounterOpts{
Subsystem: negControllerSubsystem,
Name: syncErrorKey,
Help: "Number of execution for a NEG syncer.",
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this help string needs to be changed?

pkg/neg/metrics.go Outdated
prometheus.GaugeOpts{
Subsystem: negControllerSubsystem,
Name: lastSyncTimestampKey,
Help: "Gauge measuring the timestamp of the last execution of NEG controller sync loop.",
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove "Gauge measuring" from the help string.

@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 12, 2018
@freehan freehan force-pushed the neg-metrics branch 2 times, most recently from 9fe8119 to 3022e4b Compare June 12, 2018 21:30
rramkumar1
rramkumar1 reviewed Jun 12, 2018
View reviewed changes
pkg/neg/metrics.go
Name: syncLatencyKey,
Help: "Sync latency of a NEG syncer",
},
syncMetricsLabels,
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you need the "result" label for this metric? Maybe you can just take a portion of the label slice if you don't need all of them.

@freehan freehan force-pushed the neg-metrics branch 2 times, most recently from 4a0f18d to 8f2c86b Compare June 12, 2018 22:09
@rramkumar1
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 12, 2018
@freehan freehan merged commit 1235055 into kubernetes:master Jun 12, 2018
@bowei bowei mentioned this pull request Feb 12, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rramkumar1 rramkumar1 rramkumar1 left review comments

Assignees

@rramkumar1 rramkumar1

Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@freehan @rramkumar1 @k8s-ci-robot