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

Add Openstack metric service to Settings #13918

Conversation

aufi
Copy link
Member

@aufi aufi commented Feb 14, 2017

Openstack Metrics capture service can be specified in settings.yml file. Needed for OSP9 which contains both Gnocchi and Ceilometer (but Gnocchi does not have data there).

Possible values:

  • "gnocchi"
  • "ceilometer"
  • "auto" (and any other string means automatic selection, gnocchi is preffered)

Links

end

def available_metric_services
{"gnocchi" => "metric", "ceilometer" => "metering"}
Copy link
Contributor

Choose a reason for hiding this comment

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

Do 'metric' and 'metering' need to be capitalized?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure, but capitalized, thanks for comment.

@aufi aufi force-pushed the add_openstack_metric_service_to_settings_master branch from f501f09 to 84af0e6 Compare February 14, 2017 17:38
@tzumainn
Copy link
Contributor

@miq-bot add_label euwe/yes,bug

Copy link
Member

@blomquisg blomquisg left a comment

Choose a reason for hiding this comment

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

Generally it looks fine. Couple of nitpick points that might help.

@@ -1306,6 +1306,7 @@
:ems_metrics_collector_worker_openstack_network: {}
:ems_metrics_collector_worker_redhat: {}
:ems_metrics_collector_worker_vmware: {}
:ems_metrics_openstack_default_service: "auto"
Copy link
Member

Choose a reason for hiding this comment

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

So, "auto" simply means skip over the return statement and default to gnocchi?

I think that just having "gnocchi" be the default here might be clearer.

Copy link
Contributor

Choose a reason for hiding this comment

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

Not quite - auto defaults to the current code path, where CF will attempt to connect to Gnocchi; if it can't, it'll then try Ceilometer. That means the default behavior is the same as the current behavior.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, gotcha

Copy link
Member Author

Choose a reason for hiding this comment

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

"auto" (or any other string except "gnocchi" or "ceilometer") means use gnocchi with ceilometer fallback, what might have different behavior in Openstack than without fallback.

@@ -26,6 +26,8 @@ def perf_init_openstack
raise "No EMS defined" if target.ext_management_system.nil?

metering_service, = Benchmark.realtime_block(:connect) do
return target.ext_management_system.connect(:service => available_metric_services[metric_service_from_settings]) if \
available_metric_services.keys.include? metric_service_from_settings
Copy link
Member

Choose a reason for hiding this comment

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

If I were looking in the logs trying to figure out why I wasn't seeing metrics, I would want to see that which service was selected, I think.

It's probably worth selecting the service, log (even at debug level) which service was selected, then try to connect to that service.

Copy link
Member Author

Choose a reason for hiding this comment

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

Makes sense, updated.

Openstack Metrics capture service can be specified in settings.yml file.

Valid values are "gnocchi", "ceilometer". All other values means autodetection.

Related BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1421729
@aufi aufi force-pushed the add_openstack_metric_service_to_settings_master branch from 84af0e6 to 926623f Compare February 14, 2017 19:20
@miq-bot
Copy link
Member

miq-bot commented Feb 14, 2017

Checked commit aufi@926623f with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
1 file checked, 0 offenses detected
Everything looks good. 🍰

@tzumainn
Copy link
Contributor

I've tested with the config setting set to 'auto', 'ceilometer', and 'gnocchi' and everything seems to work as expected. Thanks!

@blomquisg blomquisg merged commit 05f1669 into ManageIQ:master Feb 14, 2017
@blomquisg blomquisg added this to the Sprint 55 Ending Feb 27, 2017 milestone Feb 14, 2017
simaishi pushed a commit that referenced this pull request Feb 14, 2017
…ettings_master

Add Openstack metric service to Settings
(cherry picked from commit 05f1669)

https://bugzilla.redhat.com/show_bug.cgi?id=1422241
@simaishi
Copy link
Contributor

Euwe backport details:

$ git log -1
commit ebaba61057f5b661974b224a187258eafb241dc7
Author: Greg Blomquist <[email protected]>
Date:   Tue Feb 14 15:02:42 2017 -0500

    Merge pull request #13918 from aufi/add_openstack_metric_service_to_settings_master
    
    Add Openstack metric service to Settings
    (cherry picked from commit 05f16698e5d50e2123029a5ed40634ac02c3c8de)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1422241

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants