remove LiveMetric hardcoded column includes

[kbrock]

reports include virtual columns in includes
this is used by Hawkluar LiveMetrics to send the column list to the
server.

Changed LiveMetric virtual column detection to auto include these
columns without hardcoding the class name

tangentially related to https://www.pivotaltracker.com/story/show/137377249

---

Hola @lucasponce - We introduced this change somewhere around #10570 and I'm pretty sure this follows through in your goals.

I am aiming to reduce a bunch of this reporting code, possibly removing report includes field or finding a different way of adding these includes().

I don't want to break Hawkular

Is there an integration test we can introduce that will define a report and ensure that the correct columns get into Hawkular API (either via includes or select mechanism)?

[lucasponce]

I think this one won't work well, as it is assuming that all metrics should start with the "mw_" prefix (which might not be true).
Basically LiveMetric object has a fixed field

set_columns_hash(:timestamp => :datetime) 

And rest of the fields are prepared on:

  def self.filter_and_fetch_metrics(resource, filter, start_time, end_time, interval_name)
    filtered = resource.metrics_available.select { |metric| filter.nil? || filter.include?(metric[:name]) }
    filtered.each { |metric| set_columns_hash(metric[:name] => :float) }
    fetch_live_metrics(resource, filtered, start_time, end_time, interval_name)
  end

Which is basically doing is to add the fields that are previously parsed from a config file.

One question that perhaps might simplify, in case of LiveMetric objects, all fields are virtual, so why don't return a true ?

A complete end-to-end flow test with reports is not available, but there are some tests under

spec/models/manageiq/providers/hawkular/middleware_manager/middleware_*_spec.rb

These ones invoke LiveMetric but not from the generator.rb, sorry.

If there is an example of integration test I can take a look and put it on my todo list.

[kbrock]

Thanks @lucasponce
I had seen that regular expression in a few places, so thought that was a good test.
I'll just return true there.

hmm, :timestamp is defined, so it will work.

Guess options are to

1. just return true
2. use logic from filter_and_fetch_metrics

Curious if it makes sense to define all fields in LiveMetric. Probably not.
Not sure if we have proper discovery here

[lucasponce]

I think return true could be just enough.
Basically, on LiveMetric.find(), this is resolved, so option 2 would need to store additional state at class level.
So, at this use case has some boundaries I do not see any risk in additional use cases.
LiveMetric is mainly used for miq_reports, so if there is some wrong column, I think there will be an error on log which should be expected.

[miq-bot]

Checked commit kbrock@ff66278 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
2 files checked, 0 offenses detected
Everything looks good. ⭐

[gtanzillo]

This makes sense to me 👍

@lucasponce are you ok with me merging?

[lucasponce]

@gtanzillo sure, the changes look good to me.
Thanks @kbrock

[isimluk]

👍

[isimluk]

LGTM!!! 👍 💯 🔢 🥇 🥈 🥉