-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
receive: Hash-ring metrics #1363
Conversation
cc @brancz |
Name: "thanos_receive_hashring_tenants", | ||
Help: "The number of tenants per hashring.", | ||
}, | ||
[]string{"name"}), |
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.
Just curious on cardinality of this - this change on restart time or is fairly consistent across restarts?
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.
This is consistent across restarts. Name
comes from a manually maintained list which is fed through a configuration file.
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.
LGTM, just missed registration.
@@ -75,6 +77,18 @@ func NewConfigWatcher(logger log.Logger, r prometheus.Registerer, path string, i | |||
Name: "thanos_receive_hashrings_file_refreshes_total", | |||
Help: "The number of refreshes of the hashrings configuration file.", | |||
}), | |||
hashringNodesGauge: prometheus.NewGaugeVec( | |||
prometheus.GaugeOpts{ | |||
Name: "thanos_receive_hashring_nodes", |
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.
nodes or endpoints? (: Don't have strong opinion.
Name: "thanos_receive_hashring_tenants", | ||
Help: "The number of tenants per hashring.", | ||
}, | ||
[]string{"name"}), | ||
} | ||
|
||
if r != nil { |
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.
I think those metrics will work bit better if we would register them below ^^
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.
🤦♂ Thanks for pointing that out.
* master: iter.go: error message typo correction (thanos-io#1376) Fix usage of $GOPATH in Makefile (thanos-io#1379) Moved Prometheus 2.11.1 and TSDB to 0.9.1 (thanos-io#1380) Store latest config hash and timestamp as metrics (thanos-io#1378) pkg/receive/handler.go: log errors (thanos-io#1372) receive: Hash-ring metrics (thanos-io#1363) receiver: avoid race of hashring (thanos-io#1371) feat compact: added readiness Prober (thanos-io#1297) Add changelog entry for S3 option (thanos-io#1361) Multipart features (thanos-io#1358) Added katacoda.yaml (thanos-io#1359) Remove deprecated option from example (thanos-io#1351) Move suggestion about admin API to appropriate place (thanos-io#1355)
This PR adds new metrics to monitor hash-rings.
It exposes the number of nodes and the number of tenants per hash-rings.
Changes
thanos_receive_hashring_nodes
andthanos_receive_hashring_tenants
.Verification
By using make test and manual testing against receive API, it seems like the previous behavior kept intact.