update test case

open-telemetry · Dec 11, 2024 · f3466ce · f3466ce
1 parent cc6c926
commit f3466ce
Show file tree

Hide file tree

Showing 17 changed files with 71 additions and 57 deletions.
diff --git a/exporter/otlp-metrics/lib/opentelemetry/exporter/otlp/metrics/metrics_exporter.rb b/exporter/otlp-metrics/lib/opentelemetry/exporter/otlp/metrics/metrics_exporter.rb
@@ -26,7 +26,7 @@ module Exporter
     module OTLP
       module Metrics
         # An OpenTelemetry metrics exporter that sends metrics over HTTP as Protobuf encoded OTLP ExportMetricsServiceRequest.
-        class MetricsExporter < ::OpenTelemetry::SDK::Metrics::Export::MetricReader
+        class MetricsExporter < ::OpenTelemetry::SDK::Metrics::Export::MetricExporter
           include Util
 
           attr_reader :metric_snapshots

diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/configuration_patch.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/configuration_patch.rb
@@ -38,7 +38,7 @@ def wrapped_metric_exporters_from_env
             case exporter.strip
             when 'none' then nil
             when 'console' then OpenTelemetry.meter_provider.add_metric_reader(Metrics::Export::PeriodicMetricReader.new(exporter: Metrics::Export::ConsoleMetricPullExporter.new))
-            when 'in-memory' then OpenTelemetry.meter_provider.add_metric_reader(Metrics::Export::InMemoryMetricPullExporter.new)
+            when 'in-memory' then OpenTelemetry.meter_provider.add_metric_reader(Metrics::Export::MetricReader.new(exporter: Metrics::Export::InMemoryMetricPullExporter.new))
             when 'otlp'
               begin
                 OpenTelemetry.meter_provider.add_metric_reader(Metrics::Export::PeriodicMetricReader.new(exporter: OpenTelemetry::Exporter::OTLP::Metrics::MetricsExporter.new))

diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/export.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/export.rb
@@ -23,6 +23,7 @@ module Export
   end
 end
 
+require 'opentelemetry/sdk/metrics/export/metric_exporter'
 require 'opentelemetry/sdk/metrics/export/metric_reader'
 require 'opentelemetry/sdk/metrics/export/in_memory_metric_pull_exporter'
 require 'opentelemetry/sdk/metrics/export/console_metric_pull_exporter'

diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/export/console_metric_pull_exporter.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/export/console_metric_pull_exporter.rb
@@ -17,10 +17,6 @@ def initialize
             @stopped = false
           end
 
-          def pull
-            export(collect)
-          end
-
           def export(metrics, timeout: nil)
             return FAILURE if @stopped
 

diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/export/in_memory_metric_pull_exporter.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/export/in_memory_metric_pull_exporter.rb
@@ -19,10 +19,6 @@ def initialize
             @mutex = Mutex.new
           end
 
-          def pull
-            export(collect)
-          end
-
           def export(metrics, timeout: nil)
             @mutex.synchronize do
               @metric_snapshots.concat(Array(metrics))

diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/export/metric_exporter.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/export/metric_exporter.rb
@@ -22,6 +22,10 @@ def collect
             @metric_store.collect
           end
 
+          def pull(timeout: nil)
+            export(collect, timeout: timeout)
+          end
+
           def shutdown(timeout: nil)
             Export::SUCCESS
           end

diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/export/metric_reader.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/export/metric_reader.rb
@@ -13,9 +13,9 @@ class MetricReader
           attr_reader :exporters
 
           def initialize(exporter: nil)
-            register_exporter(exporter)
             @mutex = Mutex.new
             @exporters = []
+            register_exporter(exporter: exporter)
           end
 
           # The metrics Reader implementation supports registering metric Exporters
@@ -27,10 +27,9 @@ def register_exporter(exporter: nil)
             end
           end
 
-          # Each exporter pull will trigger its metric_store call collect;
-          # and metric_store will collect all metrics data and send for export.
-          def collect
-            @exporters.each { |exporter| exporter.pull if exporter.respond_to?(:pull) }
+          # exporter pull should trigger exporter to send out the metrics
+          def collect(timeout: nil)
+            @exporters.each { |exporter| exporter.pull(timeout: timeout) if exporter.respond_to?(:pull) }
           end
           alias pull collect
 

diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/export/periodic_metric_reader.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/export/periodic_metric_reader.rb
@@ -25,14 +25,13 @@ class PeriodicMetricReader < MetricReader
           def initialize(export_interval_millis: Float(ENV.fetch('OTEL_METRIC_EXPORT_INTERVAL', 60_000)),
                          export_timeout_millis: Float(ENV.fetch('OTEL_METRIC_EXPORT_TIMEOUT', 30_000)),
                          exporter: nil)
-            super()
+            super(exporter: exporter)
 
             @export_interval = export_interval_millis / 1000.0
             @export_timeout = export_timeout_millis / 1000.0
-            @exporter = exporter
+            # @exporter = exporter
             @thread   = nil
             @continue = false
-            @mutex = Mutex.new
             @export_mutex = Mutex.new
 
             start
@@ -44,26 +43,28 @@ def shutdown(timeout: nil)
               @thread
             end
             thread&.join(@export_interval)
-            @exporter.force_flush if @exporter.respond_to?(:force_flush)
-            @exporter.shutdown
-            Export::SUCCESS
+            # @exporter.force_flush if @exporter.respond_to?(:force_flush)
+            # @exporter.shutdown
+            super(timeout: timeout)
+            # Export::SUCCESS
           rescue StandardError => e
             OpenTelemetry.handle_error(exception: e, message: 'Fail to shutdown PeriodicMetricReader.')
             Export::FAILURE
           end
 
           def force_flush(timeout: nil)
-            export(timeout: timeout)
-            Export::SUCCESS
-          rescue StandardError
-            Export::FAILURE
+            # export(timeout: timeout)
+            super(timeout: timeout)
+            # Export::SUCCESS
+          # rescue StandardError
+          #   Export::FAILURE
           end
 
           private
 
           def start
             @continue = true
-            if @exporter.nil?
+            if @exporters.empty?
               OpenTelemetry.logger.warn 'Missing exporter in PeriodicMetricReader.'
             elsif @thread&.alive?
               OpenTelemetry.logger.warn 'PeriodicMetricReader is still running. Please shutdown it if it needs to restart.'
@@ -85,8 +86,10 @@ def start
 
           def export(timeout: nil)
             @export_mutex.synchronize do
-              collected_metrics = collect
-              @exporter.export(collected_metrics, timeout: timeout || @export_timeout) unless collected_metrics.empty?
+              # collected_metrics = collect
+              # collected_metrics = @exporters.each { |exporter| exporter.pull if exporter.respond_to?(:pull) }
+              # @exporter.export(collected_metrics, timeout: timeout || @export_timeout) unless collected_metrics.empty?
+              @exporters.each { |exporter| exporter.pull(timeout: timeout || @export_timeout) if exporter.respond_to?(:pull) }
             end
           end
 

diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/meter.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/meter.rb
@@ -31,7 +31,9 @@ def initialize(name, version, meter_provider)
         # @api private
         def add_metric_reader(metric_reader)
           @instrument_registry.each_value do |instrument|
-            instrument.register_with_new_metric_store(metric_reader.metric_store)
+            metric_reader.exporters.each do |exporter|
+              instrument.register_with_new_metric_store(exporter.metric_store)
+            end
           end
         end
 

diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb
@@ -122,7 +122,9 @@ def add_metric_reader(metric_reader)
         def register_synchronous_instrument(instrument)
           @mutex.synchronize do
             @metric_readers.each do |mr|
-              instrument.register_with_new_metric_store(mr.metric_store)
+              mr.exporters.each do |exporter|
+                instrument.register_with_new_metric_store(exporter.metric_store)
+              end
             end
           end
         end

diff --git a/metrics_sdk/test/integration/in_memory_metric_pull_exporter_test.rb b/metrics_sdk/test/integration/in_memory_metric_pull_exporter_test.rb
@@ -14,7 +14,8 @@
       OpenTelemetry::SDK.configure
 
       metric_exporter = OpenTelemetry::SDK::Metrics::Export::InMemoryMetricPullExporter.new
-      OpenTelemetry.meter_provider.add_metric_reader(metric_exporter)
+      metric_reader = OpenTelemetry::SDK::Metrics::Export::MetricReader.new(exporter: metric_exporter)
+      OpenTelemetry.meter_provider.add_metric_reader(metric_reader)
 
       meter = OpenTelemetry.meter_provider.meter('test')
       counter = meter.create_counter('counter', unit: 'smidgen', description: 'a small amount of something')
@@ -25,7 +26,7 @@
       counter.add(3, attributes: { 'b' => 'c' })
       counter.add(4, attributes: { 'd' => 'e' })
 
-      metric_exporter.pull
+      metric_reader.pull
       last_snapshot = metric_exporter.metric_snapshots
 
       _(last_snapshot).wont_be_empty

diff --git a/metrics_sdk/test/opentelemetry/sdk/metrics/configuration_patch_test.rb b/metrics_sdk/test/opentelemetry/sdk/metrics/configuration_patch_test.rb
@@ -44,7 +44,7 @@
         reader = OpenTelemetry.meter_provider.metric_readers[0]
 
         assert_instance_of OpenTelemetry::SDK::Metrics::Export::PeriodicMetricReader, reader
-        assert_instance_of OpenTelemetry::Exporter::OTLP::Metrics::MetricsExporter, reader.instance_variable_get(:@exporter)
+        assert_instance_of OpenTelemetry::Exporter::OTLP::Metrics::MetricsExporter, reader.exporters.first
       end
 
       it 'can be set by environment variable' do
@@ -57,7 +57,7 @@
         reader = OpenTelemetry.meter_provider.metric_readers[0]
 
         assert_instance_of OpenTelemetry::SDK::Metrics::Export::PeriodicMetricReader, reader
-        assert_instance_of OpenTelemetry::SDK::Metrics::Export::ConsoleMetricPullExporter, reader.instance_variable_get(:@exporter)
+        assert_instance_of OpenTelemetry::SDK::Metrics::Export::ConsoleMetricPullExporter, reader.exporters.first
       end
 
       it 'supports "none" as an environment variable' do
@@ -83,9 +83,9 @@
         reader2 = OpenTelemetry.meter_provider.metric_readers[1]
 
         assert_instance_of OpenTelemetry::SDK::Metrics::Export::PeriodicMetricReader, reader1
-        assert_instance_of OpenTelemetry::SDK::Metrics::Export::ConsoleMetricPullExporter, reader1.instance_variable_get(:@exporter)
+        assert_instance_of OpenTelemetry::SDK::Metrics::Export::ConsoleMetricPullExporter, reader1.exporters.first
 
-        assert_instance_of OpenTelemetry::SDK::Metrics::Export::InMemoryMetricPullExporter, reader2
+        assert_instance_of OpenTelemetry::SDK::Metrics::Export::InMemoryMetricPullExporter, reader2.exporters.first
       end
 
       it 'defaults to noop with invalid env var' do

diff --git a/metrics_sdk/test/opentelemetry/sdk/metrics/instrument/counter_test.rb b/metrics_sdk/test/opentelemetry/sdk/metrics/instrument/counter_test.rb
@@ -8,18 +8,19 @@
 
 describe OpenTelemetry::SDK::Metrics::Instrument::Counter do
   let(:metric_exporter) { OpenTelemetry::SDK::Metrics::Export::InMemoryMetricPullExporter.new }
+  let(:metric_reader) { OpenTelemetry::SDK::Metrics::Export::MetricReader.new(exporter: metric_exporter) }
   let(:meter) { OpenTelemetry.meter_provider.meter('test') }
   let(:counter) { meter.create_counter('counter', unit: 'smidgen', description: 'a small amount of something') }
 
   before do
     reset_metrics_sdk
     OpenTelemetry::SDK.configure
-    OpenTelemetry.meter_provider.add_metric_reader(metric_exporter)
+    OpenTelemetry.meter_provider.add_metric_reader(metric_reader)
   end
 
   it 'counts' do
     counter.add(1, attributes: { 'foo' => 'bar' })
-    metric_exporter.pull
+    metric_reader.pull
     last_snapshot = metric_exporter.metric_snapshots
 
     _(last_snapshot[0].name).must_equal('counter')

diff --git a/metrics_sdk/test/opentelemetry/sdk/metrics/instrument/histogram_test.rb b/metrics_sdk/test/opentelemetry/sdk/metrics/instrument/histogram_test.rb
@@ -8,19 +8,20 @@
 
 describe OpenTelemetry::SDK::Metrics::Instrument::Histogram do
   let(:metric_exporter) { OpenTelemetry::SDK::Metrics::Export::InMemoryMetricPullExporter.new }
+  let(:metric_reader) { OpenTelemetry::SDK::Metrics::Export::MetricReader.new(exporter: metric_exporter) }
   let(:meter) { OpenTelemetry.meter_provider.meter('test') }
   let(:histogram) { meter.create_histogram('histogram', unit: 'smidgen', description: 'a small amount of something') }
 
   before do
     reset_metrics_sdk
     OpenTelemetry::SDK.configure
-    OpenTelemetry.meter_provider.add_metric_reader(metric_exporter)
+    OpenTelemetry.meter_provider.add_metric_reader(metric_reader)
   end
 
   it 'histograms' do
     histogram.record(5, attributes: { 'foo' => 'bar' })
     histogram.record(6, attributes: { 'foo' => 'bar' })
-    metric_exporter.pull
+    metric_reader.pull
     last_snapshot = metric_exporter.metric_snapshots
 
     _(last_snapshot[0].name).must_equal('histogram')

diff --git a/metrics_sdk/test/opentelemetry/sdk/metrics/instrument/up_down_counter_test.rb b/metrics_sdk/test/opentelemetry/sdk/metrics/instrument/up_down_counter_test.rb
@@ -8,19 +8,20 @@
 
 describe OpenTelemetry::SDK::Metrics::Instrument::UpDownCounter do
   let(:metric_exporter) { OpenTelemetry::SDK::Metrics::Export::InMemoryMetricPullExporter.new }
+  let(:metric_reader) { OpenTelemetry::SDK::Metrics::Export::MetricReader.new(exporter: metric_exporter) }
   let(:meter) { OpenTelemetry.meter_provider.meter('test') }
   let(:up_down_counter) { meter.create_up_down_counter('up_down_counter', unit: 'smidgen', description: 'a small amount of something') }
 
   before do
     reset_metrics_sdk
     OpenTelemetry::SDK.configure
-    OpenTelemetry.meter_provider.add_metric_reader(metric_exporter)
+    OpenTelemetry.meter_provider.add_metric_reader(metric_reader)
   end
 
   it 'counts up and down' do
     up_down_counter.add(1, attributes: { 'foo' => 'bar' })
     up_down_counter.add(-2, attributes: { 'foo' => 'bar' })
-    metric_exporter.pull
+    metric_reader.pull
     last_snapshot = metric_exporter.metric_snapshots
 
     _(last_snapshot[0].name).must_equal('up_down_counter')

diff --git a/metrics_sdk/test/opentelemetry/sdk/metrics/meter_provider_test.rb b/metrics_sdk/test/opentelemetry/sdk/metrics/meter_provider_test.rb
@@ -112,29 +112,33 @@
     it 'associates the metric store with instruments created before the metric reader' do
       meter_a = OpenTelemetry.meter_provider.meter('a').create_counter('meter_a')
 
-      metric_reader_a = OpenTelemetry::SDK::Metrics::Export::MetricReader.new
+      metric_exporter_a = OpenTelemetry::SDK::Metrics::Export::MetricExporter.new
+      metric_reader_a = OpenTelemetry::SDK::Metrics::Export::MetricReader.new(exporter: metric_exporter_a)
       OpenTelemetry.meter_provider.add_metric_reader(metric_reader_a)
 
-      metric_reader_b = OpenTelemetry::SDK::Metrics::Export::MetricReader.new
+      metric_exporter_b = OpenTelemetry::SDK::Metrics::Export::MetricExporter.new
+      metric_reader_b = OpenTelemetry::SDK::Metrics::Export::MetricReader.new(exporter: metric_exporter_b)
       OpenTelemetry.meter_provider.add_metric_reader(metric_reader_b)
-
+      
       _(meter_a.instance_variable_get(:@metric_streams).size).must_equal(2)
-      _(metric_reader_a.metric_store.instance_variable_get(:@metric_streams).size).must_equal(1)
-      _(metric_reader_b.metric_store.instance_variable_get(:@metric_streams).size).must_equal(1)
+      _(metric_reader_a.exporters.first.metric_store.instance_variable_get(:@metric_streams).size).must_equal(1)
+      _(metric_reader_b.exporters.first.metric_store.instance_variable_get(:@metric_streams).size).must_equal(1)
     end
 
     it 'associates the metric store with instruments created after the metric reader' do
-      metric_reader_a = OpenTelemetry::SDK::Metrics::Export::MetricReader.new
+      metric_exporter_a = OpenTelemetry::SDK::Metrics::Export::MetricExporter.new
+      metric_reader_a = OpenTelemetry::SDK::Metrics::Export::MetricReader.new(exporter: metric_exporter_a)
       OpenTelemetry.meter_provider.add_metric_reader(metric_reader_a)
 
-      metric_reader_b = OpenTelemetry::SDK::Metrics::Export::MetricReader.new
+      metric_exporter_b = OpenTelemetry::SDK::Metrics::Export::MetricExporter.new
+      metric_reader_b = OpenTelemetry::SDK::Metrics::Export::MetricReader.new(exporter: metric_exporter_b)
       OpenTelemetry.meter_provider.add_metric_reader(metric_reader_b)
 
       meter_a = OpenTelemetry.meter_provider.meter('a').create_counter('meter_a')
 
       _(meter_a.instance_variable_get(:@metric_streams).size).must_equal(2)
-      _(metric_reader_a.metric_store.instance_variable_get(:@metric_streams).size).must_equal(1)
-      _(metric_reader_b.metric_store.instance_variable_get(:@metric_streams).size).must_equal(1)
+      _(metric_reader_a.exporters.first.metric_store.instance_variable_get(:@metric_streams).size).must_equal(1)
+      _(metric_reader_b.exporters.first.metric_store.instance_variable_get(:@metric_streams).size).must_equal(1)
     end
   end
 

diff --git a/metrics_sdk/test/opentelemetry/sdk/metrics/view/registered_view_test.rb b/metrics_sdk/test/opentelemetry/sdk/metrics/view/registered_view_test.rb
@@ -14,7 +14,8 @@
       OpenTelemetry::SDK.configure
 
       metric_exporter = OpenTelemetry::SDK::Metrics::Export::InMemoryMetricPullExporter.new
-      OpenTelemetry.meter_provider.add_metric_reader(metric_exporter)
+      metric_reader = OpenTelemetry::SDK::Metrics::Export::MetricReader.new(exporter: metric_exporter)
+      OpenTelemetry.meter_provider.add_metric_reader(metric_reader)
 
       meter = OpenTelemetry.meter_provider.meter('test')
       OpenTelemetry.meter_provider.add_view('counter', aggregation: ::OpenTelemetry::SDK::Metrics::Aggregation::Drop.new)
@@ -24,7 +25,7 @@
       counter.add(1)
       counter.add(2, attributes: { 'a' => 'b' })
 
-      metric_exporter.pull
+      metric_reader.pull
       last_snapshot = metric_exporter.metric_snapshots
 
       _(last_snapshot).wont_be_empty
@@ -47,7 +48,8 @@
       OpenTelemetry::SDK.configure
 
       metric_exporter = OpenTelemetry::SDK::Metrics::Export::InMemoryMetricPullExporter.new
-      OpenTelemetry.meter_provider.add_metric_reader(metric_exporter)
+      metric_reader = OpenTelemetry::SDK::Metrics::Export::MetricReader.new(exporter: metric_exporter)
+      OpenTelemetry.meter_provider.add_metric_reader(metric_reader)
 
       meter = OpenTelemetry.meter_provider.meter('test')
       OpenTelemetry.meter_provider.add_view('counter', aggregation: ::OpenTelemetry::SDK::Metrics::Aggregation::LastValue.new)
@@ -59,7 +61,7 @@
       counter.add(3)
       counter.add(4)
 
-      metric_exporter.pull
+      metric_reader.pull
       last_snapshot = metric_exporter.metric_snapshots
 
       _(last_snapshot[0].data_points).wont_be_empty
@@ -70,7 +72,8 @@
       OpenTelemetry::SDK.configure
 
       metric_exporter = OpenTelemetry::SDK::Metrics::Export::InMemoryMetricPullExporter.new
-      OpenTelemetry.meter_provider.add_metric_reader(metric_exporter)
+      metric_reader = OpenTelemetry::SDK::Metrics::Export::MetricReader.new(exporter: metric_exporter)
+      OpenTelemetry.meter_provider.add_metric_reader(metric_reader)
 
       meter = OpenTelemetry.meter_provider.meter('test')
       OpenTelemetry.meter_provider.add_view('retnuoc', aggregation: ::OpenTelemetry::SDK::Metrics::Aggregation::LastValue.new)
@@ -82,7 +85,7 @@
       counter.add(3)
       counter.add(4)
 
-      metric_exporter.pull
+      metric_reader.pull
       last_snapshot = metric_exporter.metric_snapshots
 
       _(last_snapshot[0].data_points).wont_be_empty