refactored the code

README.md

@@ -150,3 +150,4 @@ docker compose run dark-blue rake test
 - [aws-sdk-s3](https://docs.aws.amazon.com/sdk-for-ruby/v3/api/Aws/S3.html)
 - [mlibrary/sftp](https://github.com/mlibrary/sftp)
 - [Sequel](http://sequel.jeremyevans.net/)
+- [Prometheus Client](https://github.com/prometheus/client_ruby/blob/main/README.md)

docker-compose.yml

@@ -45,10 +45,9 @@ services:
       - 9090:9090
     volumes:
       - ./prometheus.yml:/etc/prometheus/prometheus.yml
-
   pushgateway:
     image: prom/pushgateway
     ports:
-      - 9091:9091   
+      - 9091:9091  
 volumes:
   database:

example.env

@@ -65,4 +65,4 @@ APTRUST_REMOTE_SETTINGS_AWS_ACCESS_KEY_ID=
 APTRUST_REMOTE_SETTINGS_AWS_SECRET_ACCESS_KEY=
 
 # Metrics push gateway url 
-PROMETHEUS_PUSH_GATEWAY=
+PROMETHEUS_PUSH_GATEWAY=http://pushgateway:9091

lib/metrics_collector.rb → lib/metrics.rb

@@ -7,7 +7,7 @@
 require_relative "../services"
 require_relative "bag_status"
 
-module DarkBlueMetrics
+module Metrics
   class PushGatewayClientError < StandardError; end
 
   class Timer
@@ -54,10 +54,6 @@ def get_failure_count(events_by_time)
       failure_count.count
     end
 
-    def get_failed_bag_ids(events_by_time)
-      events_by_time.select { |e| e.status == BagStatus::FAILED }
-    end
-
     def set_success_count(events_by_time)
       dark_blue_success_count = registry.gauge(
         :dark_blue_success_count,
@@ -74,18 +70,6 @@ def set_failed_count(events_by_time)
       dark_blue_failed_count.set(get_failure_count(events_by_time))
     end
 
-    def set_failed_bag_id(events_by_time)
-      dark_blue_failed_bag_ids = registry.counter(
-        :dark_blue_failed_bag_ids,
-        docstring: "Failed bag transfer",
-        labels: [:failed_id]
-      )
-      get_failed_ids = get_failed_bag_ids(events_by_time)
-      get_failed_ids.each do |e|
-        dark_blue_failed_bag_ids.increment(labels: {failed_id: e.bag_identifier})
-      end
-    end
-
     def set_last_successful_run
       dark_blue_last_successful_run = registry.gauge(
         :dark_blue_last_successful_run,
@@ -124,7 +108,6 @@ def set_all_metrics
       latest_events = get_latest_bag_events_by_time
       set_success_count(latest_events)
       set_failed_count(latest_events)
-      set_failed_bag_id(latest_events)
       push_metrics
     end
   end

lib/status_event_repository.rb

@@ -38,6 +38,10 @@ def get_all_for_bag_identifier(identifier)
     def get_latest_event_for_bag(bag_identifier:)
       raise NotImplementedError
     end
+
+    def get_latest_event_for_bags(start_time:)
+      raise NotImplementedError
+    end
   end
 
   class StatusEventInMemoryRepository < StatusEventRepositoryBase
@@ -69,10 +73,6 @@ def create(bag_identifier:, status:, timestamp:, note: nil)
       @status_events << event
     end
 
-    def get_by_id(id)
-      @status_events.find { |e| e.id == id }
-    end
-
     def get_all
       @status_events
     end
@@ -89,11 +89,11 @@ def get_latest_event_for_bag(bag_identifier:)
     end
 
     def get_latest_event_for_bags(start_time:)
-      events_by_id = @status_events.group_by(&:bag_identifier)
-      events_by_id.map do |id, events_for_id|
-        event_by_time = events_for_id.select { |e| e.timestamp >= start_time }
-        event_by_time.max_by(&:timestamp) unless event_by_time.empty?
-      end.compact
+      @status_events.select { |e| e.timestamp >= start_time }
+        .group_by(&:bag_identifier)
+        .transform_values { |bag_identifier| bag_identifier.max_by(&:timestamp) }
+        .values
+        .compact
     end
   end
 
@@ -152,10 +152,10 @@ def get_all_for_bag_identifier(identifier)
 
     # https://sequel.jeremyevans.net/rdoc/classes/Sequel/SQL/Window.html
     def get_latest_event_for_bags(start_time:)
-      latest_events = base_query.where { timestamp >= start_time }
+      base_query.where { timestamp >= start_time }
         .select_append { row_number.function.over(partition: :bag_id, order: Sequel.desc(:timestamp)).as(:rn) }
         .from_self.where(rn: 1)
-      latest_events.all.map { |se| convert_to_struct(se) }
+        .all.map { |se| convert_to_struct(se) }
     end
 
     def get_latest_event_for_bag(bag_identifier:)

run_dark_blue.rb

@@ -10,7 +10,7 @@
 require_relative "lib/bag_validator"
 require_relative "lib/data_transfer"
 require_relative "lib/dispatcher"
-require_relative "lib/metrics_collector"
+require_relative "lib/metrics"
 require_relative "lib/remote_client"
 require_relative "lib/repository_package_repository"
 require_relative "lib/status_event_repository"
@@ -203,13 +203,13 @@ def self.parse(options)
 
 options = DarkBlueParser.parse ARGV
 
-start_time, end_time = DarkBlueMetrics::Timer.time_processing {
+start_time, end_time = Metrics::Timer.time_processing {
   if options.packages.length > 0
     dark_blue_job.redeliver_packages(options.packages)
   else
     dark_blue_job.process
   end
 }
-metrics = DarkBlueMetrics::MetricsProvider.new(start_time: start_time, end_time: end_time,
+metrics = Metrics::MetricsProvider.new(start_time: start_time, end_time: end_time,
   status_event_repo: S.status_event_repo, push_gateway_url: config.metrics.push_gateway_url)
 metrics.set_all_metrics

test/test_metrics_collector.rb → test/test_metrics.rb

@@ -1,15 +1,13 @@
 require "minitest/autorun"
-require "minitest/pride"
-require "minitest/mock"
 
 require_relative "setup_db"
 require_relative "../lib/bag_status"
 require_relative "../lib/bag_repository"
-require_relative "../lib/metrics_collector"
+require_relative "../lib/metrics"
 require_relative "../lib/repository_package_repository"
 require_relative "../lib/status_event_repository"
 
-class DarkBlueMetricTest < Minitest::Test
+class MetricsTest < Minitest::Test
   def setup
     @time_stamp = Time.utc(2024, 1, 4, 12, 0, 0, 0)
     @start_time = @time_stamp.to_i
@@ -20,7 +18,7 @@ def setup
     @registry_mock = Minitest::Mock.new
     @gauge_mock = Minitest::Mock.new
 
-    @metrics = DarkBlueMetrics::MetricsProvider.new(
+    @metrics = Metrics::MetricsProvider.new(
       start_time: @start_time,
       end_time: @end_time,
       status_event_repo: @status_event_repo,
@@ -180,27 +178,6 @@ def test_get_failure_count
     assert_equal 1, actual_result
   end
 
-  def test_get_failed_bag_ids
-    @bag_identifier_one = "repository.context-0001"
-    @bag_identifier_two = "repository.context-0002"
-    @deposited_at = Time.utc(2024, 3, 18)
-
-    @status_event_repo.create(
-      bag_identifier: @bag_identifier_one,
-      status: BagStatus::DEPOSITED,
-      timestamp: @deposited_at
-    )
-
-    @status_event_repo.create(
-      bag_identifier: @bag_identifier_two,
-      status: BagStatus::FAILED,
-      timestamp: @deposited_at
-    )
-    events_by_time = @metrics.get_latest_bag_events_by_time
-    actual_result = @metrics.get_failed_bag_ids(events_by_time)
-    assert_equal "repository.context-0002", actual_result[0].bag_identifier
-  end
-
   def test_get_latest_bag_events_by_time_empty_array
     actual_result = @metrics.get_latest_bag_events_by_time
     assert_equal [], actual_result
@@ -217,10 +194,4 @@ def test_get_failure_count_nil
     actual_result = @metrics.get_failure_count(events_by_time)
     assert_equal 0, actual_result
   end
-
-  def test_get_failed_bag_ids_nil
-    events_by_time = @metrics.get_latest_bag_events_by_time
-    actual_result = @metrics.get_failed_bag_ids(events_by_time)
-    assert_equal [], actual_result
-  end
 end

test/test_status_event_repository.rb

@@ -85,13 +85,13 @@ def test_get_latest_event_for_bag
     mixin_repo.create(status: BagStatus::COPYING, bag_identifier: bag_identifier_one, timestamp: start_time)
     mixin_repo.create(status: BagStatus::COPIED, bag_identifier: bag_identifier_one, timestamp: start_time + 30)
     mixin_repo.create(status: BagStatus::COPYING, bag_identifier: bag_identifier_one, timestamp: start_time + 60)
-    mixin_repo.create(status: BagStatus::DEPOSITED, bag_identifier: bag_identifier_one, timestamp: start_time + 90)
+    mixin_repo.create(status: BagStatus::COPIED, bag_identifier: bag_identifier_one, timestamp: start_time + 90)
     mixin_repo.create(status: BagStatus::COPYING, bag_identifier: bag_identifier_two, timestamp: start_time + 100)
     mixin_repo.create(status: BagStatus::COPIED, bag_identifier: bag_identifier_two, timestamp: start_time + 120)
     event = mixin_repo.get_latest_event_for_bag(bag_identifier: bag_identifier_one)
     assert event.is_a?(StatusEventRepository::StatusEvent)
     assert_equal bag_identifier_one, event.bag_identifier
-    assert_equal BagStatus::DEPOSITED, event.status
+    assert_equal BagStatus::COPIED, event.status
     assert_equal start_time + 90, event.timestamp
   end
 
@@ -100,12 +100,9 @@ def test_get_latest_event_for_bag_when_nil
     refute event
   end
 
-  def test_get_latest_no_event_for_bags
+  def test_get_latest_event_for_bags_when_no_events
     start_time = Time.utc(2024, 5, 4, 12, 0, 0, 0)
     bag_events = mixin_repo.get_latest_event_for_bags(start_time: start_time)
-    bag_events.each do |bag_event|
-      assert bag_event.is_a?(StatusEventRepository::StatusEvent)
-    end
     assert_equal 0, bag_events.length
   end