Skip to content

Register a histogram that was omitted#2138

Merged
pracucci merged 1 commit intomasterfrom
register-collectors
Feb 17, 2020
Merged

Register a histogram that was omitted#2138
pracucci merged 1 commit intomasterfrom
register-collectors

Conversation

@bboreham
Copy link
Contributor

@bboreham bboreham commented Feb 14, 2020

What this PR does:

Put back a metric that got lost in #1571.

Note I used promauto for shorter diff but most HistogramCollectors in the code registered via init() functions. It's kinda relying on an implementation detail but distributor.go is already doing it and yolo.

Checklist

  • Tests updated - it would be nice to test for metrics but that's a bigger job than I had planned
  • Documentation added - NA
  • CHANGELOG.md updated

Signed-off-by: Bryan Boreham <bryan@weave.works>
@bboreham bboreham force-pushed the register-collectors branch from f7ac4b9 to d490443 Compare February 14, 2020 15:31
@bboreham bboreham changed the title Register two histograms that were omitted Register a histograms that was omitted Feb 14, 2020
@bboreham bboreham changed the title Register a histograms that was omitted Register a histogram that was omitted Feb 14, 2020
Copy link
Contributor

@cyriltovena cyriltovena left a comment

Choose a reason for hiding this comment

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

LGTM

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!

Unrelated from this bugfix, we should (actually we slowly are) move away from globally defined metrics to avoid config DB metrics are exposed also when it's not running (ie. microservices deployments).

@pracucci pracucci merged commit 01f7121 into master Feb 17, 2020
@pracucci pracucci deleted the register-collectors branch February 17, 2020 12:07
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