diff --git a/app/services/reporting_events/backfill_service.rb b/app/services/reporting_events/backfill_service.rb index b47ebc9f7..2db0a527a 100644 --- a/app/services/reporting_events/backfill_service.rb +++ b/app/services/reporting_events/backfill_service.rb @@ -49,7 +49,7 @@ class ReportingEvents::BackfillService return [] if events.empty? # Build in-memory aggregates by dimension - aggregates = build_aggregates(events, event_name) + aggregates = build_aggregates(events) # Convert to rollup row hashes aggregates.map do |(dimension_type, dimension_id, metric), data| @@ -68,24 +68,10 @@ class ReportingEvents::BackfillService end end - def build_aggregates(events, event_name) + def build_aggregates(events) aggregates = Hash.new { |h, k| h[k] = { count: 0, sum_value: 0.0, sum_value_business_hours: 0.0 } } - events.each do |event| - metrics = metrics_for_event(event_name, event) - dimensions = dimensions_for_event(event) - - metrics.each do |metric, metric_data| - dimensions.each do |dimension_type, dimension_id| - next if dimension_id.nil? - - key = [dimension_type, dimension_id, metric] - aggregates[key][:count] += metric_data[:count] - aggregates[key][:sum_value] += metric_data[:sum_value].to_f - aggregates[key][:sum_value_business_hours] += metric_data[:sum_value_business_hours].to_f - end - end - end + events.each { |event| accumulate_event_aggregates(aggregates, event) } aggregates end @@ -98,23 +84,22 @@ class ReportingEvents::BackfillService } end - def metrics_for_event(event_name, event) - case event_name - when 'conversation_resolved' - { - 'resolutions_count' => { count: 1, sum_value: 0, sum_value_business_hours: 0 }, - 'resolution_time' => { count: 1, sum_value: event.value, sum_value_business_hours: event.value_in_business_hours } - } - when 'first_response' - { 'first_response' => { count: 1, sum_value: event.value, sum_value_business_hours: event.value_in_business_hours } } - when 'reply_time' - { 'reply_time' => { count: 1, sum_value: event.value, sum_value_business_hours: event.value_in_business_hours } } - when 'conversation_bot_resolved' - { 'bot_resolutions_count' => { count: 1, sum_value: 0, sum_value_business_hours: 0 } } - when 'conversation_bot_handoff' - { 'bot_handoffs_count' => { count: 1, sum_value: 0, sum_value_business_hours: 0 } } - else - {} + def accumulate_event_aggregates(aggregates, event) + dimensions = dimensions_for_event(event) + + ReportingEvents::MetricRegistry.event_metrics_for(event).each do |metric, metric_data| + accumulate_metric_aggregates(aggregates, dimensions, metric, metric_data) + end + end + + def accumulate_metric_aggregates(aggregates, dimensions, metric, metric_data) + dimensions.each do |dimension_type, dimension_id| + next if dimension_id.nil? + + key = [dimension_type, dimension_id, metric] + aggregates[key][:count] += metric_data[:count] + aggregates[key][:sum_value] += metric_data[:sum_value].to_f + aggregates[key][:sum_value_business_hours] += metric_data[:sum_value_business_hours].to_f end end diff --git a/app/services/reporting_events/metric_registry.rb b/app/services/reporting_events/metric_registry.rb new file mode 100644 index 000000000..791bdf700 --- /dev/null +++ b/app/services/reporting_events/metric_registry.rb @@ -0,0 +1,88 @@ +module ReportingEvents::MetricRegistry + EVENT_METRICS = { + 'conversation_resolved' => lambda do |event| + { + resolutions_count: count_metric, + resolution_time: duration_metric(event) + } + end, + 'first_response' => ->(event) { { first_response: duration_metric(event) } }, + 'reply_time' => ->(event) { { reply_time: duration_metric(event) } }, + 'conversation_bot_resolved' => ->(_event) { { bot_resolutions_count: count_metric } }, + 'conversation_bot_handoff' => ->(_event) { { bot_handoffs_count: count_metric } } + }.freeze + + REPORT_METRICS = { + avg_first_response_time: { + raw_event_name: :first_response, + rollup_metric: :first_response, + aggregate: :average + }.freeze, + avg_resolution_time: { + raw_event_name: :conversation_resolved, + rollup_metric: :resolution_time, + aggregate: :average + }.freeze, + reply_time: { + raw_event_name: :reply_time, + rollup_metric: :reply_time, + aggregate: :average + }.freeze, + resolutions_count: { + raw_event_name: :conversation_resolved, + rollup_metric: :resolutions_count, + aggregate: :count + }.freeze, + bot_resolutions_count: { + raw_event_name: :conversation_bot_resolved, + rollup_metric: :bot_resolutions_count, + aggregate: :count + }.freeze, + bot_handoffs_count: { + raw_event_name: :conversation_bot_handoff, + rollup_metric: :bot_handoffs_count, + aggregate: :count, + raw_count_strategy: :distinct_conversation + }.freeze + }.freeze + + module_function + + def event_metrics_for(event) + return {} if event.blank? + + EVENT_METRICS[event.name.to_s]&.call(event) || {} + end + + def report_metric(metric) + return if metric.blank? + + REPORT_METRICS[metric.to_sym] + end + + def rollup_supported_metric?(metric) + rollup_metric_for(metric).present? + end + + def rollup_metric_for(metric) + report_metric(metric)&.dig(:rollup_metric) + end + + def raw_event_name_for(metric) + report_metric(metric)&.dig(:raw_event_name) + end + + def count_metric + { count: 1, sum_value: 0, sum_value_business_hours: 0 } + end + private_class_method :count_metric + + def duration_metric(event) + { + count: 1, + sum_value: event.value.to_f, + sum_value_business_hours: event.value_in_business_hours.to_f + } + end + private_class_method :duration_metric +end diff --git a/app/services/reporting_events/rollup_service.rb b/app/services/reporting_events/rollup_service.rb index 6bccdd709..9f4b73176 100644 --- a/app/services/reporting_events/rollup_service.rb +++ b/app/services/reporting_events/rollup_service.rb @@ -43,76 +43,18 @@ module ReportingEvents::RollupService } end - def metrics_for_event - case @reporting_event.name - when 'conversation_resolved' - conversation_resolved_metrics - when 'first_response' - first_response_metrics - when 'reply_time' - reply_time_metrics - when 'conversation_bot_resolved' - bot_resolutions_metrics - when 'conversation_bot_handoff' - bot_handoffs_metrics - else - {} - end - end - def build_rollup_rows + event_metrics = ReportingEvents::MetricRegistry.event_metrics_for(@reporting_event) + dimensions.each_with_object([]) do |(dimension_type, dimension_id), rows| next if dimension_id.nil? - metrics_for_event.each do |metric, metric_data| + event_metrics.each do |metric, metric_data| rows << rollup_attributes(dimension_type, dimension_id, metric, metric_data) end end end - def conversation_resolved_metrics - { - resolutions_count: { count: 1, sum_value: 0, sum_value_business_hours: 0 }, - resolution_time: { - count: 1, - sum_value: @reporting_event.value, - sum_value_business_hours: @reporting_event.value_in_business_hours - } - } - end - - def first_response_metrics - { - first_response: { - count: 1, - sum_value: @reporting_event.value, - sum_value_business_hours: @reporting_event.value_in_business_hours - } - } - end - - def reply_time_metrics - { - reply_time: { - count: 1, - sum_value: @reporting_event.value, - sum_value_business_hours: @reporting_event.value_in_business_hours - } - } - end - - def bot_resolutions_metrics - { - bot_resolutions_count: { count: 1, sum_value: 0, sum_value_business_hours: 0 } - } - end - - def bot_handoffs_metrics - { - bot_handoffs_count: { count: 1, sum_value: 0, sum_value_business_hours: 0 } - } - end - def upsert_rollups(rows) # rubocop:disable Rails/SkipsModelValidations ReportingEventsRollup.upsert_all( diff --git a/spec/services/reporting_events/metric_registry_spec.rb b/spec/services/reporting_events/metric_registry_spec.rb new file mode 100644 index 000000000..9014be0d1 --- /dev/null +++ b/spec/services/reporting_events/metric_registry_spec.rb @@ -0,0 +1,121 @@ +require 'rails_helper' + +RSpec.describe ReportingEvents::MetricRegistry do + describe '.event_metrics_for' do + it 'returns the emitted rollup metrics for conversation_resolved' do + event = instance_double(ReportingEvent, name: 'conversation_resolved', value: 120, value_in_business_hours: 45) + + expect(described_class.event_metrics_for(event)).to eq( + resolutions_count: { + count: 1, + sum_value: 0, + sum_value_business_hours: 0 + }, + resolution_time: { + count: 1, + sum_value: 120.0, + sum_value_business_hours: 45.0 + } + ) + end + + it 'returns the emitted rollup metrics for first_response' do + event = instance_double(ReportingEvent, name: 'first_response', value: 80, value_in_business_hours: 20) + + expect(described_class.event_metrics_for(event)).to eq( + first_response: { + count: 1, + sum_value: 80.0, + sum_value_business_hours: 20.0 + } + ) + end + + it 'returns the emitted rollup metrics for reply_time' do + event = instance_double(ReportingEvent, name: 'reply_time', value: 40, value_in_business_hours: 15) + + expect(described_class.event_metrics_for(event)).to eq( + reply_time: { + count: 1, + sum_value: 40.0, + sum_value_business_hours: 15.0 + } + ) + end + + it 'returns the emitted rollup metrics for conversation_bot_resolved' do + event = instance_double(ReportingEvent, name: 'conversation_bot_resolved') + + expect(described_class.event_metrics_for(event)).to eq( + bot_resolutions_count: { + count: 1, + sum_value: 0, + sum_value_business_hours: 0 + } + ) + end + + it 'returns the emitted rollup metrics for conversation_bot_handoff' do + event = instance_double(ReportingEvent, name: 'conversation_bot_handoff') + + expect(described_class.event_metrics_for(event)).to eq( + bot_handoffs_count: { + count: 1, + sum_value: 0, + sum_value_business_hours: 0 + } + ) + end + + it 'returns an empty hash for unsupported events' do + event = instance_double(ReportingEvent, name: 'conversation_created') + + expect(described_class.event_metrics_for(event)).to eq({}) + end + end + + describe '.report_metric' do + it 'returns the definition for avg_resolution_time' do + expect(described_class.report_metric(:avg_resolution_time)).to eq( + raw_event_name: :conversation_resolved, + rollup_metric: :resolution_time, + aggregate: :average + ) + end + + it 'locks the distinct conversation strategy for bot_handoffs_count' do + expect(described_class.report_metric(:bot_handoffs_count)).to eq( + raw_event_name: :conversation_bot_handoff, + rollup_metric: :bot_handoffs_count, + aggregate: :count, + raw_count_strategy: :distinct_conversation + ) + end + + it 'returns nil for unsupported metrics' do + expect(described_class.report_metric(:conversations_count)).to be_nil + end + end + + describe '.rollup_supported_metric?' do + it 'returns true for rollup-backed metrics' do + expect(described_class.rollup_supported_metric?(:reply_time)).to be(true) + end + + it 'returns false for unsupported metrics' do + expect(described_class.rollup_supported_metric?(:conversations_count)).to be(false) + end + end + + describe '.rollup_metric_for' do + it 'returns the rollup metric name' do + expect(described_class.rollup_metric_for(:avg_first_response_time)).to eq(:first_response) + end + end + + describe '.raw_event_name_for' do + it 'returns the raw event name' do + expect(described_class.raw_event_name_for(:bot_resolutions_count)).to eq(:conversation_bot_resolved) + end + end +end