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

TSDB: Expose shipper metrics via ingester #1983

Merged
merged 5 commits into from
Jan 17, 2020
Merged

TSDB: Expose shipper metrics via ingester #1983

merged 5 commits into from
Jan 17, 2020

Conversation

pstibrany
Copy link
Contributor

Since we have one TSDB shipper per tenant in memory, we need to gather all of these metrics and sum them together (fortunately they are all counters).

One shipper metric, thanos_shipper_upload_compacted_done ("If 1 it means shipper uploaded all compacted blocks from the filesystem.") was skipped and is not exposed, as there is no good way to group it across all tenants.

All shipper metrics now have cortex_ingester prefix instead of original thanos prefix.

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

@pstibrany pstibrany requested a review from pracucci January 15, 2020 12:09
Copy link
Contributor

@pracucci pracucci left a comment

Choose a reason for hiding this comment

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

Wow, good job @pstibrany ! I left a couple of comments.

pkg/ingester/ingester_v2.go Outdated Show resolved Hide resolved
pkg/ingester/ingester_v2.go Outdated Show resolved Hide resolved
pkg/ingester/ingester_v2.go Outdated Show resolved Hide resolved
Copy link
Contributor

@pracucci pracucci left a comment

Choose a reason for hiding this comment

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

Good job. LGTM

Copy link
Contributor

@thorfour thorfour left a comment

Choose a reason for hiding this comment

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

LGTM nice work!

pkg/ingester/metrics.go Outdated Show resolved Hide resolved
pkg/ingester/metrics.go Outdated Show resolved Hide resolved
Since we have one TSDB shipper per tenant in memory, we need to
gather all of these metrics and sum them together (fortunately
they are all counters).

One shipper metric, thanos_shipper_upload_compacted_done ("If 1 it means
shipper uploaded all compacted blocks from the filesystem.") was skipped
and is not exposed, as there is no good way to group it across all tenants.

Ingester metrics were moved to separate file, metrics.go. shipper metrics have
their own type and are only used from TSDB code.

Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>
Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>
Copy link
Contributor

@pracucci pracucci left a comment

Choose a reason for hiding this comment

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

LGTM, with a nit.

@gouthamve May you also take a look, please?

pkg/ingester/metrics.go Outdated Show resolved Hide resolved
This should never happen.

Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>
Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>
Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>
Copy link
Contributor

@pracucci pracucci left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@gouthamve
Copy link
Contributor

That was very cleverly done!

@pracucci pracucci merged commit 8355623 into cortexproject:master Jan 17, 2020
@pstibrany pstibrany deleted the tsdb-shipper-metrics branch January 18, 2020 21:53
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.

4 participants