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

Custom Metrics: implement as configuration and add docs #4

Merged
merged 3 commits into from Mar 18, 2019

Conversation

Projects
None yet
2 participants
@leklund
Copy link
Member

commented Mar 18, 2019

Summary

  • add SidekiqPrometheus.custom_metrics accessor that can be used in a configure block to define and register custom metrics
  • does some basic validation of the custom metrics
  • update the README with instructions on how to configure, register, and use custom metrics.

Closes #2

leklund added some commits Mar 18, 2019

feat: Custom Metrics
* add SidekiqPrometheus.custom_metrics accessor that can be used in a
  configure block to define and register custom metrics
* does some basic validation of the custom metrics
doc: Update README for Custom Metrics usage
* how to register custom metrics
* how to use custom metrics
* tag code blocks with `ruby`
if custom_metrics.all? { |m| (m.keys.sort & expected_keys) == expected_keys }
SidekiqPrometheus::Metrics.register_metrics(custom_metrics)
else
Sidekiq.logger.warn('[SidekiqPrometheus] Skipping custom metrics. Hash does not define required keys')

This comment has been minimized.

Copy link
@DuoSRX

DuoSRX Mar 18, 2019

Member

How about raising an exception instead of a warning? If custom metrics are skipped, the workers will crash later when trying to push data to the inexistent metrics. I'd rather have it crash during setup than later.

This comment has been minimized.

Copy link
@leklund

leklund Mar 18, 2019

Author Member

That makes sense. In that case, I can skip the "validation" and just let Prometheus::Client raise if the metrics hash is malformed.

This comment has been minimized.

Copy link
@leklund

leklund Mar 18, 2019

Author Member

@DuoSRX removed that logic in 714029a

refactor: don't raise if missing keys. let the client handle that
rather, raise only if the custom_metrics is not an array
@DuoSRX

DuoSRX approved these changes Mar 18, 2019

Copy link
Member

left a comment

👍

@leklund leklund merged commit 4a49da0 into master Mar 18, 2019

2 checks passed

Travis CI - Branch Build Passed
Details
Travis CI - Pull Request Build Passed
Details

@leklund leklund deleted the custom_metrics-fix#2 branch Mar 18, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.