From 5e54ac28a0a1632fd54e8175e048cfe1c4d2b68f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tadej=20Borov=C5=A1ak?= Date: Fri, 17 Aug 2018 09:49:27 +0200 Subject: [PATCH 1/2] Refactor event type SQL filter construction We construct filter in two steps: 1. we collect the conditions and 2. join them using and operator. This reduces the amount of string manipulation and conditional statements considerably. --- .../application_controller/timelines.rb | 54 ++++++++----------- .../timelines/options.rb | 12 +++-- 2 files changed, 31 insertions(+), 35 deletions(-) diff --git a/app/controllers/application_controller/timelines.rb b/app/controllers/application_controller/timelines.rb index 3acc7e70eec..17a622cbbd2 100644 --- a/app/controllers/application_controller/timelines.rb +++ b/app/controllers/application_controller/timelines.rb @@ -101,43 +101,35 @@ def tl_build_timeline_report_options to_dt = create_time_in_utc("#{yy}-#{mm}-#{dd} 23:59:59", session[:user_tz]) # Get tz 11pm in user's time zone end - temp_clause = @tl_record.event_where_clause(@tl_options.evt_type) - - cond = "( " - cond = cond << temp_clause[0] - params = temp_clause.slice(1, temp_clause.length) - - event_set = @tl_options.event_set - if !event_set.empty? - if @tl_options.policy_events? && @tl_options.policy.result != "both" - where_clause = [") and (timestamp >= ? and timestamp <= ?) and (event_type in (?)) and (result = ?)", - from_dt, - to_dt, - event_set.flatten, - @tl_options.policy.result] - else - where_clause = [") and (timestamp >= ? and timestamp <= ?) and (event_type in (?))", - from_dt, - to_dt, - event_set.flatten] - end - else - where_clause = [") and (timestamp >= ? and timestamp <= ?)", - from_dt, - to_dt] - end - cond << where_clause[0] + rec_cond, *rec_params = @tl_record.event_where_clause(@tl_options.evt_type) + conditions = [rec_cond, "timestamp >= ?", "timestamp <= ?"] + parameters = rec_params + [from_dt, to_dt] + + tl_add_event_type_conditions(conditions, parameters) + tl_add_policy_conditions(conditions, parameters) if @tl_options.policy_events? - params2 = where_clause.slice(1, where_clause.length - 1) - params = params.concat(params2) - @report.where_clause = [cond, *params] + condition = conditions.join(") and (") + @report.where_clause = ["(#{condition})"] + parameters @report.rpt_options ||= {} - @report.rpt_options[:categories] = - @tl_options.management_events? ? @tl_options.management.categories : @tl_options.policy.categories + @report.rpt_options[:categories] = @tl_options.categories @title = @report.title end end + def tl_add_event_type_conditions(conditions, parameters) + unless @tl_options.event_set.empty? + conditions << "event_type in (?)" + parameters << @tl_options.event_set.flatten + end + end + + def tl_add_policy_conditions(conditions, parameters) + if @tl_options.policy.result != "both" + conditions << "result = ?" + parameters << @tl_options.policy.result + end + end + def tl_build_timeline(refresh = nil) tl_build_init_options(refresh) # Intialize options(refresh) if !@report @ajax_action = "tl_chooser" diff --git a/app/controllers/application_controller/timelines/options.rb b/app/controllers/application_controller/timelines/options.rb index b8efa97166b..a034d8bb95a 100644 --- a/app/controllers/application_controller/timelines/options.rb +++ b/app/controllers/application_controller/timelines/options.rb @@ -68,16 +68,16 @@ def drop_cache @events = @event_groups = @lvl_text_value = nil end - def event_groups - @event_groups ||= EmsEvent.event_groups - end - def levels_text_and_value @lvl_text_value ||= group_levels.map { |level| [level.to_s.titleize, level] } end private + def event_groups + @event_groups ||= EmsEvent.event_groups + end + def group_levels EmsEvent::GROUP_LEVELS end @@ -143,6 +143,10 @@ def event_set (policy_events? ? policy : management).event_set end + def categories + (policy_events? ? policy : management).categories + end + def drop_cache [policy, management].each(&:drop_cache) end From d96114187e16d7b60a959061f0ba5306561aab97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tadej=20Borov=C5=A1ak?= Date: Fri, 27 Jul 2018 18:31:45 +0200 Subject: [PATCH 2/2] Allow registering events to timelines using regexes Up until now, in order to register event type to the timeline, we needed to list all of the event types in settings yaml. This commit adds another option: specify event type to group/level mapping using regular expressions, which should make mapping definitions much shorter for providers with many different event types that follow a certain pattern. Changes were made in to areas: in database querying and in event to category assignment. When creating database condition for events, we now use three sources: 1. a set of event types that should be included in result, 2. a set of event types that should not be present in result and 3. a set of regular expressions that event types can match. An event is part of the result if its type is in include set or matches any of the regular expressions and is not part of the exclude set. For example, take this fragment of settings file: :ems: :ems_dummy: :event_handling: :event_groups: :addition: :critical: - !ruby/regexp /^dummy_add_.+$/ :update: :critical: - dummy_123_update - !ruby/regexp /^dummy_.+_update$/i :detail: - dummy_abc_update - dummy_def_update - !ruby/regexp /^dummy_detail_.+_update$/ This translates into following three sources for :update category, assuming user selected :critical level in UI: * include_set: dummy_123_update * exclude_set: dummy_abc_update, dummy_def_update * regexes: ^dummy_.+_updates$ In this case, events of type dummy_abc_update will not be displayed, since they match details and are thus put in exclude set. On the other hand, if the user selected :critical and :detail levels, things are a bit different: * include_set: dummy_123_update, dummy_abc_update, dummy_def_update * exclude_set: * regexes: ^dummy_.+_updates$, ^dummy_detail_.+_update$ Regular expressions should be accepted by the PostreSQL DBMS and should not contain Ruby-specific functionality. The only exception to this rule is case sensitivity, which can make regular expressions a lot simpler in certain situations. Category assignment is done in two phases. First, we try to assign category only looking at the full name of event. If this yields no category, we try to assign one by matching against regular expressions. If we continue with categories from example above, we would get those mappings: * dummy_123_update is mapped to :update. It also matches :addition category, but since explicit names have higher priority, that regular expression is not checked at all. * dummy_nil_update is mapped to :update because it matches one of the :critical expressions. * dummy_add_event is mapped to :addition. All this is done in order to give fully spelled-out event type mappings higher priority than regular expression ones and to be consistent with how EventStream classifies events into groups and levels. --- .../application_controller/timelines.rb | 33 ++++++- .../timelines/options.rb | 60 ++++++------ lib/report_formatter/timeline.rb | 23 +++-- .../application_controller/timelines_spec.rb | 12 +-- spec/lib/report_formater/timeline_spec.rb | 93 ++++++++++++++++++- 5 files changed, 172 insertions(+), 49 deletions(-) diff --git a/app/controllers/application_controller/timelines.rb b/app/controllers/application_controller/timelines.rb index 17a622cbbd2..ff8e66fac13 100644 --- a/app/controllers/application_controller/timelines.rb +++ b/app/controllers/application_controller/timelines.rb @@ -117,9 +117,36 @@ def tl_build_timeline_report_options end def tl_add_event_type_conditions(conditions, parameters) - unless @tl_options.event_set.empty? - conditions << "event_type in (?)" - parameters << @tl_options.event_set.flatten + tl_add_event_type_inclusions(conditions, parameters) + tl_add_event_type_exclusions(conditions, parameters) + end + + def tl_add_event_type_inclusions(conditions, parameters) + expressions = [] + @tl_options.get_set(:regexes).each do |regex| + expressions << tl_get_regex_sql_expression(regex) + parameters << regex.source + end + + includes = @tl_options.get_set(:include_set) + unless includes.empty? + expressions << "event_type in (?)" + parameters << includes + end + + condition = expressions.join(") or (") + conditions << "(#{condition})" unless condition.empty? + end + + def tl_get_regex_sql_expression(regex) + regex.casefold? ? "event_type ~* ?" : "event_type ~ ?" + end + + def tl_add_event_type_exclusions(conditions, parameters) + excludes = @tl_options.get_set(:exclude_set) + unless excludes.empty? + conditions << "event_type not in (?)" + parameters << excludes end end diff --git a/app/controllers/application_controller/timelines/options.rb b/app/controllers/application_controller/timelines/options.rb index a034d8bb95a..44380f1716d 100644 --- a/app/controllers/application_controller/timelines/options.rb +++ b/app/controllers/application_controller/timelines/options.rb @@ -36,16 +36,28 @@ def update_start_end(sdate, edate) def update_from_params(params) self.levels = params[:tl_levels]&.map(&:to_sym) || group_levels self.categories = {} - if params[:tl_categories] - params[:tl_categories].each do |category| - event_group = event_groups[events[category]].clone - next unless event_group.keys.include_any?(levels) - categories[events[category]] = {:display_name => category} - unless levels.include_all?(event_group.keys) - event_group.delete_if { |key, _v| !levels.include?(key) } + params.fetch(:tl_categories, []).each do |category_display_name| + group_data = event_groups[events[category_display_name]] + category = { + :display_name => category_display_name, + :include_set => [], + :exclude_set => [], + :regexes => [] + } + + group_levels.each do |lvl| + next unless group_data[lvl] + strings, regexes = group_data[lvl].partition { |typ| typ.kind_of?(String) } + if levels.include?(lvl) + category[:include_set].push(*strings) + category[:regexes].push(*regexes) + else + category[:exclude_set].push(*strings) end - categories[events[category]][:event_groups] = event_group.values.flatten end + next if category[:include_set].empty? && category[:regexes].empty? + + categories[events[category_display_name]] = category end end @@ -56,14 +68,6 @@ def events end end - def event_set - event_set = [] - categories.each do |category| - event_set.push(category.last[:event_groups]) - end - event_set - end - def drop_cache @events = @event_groups = @lvl_text_value = nil end @@ -90,10 +94,13 @@ def group_levels def update_from_params(params) self.result = params[:tl_result] || "success" self.categories = {} - if params[:tl_categories] - params[:tl_categories].each do |category| - categories[category] = {:display_name => category, :event_groups => events[category]} - end + params.fetch(:tl_categories, []).each do |category| + categories[category] = { + :display_name => category, + :include_set => events[category], + :exclude_set => [], + :regexes => [] + } end end @@ -106,11 +113,6 @@ def events def drop_cache @events = @fltr_cache = nil end - - def event_set - categories.blank? ? [] : categories.collect { |_, e| e[:event_groups] } - end - end Options = Struct.new( @@ -139,14 +141,14 @@ def evt_type management_events? ? :event_streams : :policy_events end - def event_set - (policy_events? ? policy : management).event_set - end - def categories (policy_events? ? policy : management).categories end + def get_set(name) + categories.values.flat_map { |v| v[name] } + end + def drop_cache [policy, management].each(&:drop_cache) end diff --git a/lib/report_formatter/timeline.rb b/lib/report_formatter/timeline.rb index 3aff34f525b..16482677ec7 100644 --- a/lib/report_formatter/timeline.rb +++ b/lib/report_formatter/timeline.rb @@ -34,14 +34,23 @@ def build_document_body # some of the OOTB reports have db as EventStream or PolicyEvent, # those do not have event categories, so need to go thru else block for such reports. if (mri.db == "EventStream" || mri.db == "PolicyEvent") && mri.rpt_options.try(:[], :categories) - mri.rpt_options[:categories].each do |_, options| + event_map = mri.table.data.each_with_object({}) do |event, buckets| + bucket_name = mri.rpt_options[:categories].detect do |_, options| + options[:include_set].include?(event.event_type) + end&.last.try(:[], :display_name) + + bucket_name ||= mri.rpt_options[:categories].detect do |_, options| + options[:regexes].any? { |regex| regex.match(event.event_type) } + end.last[:display_name] + + buckets[bucket_name] ||= [] + buckets[bucket_name] << event + end + + event_map.each do |name, events| @events_data = [] - all_events = mri.table.data.select { |e| options[:event_groups].include?(e.event_type) } - all_events.each do |row| - tl_event(row, col) # Add this row to the tl event xml - end - @events.push(:name => options[:display_name], - :data => [@events_data]) + events.each { |row| tl_event(row, col) } + @events.push(:name => name, :data => [@events_data]) end else mri.table.data.each_with_index do |row, _d_idx| diff --git a/spec/controllers/application_controller/timelines_spec.rb b/spec/controllers/application_controller/timelines_spec.rb index d3f18bbb8db..27e5c25cb17 100644 --- a/spec/controllers/application_controller/timelines_spec.rb +++ b/spec/controllers/application_controller/timelines_spec.rb @@ -47,8 +47,8 @@ expect(controller).to receive(:render) controller.send(:tl_chooser) options = assigns(:tl_options) - expect(options.management[:categories][:power][:event_groups]).to include('AUTO_FAILED_SUSPEND_VM') - expect(options.management[:categories][:power][:event_groups]).to_not include('PowerOffVM_Task') + expect(options.management[:categories][:power][:include_set]).to include('AUTO_FAILED_SUSPEND_VM') + expect(options.management[:categories][:power][:include_set]).to_not include('PowerOffVM_Task') end it "selecting details option of the selectpicker in the timeline should append them to events filter list" do @@ -60,8 +60,8 @@ expect(controller).to receive(:render) controller.send(:tl_chooser) options = assigns(:tl_options) - expect(options.management[:categories][:power][:event_groups]).to include('PowerOffVM_Task') - expect(options.management[:categories][:power][:event_groups]).to_not include('AUTO_FAILED_SUSPEND_VM') + expect(options.management[:categories][:power][:include_set]).to_not include('AUTO_FAILED_SUSPEND_VM') + expect(options.management[:categories][:power][:include_set]).to include('PowerOffVM_Task') end it "selecting two options of the selectpicker in the timeline should append both to events filter list" do @@ -73,8 +73,8 @@ expect(controller).to receive(:render) controller.send(:tl_chooser) options = assigns(:tl_options) - expect(options.management[:categories][:power][:event_groups]).to include('AUTO_FAILED_SUSPEND_VM') - expect(options.management[:categories][:power][:event_groups]).to include('PowerOffVM_Task') + expect(options.management[:categories][:power][:include_set]).to include('AUTO_FAILED_SUSPEND_VM') + expect(options.management[:categories][:power][:include_set]).to include('PowerOffVM_Task') end end end diff --git a/spec/lib/report_formater/timeline_spec.rb b/spec/lib/report_formater/timeline_spec.rb index d8fa5dcb9a9..dc94566acd8 100644 --- a/spec/lib/report_formater/timeline_spec.rb +++ b/spec/lib/report_formater/timeline_spec.rb @@ -126,9 +126,11 @@ def stub_ems_event(event_type) :headers => %w(id name event_type timestamp), :timeline => {:field => "EmsEvent-timestamp", :position => "Last"}) @report.rpt_options = {:categories => {:power => {:display_name => "Power Activity", - :event_groups => %w(VmPoweredOffEvent VmPoweredOnEvent)}, + :include_set => %w(VmPoweredOffEvent VmPoweredOnEvent), + :regexes => []}, :snapshot => {:display_name => "Snapshot Activity", - :event_groups => %w(AlarmCreatedEvent AlarmRemovedEvent)}}} + :include_set => %w(AlarmCreatedEvent AlarmRemovedEvent), + :regexes => []}}} data = [] 30.times do @@ -172,6 +174,87 @@ def stub_ems_event(event_type) expect(JSON.parse(events)[0]["data"][0].length).to eq(45) end end + + describe '#events count for regex categories' do + def stub_ems_event(event_type) + ems = FactoryGirl.create(:ems_redhat) + EventStream.create!(:event_type => event_type, :ems_id => ems.id) + end + + before do + @report = FactoryGirl.create( + :miq_report, + :db => "EventStream", + :col_order => %w(id name event_type timestamp), + :headers => %w(id name event_type timestamp), + :timeline => {:field => "EmsEvent-timestamp", :position => "Last"} + ) + @report.rpt_options = { + :categories => { + :power => { + :display_name => "Power Activity", + :include_set => [], + :regexes => [/Event$/] + }, + :snapshot => { + :display_name => "Snapshot Activity", + :include_set => %w(AlarmCreatedEvent AlarmRemovedEvent), + :regexes => [] + } + } + } + + data = [] + (1..5).each do |n| + event_type = "VmPower#{n}Event" + data.push( + Ruport::Data::Record.new( + "id" => stub_ems_event(event_type).id, + "name" => "Baz", + "event_type" => event_type, + "timestamp" => Time.zone.now + ) + ) + end + + 7.times do + data.push( + Ruport::Data::Record.new( + "id" => stub_ems_event("AlarmRemovedEvent").id, + "name" => "Baz", + "event_type" => "AlarmRemovedEvent", + "timestamp" => Time.zone.now + ) + ) + end + + @report.table = Ruport::Data::Table.new( + :column_names => %w(id name event_type timestamp), + :data => data + ) + end + + it 'shows correct count of timeline events based on categories' do + allow_any_instance_of(Ruport::Controller::Options).to receive(:mri).and_return(@report) + events = ReportFormatter::ReportTimeline.new.build_document_body + expect(JSON.parse(events)[0]["data"][0].length).to eq(5) + expect(JSON.parse(events)[1]["data"][0].length).to eq(7) + end + + it 'shows correct count of timeline events together for report object with no categories' do + @report.rpt_options = {} + allow_any_instance_of(Ruport::Controller::Options).to receive(:mri).and_return(@report) + events = ReportFormatter::ReportTimeline.new.build_document_body + expect(JSON.parse(events)[0]["data"][0].length).to eq(12) + end + + it 'shows correct count of timeline events for timeline based report when rpt_options is nil' do + @report.rpt_options = nil + allow_any_instance_of(Ruport::Controller::Options).to receive(:mri).and_return(@report) + events = ReportFormatter::ReportTimeline.new.build_document_body + expect(JSON.parse(events)[0]["data"][0].length).to eq(12) + end + end end describe '#set data for headers that exist in col headers' do @@ -188,9 +271,11 @@ def stub_ems_event(event_type) :headers => %w(id name event_type timestamp vm_location), :timeline => {:field => "EmsEvent-timestamp", :position => "Last"}) @report.rpt_options = {:categories => {:power => {:display_name => "Power Activity", - :event_groups => %w(VmPoweredOffEvent VmPoweredOnEvent)}, + :include_set => %w(VmPoweredOffEvent VmPoweredOnEvent), + :regexes => []}, :snapshot => {:display_name => "Snapshot Activity", - :event_groups => %w(AlarmCreatedEvent AlarmRemovedEvent)}}} + :include_set => %w(AlarmCreatedEvent AlarmRemovedEvent), + :regexes => []}}} data = [Ruport::Data::Record.new("id" => stub_ems_event("VmPoweredOffEvent").id, "name" => "Baz",