-
Notifications
You must be signed in to change notification settings - Fork 724
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 prometheus metrics #65
Conversation
Please choose a different port than 51678. |
ipamd/datastore/data_store.go
Outdated
assignedIPs = prometheus.NewGauge( | ||
prometheus.GaugeOpts{ | ||
Name: "assigned_ip_addresses", | ||
Help: "The number of ip addresses assigned", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please capitalize initialisms IP and ENI in Help text. also eni should be plural ENIs
pkg/awsutils/awsutils.go
Outdated
awsAPILatency = prometheus.NewSummaryVec( | ||
prometheus.SummaryOpts{ | ||
Name: "aws_api_lantency_ms", | ||
Help: "aws API call latency in ms", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
capitalize AWS
pkg/awsutils/awsutils.go
Outdated
awsAPIErr = prometheus.NewCounterVec( | ||
prometheus.CounterOpts{ | ||
Name: "aws_api_error_count", | ||
Help: "the number of times aws API returns an err", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
capitalize The and AWS
if err != nil { | ||
awsAPIErrInc("TagResources", err) | ||
log.Warnf("Fail to tag the newly created eni %s %v", eniID, err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we're counting this as an Error, why do we log as a Warning? Might be a good idea to make a function logAndTallyErrorf
since we appear to always want them both together.
ipamd/datastore/data_store.go
Outdated
Help: "The number of ip addresses assigned", | ||
}, | ||
) | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It appears we're tracking and emiting metrics on eni count, total address, and assigned address. One can derived Available address as Total - Assigned, but it might be nice to emit that directly. Also would be nice to emit Max address (ENIs * IPsPerENI). Addresses go through a cool down period, might be nice to offer visibility into that too.
155dddd
to
8eb0758
Compare
Descriptions:
Add following prometheus metrics which provide metrics at localhost:61678/metrics
Also add a tool cni-metrics-helper which can collect CNI prometheus metrics from all nodes in the cluster by
Tests Performed
Deployed 4000 busybox pods over 80 nodes. Verify the prometheus metrics from all 80 nodes, that aggregated eni_allocated is 320, aggregated total_ip_addresses is 4.48k, assigned_ip_addresses is 4k
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.