From f075de5ea38786bfae7f4be4d6e1fa3b002b7260 Mon Sep 17 00:00:00 2001 From: Robert Laurin <robert.laurin@shopify.com> Date: Fri, 25 Feb 2022 14:56:51 -0500 Subject: [PATCH] fix: ruby style --- .../sdk/metrics/meter_provider.rb | 35 +++++++++++------- .../sdk/metrics/meter_provider_test.rb | 37 ++++++++++--------- 2 files changed, 41 insertions(+), 31 deletions(-) diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb index 374559ea6..ec46352c3 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb @@ -42,7 +42,6 @@ def meter(name, version = nil) # Attempts to stop all the activity for this {MeterProvider}. # # Calls MetricReader#shutdown for all registered MetricReaders. - # Calls MetricExporter#shutdown for all registered MetricExporters. # # After this is called all the newly created {Meter}s will be no-op. # @@ -56,11 +55,13 @@ def shutdown(timeout: nil) Export::FAILURE else start_time = OpenTelemetry::Common::Utilities.timeout_timestamp - results = @metric_readers.map do |metric_reader| remaining_timeout = OpenTelemetry::Common::Utilities.maybe_timeout(timeout, start_time) - break [Export::TIMEOUT] if remaining_timeout&.zero? - metric_reader.shutdown(timeout: remaining_timeout) + if remaining_timeout&.zero? + Export::TIMEOUT + else + metric_reader.shutdown(timeout: remaining_timeout) + end end @stopped = true @@ -80,16 +81,21 @@ def shutdown(timeout: nil) # a non-specific failure occurred, Export::TIMEOUT if a timeout occurred. def force_flush(timeout: nil) @mutex.synchronize do - return Export::SUCCESS if @stopped - - start_time = OpenTelemetry::Common::Utilities.timeout_timestamp - results = @metric_readers.map do |metric_reader| - remaining_timeout = OpenTelemetry::Common::Utilities.maybe_timeout(timeout, start_time) - return Export::TIMEOUT if remaining_timeout&.zero? + if @stopped + Export::SUCCESS + else + start_time = OpenTelemetry::Common::Utilities.timeout_timestamp + results = @metric_readers.map do |metric_reader| + remaining_timeout = OpenTelemetry::Common::Utilities.maybe_timeout(timeout, start_time) + if remaining_timeout&.zero? + Export::TIMEOUT + else + metric_reader.force_flush(timeout: remaining_timeout) + end + end - metric_reader.force_flush(timeout: remaining_timeout) + results.max || Export::SUCCESS end - results.max || Export::SUCCESS end end @@ -100,10 +106,11 @@ def add_metric_reader(metric_reader) @mutex.synchronize do if @stopped OpenTelemetry.logger.warn('calling MetricProvider#add_metric_reader after shutdown.') - return + else + @metric_readers.push(metric_reader) end - @metric_readers = @metric_readers.push(metric_reader) + nil end end diff --git a/metrics_sdk/test/opentelemetry/sdk/metrics/meter_provider_test.rb b/metrics_sdk/test/opentelemetry/sdk/metrics/meter_provider_test.rb index e71d8af3a..692eda7ed 100644 --- a/metrics_sdk/test/opentelemetry/sdk/metrics/meter_provider_test.rb +++ b/metrics_sdk/test/opentelemetry/sdk/metrics/meter_provider_test.rb @@ -58,17 +58,17 @@ end it 'invokes shutdown on all registered Metric Readers' do - mock_metric_reader_1 = Minitest::Mock.new - mock_metric_reader_2 = Minitest::Mock.new - mock_metric_reader_1.expect(:shutdown, nil, [{ timeout: nil }]) - mock_metric_reader_2.expect(:shutdown, nil, [{ timeout: nil }]) + mock_metric_reader1 = Minitest::Mock.new + mock_metric_reader2 = Minitest::Mock.new + mock_metric_reader1.expect(:shutdown, nil, [{ timeout: nil }]) + mock_metric_reader2.expect(:shutdown, nil, [{ timeout: nil }]) - OpenTelemetry.meter_provider.add_metric_reader(mock_metric_reader_1) - OpenTelemetry.meter_provider.add_metric_reader(mock_metric_reader_2) + OpenTelemetry.meter_provider.add_metric_reader(mock_metric_reader1) + OpenTelemetry.meter_provider.add_metric_reader(mock_metric_reader2) OpenTelemetry.meter_provider.shutdown - mock_metric_reader_1.verify - mock_metric_reader_2.verify + mock_metric_reader1.verify + mock_metric_reader2.verify end end @@ -82,17 +82,17 @@ end it 'invokes force_flush on all registered Metric Readers' do - mock_metric_reader_1 = Minitest::Mock.new - mock_metric_reader_2 = Minitest::Mock.new - mock_metric_reader_1.expect(:force_flush, nil, [{ timeout: nil }]) - mock_metric_reader_2.expect(:force_flush, nil, [{ timeout: nil }]) - OpenTelemetry.meter_provider.add_metric_reader(mock_metric_reader_1) - OpenTelemetry.meter_provider.add_metric_reader(mock_metric_reader_2) + mock_metric_reader1 = Minitest::Mock.new + mock_metric_reader2 = Minitest::Mock.new + mock_metric_reader1.expect(:force_flush, nil, [{ timeout: nil }]) + mock_metric_reader2.expect(:force_flush, nil, [{ timeout: nil }]) + OpenTelemetry.meter_provider.add_metric_reader(mock_metric_reader1) + OpenTelemetry.meter_provider.add_metric_reader(mock_metric_reader2) OpenTelemetry.meter_provider.force_flush - mock_metric_reader_1.verify - mock_metric_reader_2.verify + mock_metric_reader1.verify + mock_metric_reader2.verify end end @@ -109,6 +109,9 @@ end describe '#add_view' do - # TODO + it 'adds a view' do + # TODO + # OpenTelemetry.meter_provider.add_view + end end end