Merge pull request #17560 from lpichler/use_pluck_metric_rollup_query…

…_in_chargeback

Use pluck on  metric rollup query in chargeback
(cherry picked from commit 8180f01)

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1591939

app/models/chargeable_field.rb

@@ -77,6 +77,15 @@ def metering?
     group == 'metering' && source == 'used'
   end
 
+  def self.cols_on_metric_rollup
+    (%w(id tag_names resource_id) + chargeable_cols_on_metric_rollup).uniq
+  end
+
+  def self.col_index(column)
+    @rate_cols ||= {}
+    @rate_cols[column] ||= cols_on_metric_rollup.index(column.to_s)
+  end
+
   private
 
   def rate_name
@@ -117,6 +126,6 @@ def self.seed_data
   def self.chargeable_cols_on_metric_rollup
     existing_cols = MetricRollup.attribute_names
     chargeable_cols = pluck(:metric) & existing_cols
-    chargeable_cols.map! { |x| VIRTUAL_COL_USES[x] || x }
+    chargeable_cols.map! { |x| VIRTUAL_COL_USES[x] || x }.sort
   end
 end

app/models/chargeback/consumption_history.rb

@@ -25,7 +25,7 @@ def self.for_report(cb_class, options)
         # values are grouped by resource_id and timestamp (query_start_time...query_end_time)
 
         records.each_value do |rollup_record_ids|
-          metric_rollup_records = base_rollup.where(:id => rollup_record_ids).to_a
+          metric_rollup_records = MetricRollup.where(:id => rollup_record_ids).pluck(*ChargeableField.cols_on_metric_rollup)
           consumption = ConsumptionWithRollups.new(metric_rollup_records, query_start_time, query_end_time)
           yield(consumption) unless consumption.consumed_hours_in_interval.zero?
         end

app/models/chargeback/consumption_with_rollups.rb

@@ -1,14 +1,14 @@
 class Chargeback
   class ConsumptionWithRollups < Consumption
     delegate :timestamp, :resource, :resource_id, :resource_name, :resource_type, :parent_ems,
-             :parents_determining_rate,
+             :parents_determining_rate, :resource_current_tag_names,
              :to => :first_metric_rollup_record
 
     attr_accessor :start_time, :end_time
 
     def initialize(metric_rollup_records, start_time, end_time)
       super(start_time, end_time)
-      @rollups = metric_rollup_records
+      @rollup_array = metric_rollup_records
     end
 
     def hash_features_affecting_rate
@@ -21,14 +21,53 @@ def hash_features_affecting_rate
     end
 
     def tag_names
-      @tag_names ||= @rollups.inject([]) do |memo, rollup|
-        memo |= rollup.all_tag_names
+      @tag_names ||= @rollup_array.inject([]) do |memo, rollup|
+        memo |= all_tag_names(rollup)
         memo
       end
     end
 
+    TAG_MANAGED_PREFIX = "/tag/managed/".freeze
+
+    def tag_prefix
+      klass_prefix = case resource_type
+                     when Container.name        then 'container_image'
+                     when VmOrTemplate.name     then 'vm'
+                     when ContainerProject.name then 'container_project'
+                     end
+
+      klass_prefix + TAG_MANAGED_PREFIX
+    end
+
+    def chargeback_container_labels
+      resource.try(:container_image).try(:docker_labels).try(:collect_concat) do |l|
+        escaped_name = AssignmentMixin.escape(l.name)
+        escaped_value = AssignmentMixin.escape(l.value)
+        [
+          # The assignments in tags can appear the old way as they are, or escaped
+          "container_image/label/managed/#{l.name}/#{l.value}",
+          "container_image/label/managed/#{escaped_name}/#{escaped_value}"
+        ]
+      end || []
+    end
+
+    def container_tag_list_with_prefix
+      if resource.kind_of?(Container)
+        state = resource.vim_performance_state_for_ts(timestamp.to_s)
+        image_tag_name = "#{state.image_tag_names}|" if state
+
+        image_tag_name.split("|")
+      else
+        []
+      end
+    end
+
+    def tag_list_with_prefix_for(rollup)
+      (all_tag_names(rollup) + container_tag_list_with_prefix).uniq.reject(&:empty?).map { |x| "#{tag_prefix}#{x}" } + chargeback_container_labels
+    end
+
     def tag_list_with_prefix
-      @tag_list_with_prefix ||= @rollups.map(&:tag_list_with_prefix).flatten.uniq
+      @tag_list_with_prefix ||= @rollup_array.map { |rollup| tag_list_with_prefix_for(rollup) }.flatten.uniq
     end
 
     def sum(metric, sub_metric = nil)
@@ -44,8 +83,11 @@ def max(metric, sub_metric = nil)
     def sum_of_maxes_from_grouped_values(metric, sub_metric = nil)
       return max(metric, sub_metric) if sub_metric
       @grouped_values ||= {}
-      grouped_rollups = @rollups.group_by { |x| x.resource.id }
-      @grouped_values[metric] ||= grouped_rollups.map { |_, rollups| rollups.collect(&metric.to_sym).compact.max }.compact.sum
+      grouped_rollups = @rollup_array.group_by { |x| x[ChargeableField.col_index(:resource_id)] }
+
+      @grouped_values[metric] ||= grouped_rollups.map do |_, rollups|
+        rollups.map { |x| x[ChargeableField.col_index(metric)] }.compact.max
+      end.compact.sum
     end
 
     def avg(metric, sub_metric = nil)
@@ -67,11 +109,34 @@ def none?(metric, sub_metric)
     end
 
     def chargeback_fields_present
-      @chargeback_fields_present ||= @rollups.count(&:chargeback_fields_present?)
+      @chargeback_fields_present ||= @rollup_array.count { |rollup| chargeback_fields_present?(rollup) }
+    end
+
+    def chargeback_fields_present?(rollup_record)
+      MetricRollup::CHARGEBACK_METRIC_FIELDS.any? do |field|
+        rollup = rollup_record[ChargeableField.col_index(field)]
+        rollup.present? && rollup.nonzero?
+      end
+    end
+
+    def metering_used_fields_present?(rollup_record)
+      MetricRollup::METERING_USED_METRIC_FIELDS.any? do |field|
+        rollup = rollup_record[ChargeableField.col_index(field)]
+        rollup.present? && rollup.nonzero?
+      end
     end
 
     def metering_used_fields_present
-      @metering_used_fields_present ||= @rollups.count(&:metering_used_fields_present?)
+      @metering_used_fields_present ||= @rollup_array.count { |rollup| metering_used_fields_present?(rollup) }
+    end
+
+    def resource_tag_names(rollup)
+      tags_names = rollup[ChargeableField.col_index(:tag_names)]
+      tags_names ? tags_names.split('|') : []
+    end
+
+    def all_tag_names(rollup)
+      resource_current_tag_names | resource_tag_names(rollup)
     end
 
     private
@@ -88,11 +153,14 @@ def sub_metric_rollups(sub_metric)
 
     def values(metric, sub_metric = nil)
       @values ||= {}
-      @values["#{metric}#{sub_metric}"] ||= sub_metric ? sub_metric_rollups(sub_metric) : @rollups.collect(&metric.to_sym).compact
+      @values["#{metric}#{sub_metric}"] ||= begin
+        sub_metric ? sub_metric_rollups(sub_metric) : @rollup_array.collect { |x| x[ChargeableField.col_index(metric)] }.compact
+      end
     end
 
     def first_metric_rollup_record
-      @fmrr ||= @rollups.first
+      first_rollup_id = @rollup_array.first[ChargeableField.col_index(:id)]
+      @fmrr ||= MetricRollup.find(first_rollup_id) if first_rollup_id
     end
   end
 end

app/models/metric/chargeback_helper.rb

@@ -1,43 +1,4 @@
 module Metric::ChargebackHelper
-  TAG_MANAGED_PREFIX = "/tag/managed/".freeze
-
-  def tag_prefix
-    klass_prefix = case resource_type
-                   when Container.name        then 'container_image'
-                   when VmOrTemplate.name     then 'vm'
-                   when ContainerProject.name then 'container_project'
-                   end
-
-    klass_prefix + TAG_MANAGED_PREFIX
-  end
-
-  def chargeback_container_labels
-    resource.try(:container_image).try(:docker_labels).try(:collect_concat) do |l|
-      escaped_name = AssignmentMixin.escape(l.name)
-      escaped_value = AssignmentMixin.escape(l.value)
-      [
-        # The assignments in tags can appear the old way as they are, or escaped
-        "container_image/label/managed/#{l.name}/#{l.value}",
-        "container_image/label/managed/#{escaped_name}/#{escaped_value}"
-      ]
-    end || []
-  end
-
-  def container_tag_list_with_prefix
-    if resource.kind_of?(Container)
-      state = resource.vim_performance_state_for_ts(timestamp.to_s)
-      image_tag_name = "#{state.image_tag_names}|" if state
-
-      image_tag_name.split("|")
-    else
-      []
-    end
-  end
-
-  def tag_list_with_prefix
-    (all_tag_names + container_tag_list_with_prefix).uniq.reject(&:empty?).map { |x| "#{tag_prefix}#{x}" } + chargeback_container_labels
-  end
-
   def resource_parents
     [parent_host || resource.try(:host),
      parent_ems_cluster || resource.try(:ems_cluster),

app/models/metric_rollup.rb

@@ -67,12 +67,6 @@ def self.rollups_in_range(resource_type, resource_ids, capture_interval_name, st
     metrics.order(:resource_id, :timestamp => :desc)
   end
 
-  def chargeback_fields_present?
-    return @chargeback_fields_present if defined?(@chargeback_fields_present)
-
-    @chargeback_fields_present = CHARGEBACK_METRIC_FIELDS.any? { |field| send(field).present? && send(field).nonzero? }
-  end
-
   def metering_used_fields_present?
     @metering_used_fields_present ||= METERING_USED_METRIC_FIELDS.any? { |field| send(field).present? && send(field).nonzero? }
   end

spec/models/chargeable_field_spec.rb

@@ -6,4 +6,30 @@
     subject { field.send :rate_name }
     it { is_expected.to eq("#{group}_#{source}") }
   end
+
+  describe "#cols_on_metric_rollup" do
+    before do
+      ChargebackRateDetailMeasure.seed
+      described_class.seed
+    end
+
+    it 'returns list of columns for main chargeback metric rollup query' do
+      expected_columns = %w(
+        id
+        tag_names
+        resource_id
+        cpu_usage_rate_average
+        cpu_usagemhz_rate_average
+        derived_memory_available
+        derived_memory_used
+        derived_vm_allocated_disk_storage
+        derived_vm_numvcpus
+        derived_vm_used_disk_storage
+        disk_usage_rate_average
+        net_usage_rate_average
+      )
+
+      expect(described_class.cols_on_metric_rollup).to eq(expected_columns)
+    end
+  end
 end

spec/models/chargeback/consumption_with_rollups_spec.rb

@@ -1,6 +1,6 @@
 describe Chargeback::ConsumptionWithRollups do
-  let(:vm)          { FactoryGirl.build(:vm_microsoft) }
-  let(:consumption) { described_class.new([metric_rollup], starting_date, starting_date + 1.day) }
+  let(:vm)          { FactoryGirl.create(:vm_microsoft) }
+  let(:consumption) { described_class.new(pluck_rollup([metric_rollup]), starting_date, starting_date + 1.day) }
 
   describe '#sub_metric_rollups' do
     let(:starting_date)        { Time.parse('2012-09-01 23:59:59Z').utc }
@@ -9,6 +9,10 @@
     let!(:state)         { FactoryGirl.create(:vim_performance_state, :resource => vm, :state_data => nil, :timestamp => starting_date, :capture_interval => 3_600) }
     let!(:metric_rollup) { FactoryGirl.create(:metric_rollup_vm_hr, :timestamp => starting_date + 1.hour, :resource => vm) }
 
+    def pluck_rollup(metric_rollup_records)
+      metric_rollup_records.pluck(*ChargeableField.cols_on_metric_rollup)
+    end
+
     before do
       Timecop.travel(starting_date + 10.hours)
     end
@@ -43,6 +47,41 @@
       end
     end
 
+    let(:ems) { FactoryGirl.build(:ems_vmware) }
+
+    context "Containers" do
+      describe "#tag_list_with_prefix" do
+        let(:tag) { FactoryGirl.create(:tag, :name => "/managed/operations/analysis_failed") }
+        let(:vm) { FactoryGirl.create(:vm_vmware, :tags => [tag]) }
+        let(:metric_rollup) { FactoryGirl.create(:metric_rollup_vm_hr, :resource => vm, :tag_names => "environment/prod|environment/dev") }
+        let(:consumption) { described_class.new(pluck_rollup([metric_rollup]), starting_date, starting_date + 1.day) }
+
+        it 'returns array of tags' do
+          expect(consumption.tag_list_with_prefix).to match_array(%w(vm/tag/managed/operations/analysis_failed vm/tag/managed/environment/prod vm/tag/managed/environment/dev))
+        end
+      end
+    end
+
+    context "Containers" do
+      describe "#tag_list_with_prefix" do
+        let(:timestamp) { Time.parse('2012-09-01 23:59:59Z').utc }
+        let(:vim_performance_state) { FactoryGirl.create(:vim_performance_state, :timestamp => timestamp, :image_tag_names => "environment/stage") }
+
+        let(:image) { FactoryGirl.create(:container_image, :ext_management_system => ems, :docker_labels => [label]) }
+        let(:label) { FactoryGirl.create(:custom_attribute, :name => "version/1.2/_label-1", :value => "test/1.0.0  rc_2", :section => 'docker_labels') }
+        let(:project) { FactoryGirl.create(:container_project, :name => "my project", :ext_management_system => ems) }
+        let(:node) { FactoryGirl.create(:container_node, :name => "node") }
+        let(:group) { FactoryGirl.create(:container_group, :ext_management_system => ems, :container_project => project, :container_node => node) }
+        let(:container) { FactoryGirl.create(:kubernetes_container, :container_group => group, :container_image => image, :vim_performance_states => [vim_performance_state]) }
+        let(:metric_rollup_container) { FactoryGirl.create(:metric_rollup_vm_hr, :timestamp => timestamp, :resource => container, :tag_names => "environment/cont|environment/cust") }
+        let(:consumption) { described_class.new(pluck_rollup([metric_rollup_container]), starting_date, starting_date + 1.day) }
+
+        it 'returns array of tags' do
+          expect(consumption.tag_list_with_prefix).to match_array(%w(container_image/tag/managed/environment/cont container_image/tag/managed/environment/cust container_image/tag/managed/environment/stage container_image/label/managed/version/1.2/_label-1/test/1.0.0\ \ rc_2 container_image/label/managed/escaped:{version%2F1%2E2%2F%5Flabel%2D1}/escaped:{test%2F1%2E0%2E0%20%20rc%5F2}))
+        end
+      end
+    end
+
     after do
       Timecop.return
     end