-
Notifications
You must be signed in to change notification settings - Fork 358
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
Add prometheus view to ad hoc metrics #1677
Conversation
@miq-bot add_label compute/containers wip because it requires: ManageIQ/manageiq-providers-kubernetes#71 @simon3z @cben @moolitayer @nimrodshn please review |
135dd85
to
844a0c0
Compare
844a0c0
to
e6e88a3
Compare
This pull request is not mergeable. Please rebase and repush. |
e6e88a3
to
1fc036f
Compare
ManageIQ/manageiq-providers-kubernetes#71 is merged @simon3z @nimrodshn @zeari @himdel please review |
1fc036f
to
32fc71f
Compare
cc @ilackarms this PR is updated |
@joelddiaz this is our take on Prometheus metrics view in Manageiq, please review cc @simon3z |
tested working. LGTM! |
@@ -78,7 +82,7 @@ def data(query) | |||
rescue StandardError => e | |||
{ | |||
:parameters => params, | |||
:error => ActionView::Base.full_sanitizer.sanitize(e.to_s) + " " + _("(Please check your Hawkular server)") | |||
:error => ActionView::Base.full_sanitizer.sanitize(e.to_s) + " " + _("(Please check your #{@db_name} server)") |
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 can't work, you need to _("(Please check your %{type} server)") % {:type => @db_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.
👍 Thanks, I do not think i18n ... , copied your fix :-)
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.
:) The only thing to keep it in mind really is that the _
function can only translate it if it finds an exact match in its db. So any interpolation has to be done on the result of _
, not before.
jobs.map { |id| { :label => labelize(id), :value => id } } | ||
end | ||
|
||
def add_last_readings(metrics) |
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.
Dead method? (Or just useless? :))
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.
👍 done, moved implementation to parent class
"default" => "Default", | ||
"admin" => "Admin", | ||
"openshift-infra" => "OpenShift Infra" | ||
"_system" => "System", |
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.
any chance these should be translated?
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 makes seance.
@yaacov Trying to test in ui, but the current set-miq-providers fails with
Can you take a look please? From evm.log:
(And the old provider showed a long message about it not being available, but I can't get to it again, sorry.) |
sorry, all my machines are down/broken because for bad openshit 3.7 images ..., re-spinning 3.6 images now, will ping you when they will be running. |
@himdel ^^ |
32fc71f
to
ef8a881
Compare
first machine is up (with prometheus metrics) working on the |
Checked commits yaacov/manageiq-ui-classic@6db8abe~...b1ea37f with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 |
@himdel what to do about codeclimate ? |
@yaacov No need to do anything, the threshold is set too low for ruby (18), we were discussing increasing the threshold today .. Cc @martinpovolny: mass 25 is definitely too low :)
and
are definitely not similar enough :). |
Tested in the UI, LGTM :) |
Description
Add Prometheus as an external data source
Screenshot