From 309abb695435aa9cd2fab4a7c33c5edf44f9c292 Mon Sep 17 00:00:00 2001 From: Jayamala Date: Fri, 17 May 2024 13:55:07 -0400 Subject: [PATCH] refactored the code --- README.md | 1 + docker-compose.yml | 3 +- example.env | 2 +- lib/{metrics_collector.rb => metrics.rb} | 19 +--------- lib/status_event_repository.rb | 22 ++++++------ run_dark_blue.rb | 6 ++-- ...t_metrics_collector.rb => test_metrics.rb} | 35 ++----------------- test/test_status_event_repository.rb | 9 ++--- 8 files changed, 24 insertions(+), 73 deletions(-) rename lib/{metrics_collector.rb => metrics.rb} (84%) rename test/{test_metrics_collector.rb => test_metrics.rb} (84%) diff --git a/README.md b/README.md index 3bc6ef4..798e455 100644 --- a/README.md +++ b/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) diff --git a/docker-compose.yml b/docker-compose.yml index 8880814..090cfea 100644 --- a/docker-compose.yml +++ b/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: diff --git a/example.env b/example.env index 733d0e8..fe4f891 100644 --- a/example.env +++ b/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 diff --git a/lib/metrics_collector.rb b/lib/metrics.rb similarity index 84% rename from lib/metrics_collector.rb rename to lib/metrics.rb index a21c772..73cdd0d 100644 --- a/lib/metrics_collector.rb +++ b/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 diff --git a/lib/status_event_repository.rb b/lib/status_event_repository.rb index 9030415..b41e202 100644 --- a/lib/status_event_repository.rb +++ b/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:) diff --git a/run_dark_blue.rb b/run_dark_blue.rb index 98fadd9..59c7ecc 100644 --- a/run_dark_blue.rb +++ b/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 diff --git a/test/test_metrics_collector.rb b/test/test_metrics.rb similarity index 84% rename from test/test_metrics_collector.rb rename to test/test_metrics.rb index dac778d..224d679 100644 --- a/test/test_metrics_collector.rb +++ b/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 diff --git a/test/test_status_event_repository.rb b/test/test_status_event_repository.rb index 1cefdbd..e6c8dbb 100644 --- a/test/test_status_event_repository.rb +++ b/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