Enable custom attributes for chargeback reports #11794

zeari · 2016-10-10T15:13:31Z

BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1382720
When the report generator asks for the newly defined CustomAttribute method, direct it to the entity class instead of the chargeback class. Each chargeback line will contain the entity(Vm/Project) required for this.

[root@mtayer-centos7-3 ~]# oc describe project demo2
Name:           demo2
Created:        4 days ago
Labels:         zone=east   <---------

zone becomes an available column:

yields report(blank where zone not defined):

@lpichler @gtanzillo Please review
cc @simon3z

zeari · 2016-10-10T15:18:56Z

@miq-bot add_label providers/containers, chargeback, euwe/yes

lpichler · 2016-10-10T15:19:05Z

@miq_bot assign @lpichler

zeari · 2016-10-10T15:30:51Z

@miq_bot assign @lpichler

@miq-bot assign @lpichler

miq-bot · 2016-10-10T20:21:27Z

<pr_mergeability_checker />This pull request is not mergeable. Please rebase and repush.

lpichler · 2016-10-13T11:55:24Z

tested, LGTM 👍

zeari · 2016-10-13T12:06:39Z

Thanks for all the help @lpichler!
@gtanzillo Please review\merge

zeari · 2016-10-13T13:55:04Z

@simon3z Added better description with screenshots

simon3z · 2016-10-13T14:03:07Z

@simon3z Added better description with screenshots

@zeari nice thanks! Only comment from my side would be to have a prefix to understand that that's a label (Label : zone)

zeari · 2016-10-13T14:31:25Z

@zeari nice thanks! Only comment from my side would be to have a prefix to understand that that's a label (Label : zone)

Well it looks like I can but itll apply to any CustomAttribute. We will need to find a better solution for that and also apply it to the rest of reporting in general. Lets make that happen in another PR.

simon3z · 2016-10-13T14:45:39Z

Well it looks like I can but itll apply to any CustomAttribute. We will need to find a better solution for that and also apply it to the rest of reporting in general. Lets make that happen in another PR.

@zeari can't you use the CustomAttribute section? E.g. Attribute : label : zone

zeari · 2016-10-13T14:54:30Z

@zeari can't you use the CustomAttribute section? E.g. Attribute : label : zone

@simon3z Yes, but lets argue about UI in another PR instead of here: #11916

gtanzillo

👍 LGTM

Enable custom attributes for chargeback reports (cherry picked from commit 9b09682) https://bugzilla.redhat.com/show_bug.cgi?id=1382720

chessbyte · 2016-10-14T15:39:17Z

Euwe Backport details:

$ git log -1
commit 5a3f888d8d1d32a2e32ad74da8c0b8fddf860293
Author: Gregg Tanzillo <gtanzill@redhat.com>
Date:   Thu Oct 13 20:21:23 2016 -0400

    Merge pull request #11794 from zeari/chargeback_kubernetes_labels

    Enable custom attributes for chargeback reports
    (cherry picked from commit 9b096826f16cef9f6b026dddf73cee96e6c69430)

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

chessbyte · 2016-11-03T20:00:32Z

Darga Backport conflict:

$ git cherry-pick -e -x -m 1 9b09682   
error: could not apply 9b09682... Merge pull request #11794 from zeari/chargeback_kubernetes_labels
hint: after resolving the conflicts, mark the corrected paths
hint: with 'git add <paths>' or 'git rm <paths>'
hint: and commit the result with 'git commit'

$ git status
On branch darga
Your branch is up-to-date with 'upstream/darga'.
You are currently cherry-picking commit 9b09682.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:

    modified:   app/controllers/report_controller/reports/editor.rb
    modified:   app/models/miq_expression.rb

Unmerged paths:
  (use "git add/rm <file>..." as appropriate to mark resolution)

    both modified:   app/models/chargeback.rb
    deleted by us:   app/models/chargeback_container_image.rb
    both modified:   app/models/chargeback_container_project.rb
    both modified:   app/models/chargeback_vm.rb
    both modified:   app/models/miq_report.rb

$ git diff

diff --cc app/models/chargeback.rb
index c7fb122,7cba6ff..0000000
--- a/app/models/chargeback.rb
+++ b/app/models/chargeback.rb
@@@ -14,8 -14,12 +14,12 @@@ class Chargeback < ActsAsArMode

      options[:ext_options] ||= {}

 -    if @options[:groupby_tag]
 -      @tag_hash = Classification.hash_all_by_type_and_name[@options[:groupby_tag]][:entry]
 -    end
 -
      base_rollup = MetricRollup.includes(
++<<<<<<< HEAD
 +      :resource           => [:hardware, :tenant],
++=======
+       :resource           => [:hardware, :tenant, :tags, :vim_performance_states, :custom_attributes],
++>>>>>>> 9b09682... Merge pull request #11794 from zeari/chargeback_kubernetes_labels
        :parent_host        => :tags,
        :parent_ems_cluster => :tags,
        :parent_storage     => :tags,
@@@ -64,6 -64,41 +68,44 @@@
      [data.map { |r| new(r.last) }]
    end

++<<<<<<< HEAD
++=======
+   def self.key_and_fields(metric_rollup_record, interval, tz)
+     ts_key = get_group_key_ts(metric_rollup_record, interval, tz)
+ 
+     key, extra_fields = if @options[:groupby_tag].present?
+                           get_tag_keys_and_fields(metric_rollup_record, ts_key)
+                         else
+                           get_keys_and_extra_fields(metric_rollup_record, ts_key)
+                         end
+ 
+     [key, date_fields(metric_rollup_record, interval, tz).merge(extra_fields)]
+   end
+ 
+   def self.date_fields(metric_rollup_record, interval, tz)
+     start_ts, end_ts, display_range = get_time_range(metric_rollup_record, interval, tz)
+ 
+     {
+       'start_date'       => start_ts,
+       'end_date'         => end_ts,
+       'display_range'    => display_range,
+       'interval_name'    => interval,
+       'chargeback_rates' => '',
+       'entity'           => metric_rollup_record.resource
+     }
+   end
+ 
+   def self.get_tag_keys_and_fields(perf, ts_key)
+     tag = perf.tag_names.split("|").select { |x| x.starts_with?(@options[:groupby_tag]) }.first # 'department/*'
+     tag = tag.split('/').second unless tag.blank? # 'department/finance' -> 'finance'
+     classification = @tag_hash[tag]
+     classification_id = classification.present? ? classification.id : 'none'
+     key = "#{classification_id}_#{ts_key}"
+     extra_fields = { "tag_name" => classification.present? ? classification.description : _('<Empty>') }
+     [key, extra_fields]
+   end
+ 
++>>>>>>> 9b09682... Merge pull request #11794 from zeari/chargeback_kubernetes_labels
    def get_rates(perf)
      @rates ||= {}
      @enterprise ||= MiqEnterprise.my_enterprise
@@@ -189,4 -236,71 +231,74 @@@
    def self.report_tag_field
      "tag_name"
    end
++<<<<<<< HEAD
++=======
+ 
+   def self.get_rate_parents
+     raise "Chargeback: get_rate_parents must be implemented in child class."
+   end
+ 
+   def self.set_chargeback_report_options(rpt, edit)
+     rpt.cols = %w(start_date display_range)
+ 
+     static_cols = report_static_cols
+     if edit[:new][:cb_groupby] == "date"
+       rpt.cols += static_cols
+       rpt.col_order = ["display_range"] + static_cols
+       rpt.sortby = ["start_date"] + static_cols
+     elsif edit[:new][:cb_groupby] == "vm"
+       rpt.cols += static_cols
+       rpt.col_order = static_cols + ["display_range"]
+       rpt.sortby = static_cols + ["start_date"]
+     elsif edit[:new][:cb_groupby] == "tag"
+       tag_col = report_tag_field
+       rpt.cols += tag_col
+       rpt.col_order = [tag_col, "display_range"]
+       rpt.sortby = [tag_col, "start_date"]
+     elsif edit[:new][:cb_groupby] == "project"
+       static_cols -= ["image_name"]
+       rpt.cols += static_cols
+       rpt.col_order = static_cols + ["display_range"]
+       rpt.sortby = static_cols + ["start_date"]
+     end
+     rpt.col_order.each do |c|
+       if c == tag_col
+         header = edit[:cb_cats][edit[:new][:cb_groupby_tag]]
+         rpt.headers.push(Dictionary.gettext(header, :type => :column, :notfound => :titleize))
+       else
+         rpt.headers.push(Dictionary.gettext(c, :type => :column, :notfound => :titleize))
+       end
+ 
+       rpt.col_formats.push(nil) # No formatting needed on the static cols
+     end
+ 
+     rpt.col_options = report_col_options
+     rpt.order = "Ascending"
+     rpt.group = "y"
+     rpt.tz = edit[:new][:tz]
+     rpt
+   end
+ 
+   def tags
+     entity.try(:tags).to_a
+   end
+ 
+   def self.load_custom_attributes_for(cols)
+     chargeback_klass = report_cb_model(self.to_s).safe_constantize
+     chargeback_klass.load_custom_attributes_for(cols)
+     cols.each do |x|
+       next unless x.include?(CustomAttributeMixin::CUSTOM_ATTRIBUTES_PREFIX)
+ 
+       load_custom_attribute(x)
+     end
+   end
+ 
+   def self.load_custom_attribute(custom_attribute)
+     virtual_column(custom_attribute.to_sym, :type => :string)
+ 
+     define_method(custom_attribute.to_sym) do
+       entity.send(custom_attribute)
+     end
+   end
++>>>>>>> 9b09682... Merge pull request #11794 from zeari/chargeback_kubernetes_labels
  end # class Chargeback
diff --cc app/models/chargeback_container_project.rb
index 9e203e6,bc84ad4..0000000
--- a/app/models/chargeback_container_project.rb
+++ b/app/models/chargeback_container_project.rb
@@@ -20,9 -22,8 +20,14 @@@ class ChargebackContainerProject < Char
      :memory_used_metric    => :float,
      :net_io_used_cost      => :float,
      :net_io_used_metric    => :float,
++<<<<<<< HEAD
 +    :net_io_cost           => :float,
 +    :net_io_metric         => :float,
 +    :total_cost            => :float
++=======
+     :total_cost            => :float,
+     :entity                => :binary
++>>>>>>> 9b09682... Merge pull request #11794 from zeari/chargeback_kubernetes_labels
    )

    def self.build_results_for_report_ChargebackContainerProject(options)
@@@ -115,7 -100,8 +120,13 @@@
      }
    end

++<<<<<<< HEAD
 +  def tags
 +    ContainerProject.includes(:tags).find_by_ems_ref(project_uid).try(:tags).to_a
++=======
+   def get_rate_parents(perf)
+     # Get rate from assigned containers providers only
+     [perf.parent_ems]
++>>>>>>> 9b09682... Merge pull request #11794 from zeari/chargeback_kubernetes_labels
    end
  end # class Chargeback
diff --cc app/models/chargeback_vm.rb
index ab38d38,d57ce6b..0000000
--- a/app/models/chargeback_vm.rb
+++ b/app/models/chargeback_vm.rb
@@@ -152,7 -164,8 +153,13 @@@ class ChargebackVm < Chargebac
      }
    end

++<<<<<<< HEAD
 +  def tags
 +    Vm.includes(:tags).find_by_ems_ref(vm_uid).try(:tags).to_a
++=======
+   def get_rate_parents(perf)
+     @enterprise ||= MiqEnterprise.my_enterprise
+     [perf.parent_host, perf.parent_ems_cluster, perf.parent_storage, perf.parent_ems, @enterprise, perf.resource.try(:tenant)]
++>>>>>>> 9b09682... Merge pull request #11794 from zeari/chargeback_kubernetes_labels
    end
  end # class Chargeback
diff --cc app/models/miq_report.rb
index aa0f6dd,84b162d..0000000
--- a/app/models/miq_report.rb
+++ b/app/models/miq_report.rb
@@@ -165,4 -201,26 +165,29 @@@ class MiqReport < ApplicationRecor
    def page_size
      rpt_options.try(:fetch_path, :pdf, :page_size) || "a4"
    end
++<<<<<<< HEAD
++=======
+ 
+   def load_custom_attributes
+     klass = db.safe_constantize
+     return unless klass < CustomAttributeMixin || Chargeback.db_is_chargeback?(db)
+ 
+     klass.load_custom_attributes_for(cols.uniq)
+   end
+ 
+   # this method adds :custom_attributes => {} to MiqReport#include
+   # when report with virtual custom attributes is stored
+   # we need preload custom_attributes table to main query for building report for elimination of superfluous queries
+   def add_includes_for_virtual_custom_attributes
+     include[:custom_attributes] ||= {} if CustomAttributeMixin.select_virtual_custom_attributes(cols).present?
+   end
+ 
+   # this method removes loading (:custom_attributes => {}) relations for custom_attributes before report is built
+   # :custom_attributes => {} was added in method add_includes_for_virtual_custom_attributes in MiqReport#include
+   # vc_attributes == Virtual Custom Attributes
+   def remove_loading_relations_for_virtual_custom_attributes
+     vc_attributes = CustomAttributeMixin.select_virtual_custom_attributes(cols).present?
+     include.delete(:custom_attributes) if vc_attributes.present? && include && include[:custom_attributes].blank?
+   end
++>>>>>>> 9b09682... Merge pull request #11794 from zeari/chargeback_kubernetes_labels
  end
* Unmerged path app/models/chargeback_container_image.rb

chessbyte · 2016-11-03T20:01:31Z

@zeari @simon3z Need Darga-specific PR or suggest other PRs to backport first.

zeari · 2016-11-07T12:20:30Z

This isnt really darga compatible since we dont have support for custom attributes in darga.
@gtanzillo Do you remember why you marked this darga/yes?

simaishi · 2016-11-14T19:52:53Z

@zeari The darga/yes label was added as per https://bugzilla.redhat.com/show_bug.cgi?id=1382720#c12. As per Loic, the fix is still needed in 5.6.z (just confirmed), so adding the label back - please discuss with him if there is any issue making this available for darga.

zeari · 2016-11-15T09:21:27Z

@simaishi There might be more dependencies for this.
@gtanzillo @lpichler Were custom attributes for reports backported to darga?

gtanzillo · 2016-11-15T12:14:35Z

@zeari As far as I can tell it was not backported to Darga. These are the PRs I found #9451, #9807, #11369. I'm not sure if there are others. Does this PR need to go to Darga? I see the BZ is not flagged for 5.6.

simaishi · 2016-11-15T12:25:38Z

@gtanzillo it's already cloned to 5.6.z: https://bugzilla.redhat.com/show_bug.cgi?id=1390713.

lpichler · 2016-11-15T12:31:31Z

I'm not sure if there are others.

it should be all what about reporting custom attributes but I can imagine that there is more PRs related to chargeback refactoring, etc. Some of them was backported in already #12093

simaishi · 2017-01-12T03:23:39Z

5.6.z clone was closed as WONTFIX.

zeari force-pushed the chargeback_kubernetes_labels branch from 1519a48 to e17d03d Compare October 10, 2016 15:14

miq-bot added core/chargeback euwe/yes providers/containers labels Oct 10, 2016

zeari force-pushed the chargeback_kubernetes_labels branch from e17d03d to 50b451d Compare October 10, 2016 15:20

chessbyte assigned simon3z Oct 10, 2016

chessbyte added the enhancement label Oct 10, 2016

miq-bot assigned lpichler and unassigned simon3z Oct 10, 2016

zeari force-pushed the chargeback_kubernetes_labels branch from 50b451d to 0a44a02 Compare October 10, 2016 16:14

zeari force-pushed the chargeback_kubernetes_labels branch 4 times, most recently from 1c83ca7 to d4ee12c Compare October 13, 2016 10:13

Enable custom attributes for chargeback reports

d4ee12c

zeari closed this Oct 13, 2016

zeari reopened this Oct 13, 2016

gtanzillo approved these changes Oct 14, 2016

View reviewed changes

gtanzillo added this to the Sprint 48 Ending Oct 24, 2016 milestone Oct 14, 2016

gtanzillo merged commit 9b09682 into ManageIQ:master Oct 14, 2016

chessbyte pushed a commit that referenced this pull request Oct 14, 2016

Merge pull request #11794 from zeari/chargeback_kubernetes_labels

5a3f888

Enable custom attributes for chargeback reports (cherry picked from commit 9b09682) https://bugzilla.redhat.com/show_bug.cgi?id=1382720

chessbyte added euwe/backported and removed euwe/yes labels Oct 14, 2016

gtanzillo added the darga/yes label Nov 1, 2016

simon3z mentioned this pull request Nov 2, 2016

Descriptive custom attribute columns #11916

Closed

chessbyte added the darga/conflict label Nov 3, 2016

gtanzillo removed darga/conflict labels Nov 7, 2016

simaishi added the darga/yes label Nov 14, 2016

chessbyte added the darga/conflict label Dec 13, 2016

simaishi added darga/no and removed darga/conflict labels Jan 12, 2017

zeari deleted the chargeback_kubernetes_labels branch August 24, 2017 07:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable custom attributes for chargeback reports #11794

Enable custom attributes for chargeback reports #11794

zeari commented Oct 10, 2016 •

edited

Loading

zeari commented Oct 10, 2016

lpichler commented Oct 10, 2016 •

edited

Loading

zeari commented Oct 10, 2016

miq-bot commented Oct 10, 2016

lpichler commented Oct 13, 2016

zeari commented Oct 13, 2016 •

edited

Loading

zeari commented Oct 13, 2016

simon3z commented Oct 13, 2016

zeari commented Oct 13, 2016

simon3z commented Oct 13, 2016

zeari commented Oct 13, 2016 •

edited

Loading

gtanzillo left a comment

chessbyte commented Oct 14, 2016

chessbyte commented Nov 3, 2016

chessbyte commented Nov 3, 2016 •

edited

Loading

zeari commented Nov 7, 2016

simaishi commented Nov 14, 2016 •

edited

Loading

zeari commented Nov 15, 2016

gtanzillo commented Nov 15, 2016

simaishi commented Nov 15, 2016

lpichler commented Nov 15, 2016

simaishi commented Jan 12, 2017

Enable custom attributes for chargeback reports #11794

Enable custom attributes for chargeback reports #11794

Conversation

zeari commented Oct 10, 2016 • edited Loading

zeari commented Oct 10, 2016

lpichler commented Oct 10, 2016 • edited Loading

zeari commented Oct 10, 2016

miq-bot commented Oct 10, 2016

lpichler commented Oct 13, 2016

zeari commented Oct 13, 2016 • edited Loading

zeari commented Oct 13, 2016

simon3z commented Oct 13, 2016

zeari commented Oct 13, 2016

simon3z commented Oct 13, 2016

zeari commented Oct 13, 2016 • edited Loading

gtanzillo left a comment

Choose a reason for hiding this comment

chessbyte commented Oct 14, 2016

chessbyte commented Nov 3, 2016

chessbyte commented Nov 3, 2016 • edited Loading

zeari commented Nov 7, 2016

simaishi commented Nov 14, 2016 • edited Loading

zeari commented Nov 15, 2016

gtanzillo commented Nov 15, 2016

simaishi commented Nov 15, 2016

lpichler commented Nov 15, 2016

simaishi commented Jan 12, 2017

zeari commented Oct 10, 2016 •

edited

Loading

lpichler commented Oct 10, 2016 •

edited

Loading

zeari commented Oct 13, 2016 •

edited

Loading

zeari commented Oct 13, 2016 •

edited

Loading

chessbyte commented Nov 3, 2016 •

edited

Loading

simaishi commented Nov 14, 2016 •

edited

Loading