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.

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

Merged
merged 4 commits into from
Aug 5, 2019
Merged

Conversation

kakkoyun
Copy link
Member

@kakkoyun kakkoyun commented Jul 31, 2019

This PR adds new metrics to monitor hash-rings.
It exposes the number of nodes and the number of tenants per hash-rings.

Changes

  • Adds two new metrics to receive thanos_receive_hashring_nodes and thanos_receive_hashring_tenants.

Verification

By using make test and manual testing against receive API, it seems like the previous behavior kept intact.

@kakkoyun
Copy link
Member Author

kakkoyun commented Aug 5, 2019

cc @brancz

Name: "thanos_receive_hashring_tenants",
Help: "The number of tenants per hashring.",
},
[]string{"name"}),
Copy link
Member

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?

Copy link
Member Author

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.

Copy link
Member

@bwplotka bwplotka left a 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",
Copy link
Member

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 {
Copy link
Member

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 ^^

Copy link
Member Author

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.

@brancz brancz merged commit c49af2e into thanos-io:master Aug 5, 2019
paulfantom added a commit to paulfantom/thanos that referenced this pull request Aug 7, 2019
* 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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants