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 CPU, CPU cores and MEMORY metering allocation to Metering reports #17938

Merged

Conversation

lpichler
Copy link
Contributor

@lpichler lpichler commented Sep 4, 2018

we have already Metering Used Metric and this PR is adding same thing but for allocation and concretely for CPU (VMs),CPU Cores(ContainerProject/Image) and Memory.

In Report Definitoin it looks like:
screen shot 2018-09-04 at 10 10 37
screen shot 2018-09-04 at 10 03 24

In Report Result:
screen shot 2018-09-04 at 10 17 46

Links

@miq-bot add_label enhancement

@miq-bot assign @gtanzillo

@lpichler lpichler force-pushed the add_allocation_to_metering_report branch from d8dd900 to 464fbcc Compare September 4, 2018 12:31
@@ -129,6 +129,14 @@ def metering_used_fields_present
@metering_used_fields_present ||= @rollup_array.count { |rollup| metering_used_fields_present?(rollup) }
end

def metering_allocated_for(metric)
Copy link
Member

Choose a reason for hiding this comment

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

I'm not that familiar with chargeback reports...

But this looks like a rollup.
Is there a reason to not just do a count(:column) in sql?
I guess the non-zero clause could be a little tricky.

Copy link
Contributor Author

@lpichler lpichler Sep 11, 2018

Choose a reason for hiding this comment

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

thanks but I cannot use it - it is rollup but after method pluck so there are just values.

Copy link
Member

Choose a reason for hiding this comment

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

darn. especially since these all look like sql group by rollups.

GL

@@ -40,7 +57,7 @@ def calculate_costs(consumption, _)
end

chargable_field = ChargeableField.find_by(:group => group, :source => source)
next if field == "existence_hours_metric" || field == "fixed_compute_metric" || chargable_field && chargable_field.metering?
next if field == 'metering_allocated_cpu_cores_metric' || field == 'metering_allocated_cpu_metric' || field == 'metering_allocated_memory_metric' || field == "existence_hours_metric" || field == "fixed_compute_metric" || chargable_field&.metering?
Copy link
Member

Choose a reason for hiding this comment

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

Maybe you can simplify this by putting the list of a"allocated" fields in an array an doing a look up of field in ti?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes it is better, I added it 👍

@lpichler lpichler force-pushed the add_allocation_to_metering_report branch 2 times, most recently from 36682ad to 04122fa Compare September 11, 2018 08:02
@@ -24,6 +24,8 @@ class ChargebackVm < Chargeback
:memory_used_cost => :float,
:memory_used_metric => :float,
:memory_cost => :float,
:metering_used_metric => :integer,
Copy link
Member

Choose a reason for hiding this comment

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

This should be removed per our discussion

@lpichler lpichler force-pushed the add_allocation_to_metering_report branch 2 times, most recently from 19fdff7 to d167ced Compare September 12, 2018 04:30
@lpichler lpichler closed this Sep 13, 2018
@lpichler lpichler reopened this Sep 13, 2018
@lpichler lpichler force-pushed the add_allocation_to_metering_report branch from d167ced to b058a02 Compare September 13, 2018 06:48
@miq-bot
Copy link
Member

miq-bot commented Sep 13, 2018

Checked commit lpichler@b058a02 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
9 files checked, 0 offenses detected
Everything looks fine. 👍

@gtanzillo gtanzillo added this to the Sprint 95 Ending Sep 24, 2018 milestone Sep 24, 2018
@gtanzillo gtanzillo merged commit 4240c99 into ManageIQ:master Sep 24, 2018
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.

4 participants