From d6c26fde359b22a102d90f70bcedbb98f24f383f Mon Sep 17 00:00:00 2001 From: Elias Rodrigues Date: Fri, 30 Jun 2023 16:11:08 -0300 Subject: [PATCH 01/32] Move attr_reader from SDK to API in MeterProvider --- metrics_api/lib/opentelemetry/metrics/meter_provider.rb | 2 ++ metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb | 2 -- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/metrics_api/lib/opentelemetry/metrics/meter_provider.rb b/metrics_api/lib/opentelemetry/metrics/meter_provider.rb index 3cd53b110c..371d764a47 100644 --- a/metrics_api/lib/opentelemetry/metrics/meter_provider.rb +++ b/metrics_api/lib/opentelemetry/metrics/meter_provider.rb @@ -12,6 +12,8 @@ class MeterProvider private_constant :NOOP_METER + attr_reader :resource, :metric_readers + def initialize(resource: nil) @resource = resource diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb index 205ff5db0d..c21a3630c1 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb @@ -14,8 +14,6 @@ class MeterProvider < OpenTelemetry::Metrics::MeterProvider Key = Struct.new(:name, :version) private_constant(:Key) - attr_reader :resource, :metric_readers - def initialize(resource: OpenTelemetry::SDK::Resources::Resource.create) @mutex = Mutex.new @meter_registry = {} From 087640892c59dd899b43021b44ea6a3a7b0b7e3d Mon Sep 17 00:00:00 2001 From: Elias Rodrigues Date: Fri, 30 Jun 2023 16:11:46 -0300 Subject: [PATCH 02/32] Call initialize from API in SDK MeterProvider and set Resource to default See https://opentelemetry.io/docs/specs/otel/resource/sdk/ --- .../lib/opentelemetry/sdk/metrics/meter_provider.rb | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb index c21a3630c1..6d8c611933 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb @@ -14,12 +14,8 @@ class MeterProvider < OpenTelemetry::Metrics::MeterProvider Key = Struct.new(:name, :version) private_constant(:Key) - def initialize(resource: OpenTelemetry::SDK::Resources::Resource.create) - @mutex = Mutex.new - @meter_registry = {} - @stopped = false - @metric_readers = [] - @resource = resource + def initialize(resource: OpenTelemetry::SDK::Resources::Resource.default) + super end # Returns a {Meter} instance. From 1dd9e6f4afe93960034daab3674f6b5f9dcfa42c Mon Sep 17 00:00:00 2001 From: Elias Rodrigues Date: Fri, 30 Jun 2023 16:20:48 -0300 Subject: [PATCH 03/32] Move private_constant :Key from SDK to API in MeterProvider --- metrics_api/lib/opentelemetry/metrics/meter_provider.rb | 3 ++- metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb | 3 --- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/metrics_api/lib/opentelemetry/metrics/meter_provider.rb b/metrics_api/lib/opentelemetry/metrics/meter_provider.rb index 371d764a47..23198f9bf5 100644 --- a/metrics_api/lib/opentelemetry/metrics/meter_provider.rb +++ b/metrics_api/lib/opentelemetry/metrics/meter_provider.rb @@ -9,8 +9,9 @@ module Metrics # No-op implementation of a meter provider. class MeterProvider NOOP_METER = Meter.new('no-op') + Key = Struct.new(:name, :version) - private_constant :NOOP_METER + private_constant :NOOP_METER, :Key attr_reader :resource, :metric_readers diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb index 6d8c611933..8830a56b2a 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb @@ -11,9 +11,6 @@ module SDK module Metrics # {MeterProvider} is the SDK implementation of {OpenTelemetry::Metrics::MeterProvider}. class MeterProvider < OpenTelemetry::Metrics::MeterProvider - Key = Struct.new(:name, :version) - private_constant(:Key) - def initialize(resource: OpenTelemetry::SDK::Resources::Resource.default) super end From 20ead90a1c4cf8f45c3db9841d174ccb783ff935 Mon Sep 17 00:00:00 2001 From: Elias Rodrigues Date: Fri, 30 Jun 2023 16:43:03 -0300 Subject: [PATCH 04/32] Update InstrumentationScope to include schema_url and attributes See https://opentelemetry.io/docs/specs/otel/metrics/sdk/#meter-creation --- sdk/lib/opentelemetry/sdk/instrumentation_scope.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/sdk/lib/opentelemetry/sdk/instrumentation_scope.rb b/sdk/lib/opentelemetry/sdk/instrumentation_scope.rb index 606738ebbc..413e6a7298 100644 --- a/sdk/lib/opentelemetry/sdk/instrumentation_scope.rb +++ b/sdk/lib/opentelemetry/sdk/instrumentation_scope.rb @@ -7,7 +7,11 @@ module OpenTelemetry module SDK # InstrumentationScope is a struct containing scope information for export. - InstrumentationScope = Struct.new(:name, - :version) + InstrumentationScope = Struct.new( + :name, + :version, + :schema_url, + :attributes + ) end end From ca7d03b5dce64e157ef552c72eb5799999f32a77 Mon Sep 17 00:00:00 2001 From: Elias Rodrigues Date: Fri, 30 Jun 2023 16:43:57 -0300 Subject: [PATCH 05/32] Add @instrumentation_scope to Meter in API --- metrics_api/lib/opentelemetry/metrics/meter.rb | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/metrics_api/lib/opentelemetry/metrics/meter.rb b/metrics_api/lib/opentelemetry/metrics/meter.rb index 37616415fe..d0c49293e5 100644 --- a/metrics_api/lib/opentelemetry/metrics/meter.rb +++ b/metrics_api/lib/opentelemetry/metrics/meter.rb @@ -33,6 +33,13 @@ def initialize(name, version: nil, schema_url: nil, attributes: nil) @schema_url = schema_url || '' @attributes = attributes || {} + @instrumentation_scope = InstrumentationScope.new( + @name, + @version, + @schema_url, + @attributes + ) + @mutex = Mutex.new @instrument_registry = {} end From 1555a62a4de3df2f74068a3b847339ca0631f8c8 Mon Sep 17 00:00:00 2001 From: Elias Rodrigues Date: Fri, 30 Jun 2023 16:44:23 -0300 Subject: [PATCH 06/32] Update Key, the Meter identifier, to include schema_url See https://opentelemetry.io/docs/specs/otel/metrics/api/#get-a-meter > Meters are identified by `name`, `version`, and `schema_url` fields. --- metrics_api/lib/opentelemetry/metrics/meter_provider.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/metrics_api/lib/opentelemetry/metrics/meter_provider.rb b/metrics_api/lib/opentelemetry/metrics/meter_provider.rb index 23198f9bf5..2e959162c7 100644 --- a/metrics_api/lib/opentelemetry/metrics/meter_provider.rb +++ b/metrics_api/lib/opentelemetry/metrics/meter_provider.rb @@ -9,7 +9,7 @@ module Metrics # No-op implementation of a meter provider. class MeterProvider NOOP_METER = Meter.new('no-op') - Key = Struct.new(:name, :version) + Key = Struct.new(:name, :version, :schema_url) private_constant :NOOP_METER, :Key From f5d3f0305386af6953f18bd0cc981423db148c42 Mon Sep 17 00:00:00 2001 From: Elias Rodrigues Date: Fri, 30 Jun 2023 16:50:05 -0300 Subject: [PATCH 07/32] Update MeterProvider#meter in SDK --- .../lib/opentelemetry/sdk/metrics/meter.rb | 15 ------- .../sdk/metrics/meter_provider.rb | 42 +++++++++++++++---- 2 files changed, 33 insertions(+), 24 deletions(-) diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/meter.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/meter.rb index faf8e9adb1..47ffad55b4 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/meter.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/meter.rb @@ -11,21 +11,6 @@ module SDK module Metrics # {Meter} is the SDK implementation of {OpenTelemetry::Metrics::Meter}. class Meter < OpenTelemetry::Metrics::Meter - # @api private - # - # Returns a new {Meter} instance. - # - # @param [String] name Instrumentation package name - # @param [String] version Instrumentation package version - # - # @return [Meter] - def initialize(name, version, meter_provider) - @mutex = Mutex.new - @instrument_registry = {} - @instrumentation_scope = InstrumentationScope.new(name, version) - @meter_provider = meter_provider - end - # @api private def add_metric_reader(metric_reader) @instrument_registry.each do |_n, instrument| diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb index 8830a56b2a..0534475782 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb @@ -15,19 +15,43 @@ def initialize(resource: OpenTelemetry::SDK::Resources::Resource.default) super end - # Returns a {Meter} instance. + # @param [String] name + # Uniquely identifies the instrumentation scope, such as the instrumentation library + # (e.g. io.opentelemetry.contrib.mongodb), package, module or class name + # @param [optional String] version + # Version of the instrumentation scope if the scope has a version (e.g. a library version) + # @param [optional String] schema_url + # Schema URL that should be recorded in the emitted telemetry + # @param [optional Hash{String => String, Numeric, Boolean, Array}] attributes + # Instrumentation scope attributes to associate with emitted telemetry # - # @param [String] name Instrumentation package name - # @param [optional String] version Instrumentation package version - # - # @return [Meter] - def meter(name, version: nil) - version ||= '' + # @return [SDK::Metrics::Meter] + def meter(name, version: nil, schema_url: nil, attributes: nil) if @stopped OpenTelemetry.logger.warn 'calling MeterProvider#meter after shutdown, a noop meter will be returned.' - OpenTelemetry::Metrics::Meter.new + + NOOP_METER else - @mutex.synchronize { @meter_registry[Key.new(name, version)] ||= Meter.new(name, version, self) } + if name.nil? || name.empty? + OpenTelemetry.logger.warn 'Invalid name provided to MeterProvider#meter: nil or empty' + end + + # TODO: Check if we should pass self to Meter.new + meter = Meter.new( + name, + version: version, + schema_url: schema_url, + attributes: attributes + ) + key = Key.new( + meter.name, + meter.version, + meter.schema_url + ) + + @mutex.synchronize do + @meter_registry[key] ||= meter + end end end From 159341de880f250e8f8842cdb19e7cebe99e565b Mon Sep 17 00:00:00 2001 From: Elias Rodrigues Date: Fri, 30 Jun 2023 17:39:41 -0300 Subject: [PATCH 08/32] Update ProxyMeterProvider to match API --- .../internal/proxy_meter_provider.rb | 67 ++++++++++++------- 1 file changed, 41 insertions(+), 26 deletions(-) diff --git a/metrics_api/lib/opentelemetry/internal/proxy_meter_provider.rb b/metrics_api/lib/opentelemetry/internal/proxy_meter_provider.rb index 915c4994bc..ed04823ef9 100644 --- a/metrics_api/lib/opentelemetry/internal/proxy_meter_provider.rb +++ b/metrics_api/lib/opentelemetry/internal/proxy_meter_provider.rb @@ -13,45 +13,60 @@ module Internal # It delegates to a "real" MeterProvider after the global meter provider is registered. # It returns {ProxyMeter} instances until the delegate is installed. class ProxyMeterProvider < Metrics::MeterProvider - Key = Struct.new(:name, :version) - private_constant(:Key) + def initialize(resource: nil) + super - # Returns a new {ProxyMeterProvider} instance. - # - # @return [ProxyMeterProvider] - def initialize - @mutex = Mutex.new - @registry = {} @delegate = nil end # Set the delegate Meter provider. If this is called more than once, a warning will # be logged and superfluous calls will be ignored. # - # @param [MeterProvider] provider The Meter provider to delegate to - def delegate=(provider) - unless @delegate.nil? - OpenTelemetry.logger.warn 'Attempt to reset delegate in ProxyMeterProvider ignored.' - return - end - + # @param [MeterProvider] meter_provider The Meter provider to delegate to + def delegate=(meter_provider) @mutex.synchronize do - @delegate = provider - @registry.each { |key, meter| meter.delegate = provider.meter(key.name, version: key.version) } + if @delegate.nil? + @delegate = meter_provider + + @meter_registry.each do |key, proxy_meter| + proxy_meter.delegate = meter_provider.meter( + proxy_meter.name, + version: proxy_meter.version, + schema_url: proxy_meter.schema_url, + attributes: proxy_meter.attributes, + ) + end + else + OpenTelemetry.logger.warn 'Attempt to reset delegate in ProxyMeterProvider ignored.' + end end end - # Returns a {Meter} instance. - # - # @param [optional String] name Instrumentation package name - # @param [optional String] version Instrumentation package version - # - # @return [Meter] - def meter(name = nil, version: nil) + # @api private + def meter(name, version: nil, schema_url: nil, attributes: nil) @mutex.synchronize do - return @delegate.meter(name, version: version) unless @delegate.nil? + if @delegate.nil? + proxy_meter = ProxyMeter.new( + name, + version: version, + schema_url: schema_url, + attributes: attributes + ) + key = Key.new( + proxy_meter.name, + proxy_meter.version, + proxy_meter.schema_url + ) - @registry[Key.new(name, version)] ||= ProxyMeter.new + @meter_registry[key] ||= proxy_meter + else + @delegate.meter( + name, + version: version, + schema_url: schema_url, + attributes: attributes + ) + end end end end From c33b1372b35226a66631a4dfa39ad3026fbe2ebe Mon Sep 17 00:00:00 2001 From: Elias Rodrigues Date: Fri, 30 Jun 2023 17:52:40 -0300 Subject: [PATCH 09/32] Update ProxyMeter to match API --- .../lib/opentelemetry/internal/proxy_meter.rb | 72 ++++++++++++++----- 1 file changed, 54 insertions(+), 18 deletions(-) diff --git a/metrics_api/lib/opentelemetry/internal/proxy_meter.rb b/metrics_api/lib/opentelemetry/internal/proxy_meter.rb index 74d497c684..35f8984c18 100644 --- a/metrics_api/lib/opentelemetry/internal/proxy_meter.rb +++ b/metrics_api/lib/opentelemetry/internal/proxy_meter.rb @@ -8,16 +8,14 @@ module OpenTelemetry module Internal # @api private # - # {ProxyMeter} is an implementation of {OpenTelemetry::Trace::Meter}. It is returned from - # the ProxyMeterProvider until a delegate meter provider is installed. After the delegate - # meter provider is installed, the ProxyMeter will delegate to the corresponding "real" - # meter. + # {ProxyMeter} is an implementation of {OpenTelemetry::Metrics::Meter}. + # It is returned from the ProxyMeterProvider until a delegate meter provider + # is installed. After the delegate meter provider is installed, + # the ProxyMeter will delegate to the corresponding "real" meter. class ProxyMeter < Metrics::Meter - # Returns a new {ProxyMeter} instance. - # - # @return [ProxyMeter] - def initialize + def initialize(name, version: nil, schema_url: nil, attributes: nil) super + @delegate = nil end @@ -38,17 +36,55 @@ def delegate=(meter) private - def create_instrument(kind, name, unit, description, callback) + def create_instrument(kind, name, unit, description, advice, callbacks) super do - next ProxyInstrument.new(kind, name, unit, description, callback) if @delegate.nil? - - case kind - when :counter then @delegate.create_counter(name, unit: unit, description: description) - when :histogram then @delegate.create_histogram(name, unit: unit, description: description) - when :up_down_counter then @delegate.create_up_down_counter(name, unit: unit, description: description) - when :observable_counter then @delegate.create_observable_counter(name, unit: unit, description: description, callback: callback) - when :observable_gauge then @delegate.create_observable_gauge(name, unit: unit, description: description, callback: callback) - when :observable_up_down_counter then @delegate.create_observable_up_down_counter(name, unit: unit, description: description, callback: callback) + if @delegate.nil? + ProxyInstrument.new(kind, name, unit, description, advice, callbacks) + else + case kind + when :counter + @delegate.create_counter( + name, + unit: unit, + description: description, + advice: advice + ) + when :histogram + @delegate.create_histogram( + name, + unit: unit, + description: description, + advice: advice + ) + when :up_down_counter + @delegate.create_up_down_counter( + name, + unit: unit, + description: description, + advice: advice + ) + when :observable_counter + @delegate.create_observable_counter( + name, + unit: unit, + description: description, + callbacks: callbacks + ) + when :observable_gauge + @delegate.create_observable_gauge( + name, + unit: unit, + description: description, + callbacks: callbacks + ) + when :observable_up_down_counter + @delegate.create_observable_up_down_counter( + name, + unit: unit, + description: description, + callbacks: callbacks + ) + end end end end From f25ce0b812182c79c3011865caae447606d8450e Mon Sep 17 00:00:00 2001 From: Elias Rodrigues Date: Fri, 30 Jun 2023 19:23:23 -0300 Subject: [PATCH 10/32] Make Proxy Instruments inherit from API Instruments --- .../internal/proxy_instrument.rb | 38 ----- .../internal/proxy_instrument/counter.rb | 24 +++ .../delegate_asynchronous_instrument.rb | 49 ++++++ .../delegate_synchronous_instrument.rb | 33 ++++ .../internal/proxy_instrument/histogram.rb | 24 +++ .../proxy_instrument/observable_counter.rb | 16 ++ .../proxy_instrument/observable_gauge.rb | 16 ++ .../observable_up_down_counter.rb | 16 ++ .../proxy_instrument/up_down_counter.rb | 24 +++ .../lib/opentelemetry/internal/proxy_meter.rb | 156 ++++++++++++++++-- .../internal/proxy_meter_provider.rb | 2 +- .../lib/opentelemetry/metrics/meter.rb | 14 +- .../lib/opentelemetry/sdk/metrics/meter.rb | 2 +- 13 files changed, 351 insertions(+), 63 deletions(-) delete mode 100644 metrics_api/lib/opentelemetry/internal/proxy_instrument.rb create mode 100644 metrics_api/lib/opentelemetry/internal/proxy_instrument/counter.rb create mode 100644 metrics_api/lib/opentelemetry/internal/proxy_instrument/delegate_asynchronous_instrument.rb create mode 100644 metrics_api/lib/opentelemetry/internal/proxy_instrument/delegate_synchronous_instrument.rb create mode 100644 metrics_api/lib/opentelemetry/internal/proxy_instrument/histogram.rb create mode 100644 metrics_api/lib/opentelemetry/internal/proxy_instrument/observable_counter.rb create mode 100644 metrics_api/lib/opentelemetry/internal/proxy_instrument/observable_gauge.rb create mode 100644 metrics_api/lib/opentelemetry/internal/proxy_instrument/observable_up_down_counter.rb create mode 100644 metrics_api/lib/opentelemetry/internal/proxy_instrument/up_down_counter.rb diff --git a/metrics_api/lib/opentelemetry/internal/proxy_instrument.rb b/metrics_api/lib/opentelemetry/internal/proxy_instrument.rb deleted file mode 100644 index 117a400778..0000000000 --- a/metrics_api/lib/opentelemetry/internal/proxy_instrument.rb +++ /dev/null @@ -1,38 +0,0 @@ -# frozen_string_literal: true - -# Copyright The OpenTelemetry Authors -# -# SPDX-License-Identifier: Apache-2.0 - -module OpenTelemetry - module Internal - # @api private - class ProxyInstrument - def initialize(kind, name, unit, desc, callable) - @kind = kind - @name = name - @unit = unit - @desc = desc - @callable = callable - @delegate = nil - end - - def upgrade_with(meter) - @delegate = case @kind - when :counter, :histogram, :up_down_counter - meter.send("create_#{@kind}", @name, unit: @unit, description: @desc) - when :observable_counter, :observable_gauge, :observable_up_down_counter - meter.send("create_#{@kind}", @name, unit: @unit, description: @desc, callback: @callback) - end - end - - def add(amount, attributes: nil) - @delegate&.add(amount, attributes: attributes) - end - - def record(amount, attributes: nil) - @delegate&.record(amount, attributes: attributes) - end - end - end -end diff --git a/metrics_api/lib/opentelemetry/internal/proxy_instrument/counter.rb b/metrics_api/lib/opentelemetry/internal/proxy_instrument/counter.rb new file mode 100644 index 0000000000..e6cc612bb5 --- /dev/null +++ b/metrics_api/lib/opentelemetry/internal/proxy_instrument/counter.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +# Copyright The OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +module OpenTelemetry + module Internal + module ProxyInstrument + # @api private + class Counter < Metrics::Instrument::Counter + include DelegateSynchronousInstrument + + def add(increment, attributes: nil) + if @delegate.nil? + super + else + @delegate.add(increment, attributes: attributes) + end + end + end + end + end +end diff --git a/metrics_api/lib/opentelemetry/internal/proxy_instrument/delegate_asynchronous_instrument.rb b/metrics_api/lib/opentelemetry/internal/proxy_instrument/delegate_asynchronous_instrument.rb new file mode 100644 index 0000000000..605b65e184 --- /dev/null +++ b/metrics_api/lib/opentelemetry/internal/proxy_instrument/delegate_asynchronous_instrument.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +# Copyright The OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +module OpenTelemetry + module Internal + module ProxyInstrument + # @api private + module DelegateAsynchronousInstrument + def initialize(*args, **kwargs) + super + + @mutex = Mutex.new + @delegate = nil + end + + def delegate=(instrument) + @mutex.synchronize do + if @delegate.nil? + @delegate = instrument + else + OpenTelemetry.logger.warn( + 'Attempt to reset delegate in Asynchronous ProxyInstrument ignored' + ) + end + end + end + + def register_callbacks(*callbacks) + if @delegate.nil? + super + else + @delegate.register_callbacks(*callbacks) + end + end + + def unregister_callbacks(*callbacks) + if @delegate.nil? + super + else + @delegate.unregister_callbacks(*callbacks) + end + end + end + end + end +end diff --git a/metrics_api/lib/opentelemetry/internal/proxy_instrument/delegate_synchronous_instrument.rb b/metrics_api/lib/opentelemetry/internal/proxy_instrument/delegate_synchronous_instrument.rb new file mode 100644 index 0000000000..5914701d2f --- /dev/null +++ b/metrics_api/lib/opentelemetry/internal/proxy_instrument/delegate_synchronous_instrument.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +# Copyright The OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +module OpenTelemetry + module Internal + module ProxyInstrument + # @api private + module DelegateSynchronousInstrument + def initialize(*args, **kwargs) + super + + @mutex = Mutex.new + @delegate = nil + end + + def delegate=(instrument) + @mutex.synchronize do + if @delegate.nil? + @delegate = instrument + else + OpenTelemetry.logger.warn(' + Attempt to reset delegate in Synchronous ProxyInstrument ignored' + ) + end + end + end + end + end + end +end diff --git a/metrics_api/lib/opentelemetry/internal/proxy_instrument/histogram.rb b/metrics_api/lib/opentelemetry/internal/proxy_instrument/histogram.rb new file mode 100644 index 0000000000..0ab071bc4a --- /dev/null +++ b/metrics_api/lib/opentelemetry/internal/proxy_instrument/histogram.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +# Copyright The OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +module OpenTelemetry + module Internal + module ProxyInstrument + # @api private + class Histogram < Metrics::Instrument::Histogram + include DelegateSynchronousInstrument + + def record(amount, attributes: nil) + if @delegate.nil? + super + else + @delegate.record(amount, attributes: attributes) + end + end + end + end + end +end diff --git a/metrics_api/lib/opentelemetry/internal/proxy_instrument/observable_counter.rb b/metrics_api/lib/opentelemetry/internal/proxy_instrument/observable_counter.rb new file mode 100644 index 0000000000..6747e27f97 --- /dev/null +++ b/metrics_api/lib/opentelemetry/internal/proxy_instrument/observable_counter.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +# Copyright The OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +module OpenTelemetry + module Internal + module ProxyInstrument + # @api private + class ObservableCounter < Metrics::Instrument::ObservableCounter + include DelegateAsynchronousInstrument + end + end + end +end diff --git a/metrics_api/lib/opentelemetry/internal/proxy_instrument/observable_gauge.rb b/metrics_api/lib/opentelemetry/internal/proxy_instrument/observable_gauge.rb new file mode 100644 index 0000000000..9862458a67 --- /dev/null +++ b/metrics_api/lib/opentelemetry/internal/proxy_instrument/observable_gauge.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +# Copyright The OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +module OpenTelemetry + module Internal + module ProxyInstrument + # @api private + class ObservableGauge < Metrics::Instrument::ObservableGauge + include DelegateAsynchronousInstrument + end + end + end +end diff --git a/metrics_api/lib/opentelemetry/internal/proxy_instrument/observable_up_down_counter.rb b/metrics_api/lib/opentelemetry/internal/proxy_instrument/observable_up_down_counter.rb new file mode 100644 index 0000000000..d31d3f2f9b --- /dev/null +++ b/metrics_api/lib/opentelemetry/internal/proxy_instrument/observable_up_down_counter.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +# Copyright The OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +module OpenTelemetry + module Internal + module ProxyInstrument + # @api private + class ObservableUpDownCounter < Metrics::Instrument::ObservableUpDownCounter + include DelegateAsynchronousInstrument + end + end + end +end diff --git a/metrics_api/lib/opentelemetry/internal/proxy_instrument/up_down_counter.rb b/metrics_api/lib/opentelemetry/internal/proxy_instrument/up_down_counter.rb new file mode 100644 index 0000000000..a39eabcd85 --- /dev/null +++ b/metrics_api/lib/opentelemetry/internal/proxy_instrument/up_down_counter.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +# Copyright The OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +module OpenTelemetry + module Internal + module ProxyInstrument + # @api private + class UpDownCounter < Metrics::Instrument::UpDownCounter + include DelegateSynchronousInstrument + + def add(amount, attributes: nil) + if @delegate.nil? + super + else + @delegate.add(amount, attributes: attributes) + end + end + end + end + end +end diff --git a/metrics_api/lib/opentelemetry/internal/proxy_meter.rb b/metrics_api/lib/opentelemetry/internal/proxy_meter.rb index 35f8984c18..cb1392e480 100644 --- a/metrics_api/lib/opentelemetry/internal/proxy_meter.rb +++ b/metrics_api/lib/opentelemetry/internal/proxy_meter.rb @@ -13,7 +13,7 @@ module Internal # is installed. After the delegate meter provider is installed, # the ProxyMeter will delegate to the corresponding "real" meter. class ProxyMeter < Metrics::Meter - def initialize(name, version: nil, schema_url: nil, attributes: nil) + def initialize(*args, **kwargs) super @delegate = nil @@ -27,57 +27,181 @@ def delegate=(meter) @mutex.synchronize do if @delegate.nil? @delegate = meter - @instrument_registry.each_value { |instrument| instrument.upgrade_with(meter) } + + @instrument_registry.each_value do |proxy_instrument| + proxy_instrument.delegate = + case proxy_instrument + when ProxyInstrument::Counter + meter.create_counter( + proxy_instrument.name, + unit: proxy_instrument.unit, + description: proxy_instrument.description, + advice: proxy_instrument.advice + ) + when ProxyInstrument::Histogram + meter.create_histogram( + proxy_instrument.name, + unit: proxy_instrument.unit, + description: proxy_instrument.description, + advice: proxy_instrument.advice + ) + when ProxyInstrument::UpDownCounter + meter.create_up_down_counter( + proxy_instrument.name, + unit: proxy_instrument.unit, + description: proxy_instrument.description, + advice: proxy_instrument.advice + ) + when ProxyInstrument::ObservableCounter + meter.create_observable_counter( + proxy_instrument.name, + unit: proxy_instrument.unit, + description: proxy_instrument.description, + callbacks: proxy_instrument.callbacks + ) + when ProxyInstrument::ObservableGauge + meter.create_observable_gauge( + proxy_instrument.name, + unit: proxy_instrument.unit, + description: proxy_instrument.description, + callbacks: proxy_instrument.callbacks + ) + when ProxyInstrument::ObservableUpDownCounter + meter.create_observable_up_down_counter( + proxy_instrument.name, + unit: proxy_instrument.unit, + description: proxy_instrument.description, + callbacks: proxy_instrument.callbacks + ) + end + end else OpenTelemetry.logger.warn 'Attempt to reset delegate in ProxyMeter ignored.' end end end - private - - def create_instrument(kind, name, unit, description, advice, callbacks) - super do - if @delegate.nil? - ProxyInstrument.new(kind, name, unit, description, advice, callbacks) - else - case kind - when :counter + def create_counter(name, unit: nil, description: nil, advice: nil) + register_instrument(name) do + @mutex.synchronize do + if @delegate.nil? + ProxyInstrument::Counter.new( + name, + unit: unit, + description: description, + advice: advice + ) + else @delegate.create_counter( name, unit: unit, description: description, advice: advice ) - when :histogram + end + end + end + end + + def create_histogram(name, unit: nil, description: nil, advice: nil) + register_instrument(name) do + @mutex.synchronize do + if @delegate.nil? + ProxyInstrument::Histogram.new( + name, + unit: unit, + description: description, + advice: advice + ) + else @delegate.create_histogram( name, unit: unit, description: description, advice: advice ) - when :up_down_counter + end + end + end + end + + def create_up_down_counter(name, unit: nil, description: nil, advice: nil) + register_instrument(name) do + @mutex.synchronize do + if @delegate.nil? + ProxyInstrument::UpDownCounter.new( + name, + unit: unit, + description: description, + advice: advice + ) + else @delegate.create_up_down_counter( name, unit: unit, description: description, advice: advice ) - when :observable_counter + end + end + end + end + + def create_observable_counter(name, unit: nil, description: nil, callbacks: nil) + register_instrument(name) do + @mutex.synchronize do + if @delegate.nil? + ProxyInstrument::ObservableCounter.new( + name, + unit: unit, + description: description, + callbacks: callbacks + ) + else @delegate.create_observable_counter( name, unit: unit, description: description, callbacks: callbacks ) - when :observable_gauge + end + end + end + end + + def create_observable_gauge(name, unit: nil, description: nil, callbacks: nil) + register_instrument(name) do + @mutex.synchronize do + if @delegate.nil? + ProxyInstrument::ObservableGauge.new( + name, + unit: unit, + description: description, + callbacks: callbacks + ) + else @delegate.create_observable_gauge( name, unit: unit, description: description, callbacks: callbacks ) - when :observable_up_down_counter + end + end + end + end + + def create_observable_up_down_counter(name, unit: nil, description: nil, callbacks: nil) + register_instrument(name) do + @mutex.synchronize do + if @delegate.nil? + ProxyInstrument::ObservableUpDownCounter.new( + name, + unit: unit, + description: description, + callbacks: callbacks + ) + else @delegate.create_observable_up_down_counter( name, unit: unit, diff --git a/metrics_api/lib/opentelemetry/internal/proxy_meter_provider.rb b/metrics_api/lib/opentelemetry/internal/proxy_meter_provider.rb index ed04823ef9..76c2bcf181 100644 --- a/metrics_api/lib/opentelemetry/internal/proxy_meter_provider.rb +++ b/metrics_api/lib/opentelemetry/internal/proxy_meter_provider.rb @@ -13,7 +13,7 @@ module Internal # It delegates to a "real" MeterProvider after the global meter provider is registered. # It returns {ProxyMeter} instances until the delegate is installed. class ProxyMeterProvider < Metrics::MeterProvider - def initialize(resource: nil) + def initialize(*args, **kwargs) super @delegate = nil diff --git a/metrics_api/lib/opentelemetry/metrics/meter.rb b/metrics_api/lib/opentelemetry/metrics/meter.rb index d0c49293e5..31cf66381c 100644 --- a/metrics_api/lib/opentelemetry/metrics/meter.rb +++ b/metrics_api/lib/opentelemetry/metrics/meter.rb @@ -58,7 +58,7 @@ def initialize(name, version: nil, schema_url: nil, attributes: nil) # # @return [Instrument::Counter] def create_counter(name, unit: nil, description: nil, advice: nil) - create_instrument(:counter, name, unit, description, advice, nil) { NOOP_COUNTER } + register_instrument(name) { NOOP_COUNTER } end # @param name [String] @@ -75,7 +75,7 @@ def create_counter(name, unit: nil, description: nil, advice: nil) # # @return [Instrument::Histogram] def create_histogram(name, unit: nil, description: nil, advice: nil) - create_instrument(:histogram, name, unit, description, advice, nil) { NOOP_HISTOGRAM } + register_instrument(name) { NOOP_HISTOGRAM } end # @param name [String] @@ -92,7 +92,7 @@ def create_histogram(name, unit: nil, description: nil, advice: nil) # # @return [Instrument::UpDownCounter] def create_up_down_counter(name, unit: nil, description: nil, advice: nil) - create_instrument(:up_down_counter, name, unit, description, advice, nil) { NOOP_UP_DOWN_COUNTER } + register_instrument(name) { NOOP_UP_DOWN_COUNTER } end # @param name [String] @@ -113,7 +113,7 @@ def create_up_down_counter(name, unit: nil, description: nil, advice: nil) # # @return [Instrument::ObservableCounter] def create_observable_counter(name, unit: nil, description: nil, callbacks: nil) - create_instrument(:observable_counter, name, unit, description, nil, callbacks) { NOOP_OBSERVABLE_COUNTER } + register_instrument(name) { NOOP_OBSERVABLE_COUNTER } end # @param name [String] @@ -134,7 +134,7 @@ def create_observable_counter(name, unit: nil, description: nil, callbacks: nil) # # @return [Instrument::ObservableGauge] def create_observable_gauge(name, unit: nil, description: nil, callbacks: nil) - create_instrument(:observable_gauge, name, unit, description, nil, callbacks) { NOOP_OBSERVABLE_GAUGE } + register_instrument(name) { NOOP_OBSERVABLE_GAUGE } end # @param name [String] @@ -155,12 +155,12 @@ def create_observable_gauge(name, unit: nil, description: nil, callbacks: nil) # # @return [Instrument::ObservableUpDownCounter] def create_observable_up_down_counter(name, unit: nil, description: nil, callbacks: nil) - create_instrument(:observable_up_down_counter, name, unit, description, nil, callbacks) { NOOP_OBSERVABLE_UP_DOWN_COUNTER } + register_instrument(name) { NOOP_OBSERVABLE_UP_DOWN_COUNTER } end private - def create_instrument(kind, name, unit, description, advice, callbacks) + def register_instrument(name) name = name.downcase @mutex.synchronize do diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/meter.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/meter.rb index 47ffad55b4..7e65c9d482 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/meter.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/meter.rb @@ -18,7 +18,7 @@ def add_metric_reader(metric_reader) end end - def create_instrument(kind, name, unit, description, callback) + def register_instrument(kind, name, unit, description, callback) super do case kind when :counter then OpenTelemetry::SDK::Metrics::Instrument::Counter.new(name, unit, description, @instrumentation_scope, @meter_provider) From f21c3031b5f4d0dd0c8fddbaa605e2a5bae444e2 Mon Sep 17 00:00:00 2001 From: Elias Rodrigues Date: Fri, 30 Jun 2023 19:26:22 -0300 Subject: [PATCH 11/32] Require ProxyInstrument files --- .../internal/proxy_instrument.rb | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 metrics_api/lib/opentelemetry/internal/proxy_instrument.rb diff --git a/metrics_api/lib/opentelemetry/internal/proxy_instrument.rb b/metrics_api/lib/opentelemetry/internal/proxy_instrument.rb new file mode 100644 index 0000000000..365446ad59 --- /dev/null +++ b/metrics_api/lib/opentelemetry/internal/proxy_instrument.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +# Copyright The OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +module OpenTelemetry + module Metrics + module ProxyInstrument + end + end +end + +require 'opentelemetry/internal/proxy_instrument/delegate_synchronous_instrument' +require 'opentelemetry/internal/proxy_instrument/counter' +require 'opentelemetry/internal/proxy_instrument/histogram' +require 'opentelemetry/internal/proxy_instrument/up_down_counter' + +require 'opentelemetry/internal/proxy_instrument/delegate_asynchronous_instrument' +require 'opentelemetry/internal/proxy_instrument/observable_counter' +require 'opentelemetry/internal/proxy_instrument/observable_gauge' +require 'opentelemetry/internal/proxy_instrument/observable_up_down_counter' From 30854b37d7fa0545515ee2d564243b7250cc159a Mon Sep 17 00:00:00 2001 From: Elias Rodrigues Date: Sat, 1 Jul 2023 16:19:23 -0300 Subject: [PATCH 12/32] Adjust SDK Meter to use register_instrument --- .../lib/opentelemetry/sdk/metrics/meter.rb | 81 ++++++++++++++++--- 1 file changed, 71 insertions(+), 10 deletions(-) diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/meter.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/meter.rb index 7e65c9d482..c0b1a8422d 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/meter.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/meter.rb @@ -18,16 +18,77 @@ def add_metric_reader(metric_reader) end end - def register_instrument(kind, name, unit, description, callback) - super do - case kind - when :counter then OpenTelemetry::SDK::Metrics::Instrument::Counter.new(name, unit, description, @instrumentation_scope, @meter_provider) - when :observable_counter then OpenTelemetry::SDK::Metrics::Instrument::ObservableCounter.new(name, unit, description, callback, @instrumentation_scope, @meter_provider) - when :histogram then OpenTelemetry::SDK::Metrics::Instrument::Histogram.new(name, unit, description, @instrumentation_scope, @meter_provider) - when :observable_gauge then OpenTelemetry::SDK::Metrics::Instrument::ObservableGauge.new(name, unit, description, callback, @instrumentation_scope, @meter_provider) - when :up_down_counter then OpenTelemetry::SDK::Metrics::Instrument::UpDownCounter.new(name, unit, description, @instrumentation_scope, @meter_provider) - when :observable_up_down_counter then OpenTelemetry::SDK::Metrics::Instrument::ObservableUpDownCounter.new(name, unit, description, callback, @instrumentation_scope, @meter_provider) - end + # TODO: refer yard doc comments to API + + def create_counter(name, unit: nil, description: nil, advice: nil) + register_instrument(name) do + SDK::Metrics::Instrument::Counter.new( + name, + unit: unit, + description: description, + advice: advice, + # @meter_provider # TODO: Check if we should pass meter/meter provider to Instrument + ) + end + end + + def create_histogram(name, unit: nil, description: nil, advice: nil) + register_instrument(name) do + SDK::Metrics::Instrument::Histogram.new( + name, + unit: unit, + description: description, + advice: advice, + # @meter_provider + ) + end + end + + def create_up_down_counter(name, unit: nil, description: nil, advice: nil) + register_instrument(name) do + SDK::Metrics::Instrument::UpDownCounter.new( + name, + unit: unit, + description: description, + advice: advice, + # @meter_provider + ) + end + end + + def create_observable_counter(name, unit: nil, description: nil, callbacks: nil) + register_instrument(name) do + SDK::Metrics::Instrument::ObservableCounter.new( + name, + unit: unit, + description: description, + callbacks: callbacks, + # @meter_provider + ) + end + end + + def create_observable_gauge(name, unit: nil, description: nil, callbacks: nil) + register_instrument(name) do + SDK::Metrics::Instrument::ObservableGauge.new( + name, + unit: unit, + description: description, + callbacks: callbacks, + # @meter_provider + ) + end + end + + def create_observable_up_down_counter(name, unit: nil, description: nil, callbacks: nil) + register_instrument(name) do + SDK::Metrics::Instrument::ObservableUpDownCounter.new( + name, + unit: unit, + description: description, + callbacks: callbacks, + # @meter_provider + ) end end end From 508e19f32ec76f975132784d2d9dd409f97aa6aa Mon Sep 17 00:00:00 2001 From: Elias Rodrigues Date: Sat, 1 Jul 2023 16:24:03 -0300 Subject: [PATCH 13/32] Add method #build_key_for_meter --- .../opentelemetry/metrics/meter_provider.rb | 2 +- .../sdk/metrics/meter_provider.rb | 24 ++++++++++--------- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/metrics_api/lib/opentelemetry/metrics/meter_provider.rb b/metrics_api/lib/opentelemetry/metrics/meter_provider.rb index 2e959162c7..3798f36c5f 100644 --- a/metrics_api/lib/opentelemetry/metrics/meter_provider.rb +++ b/metrics_api/lib/opentelemetry/metrics/meter_provider.rb @@ -19,8 +19,8 @@ def initialize(resource: nil) @resource = resource @mutex = Mutex.new - @meter_registry = {} @stopped = false + @meter_registry = {} @metric_readers = [] end diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb index 0534475782..47e946d73d 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb @@ -32,21 +32,13 @@ def meter(name, version: nil, schema_url: nil, attributes: nil) NOOP_METER else - if name.nil? || name.empty? - OpenTelemetry.logger.warn 'Invalid name provided to MeterProvider#meter: nil or empty' - end - - # TODO: Check if we should pass self to Meter.new + key = build_key_for_meter(name, version, schema_url) meter = Meter.new( name, version: version, schema_url: schema_url, - attributes: attributes - ) - key = Key.new( - meter.name, - meter.version, - meter.schema_url + attributes: attributes, + # TODO: Check if we should pass self to Meter.new ) @mutex.synchronize do @@ -148,6 +140,16 @@ def register_synchronous_instrument(instrument) def add_view # TODO: For each meter add this view to all applicable instruments end + + private + + def build_key_for_meter(name, version, schema_url) + if name.nil? || name.empty? + OpenTelemetry.logger.warn 'Invalid name provided to MeterProvider#meter: nil or empty' + end + + Key.new(name, version, schema_url) + end end end end From 5db4749c9265c280a3ac6f440f74a0846a954d4f Mon Sep 17 00:00:00 2001 From: Elias Rodrigues Date: Sat, 1 Jul 2023 17:20:17 -0300 Subject: [PATCH 14/32] Remove SDK's SynchronousInstrument class --- .../instrument/synchronous_instrument.rb | 20 ++++++++ .../lib/opentelemetry/metrics/meter.rb | 17 ++++++- .../opentelemetry/metrics/meter_provider.rb | 2 +- .../instrument/synchronous_instrument.rb | 49 ------------------- .../lib/opentelemetry/sdk/metrics/meter.rb | 7 --- .../sdk/metrics/meter_provider.rb | 33 ++++++++----- 6 files changed, 59 insertions(+), 69 deletions(-) delete mode 100644 metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/synchronous_instrument.rb diff --git a/metrics_api/lib/opentelemetry/metrics/instrument/synchronous_instrument.rb b/metrics_api/lib/opentelemetry/metrics/instrument/synchronous_instrument.rb index 7452fec049..dc1d3c141a 100644 --- a/metrics_api/lib/opentelemetry/metrics/instrument/synchronous_instrument.rb +++ b/metrics_api/lib/opentelemetry/metrics/instrument/synchronous_instrument.rb @@ -17,6 +17,26 @@ def initialize(name, unit: nil, description: nil, advice: nil) @unit = unit || '' @description = description || '' @advice = advice || {} + + @mutex = Mutex.enw + @metric_streams = [] + end + + # @api private + def add_metric_stream(metric_stream) + @mutex.synchronize do + @metric_streams.push(metric_stream) + end + end + + private + + def update(value, attributes) + @mutex.synchronize do + @metric_streams.each do |metric_stream| + metric_stream.update(value, attributes) + end + end end end end diff --git a/metrics_api/lib/opentelemetry/metrics/meter.rb b/metrics_api/lib/opentelemetry/metrics/meter.rb index 31cf66381c..ec59a38211 100644 --- a/metrics_api/lib/opentelemetry/metrics/meter.rb +++ b/metrics_api/lib/opentelemetry/metrics/meter.rb @@ -24,7 +24,13 @@ class Meter :NOOP_OBSERVABLE_UP_DOWN_COUNTER ) - attr_reader :name, :version, :schema_url, :attributes + attr_reader( + :name, + :version, + :schema_url, + :attributes, + :instrumentation_scope + ) # @api private def initialize(name, version: nil, schema_url: nil, attributes: nil) @@ -158,6 +164,15 @@ def create_observable_up_down_counter(name, unit: nil, description: nil, callbac register_instrument(name) { NOOP_OBSERVABLE_UP_DOWN_COUNTER } end + # @api private + def each_instrument + @mutex.synchronize do + instrument_registry.each do |name, instrument| + yield(name, instrument) + end + end + end + private def register_instrument(name) diff --git a/metrics_api/lib/opentelemetry/metrics/meter_provider.rb b/metrics_api/lib/opentelemetry/metrics/meter_provider.rb index 3798f36c5f..d0a4aab683 100644 --- a/metrics_api/lib/opentelemetry/metrics/meter_provider.rb +++ b/metrics_api/lib/opentelemetry/metrics/meter_provider.rb @@ -20,8 +20,8 @@ def initialize(resource: nil) @mutex = Mutex.new @stopped = false - @meter_registry = {} @metric_readers = [] + @meter_registry = {} end # @param [String] name diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/synchronous_instrument.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/synchronous_instrument.rb deleted file mode 100644 index f5bf321f0a..0000000000 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/synchronous_instrument.rb +++ /dev/null @@ -1,49 +0,0 @@ -# frozen_string_literal: true - -# Copyright The OpenTelemetry Authors -# -# SPDX-License-Identifier: Apache-2.0 - -module OpenTelemetry - module SDK - module Metrics - module Instrument - # {SynchronousInstrument} contains the common functionality shared across - # the synchronous instruments SDK instruments. - class SynchronousInstrument - def initialize(name, unit, description, instrumentation_scope, meter_provider) - @name = name - @unit = unit - @description = description - @instrumentation_scope = instrumentation_scope - @meter_provider = meter_provider - @metric_streams = [] - - meter_provider.register_synchronous_instrument(self) - end - - # @api private - def register_with_new_metric_store(metric_store, aggregation: default_aggregation) - ms = OpenTelemetry::SDK::Metrics::State::MetricStream.new( - @name, - @description, - @unit, - instrument_kind, - @meter_provider, - @instrumentation_scope, - aggregation - ) - @metric_streams << ms - metric_store.add_metric_stream(ms) - end - - private - - def update(value, attributes) - @metric_streams.each { |ms| ms.update(value, attributes) } - end - end - end - end - end -end diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/meter.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/meter.rb index c0b1a8422d..c8f8ecf492 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/meter.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/meter.rb @@ -11,13 +11,6 @@ module SDK module Metrics # {Meter} is the SDK implementation of {OpenTelemetry::Metrics::Meter}. class Meter < OpenTelemetry::Metrics::Meter - # @api private - def add_metric_reader(metric_reader) - @instrument_registry.each do |_n, instrument| - instrument.register_with_new_metric_store(metric_reader.metric_store) - end - end - # TODO: refer yard doc comments to API def create_counter(name, unit: nil, description: nil, advice: nil) diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb index 47e946d73d..0081c4e50d 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb @@ -110,25 +110,24 @@ def force_flush(timeout: nil) # Adds a new MetricReader to this {MeterProvider}. # # @param metric_reader the new MetricReader to be added. - def add_metric_reader(metric_reader) + def add_metric_reader(metric_reader, aggregation: default_aggregation) @mutex.synchronize do if @stopped OpenTelemetry.logger.warn('calling MetricProvider#add_metric_reader after shutdown.') else @metric_readers.push(metric_reader) - @meter_registry.each_value { |meter| meter.add_metric_reader(metric_reader) } - end - nil - end - end + @meter_registry.each_value do |meter| + meter.each_instrument do |_name, instrument| + metric_stream = build_metric_stream(meter, instrument, aggregation) - # @api private - def register_synchronous_instrument(instrument) - @mutex.synchronize do - @metric_readers.each do |mr| - instrument.register_with_new_metric_store(mr.metric_store) + instrument.add_metric_stream(metric_stream) + metric_reader.metric_store.add_metric_stream(metric_stream) + end + end end + + nil end end @@ -150,6 +149,18 @@ def build_key_for_meter(name, version, schema_url) Key.new(name, version, schema_url) end + + def build_metric_stream(meter, instrument, aggregation) + SDK::Metrics::State::MetricStream.new( + instrument.name, + instrument.description, + instrument.unit, + instrument.kind, + self, + meter.instrumentation_scope, + aggregation + ) + end end end end From d8d929be50b053bdf0c22cc19c2f2e4db9b9b5fd Mon Sep 17 00:00:00 2001 From: Elias Rodrigues Date: Sat, 1 Jul 2023 17:23:23 -0300 Subject: [PATCH 15/32] Add #kind method to instruments --- metrics_api/lib/opentelemetry/metrics/instrument/counter.rb | 4 ++++ metrics_api/lib/opentelemetry/metrics/instrument/histogram.rb | 4 ++++ .../opentelemetry/metrics/instrument/observable_counter.rb | 3 +++ .../lib/opentelemetry/metrics/instrument/observable_gauge.rb | 3 +++ .../metrics/instrument/observable_up_down_counter.rb | 3 +++ .../lib/opentelemetry/metrics/instrument/up_down_counter.rb | 4 ++++ 6 files changed, 21 insertions(+) diff --git a/metrics_api/lib/opentelemetry/metrics/instrument/counter.rb b/metrics_api/lib/opentelemetry/metrics/instrument/counter.rb index e3ac20c278..d9d5e42b2c 100644 --- a/metrics_api/lib/opentelemetry/metrics/instrument/counter.rb +++ b/metrics_api/lib/opentelemetry/metrics/instrument/counter.rb @@ -9,6 +9,10 @@ module Metrics module Instrument # No-op implementation of Counter. class Counter < SynchronousInstrument + def kind + :counter + end + # Increment the Counter by a fixed amount. # # @param [Numeric] increment The increment amount, which MUST be a non-negative numeric value. diff --git a/metrics_api/lib/opentelemetry/metrics/instrument/histogram.rb b/metrics_api/lib/opentelemetry/metrics/instrument/histogram.rb index 1b514094e0..dfab54bafa 100644 --- a/metrics_api/lib/opentelemetry/metrics/instrument/histogram.rb +++ b/metrics_api/lib/opentelemetry/metrics/instrument/histogram.rb @@ -9,6 +9,10 @@ module Metrics module Instrument # No-op implementation of Histogram. class Histogram < SynchronousInstrument + def kind + :histogram + end + # Updates the statistics with the specified amount. # # @param [Numeric] amount The amount of the Measurement, which MUST be a non-negative numeric value. diff --git a/metrics_api/lib/opentelemetry/metrics/instrument/observable_counter.rb b/metrics_api/lib/opentelemetry/metrics/instrument/observable_counter.rb index 34d63d3969..bee63005c7 100644 --- a/metrics_api/lib/opentelemetry/metrics/instrument/observable_counter.rb +++ b/metrics_api/lib/opentelemetry/metrics/instrument/observable_counter.rb @@ -9,6 +9,9 @@ module Metrics module Instrument # No-op implementation of ObservableCounter. class ObservableCounter < AsynchronousInstrument + def kind + :observable_counter + end end end end diff --git a/metrics_api/lib/opentelemetry/metrics/instrument/observable_gauge.rb b/metrics_api/lib/opentelemetry/metrics/instrument/observable_gauge.rb index a604775959..117511aff6 100644 --- a/metrics_api/lib/opentelemetry/metrics/instrument/observable_gauge.rb +++ b/metrics_api/lib/opentelemetry/metrics/instrument/observable_gauge.rb @@ -9,6 +9,9 @@ module Metrics module Instrument # No-op implementation of ObservableGauge. class ObservableGauge < AsynchronousInstrument + def kind + :observable_gauge + end end end end diff --git a/metrics_api/lib/opentelemetry/metrics/instrument/observable_up_down_counter.rb b/metrics_api/lib/opentelemetry/metrics/instrument/observable_up_down_counter.rb index 8f685906c0..9c44230295 100644 --- a/metrics_api/lib/opentelemetry/metrics/instrument/observable_up_down_counter.rb +++ b/metrics_api/lib/opentelemetry/metrics/instrument/observable_up_down_counter.rb @@ -9,6 +9,9 @@ module Metrics module Instrument # No-op implementation of ObservableUpDownCounter. class ObservableUpDownCounter < AsynchronousInstrument + def kind + :observable_up_down_counter + end end end end diff --git a/metrics_api/lib/opentelemetry/metrics/instrument/up_down_counter.rb b/metrics_api/lib/opentelemetry/metrics/instrument/up_down_counter.rb index dc90493504..e4984a3240 100644 --- a/metrics_api/lib/opentelemetry/metrics/instrument/up_down_counter.rb +++ b/metrics_api/lib/opentelemetry/metrics/instrument/up_down_counter.rb @@ -9,6 +9,10 @@ module Metrics module Instrument # No-op implementation of UpDownCounter. class UpDownCounter < SynchronousInstrument + def kind + :up_down_counter + end + # Increment or decrement the UpDownCounter by a fixed amount. # # @param [Numeric] amount The amount to be added, can be positive, negative or zero. From f7041b41fab87dc1fb3b6df826288279b42a0075 Mon Sep 17 00:00:00 2001 From: Elias Rodrigues Date: Sat, 1 Jul 2023 17:24:28 -0300 Subject: [PATCH 16/32] Not passing meter_provider down to meter and instruments --- .../lib/opentelemetry/sdk/metrics/meter.rb | 18 ++++++------------ .../sdk/metrics/meter_provider.rb | 3 +-- 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/meter.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/meter.rb index c8f8ecf492..c8e534d563 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/meter.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/meter.rb @@ -19,8 +19,7 @@ def create_counter(name, unit: nil, description: nil, advice: nil) name, unit: unit, description: description, - advice: advice, - # @meter_provider # TODO: Check if we should pass meter/meter provider to Instrument + advice: advice ) end end @@ -31,8 +30,7 @@ def create_histogram(name, unit: nil, description: nil, advice: nil) name, unit: unit, description: description, - advice: advice, - # @meter_provider + advice: advice ) end end @@ -43,8 +41,7 @@ def create_up_down_counter(name, unit: nil, description: nil, advice: nil) name, unit: unit, description: description, - advice: advice, - # @meter_provider + advice: advice ) end end @@ -55,8 +52,7 @@ def create_observable_counter(name, unit: nil, description: nil, callbacks: nil) name, unit: unit, description: description, - callbacks: callbacks, - # @meter_provider + callbacks: callbacks ) end end @@ -67,8 +63,7 @@ def create_observable_gauge(name, unit: nil, description: nil, callbacks: nil) name, unit: unit, description: description, - callbacks: callbacks, - # @meter_provider + callbacks: callbacks ) end end @@ -79,8 +74,7 @@ def create_observable_up_down_counter(name, unit: nil, description: nil, callbac name, unit: unit, description: description, - callbacks: callbacks, - # @meter_provider + callbacks: callbacks ) end end diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb index 0081c4e50d..f4c7d3c35d 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb @@ -37,8 +37,7 @@ def meter(name, version: nil, schema_url: nil, attributes: nil) name, version: version, schema_url: schema_url, - attributes: attributes, - # TODO: Check if we should pass self to Meter.new + attributes: attributes ) @mutex.synchronize do From d442a9d07fa47e790df7496c0c0ba60fc4f9e2f8 Mon Sep 17 00:00:00 2001 From: Elias Rodrigues Date: Sat, 1 Jul 2023 17:31:59 -0300 Subject: [PATCH 17/32] Move default_aggregation to MeterProvider --- .../sdk/metrics/instrument/counter.rb | 6 ------ .../sdk/metrics/instrument/histogram.rb | 6 ------ .../sdk/metrics/instrument/up_down_counter.rb | 6 ------ .../sdk/metrics/meter_provider.rb | 21 ++++++++++++++++++- 4 files changed, 20 insertions(+), 19 deletions(-) diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/counter.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/counter.rb index 6af04b1937..2a42f8f29b 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/counter.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/counter.rb @@ -38,12 +38,6 @@ def add(increment, attributes: {}) OpenTelemetry.handle_error(exception: e) nil end - - private - - def default_aggregation - OpenTelemetry::SDK::Metrics::Aggregation::Sum.new - end end end end diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/histogram.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/histogram.rb index 9cdcf60d80..fcdf2ef134 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/histogram.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/histogram.rb @@ -31,12 +31,6 @@ def record(amount, attributes: nil) OpenTelemetry.handle_error(exception: e) nil end - - private - - def default_aggregation - OpenTelemetry::SDK::Metrics::Aggregation::ExplicitBucketHistogram.new - end end end end diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/up_down_counter.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/up_down_counter.rb index cf2dc0d8b4..0252eef66d 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/up_down_counter.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/up_down_counter.rb @@ -31,12 +31,6 @@ def add(amount, attributes: nil) OpenTelemetry.handle_error(exception: e) nil end - - private - - def default_aggregation - OpenTelemetry::SDK::Metrics::Aggregation::Sum.new - end 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 index f4c7d3c35d..9f354a674a 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb @@ -109,7 +109,7 @@ def force_flush(timeout: nil) # Adds a new MetricReader to this {MeterProvider}. # # @param metric_reader the new MetricReader to be added. - def add_metric_reader(metric_reader, aggregation: default_aggregation) + def add_metric_reader(metric_reader, aggregation: nil) @mutex.synchronize do if @stopped OpenTelemetry.logger.warn('calling MetricProvider#add_metric_reader after shutdown.') @@ -150,6 +150,8 @@ def build_key_for_meter(name, version, schema_url) end def build_metric_stream(meter, instrument, aggregation) + aggregation ||= default_aggregation_for(instrument) + SDK::Metrics::State::MetricStream.new( instrument.name, instrument.description, @@ -160,6 +162,23 @@ def build_metric_stream(meter, instrument, aggregation) aggregation ) end + + def build_default_aggregation_for(instrument) + case instrument + when Counter + Aggregation::Sum.new + when Histogram + Aggregation::ExplicitBucketHistogram.new + when UpDownCounter + Aggregation::Sum.new + when ObservableCounter + # TODO: ? + when ObservableGauge + # TODO: ? + when ObservableUpDownCounter + # TODO: ? + end + end end end end From 94bcd68b43bed38828c80c4fd328b36afc51fedc Mon Sep 17 00:00:00 2001 From: Elias Rodrigues Date: Sat, 1 Jul 2023 17:33:31 -0300 Subject: [PATCH 18/32] Make SDK metric Instruments inherit from API Instruments --- metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/counter.rb | 2 +- .../lib/opentelemetry/sdk/metrics/instrument/histogram.rb | 2 +- .../lib/opentelemetry/sdk/metrics/instrument/up_down_counter.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/counter.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/counter.rb index 2a42f8f29b..b910856d3f 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/counter.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/counter.rb @@ -9,7 +9,7 @@ module SDK module Metrics module Instrument # {Counter} is the SDK implementation of {OpenTelemetry::Metrics::Counter}. - class Counter < OpenTelemetry::SDK::Metrics::Instrument::SynchronousInstrument + class Counter < OpenTelemetry::Metrics::Instrument::Counter # Returns the instrument kind as a Symbol # # @return [Symbol] diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/histogram.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/histogram.rb index fcdf2ef134..fa01e39667 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/histogram.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/histogram.rb @@ -9,7 +9,7 @@ module SDK module Metrics module Instrument # {Histogram} is the SDK implementation of {OpenTelemetry::Metrics::Histogram}. - class Histogram < OpenTelemetry::SDK::Metrics::Instrument::SynchronousInstrument + class Histogram < OpenTelemetry::Metrics::Instrument::Histogram # Returns the instrument kind as a Symbol # # @return [Symbol] diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/up_down_counter.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/up_down_counter.rb index 0252eef66d..aeb153390b 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/up_down_counter.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/up_down_counter.rb @@ -9,7 +9,7 @@ module SDK module Metrics module Instrument # {UpDownCounter} is the SDK implementation of {OpenTelemetry::Metrics::UpDownCounter}. - class UpDownCounter < OpenTelemetry::SDK::Metrics::Instrument::SynchronousInstrument + class UpDownCounter < OpenTelemetry::Metrics::Instrument::UpDownCounter # Returns the instrument kind as a Symbol # # @return [Symbol] From 7590c1d651ba8b6757593b962d35cd6c774af44d Mon Sep 17 00:00:00 2001 From: Elias Rodrigues Date: Sat, 1 Jul 2023 17:34:48 -0300 Subject: [PATCH 19/32] Remove initialize methods from SDK async instruments And instrument_kind methods --- .../lib/opentelemetry/sdk/metrics/instrument/counter.rb | 7 ------- .../opentelemetry/sdk/metrics/instrument/histogram.rb | 7 ------- .../sdk/metrics/instrument/observable_counter.rb | 9 --------- .../sdk/metrics/instrument/observable_gauge.rb | 9 --------- .../sdk/metrics/instrument/observable_up_down_counter.rb | 9 --------- .../sdk/metrics/instrument/up_down_counter.rb | 7 ------- 6 files changed, 48 deletions(-) diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/counter.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/counter.rb index b910856d3f..b19f193d4b 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/counter.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/counter.rb @@ -10,13 +10,6 @@ module Metrics module Instrument # {Counter} is the SDK implementation of {OpenTelemetry::Metrics::Counter}. class Counter < OpenTelemetry::Metrics::Instrument::Counter - # Returns the instrument kind as a Symbol - # - # @return [Symbol] - def instrument_kind - :counter - end - # Increment the Counter by a fixed amount. # # @param [numeric] increment The increment amount, which MUST be a non-negative numeric value. diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/histogram.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/histogram.rb index fa01e39667..dd9629ee1e 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/histogram.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/histogram.rb @@ -10,13 +10,6 @@ module Metrics module Instrument # {Histogram} is the SDK implementation of {OpenTelemetry::Metrics::Histogram}. class Histogram < OpenTelemetry::Metrics::Instrument::Histogram - # Returns the instrument kind as a Symbol - # - # @return [Symbol] - def instrument_kind - :histogram - end - # Updates the statistics with the specified amount. # # @param [numeric] amount The amount of the Measurement, which MUST be a non-negative numeric value. diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/observable_counter.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/observable_counter.rb index 4e5d49102c..bba6f478fb 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/observable_counter.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/observable_counter.rb @@ -10,15 +10,6 @@ module Metrics module Instrument # {ObservableCounter} is the SDK implementation of {OpenTelemetry::Metrics::ObservableCounter}. class ObservableCounter < OpenTelemetry::Metrics::Instrument::ObservableCounter - attr_reader :name, :unit, :description - - def initialize(name, unit, description, callback, meter) - @name = name - @unit = unit - @description = description - @callback = callback - @meter = meter - end end end end diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/observable_gauge.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/observable_gauge.rb index 7a122184f4..c1240b4028 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/observable_gauge.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/observable_gauge.rb @@ -10,15 +10,6 @@ module Metrics module Instrument # {ObservableGauge} is the SDK implementation of {OpenTelemetry::Metrics::ObservableGauge}. class ObservableGauge < OpenTelemetry::Metrics::Instrument::ObservableGauge - attr_reader :name, :unit, :description - - def initialize(name, unit, description, callback, meter) - @name = name - @unit = unit - @description = description - @callback = callback - @meter = meter - end end end end diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/observable_up_down_counter.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/observable_up_down_counter.rb index 8eb812c5fb..b41325efe7 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/observable_up_down_counter.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/observable_up_down_counter.rb @@ -10,15 +10,6 @@ module Metrics module Instrument # {ObservableUpDownCounter} is the SDK implementation of {OpenTelemetry::Metrics::ObservableUpDownCounter}. class ObservableUpDownCounter < OpenTelemetry::Metrics::Instrument::ObservableUpDownCounter - attr_reader :name, :unit, :description - - def initialize(name, unit, description, callback, meter) - @name = name - @unit = unit - @description = description - @callback = callback - @meter = meter - end end end end diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/up_down_counter.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/up_down_counter.rb index aeb153390b..38c9636f2e 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/up_down_counter.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/up_down_counter.rb @@ -10,13 +10,6 @@ module Metrics module Instrument # {UpDownCounter} is the SDK implementation of {OpenTelemetry::Metrics::UpDownCounter}. class UpDownCounter < OpenTelemetry::Metrics::Instrument::UpDownCounter - # Returns the instrument kind as a Symbol - # - # @return [Symbol] - def instrument_kind - :up_down_counter - end - # Increment or decrement the UpDownCounter by a fixed amount. # # @param [Numeric] amount The amount to be added, can be positive, negative or zero. From 838c51635d1dd068283973e464e750c0582ce75a Mon Sep 17 00:00:00 2001 From: Elias Rodrigues Date: Sat, 1 Jul 2023 17:38:15 -0300 Subject: [PATCH 20/32] Remove require for SDK synchronous_instrument --- metrics_sdk/lib/opentelemetry/sdk/metrics/instrument.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument.rb index c440634c8c..5d91f95bad 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument.rb @@ -13,10 +13,10 @@ module Instrument end end -require 'opentelemetry/sdk/metrics/instrument/synchronous_instrument' require 'opentelemetry/sdk/metrics/instrument/counter' require 'opentelemetry/sdk/metrics/instrument/histogram' +require 'opentelemetry/sdk/metrics/instrument/up_down_counter' + require 'opentelemetry/sdk/metrics/instrument/observable_counter' require 'opentelemetry/sdk/metrics/instrument/observable_gauge' require 'opentelemetry/sdk/metrics/instrument/observable_up_down_counter' -require 'opentelemetry/sdk/metrics/instrument/up_down_counter' From a216f5173d7a531cb16b2d88febfd09615ce93e4 Mon Sep 17 00:00:00 2001 From: Elias Rodrigues Date: Fri, 7 Jul 2023 14:56:09 -0300 Subject: [PATCH 21/32] Add comment with agregation link to docs --- metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb index 9f354a674a..698ede6f96 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb @@ -163,6 +163,7 @@ def build_metric_stream(meter, instrument, aggregation) ) end + # https://opentelemetry.io/docs/specs/otel/metrics/sdk/#default-aggregation def build_default_aggregation_for(instrument) case instrument when Counter From 06b3c01958567e1ceb9f707486b96a96292ef68c Mon Sep 17 00:00:00 2001 From: Elias Rodrigues Date: Fri, 7 Jul 2023 15:31:23 -0300 Subject: [PATCH 22/32] Keep InstrumentationScope in SDK --- .../metrics/instrument/synchronous_instrument.rb | 2 +- metrics_api/lib/opentelemetry/metrics/meter.rb | 9 ++------- metrics_sdk/lib/opentelemetry/sdk/metrics/meter.rb | 11 +++++++++++ 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/metrics_api/lib/opentelemetry/metrics/instrument/synchronous_instrument.rb b/metrics_api/lib/opentelemetry/metrics/instrument/synchronous_instrument.rb index dc1d3c141a..a533716afa 100644 --- a/metrics_api/lib/opentelemetry/metrics/instrument/synchronous_instrument.rb +++ b/metrics_api/lib/opentelemetry/metrics/instrument/synchronous_instrument.rb @@ -18,7 +18,7 @@ def initialize(name, unit: nil, description: nil, advice: nil) @description = description || '' @advice = advice || {} - @mutex = Mutex.enw + @mutex = Mutex.new @metric_streams = [] end diff --git a/metrics_api/lib/opentelemetry/metrics/meter.rb b/metrics_api/lib/opentelemetry/metrics/meter.rb index ec59a38211..7212ba5c60 100644 --- a/metrics_api/lib/opentelemetry/metrics/meter.rb +++ b/metrics_api/lib/opentelemetry/metrics/meter.rb @@ -39,15 +39,10 @@ def initialize(name, version: nil, schema_url: nil, attributes: nil) @schema_url = schema_url || '' @attributes = attributes || {} - @instrumentation_scope = InstrumentationScope.new( - @name, - @version, - @schema_url, - @attributes - ) - @mutex = Mutex.new @instrument_registry = {} + + @instrumentation_scope = nil end # @param name [String] diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/meter.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/meter.rb index c8e534d563..1a21ac1659 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/meter.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/meter.rb @@ -13,6 +13,17 @@ module Metrics class Meter < OpenTelemetry::Metrics::Meter # TODO: refer yard doc comments to API + def initialize(*args, **kwargs) + super + + @instrumentation_scope = InstrumentationScope.new( + @name, + @version, + @schema_url, + @attributes + ) + end + def create_counter(name, unit: nil, description: nil, advice: nil) register_instrument(name) do SDK::Metrics::Instrument::Counter.new( From a2ebb481bef0945c5129e365f68015e9334099dc Mon Sep 17 00:00:00 2001 From: Elias Rodrigues Date: Fri, 7 Jul 2023 15:31:40 -0300 Subject: [PATCH 23/32] Add mutex for @delegate --- .../internal/proxy_instrument/counter.rb | 10 ++++---- .../delegate_asynchronous_instrument.rb | 24 +++++++++++-------- .../delegate_synchronous_instrument.rb | 4 ++-- .../internal/proxy_instrument/histogram.rb | 10 ++++---- .../proxy_instrument/up_down_counter.rb | 10 ++++---- .../lib/opentelemetry/internal/proxy_meter.rb | 15 ++++++------ .../internal/proxy_meter_provider.rb | 5 ++-- .../sdk/metrics/meter_provider.rb | 2 +- .../test/opentelemetry/metrics_sdk_test.rb | 2 +- 9 files changed, 47 insertions(+), 35 deletions(-) diff --git a/metrics_api/lib/opentelemetry/internal/proxy_instrument/counter.rb b/metrics_api/lib/opentelemetry/internal/proxy_instrument/counter.rb index e6cc612bb5..a44cfd5795 100644 --- a/metrics_api/lib/opentelemetry/internal/proxy_instrument/counter.rb +++ b/metrics_api/lib/opentelemetry/internal/proxy_instrument/counter.rb @@ -12,10 +12,12 @@ class Counter < Metrics::Instrument::Counter include DelegateSynchronousInstrument def add(increment, attributes: nil) - if @delegate.nil? - super - else - @delegate.add(increment, attributes: attributes) + @delegate_mutex.synchronize do + if @delegate.nil? + super + else + @delegate.add(increment, attributes: attributes) + end end end end diff --git a/metrics_api/lib/opentelemetry/internal/proxy_instrument/delegate_asynchronous_instrument.rb b/metrics_api/lib/opentelemetry/internal/proxy_instrument/delegate_asynchronous_instrument.rb index 605b65e184..021682e08b 100644 --- a/metrics_api/lib/opentelemetry/internal/proxy_instrument/delegate_asynchronous_instrument.rb +++ b/metrics_api/lib/opentelemetry/internal/proxy_instrument/delegate_asynchronous_instrument.rb @@ -12,12 +12,12 @@ module DelegateAsynchronousInstrument def initialize(*args, **kwargs) super - @mutex = Mutex.new + @delegate_mutex = Mutex.new @delegate = nil end def delegate=(instrument) - @mutex.synchronize do + @delegate_mutex.synchronize do if @delegate.nil? @delegate = instrument else @@ -29,18 +29,22 @@ def delegate=(instrument) end def register_callbacks(*callbacks) - if @delegate.nil? - super - else - @delegate.register_callbacks(*callbacks) + @delegate_mutex.synchronize do + if @delegate.nil? + super + else + @delegate.register_callbacks(*callbacks) + end end end def unregister_callbacks(*callbacks) - if @delegate.nil? - super - else - @delegate.unregister_callbacks(*callbacks) + @delegate_mutex.synchronize do + if @delegate.nil? + super + else + @delegate.unregister_callbacks(*callbacks) + end end end end diff --git a/metrics_api/lib/opentelemetry/internal/proxy_instrument/delegate_synchronous_instrument.rb b/metrics_api/lib/opentelemetry/internal/proxy_instrument/delegate_synchronous_instrument.rb index 5914701d2f..6f89a2fd6c 100644 --- a/metrics_api/lib/opentelemetry/internal/proxy_instrument/delegate_synchronous_instrument.rb +++ b/metrics_api/lib/opentelemetry/internal/proxy_instrument/delegate_synchronous_instrument.rb @@ -12,12 +12,12 @@ module DelegateSynchronousInstrument def initialize(*args, **kwargs) super - @mutex = Mutex.new + @delegate_mutex = Mutex.new @delegate = nil end def delegate=(instrument) - @mutex.synchronize do + @delegate_mutex.synchronize do if @delegate.nil? @delegate = instrument else diff --git a/metrics_api/lib/opentelemetry/internal/proxy_instrument/histogram.rb b/metrics_api/lib/opentelemetry/internal/proxy_instrument/histogram.rb index 0ab071bc4a..fd4e09b748 100644 --- a/metrics_api/lib/opentelemetry/internal/proxy_instrument/histogram.rb +++ b/metrics_api/lib/opentelemetry/internal/proxy_instrument/histogram.rb @@ -12,10 +12,12 @@ class Histogram < Metrics::Instrument::Histogram include DelegateSynchronousInstrument def record(amount, attributes: nil) - if @delegate.nil? - super - else - @delegate.record(amount, attributes: attributes) + @delegate_mutex.synchronize do + if @delegate.nil? + super + else + @delegate.record(amount, attributes: attributes) + end end end end diff --git a/metrics_api/lib/opentelemetry/internal/proxy_instrument/up_down_counter.rb b/metrics_api/lib/opentelemetry/internal/proxy_instrument/up_down_counter.rb index a39eabcd85..0df4f18265 100644 --- a/metrics_api/lib/opentelemetry/internal/proxy_instrument/up_down_counter.rb +++ b/metrics_api/lib/opentelemetry/internal/proxy_instrument/up_down_counter.rb @@ -12,10 +12,12 @@ class UpDownCounter < Metrics::Instrument::UpDownCounter include DelegateSynchronousInstrument def add(amount, attributes: nil) - if @delegate.nil? - super - else - @delegate.add(amount, attributes: attributes) + @delegate_mutex.synchronize do + if @delegate.nil? + super + else + @delegate.add(amount, attributes: attributes) + end end end end diff --git a/metrics_api/lib/opentelemetry/internal/proxy_meter.rb b/metrics_api/lib/opentelemetry/internal/proxy_meter.rb index cb1392e480..da66e35940 100644 --- a/metrics_api/lib/opentelemetry/internal/proxy_meter.rb +++ b/metrics_api/lib/opentelemetry/internal/proxy_meter.rb @@ -16,6 +16,7 @@ class ProxyMeter < Metrics::Meter def initialize(*args, **kwargs) super + @delegate_mutex = Mutex.new @delegate = nil end @@ -24,7 +25,7 @@ def initialize(*args, **kwargs) # # @param [Meter] meter The Meter to delegate to def delegate=(meter) - @mutex.synchronize do + @delegate_mutex.synchronize do if @delegate.nil? @delegate = meter @@ -83,7 +84,7 @@ def delegate=(meter) def create_counter(name, unit: nil, description: nil, advice: nil) register_instrument(name) do - @mutex.synchronize do + @delegate_mutex.synchronize do if @delegate.nil? ProxyInstrument::Counter.new( name, @@ -105,7 +106,7 @@ def create_counter(name, unit: nil, description: nil, advice: nil) def create_histogram(name, unit: nil, description: nil, advice: nil) register_instrument(name) do - @mutex.synchronize do + @delegate_mutex.synchronize do if @delegate.nil? ProxyInstrument::Histogram.new( name, @@ -127,7 +128,7 @@ def create_histogram(name, unit: nil, description: nil, advice: nil) def create_up_down_counter(name, unit: nil, description: nil, advice: nil) register_instrument(name) do - @mutex.synchronize do + @delegate_mutex.synchronize do if @delegate.nil? ProxyInstrument::UpDownCounter.new( name, @@ -149,7 +150,7 @@ def create_up_down_counter(name, unit: nil, description: nil, advice: nil) def create_observable_counter(name, unit: nil, description: nil, callbacks: nil) register_instrument(name) do - @mutex.synchronize do + @delegate_mutex.synchronize do if @delegate.nil? ProxyInstrument::ObservableCounter.new( name, @@ -171,7 +172,7 @@ def create_observable_counter(name, unit: nil, description: nil, callbacks: nil) def create_observable_gauge(name, unit: nil, description: nil, callbacks: nil) register_instrument(name) do - @mutex.synchronize do + @delegate_mutex.synchronize do if @delegate.nil? ProxyInstrument::ObservableGauge.new( name, @@ -193,7 +194,7 @@ def create_observable_gauge(name, unit: nil, description: nil, callbacks: nil) def create_observable_up_down_counter(name, unit: nil, description: nil, callbacks: nil) register_instrument(name) do - @mutex.synchronize do + @delegate_mutex.synchronize do if @delegate.nil? ProxyInstrument::ObservableUpDownCounter.new( name, diff --git a/metrics_api/lib/opentelemetry/internal/proxy_meter_provider.rb b/metrics_api/lib/opentelemetry/internal/proxy_meter_provider.rb index 76c2bcf181..99a122ee8b 100644 --- a/metrics_api/lib/opentelemetry/internal/proxy_meter_provider.rb +++ b/metrics_api/lib/opentelemetry/internal/proxy_meter_provider.rb @@ -16,6 +16,7 @@ class ProxyMeterProvider < Metrics::MeterProvider def initialize(*args, **kwargs) super + @delegate_mutex = Mutex.new @delegate = nil end @@ -24,7 +25,7 @@ def initialize(*args, **kwargs) # # @param [MeterProvider] meter_provider The Meter provider to delegate to def delegate=(meter_provider) - @mutex.synchronize do + @delegate_mutex.synchronize do if @delegate.nil? @delegate = meter_provider @@ -44,7 +45,7 @@ def delegate=(meter_provider) # @api private def meter(name, version: nil, schema_url: nil, attributes: nil) - @mutex.synchronize do + @delegate_mutex.synchronize do if @delegate.nil? proxy_meter = ProxyMeter.new( name, diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb index 698ede6f96..52083aecd7 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb @@ -159,7 +159,7 @@ def build_metric_stream(meter, instrument, aggregation) instrument.kind, self, meter.instrumentation_scope, - aggregation + aggregation || build_default_aggregation_for(instrument) ) end diff --git a/metrics_sdk/test/opentelemetry/metrics_sdk_test.rb b/metrics_sdk/test/opentelemetry/metrics_sdk_test.rb index 62a9ecf5c6..2911a6ab3b 100644 --- a/metrics_sdk/test/opentelemetry/metrics_sdk_test.rb +++ b/metrics_sdk/test/opentelemetry/metrics_sdk_test.rb @@ -18,7 +18,7 @@ # Calls before the SDK is configured return Proxy implementations _(meter_provider).must_be_instance_of OpenTelemetry::Internal::ProxyMeterProvider _(meter).must_be_instance_of OpenTelemetry::Internal::ProxyMeter - _(instrument).must_be_instance_of OpenTelemetry::Internal::ProxyInstrument + _(instrument).must_be_instance_of OpenTelemetry::Internal::ProxyInstrument::Counter OpenTelemetry::SDK.configure From f438bafed077db716a3e113661091548c8a01feb Mon Sep 17 00:00:00 2001 From: Elias Rodrigues Date: Fri, 7 Jul 2023 17:46:55 -0300 Subject: [PATCH 24/32] Trying to understando MetricData --- .../aggregation/histogram_data_point.rb | 22 ++++++----- .../metrics/aggregation/number_data_point.rb | 12 +++--- .../sdk/metrics/aggregation/sum.rb | 17 ++++++--- .../sdk/metrics/meter_provider.rb | 38 ++++++++++--------- .../sdk/metrics/state/metric_data.rb | 20 +++++----- .../sdk/metrics/state/metric_store.rb | 8 +++- .../sdk/metrics/state/metric_stream.rb | 21 +++++++--- 7 files changed, 84 insertions(+), 54 deletions(-) diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/aggregation/histogram_data_point.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/aggregation/histogram_data_point.rb index 212c5ccd0e..35ecdf245b 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/aggregation/histogram_data_point.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/aggregation/histogram_data_point.rb @@ -10,16 +10,18 @@ module Metrics module Aggregation # TODO: Deal with this later # rubocop:disable Lint/StructNewOverride - HistogramDataPoint = Struct.new(:attributes, # optional Hash{String => String, Numeric, Boolean, Array} - :start_time_unix_nano, # Integer nanoseconds since Epoch - :time_unix_nano, # Integer nanoseconds since Epoch - :count, # Integer count is the number of values in the population. Must be non-negative. - :sum, # Integer sum of the values in the population. If count is zero then this field then this field must be zero - :bucket_counts, # optional Array[Integer] field contains the count values of histogram for each bucket. - :explicit_bounds, # Array[Float] specifies buckets with explicitly defined bounds for values. - :exemplars, # optional List of exemplars collected from measurements that were used to form the data point - :min, # optional Float min is the minimum value over (start_time, end_time]. - :max) # optional Float max is the maximum value over (start_time, end_time]. + HistogramDataPoint = Struct.new( + :attributes, # optional Hash{String => String, Numeric, Boolean, Array} + :start_time_unix_nano, # Integer nanoseconds since Epoch + :time_unix_nano, # Integer nanoseconds since Epoch + :count, # Integer count is the number of values in the population. Must be non-negative. + :sum, # Integer sum of the values in the population. If count is zero then this field then this field must be zero + :bucket_counts, # optional Array[Integer] field contains the count values of histogram for each bucket. + :explicit_bounds, # Array[Float] specifies buckets with explicitly defined bounds for values. + :exemplars, # optional List of exemplars collected from measurements that were used to form the data point + :min, # optional Float min is the minimum value over (start_time, end_time]. + :max # optional Float max is the maximum value over (start_time, end_time]. + ) # rubocop:enable Lint/StructNewOverride end end diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/aggregation/number_data_point.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/aggregation/number_data_point.rb index ae6e37eac1..518d405475 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/aggregation/number_data_point.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/aggregation/number_data_point.rb @@ -8,11 +8,13 @@ module OpenTelemetry module SDK module Metrics module Aggregation - NumberDataPoint = Struct.new(:attributes, # Hash{String => String, Numeric, Boolean, Array} - :start_time_unix_nano, # Integer nanoseconds since Epoch - :time_unix_nano, # Integer nanoseconds since Epoch - :value, # Integer - :exemplars) # optional List of exemplars collected from measurements that were used to form the data point + NumberDataPoint = Struct.new( + :attributes, # Hash{String => String, Numeric, Boolean, Array} + :start_time_unix_nano, # Integer nanoseconds since Epoch + :time_unix_nano, # Integer nanoseconds since Epoch + :value, # Integer + :exemplars # optional List of exemplars collected from measurements that were used to form the data point + ) end end end diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/aggregation/sum.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/aggregation/sum.rb index a4bb50f71c..96bcd6ab10 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/aggregation/sum.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/aggregation/sum.rb @@ -11,8 +11,10 @@ module Aggregation # Contains the implementation of the Sum aggregation # https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#sum-aggregation class Sum - def initialize(aggregation_temporality: :delta) + def initialize(aggregation_temporality: :delta, monotonic: true) @aggregation_temporality = aggregation_temporality + @monotonic = monotonic + @data_points = {} end @@ -36,16 +38,21 @@ def collect(start_time, end_time) end def update(increment, attributes) - ndp = @data_points[attributes] || @data_points[attributes] = NumberDataPoint.new( + @data_points[attributes] ||= build_data_point(attributes) + @data_points[attributes].value += increment + nil + end + + private + + def build_data_point(attributes) + NumberDataPoint.new( attributes, nil, nil, 0, nil ) - - ndp.value += increment - nil 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 index 52083aecd7..b52cadeace 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb @@ -27,20 +27,20 @@ def initialize(resource: OpenTelemetry::SDK::Resources::Resource.default) # # @return [SDK::Metrics::Meter] def meter(name, version: nil, schema_url: nil, attributes: nil) - if @stopped - OpenTelemetry.logger.warn 'calling MeterProvider#meter after shutdown, a noop meter will be returned.' - - NOOP_METER - else - key = build_key_for_meter(name, version, schema_url) - meter = Meter.new( - name, - version: version, - schema_url: schema_url, - attributes: attributes - ) - - @mutex.synchronize do + @mutex.synchronize do + if @stopped + OpenTelemetry.logger.warn 'calling MeterProvider#meter after shutdown, a noop meter will be returned.' + + NOOP_METER + else + key = build_key_for_meter(name, version, schema_url) + meter = Meter.new( + name, + version: version, + schema_url: schema_url, + attributes: attributes + ) + @meter_registry[key] ||= meter end end @@ -118,7 +118,11 @@ def add_metric_reader(metric_reader, aggregation: nil) @meter_registry.each_value do |meter| meter.each_instrument do |_name, instrument| - metric_stream = build_metric_stream(meter, instrument, aggregation) + metric_stream = build_metric_stream( + meter, + instrument, + aggregation || build_default_aggregation_for(instrument) + ) instrument.add_metric_stream(metric_stream) metric_reader.metric_store.add_metric_stream(metric_stream) @@ -157,9 +161,9 @@ def build_metric_stream(meter, instrument, aggregation) instrument.description, instrument.unit, instrument.kind, - self, + @resource, meter.instrumentation_scope, - aggregation || build_default_aggregation_for(instrument) + aggregation ) end diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/state/metric_data.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/state/metric_data.rb index 9885deba71..38d1167685 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/state/metric_data.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/state/metric_data.rb @@ -9,15 +9,17 @@ module SDK module Metrics module State # MetricData is a Struct containing {MetricStream} data for export. - MetricData = Struct.new(:name, # String - :description, # String - :unit, # String - :instrument_kind, # Symbol - :resource, # OpenTelemetry::SDK::Resources::Resource - :instrumentation_scope, # OpenTelemetry::SDK::InstrumentationScope - :data_points, # Hash{Hash{String => String, Numeric, Boolean, Array} => Numeric} - :start_time_unix_nano, # Integer nanoseconds since Epoch - :time_unix_nano) # Integer nanoseconds since Epoch + MetricData = Struct.new( + :name, # String + :description, # String + :unit, # String + :instrument_kind, # Symbol + :resource, # OpenTelemetry::SDK::Resources::Resource + :instrumentation_scope, # OpenTelemetry::SDK::InstrumentationScope + :data_points, # Hash{Hash{String => String, Numeric, Boolean, Array} => Numeric} + :start_time_unix_nano, # Integer nanoseconds since Epoch + :time_unix_nano # Integer nanoseconds since Epoch + ) end end end diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/state/metric_store.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/state/metric_store.rb index de2ecb83b6..d4caaaeac6 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/state/metric_store.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/state/metric_store.rb @@ -14,17 +14,21 @@ module State # public API. class MetricStore def initialize - @mutex = Mutex.new @epoch_start_time = now_in_nano @epoch_end_time = nil + + @mutex = Mutex.new @metric_streams = [] end def collect @mutex.synchronize do @epoch_end_time = now_in_nano - snapshot = @metric_streams.map { |ms| ms.collect(@epoch_start_time, @epoch_end_time) } + snapshot = @metric_streams.map do |metric_stream| + metric_stream.collect(@epoch_start_time, @epoch_end_time) + end @epoch_start_time = @epoch_end_time + snapshot end end diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/state/metric_stream.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/state/metric_stream.rb index d79d4a51ba..72ed09abf0 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/state/metric_stream.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/state/metric_stream.rb @@ -13,14 +13,22 @@ module State # The MetricStream class provides SDK internal functionality that is not a part of the # public API. class MetricStream - attr_reader :name, :description, :unit, :instrument_kind, :instrumentation_scope, :data_points + attr_reader( + :name, + :description, + :unit, + :instrument_kind, + :resource, + :instrumentation_scope, + :data_points # ?? + ) def initialize( name, description, unit, - instrument_kind, - meter_provider, + instrument_kind, # TODO: remove and replace with data_point_type?? + resource, instrumentation_scope, aggregation ) @@ -28,10 +36,11 @@ def initialize( @description = description @unit = unit @instrument_kind = instrument_kind - @meter_provider = meter_provider + @resource = resource @instrumentation_scope = instrumentation_scope @aggregation = aggregation + @data_points = [] # ?? @mutex = Mutex.new end @@ -41,8 +50,8 @@ def collect(start_time, end_time) @name, @description, @unit, - @instrument_kind, - @meter_provider.resource, + @instrument_kind, # TODO: remove and replace with data_point_type?? + @resource, @instrumentation_scope, @aggregation.collect(start_time, end_time), start_time, From 722035bef6a3c3f8be64bd13087b0014572341ba Mon Sep 17 00:00:00 2001 From: Elias Rodrigues Date: Fri, 11 Aug 2023 13:57:13 -0300 Subject: [PATCH 25/32] Add more details to SDK Meter test --- .../lib/opentelemetry/sdk/metrics/meter.rb | 2 +- .../opentelemetry/sdk/metrics/meter_test.rb | 122 ++++++++++++++---- 2 files changed, 100 insertions(+), 24 deletions(-) diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/meter.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/meter.rb index 1a21ac1659..e2917932db 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/meter.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/meter.rb @@ -11,7 +11,7 @@ module SDK module Metrics # {Meter} is the SDK implementation of {OpenTelemetry::Metrics::Meter}. class Meter < OpenTelemetry::Metrics::Meter - # TODO: refer yard doc comments to API + attr_reader :instrumentation_scope def initialize(*args, **kwargs) super diff --git a/metrics_sdk/test/opentelemetry/sdk/metrics/meter_test.rb b/metrics_sdk/test/opentelemetry/sdk/metrics/meter_test.rb index 6a6cba2fdc..b4fa39e338 100644 --- a/metrics_sdk/test/opentelemetry/sdk/metrics/meter_test.rb +++ b/metrics_sdk/test/opentelemetry/sdk/metrics/meter_test.rb @@ -9,53 +9,129 @@ describe OpenTelemetry::SDK::Metrics::Meter do before { OpenTelemetry::SDK.configure } - let(:meter) { OpenTelemetry.meter_provider.meter('new_meter') } + describe '#instrumentation_scope' do + it 'creates an instrumentation_scope' do + meter = build_meter( + 'test-meter', + version: '1.0.0', + schema_url: 'https://example.com/schema/1.0.0', + attributes: { 'key' => 'value' } + ) + + assert(meter.instrumentation_scope.instance_of?(OpenTelemetry::SDK::InstrumentationScope)) + assert(meter.instrumentation_scope.name == 'test-meter') + assert(meter.instrumentation_scope.version == '1.0.0') + assert(meter.instrumentation_scope.schema_url == 'https://example.com/schema/1.0.0') + assert(meter.instrumentation_scope.attributes == { 'key' => 'value' }) + end + end describe '#create_counter' do it 'creates a counter instrument' do - instrument = meter.create_counter('a_counter', unit: 'minutes', description: 'useful description') - _(instrument).must_be_instance_of OpenTelemetry::SDK::Metrics::Instrument::Counter + instrument = build_meter.create_counter( + 'test-instrument', + unit: 'b', + description: 'bytes received', + advice: { some: { value: 123 }} + ) + + assert(instrument.instance_of?(OpenTelemetry::SDK::Metrics::Instrument::Counter)) + assert(instrument.name == 'test-instrument') + assert(instrument.unit == 'b') + assert(instrument.description == 'bytes received') + assert(instrument.advice == { some: { value: 123 }}) end end describe '#create_histogram' do it 'creates a histogram instrument' do - instrument = meter.create_histogram('a_histogram', unit: 'minutes', description: 'useful description') - _(instrument).must_be_instance_of OpenTelemetry::SDK::Metrics::Instrument::Histogram + instrument = build_meter.create_histogram( + 'test-instrument', + unit: 'seconds', + description: 'request duration', + advice: { some: { value: 123 }} + ) + + assert(instrument.instance_of?(OpenTelemetry::SDK::Metrics::Instrument::Histogram)) + assert(instrument.name == 'test-instrument') + assert(instrument.unit == 'seconds') + assert(instrument.description == 'request duration') + assert(instrument.advice == { some: { value: 123 }}) end end describe '#create_up_down_counter' do - it 'creates a up_down_counter instrument' do - instrument = meter.create_up_down_counter('a_up_down_counter', unit: 'minutes', description: 'useful description') - _(instrument).must_be_instance_of OpenTelemetry::SDK::Metrics::Instrument::UpDownCounter + it 'creates an up_down_counter instrument' do + instrument = build_meter.create_up_down_counter( + 'test-instrument', + unit: 'jobs', + description: 'number of jobs in a queue', + advice: { some: { value: 123 }} + ) + + assert(instrument.instance_of?(OpenTelemetry::SDK::Metrics::Instrument::UpDownCounter)) + assert(instrument.name == 'test-instrument') + assert(instrument.unit == 'jobs') + assert(instrument.description == 'number of jobs in a queue') + assert(instrument.advice == { some: { value: 123 }}) end end describe '#create_observable_counter' do - it 'creates a observable_counter instrument' do - # TODO: Implement observable instruments - skip - instrument = meter.create_observable_counter('a_observable_counter', unit: 'minutes', description: 'useful description', callback: nil) - _(instrument).must_be_instance_of OpenTelemetry::SDK::Metrics::Instrument::ObservableCounter + it 'creates an observable_counter instrument' do + callback = ->{} + instrument = build_meter.create_observable_counter( + 'test-instrument', + unit: 'faults', + description: 'number of page faults', + callbacks: [callback] + ) + + assert(instrument.instance_of?(OpenTelemetry::SDK::Metrics::Instrument::ObservableCounter)) + assert(instrument.name == 'test-instrument') + assert(instrument.unit == 'faults') + assert(instrument.description == 'number of page faults') + assert(instrument.callbacks == [callback]) end end describe '#create_observable_gauge' do - it 'creates a observable_gauge instrument' do - # TODO: Implement observable instruments - skip - instrument = meter.create_observable_gauge('a_observable_gauge', unit: 'minutes', description: 'useful description', callback: nil) - _(instrument).must_be_instance_of OpenTelemetry::SDK::Metrics::Instrument::ObservableGauge + it 'creates an observable_gauge instrument' do + callback = ->{} + instrument = build_meter.create_observable_gauge( + 'test-instrument', + unit: 'celsius', + description: 'room temperature', + callbacks: [callback] + ) + + assert(instrument.instance_of?(OpenTelemetry::SDK::Metrics::Instrument::ObservableGauge)) + assert(instrument.name == 'test-instrument') + assert(instrument.unit == 'celsius') + assert(instrument.description == 'room temperature') + assert(instrument.callbacks == [callback]) end end describe '#create_observable_up_down_counter' do - it 'creates a observable_up_down_counter instrument' do - # TODO: Implement observable instruments - skip - instrument = meter.create_observable_up_down_counter('a_observable_up_down_counter', unit: 'minutes', description: 'useful description', callback: nil) - _(instrument).must_be_instance_of OpenTelemetry::SDK::Metrics::Instrument::ObservableUpDownCounter + it 'creates an observable_up_down_counter instrument' do + callback = ->{} + instrument = build_meter.create_observable_up_down_counter( + 'test-instrument', + unit: 'b', + description: 'process heap size', + callbacks: [callback] + ) + + assert(instrument.instance_of?(OpenTelemetry::SDK::Metrics::Instrument::ObservableUpDownCounter)) + assert(instrument.name == 'test-instrument') + assert(instrument.unit == 'b') + assert(instrument.description == 'process heap size') + assert(instrument.callbacks == [callback]) end end + + def build_meter(name = 'test-meter', **kwargs) + OpenTelemetry::SDK::Metrics::Meter.new(name, **kwargs) + end end From acf5815857460f716a6f0cc6189569933dce8eb5 Mon Sep 17 00:00:00 2001 From: Elias Rodrigues Date: Fri, 11 Aug 2023 15:38:03 -0300 Subject: [PATCH 26/32] Add more details to MeterProvider test And I need to make the last test case work again. I thought we could avoid making Meter hold a reference to MeterProvider but that's no possible due to add_metric_reader --- .../lib/opentelemetry/metrics/meter.rb | 2 +- .../sdk/metrics/meter_provider.rb | 17 ++-- .../sdk/metrics/meter_provider_test.rb | 83 +++++++++++-------- 3 files changed, 59 insertions(+), 43 deletions(-) diff --git a/metrics_api/lib/opentelemetry/metrics/meter.rb b/metrics_api/lib/opentelemetry/metrics/meter.rb index 7212ba5c60..2dd9b73b85 100644 --- a/metrics_api/lib/opentelemetry/metrics/meter.rb +++ b/metrics_api/lib/opentelemetry/metrics/meter.rb @@ -162,7 +162,7 @@ def create_observable_up_down_counter(name, unit: nil, description: nil, callbac # @api private def each_instrument @mutex.synchronize do - instrument_registry.each do |name, instrument| + @instrument_registry.each do |name, instrument| yield(name, instrument) end end diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb index b52cadeace..441ef1feb7 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb @@ -34,14 +34,13 @@ def meter(name, version: nil, schema_url: nil, attributes: nil) NOOP_METER else key = build_key_for_meter(name, version, schema_url) - meter = Meter.new( + + @meter_registry[key] ||= Meter.new( name, version: version, schema_url: schema_url, attributes: attributes ) - - @meter_registry[key] ||= meter end end end @@ -170,17 +169,17 @@ def build_metric_stream(meter, instrument, aggregation) # https://opentelemetry.io/docs/specs/otel/metrics/sdk/#default-aggregation def build_default_aggregation_for(instrument) case instrument - when Counter + when Instrument::Counter Aggregation::Sum.new - when Histogram + when Instrument::Histogram Aggregation::ExplicitBucketHistogram.new - when UpDownCounter + when Instrument::UpDownCounter Aggregation::Sum.new - when ObservableCounter + when Instrument::ObservableCounter # TODO: ? - when ObservableGauge + when Instrument::ObservableGauge # TODO: ? - when ObservableUpDownCounter + when Instrument::ObservableUpDownCounter # TODO: ? 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 f781d613b3..8856556144 100644 --- a/metrics_sdk/test/opentelemetry/sdk/metrics/meter_provider_test.rb +++ b/metrics_sdk/test/opentelemetry/sdk/metrics/meter_provider_test.rb @@ -14,20 +14,38 @@ describe '#meter' do it 'requires a meter name' do - _(-> { OpenTelemetry.meter_provider.meter }).must_raise(ArgumentError) + _(-> { build_meter_provider.meter }).must_raise(ArgumentError) end it 'creates a new meter' do - meter = OpenTelemetry.meter_provider.meter('test') + meter = build_meter_provider.meter('test-meter') _(meter).must_be_instance_of(OpenTelemetry::SDK::Metrics::Meter) end - it 'repeated calls does not recreate a meter of the same name' do - meter_a = OpenTelemetry.meter_provider.meter('test') - meter_b = OpenTelemetry.meter_provider.meter('test') + it 'repeated calls do not recreate a meter of the same name' do + meter_provider = build_meter_provider - _(meter_a).must_equal(meter_b) + meter_a = meter_provider.meter('test-meter') + meter_b = meter_provider.meter('test-meter') + + _(meter_a.object_id).must_equal(meter_b.object_id) + end + + describe 'when meter_provider is shutdown' do + it 'returns a noop meter from API and logs a message' do + OpenTelemetry::TestHelpers.with_test_logger do |log_stream| + meter_provider = build_meter_provider + meter_provider.shutdown + + meter = meter_provider.meter('test-meter') + + assert(meter.instance_of?(OpenTelemetry::Metrics::Meter)) + assert(log_stream.string.match?( + /calling MeterProvider#meter after shutdown, a noop meter will be returned./ + )) + end + end end end @@ -40,15 +58,6 @@ end end - it 'returns a no-op meter after being shutdown' do - with_test_logger do |log_stream| - OpenTelemetry.meter_provider.shutdown - - _(OpenTelemetry.meter_provider.meter('test')).must_be_instance_of(OpenTelemetry::Metrics::Meter) - _(log_stream.string).must_match(/calling MeterProvider#meter after shutdown, a noop meter will be returned/) - end - end - it 'returns a timeout response when it times out' do mock_metric_reader = new_mock_reader mock_metric_reader.expect(:nothing_gets_called_because_it_times_out_first, nil) @@ -98,7 +107,7 @@ describe '#add_metric_reader' do it 'adds a metric reader' do - metric_reader = OpenTelemetry::SDK::Metrics::Export::MetricReader.new + metric_reader = build_metric_reader OpenTelemetry.meter_provider.add_metric_reader(metric_reader) @@ -106,41 +115,49 @@ end it 'associates the metric store with instruments created before the metric reader' do - meter_a = OpenTelemetry.meter_provider.meter('a').create_counter('meter_a') + instrument = OpenTelemetry.meter_provider.meter('test-meter').create_counter('test-instrument') - metric_reader_a = OpenTelemetry::SDK::Metrics::Export::MetricReader.new + metric_reader_a = build_metric_reader OpenTelemetry.meter_provider.add_metric_reader(metric_reader_a) - metric_reader_b = OpenTelemetry::SDK::Metrics::Export::MetricReader.new + metric_reader_b = build_metric_reader OpenTelemetry.meter_provider.add_metric_reader(metric_reader_b) - _(meter_a.instance_variable_get(:@metric_streams).size).must_equal(2) + _(instrument.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) end - it 'associates the metric store with instruments created after the metric reader' do - metric_reader_a = OpenTelemetry::SDK::Metrics::Export::MetricReader.new - OpenTelemetry.meter_provider.add_metric_reader(metric_reader_a) + # it 'associates the metric store with instruments created after the metric reader' do + # metric_reader_a = build_metric_reader + # OpenTelemetry.meter_provider.add_metric_reader(metric_reader_a) - metric_reader_b = OpenTelemetry::SDK::Metrics::Export::MetricReader.new - OpenTelemetry.meter_provider.add_metric_reader(metric_reader_b) + # metric_reader_b = build_metric_reader + # OpenTelemetry.meter_provider.add_metric_reader(metric_reader_b) - meter_a = OpenTelemetry.meter_provider.meter('a').create_counter('meter_a') + # instrument = OpenTelemetry.meter_provider.meter('test-meter').create_counter('test-instrument') - _(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) - end + # _(instrument.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) + # end end - # TODO: OpenTelemetry.meter_provider.add_view - describe '#add_view' do - end + # # TODO: OpenTelemetry.meter_provider.add_view + # describe '#add_view' do + # end private def new_mock_reader Minitest::Mock.new(OpenTelemetry::SDK::Metrics::Export::MetricReader.new) end + + def build_metric_reader + OpenTelemetry::SDK::Metrics::Export::MetricReader.new + end + + def build_meter_provider + OpenTelemetry::SDK::Metrics::MeterProvider.new + end end From 34fa2208936f41befcb4bca20b386d36ee3318d3 Mon Sep 17 00:00:00 2001 From: Elias Rodrigues Date: Fri, 11 Aug 2023 15:47:22 -0300 Subject: [PATCH 27/32] Make Meter hold a reference to MeterProvider and Instruments hold a reference to Meter --- .../instrument/asynchronous_instrument.rb | 4 +++- .../instrument/synchronous_instrument.rb | 11 +++-------- metrics_api/lib/opentelemetry/metrics/meter.rb | 4 +++- .../lib/opentelemetry/sdk/metrics/meter.rb | 18 ++++++++++++------ .../sdk/metrics/meter_provider.rb | 1 + 5 files changed, 22 insertions(+), 16 deletions(-) diff --git a/metrics_api/lib/opentelemetry/metrics/instrument/asynchronous_instrument.rb b/metrics_api/lib/opentelemetry/metrics/instrument/asynchronous_instrument.rb index 0d05809bab..4af592b16f 100644 --- a/metrics_api/lib/opentelemetry/metrics/instrument/asynchronous_instrument.rb +++ b/metrics_api/lib/opentelemetry/metrics/instrument/asynchronous_instrument.rb @@ -12,11 +12,13 @@ class AsynchronousInstrument attr_reader :name, :unit, :description, :callbacks # @api private - def initialize(name, unit: nil, description: nil, callbacks: nil) + def initialize(name, unit: nil, description: nil, callbacks: nil, meter: nil) @name = name @unit = unit || '' @description = description || '' @callbacks = callbacks ? Array(callbacks) : [] + + @meter = meter end # @param callbacks [Proc, Array] diff --git a/metrics_api/lib/opentelemetry/metrics/instrument/synchronous_instrument.rb b/metrics_api/lib/opentelemetry/metrics/instrument/synchronous_instrument.rb index a533716afa..5595f6090e 100644 --- a/metrics_api/lib/opentelemetry/metrics/instrument/synchronous_instrument.rb +++ b/metrics_api/lib/opentelemetry/metrics/instrument/synchronous_instrument.rb @@ -12,23 +12,18 @@ class SynchronousInstrument attr_reader :name, :unit, :description, :advice # @api private - def initialize(name, unit: nil, description: nil, advice: nil) + def initialize(name, unit: nil, description: nil, advice: nil, meter: nil) @name = name @unit = unit || '' @description = description || '' @advice = advice || {} + @meter = meter + @mutex = Mutex.new @metric_streams = [] end - # @api private - def add_metric_stream(metric_stream) - @mutex.synchronize do - @metric_streams.push(metric_stream) - end - end - private def update(value, attributes) diff --git a/metrics_api/lib/opentelemetry/metrics/meter.rb b/metrics_api/lib/opentelemetry/metrics/meter.rb index 2dd9b73b85..748be5e775 100644 --- a/metrics_api/lib/opentelemetry/metrics/meter.rb +++ b/metrics_api/lib/opentelemetry/metrics/meter.rb @@ -33,12 +33,14 @@ class Meter ) # @api private - def initialize(name, version: nil, schema_url: nil, attributes: nil) + def initialize(name, version: nil, schema_url: nil, attributes: nil, meter_provider: nil) @name = name @version = version || '' @schema_url = schema_url || '' @attributes = attributes || {} + @meter_provider = meter_provider + @mutex = Mutex.new @instrument_registry = {} diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/meter.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/meter.rb index e2917932db..04dec5b7ce 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/meter.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/meter.rb @@ -30,7 +30,8 @@ def create_counter(name, unit: nil, description: nil, advice: nil) name, unit: unit, description: description, - advice: advice + advice: advice, + meter_provider: self ) end end @@ -41,7 +42,8 @@ def create_histogram(name, unit: nil, description: nil, advice: nil) name, unit: unit, description: description, - advice: advice + advice: advice, + meter_provider: self ) end end @@ -52,7 +54,8 @@ def create_up_down_counter(name, unit: nil, description: nil, advice: nil) name, unit: unit, description: description, - advice: advice + advice: advice, + meter_provider: self ) end end @@ -63,7 +66,8 @@ def create_observable_counter(name, unit: nil, description: nil, callbacks: nil) name, unit: unit, description: description, - callbacks: callbacks + callbacks: callbacks, + meter_provider: self ) end end @@ -74,7 +78,8 @@ def create_observable_gauge(name, unit: nil, description: nil, callbacks: nil) name, unit: unit, description: description, - callbacks: callbacks + callbacks: callbacks, + meter_provider: self ) end end @@ -85,7 +90,8 @@ def create_observable_up_down_counter(name, unit: nil, description: nil, callbac name, unit: unit, description: description, - callbacks: callbacks + callbacks: callbacks, + meter_provider: self ) end end diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb index 441ef1feb7..05128bc5c8 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb @@ -40,6 +40,7 @@ def meter(name, version: nil, schema_url: nil, attributes: nil) version: version, schema_url: schema_url, attributes: attributes + meter_provider: self ) end end From cef12947008ef636a79a1eef3899a179274aaf12 Mon Sep 17 00:00:00 2001 From: Elias Rodrigues Date: Fri, 11 Aug 2023 16:36:29 -0300 Subject: [PATCH 28/32] Build and create metric stream in Meter whenever a sync instrument is created --- .../lib/opentelemetry/internal/proxy_meter.rb | 204 +++++++++--------- .../instrument/synchronous_instrument.rb | 7 + .../lib/opentelemetry/metrics/meter.rb | 16 +- .../opentelemetry/metrics/meter_provider.rb | 9 + .../lib/opentelemetry/sdk/metrics/meter.rb | 159 +++++++++----- .../sdk/metrics/meter_provider.rb | 45 +--- .../sdk/metrics/meter_provider_test.rb | 20 +- 7 files changed, 247 insertions(+), 213 deletions(-) diff --git a/metrics_api/lib/opentelemetry/internal/proxy_meter.rb b/metrics_api/lib/opentelemetry/internal/proxy_meter.rb index da66e35940..04f8da2e8b 100644 --- a/metrics_api/lib/opentelemetry/internal/proxy_meter.rb +++ b/metrics_api/lib/opentelemetry/internal/proxy_meter.rb @@ -83,135 +83,135 @@ def delegate=(meter) end def create_counter(name, unit: nil, description: nil, advice: nil) - register_instrument(name) do - @delegate_mutex.synchronize do - if @delegate.nil? - ProxyInstrument::Counter.new( - name, - unit: unit, - description: description, - advice: advice - ) - else - @delegate.create_counter( - name, - unit: unit, - description: description, - advice: advice - ) - end + instrument = @delegate_mutex.synchronize do + if @delegate.nil? + ProxyInstrument::Counter.new( + name, + unit: unit, + description: description, + advice: advice + ) + else + @delegate.create_counter( + name, + unit: unit, + description: description, + advice: advice + ) end end + + register_instrument(name, instrument) end def create_histogram(name, unit: nil, description: nil, advice: nil) - register_instrument(name) do - @delegate_mutex.synchronize do - if @delegate.nil? - ProxyInstrument::Histogram.new( - name, - unit: unit, - description: description, - advice: advice - ) - else - @delegate.create_histogram( - name, - unit: unit, - description: description, - advice: advice - ) - end + instrument = @delegate_mutex.synchronize do + if @delegate.nil? + ProxyInstrument::Histogram.new( + name, + unit: unit, + description: description, + advice: advice + ) + else + @delegate.create_histogram( + name, + unit: unit, + description: description, + advice: advice + ) end end + + register_instrument(name, instrument) end def create_up_down_counter(name, unit: nil, description: nil, advice: nil) - register_instrument(name) do - @delegate_mutex.synchronize do - if @delegate.nil? - ProxyInstrument::UpDownCounter.new( - name, - unit: unit, - description: description, - advice: advice - ) - else - @delegate.create_up_down_counter( - name, - unit: unit, - description: description, - advice: advice - ) - end + instrument = @delegate_mutex.synchronize do + if @delegate.nil? + ProxyInstrument::UpDownCounter.new( + name, + unit: unit, + description: description, + advice: advice + ) + else + @delegate.create_up_down_counter( + name, + unit: unit, + description: description, + advice: advice + ) end end + + register_instrument(name, instrument) end def create_observable_counter(name, unit: nil, description: nil, callbacks: nil) - register_instrument(name) do - @delegate_mutex.synchronize do - if @delegate.nil? - ProxyInstrument::ObservableCounter.new( - name, - unit: unit, - description: description, - callbacks: callbacks - ) - else - @delegate.create_observable_counter( - name, - unit: unit, - description: description, - callbacks: callbacks - ) - end + instrument = @delegate_mutex.synchronize do + if @delegate.nil? + ProxyInstrument::ObservableCounter.new( + name, + unit: unit, + description: description, + callbacks: callbacks + ) + else + @delegate.create_observable_counter( + name, + unit: unit, + description: description, + callbacks: callbacks + ) end end + + register_instrument(name, instrument) end def create_observable_gauge(name, unit: nil, description: nil, callbacks: nil) - register_instrument(name) do - @delegate_mutex.synchronize do - if @delegate.nil? - ProxyInstrument::ObservableGauge.new( - name, - unit: unit, - description: description, - callbacks: callbacks - ) - else - @delegate.create_observable_gauge( - name, - unit: unit, - description: description, - callbacks: callbacks - ) - end + instrument = @delegate_mutex.synchronize do + if @delegate.nil? + ProxyInstrument::ObservableGauge.new( + name, + unit: unit, + description: description, + callbacks: callbacks + ) + else + @delegate.create_observable_gauge( + name, + unit: unit, + description: description, + callbacks: callbacks + ) end end + + register_instrument(name, instrument) end def create_observable_up_down_counter(name, unit: nil, description: nil, callbacks: nil) - register_instrument(name) do - @delegate_mutex.synchronize do - if @delegate.nil? - ProxyInstrument::ObservableUpDownCounter.new( - name, - unit: unit, - description: description, - callbacks: callbacks - ) - else - @delegate.create_observable_up_down_counter( - name, - unit: unit, - description: description, - callbacks: callbacks - ) - end + instrument = @delegate_mutex.synchronize do + if @delegate.nil? + ProxyInstrument::ObservableUpDownCounter.new( + name, + unit: unit, + description: description, + callbacks: callbacks + ) + else + @delegate.create_observable_up_down_counter( + name, + unit: unit, + description: description, + callbacks: callbacks + ) end end + + register_instrument(name, instrument) end end end diff --git a/metrics_api/lib/opentelemetry/metrics/instrument/synchronous_instrument.rb b/metrics_api/lib/opentelemetry/metrics/instrument/synchronous_instrument.rb index 5595f6090e..a7cfaa6082 100644 --- a/metrics_api/lib/opentelemetry/metrics/instrument/synchronous_instrument.rb +++ b/metrics_api/lib/opentelemetry/metrics/instrument/synchronous_instrument.rb @@ -24,6 +24,13 @@ def initialize(name, unit: nil, description: nil, advice: nil, meter: nil) @metric_streams = [] end + # @api private + def add_metric_stream(metric_stream) + @mutex.synchronize do + @metric_streams.push(metric_stream) + end + end + private def update(value, attributes) diff --git a/metrics_api/lib/opentelemetry/metrics/meter.rb b/metrics_api/lib/opentelemetry/metrics/meter.rb index 748be5e775..16304fbcc2 100644 --- a/metrics_api/lib/opentelemetry/metrics/meter.rb +++ b/metrics_api/lib/opentelemetry/metrics/meter.rb @@ -61,7 +61,7 @@ def initialize(name, version: nil, schema_url: nil, attributes: nil, meter_provi # # @return [Instrument::Counter] def create_counter(name, unit: nil, description: nil, advice: nil) - register_instrument(name) { NOOP_COUNTER } + register_instrument(name, NOOP_COUNTER) end # @param name [String] @@ -78,7 +78,7 @@ def create_counter(name, unit: nil, description: nil, advice: nil) # # @return [Instrument::Histogram] def create_histogram(name, unit: nil, description: nil, advice: nil) - register_instrument(name) { NOOP_HISTOGRAM } + register_instrument(name, NOOP_HISTOGRAM) end # @param name [String] @@ -95,7 +95,7 @@ def create_histogram(name, unit: nil, description: nil, advice: nil) # # @return [Instrument::UpDownCounter] def create_up_down_counter(name, unit: nil, description: nil, advice: nil) - register_instrument(name) { NOOP_UP_DOWN_COUNTER } + register_instrument(name, NOOP_UP_DOWN_COUNTER) end # @param name [String] @@ -116,7 +116,7 @@ def create_up_down_counter(name, unit: nil, description: nil, advice: nil) # # @return [Instrument::ObservableCounter] def create_observable_counter(name, unit: nil, description: nil, callbacks: nil) - register_instrument(name) { NOOP_OBSERVABLE_COUNTER } + register_instrument(name, NOOP_OBSERVABLE_COUNTER) end # @param name [String] @@ -137,7 +137,7 @@ def create_observable_counter(name, unit: nil, description: nil, callbacks: nil) # # @return [Instrument::ObservableGauge] def create_observable_gauge(name, unit: nil, description: nil, callbacks: nil) - register_instrument(name) { NOOP_OBSERVABLE_GAUGE } + register_instrument(name, NOOP_OBSERVABLE_GAUGE) end # @param name [String] @@ -158,7 +158,7 @@ def create_observable_gauge(name, unit: nil, description: nil, callbacks: nil) # # @return [Instrument::ObservableUpDownCounter] def create_observable_up_down_counter(name, unit: nil, description: nil, callbacks: nil) - register_instrument(name) { NOOP_OBSERVABLE_UP_DOWN_COUNTER } + register_instrument(name, NOOP_OBSERVABLE_UP_DOWN_COUNTER) end # @api private @@ -172,7 +172,7 @@ def each_instrument private - def register_instrument(name) + def register_instrument(name, instrument) name = name.downcase @mutex.synchronize do @@ -180,7 +180,7 @@ def register_instrument(name) OpenTelemetry.logger.warn("duplicate instrument registration occurred for #{name}") end - @instrument_registry[name] = yield + @instrument_registry[name] = instrument end end end diff --git a/metrics_api/lib/opentelemetry/metrics/meter_provider.rb b/metrics_api/lib/opentelemetry/metrics/meter_provider.rb index d0a4aab683..3a851d1d4a 100644 --- a/metrics_api/lib/opentelemetry/metrics/meter_provider.rb +++ b/metrics_api/lib/opentelemetry/metrics/meter_provider.rb @@ -38,6 +38,15 @@ def initialize(resource: nil) def meter(name, version: nil, schema_url: nil, attributes: nil) NOOP_METER end + + # @api private + def each_metric_reader + @mutex.synchronize do + @metric_readers.each do |metric_reader| + yield(metric_reader) + end + end + end end end end diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/meter.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/meter.rb index 04dec5b7ce..77558ba02c 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/meter.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/meter.rb @@ -25,74 +25,133 @@ def initialize(*args, **kwargs) end def create_counter(name, unit: nil, description: nil, advice: nil) - register_instrument(name) do - SDK::Metrics::Instrument::Counter.new( - name, - unit: unit, - description: description, - advice: advice, - meter_provider: self - ) + instrument = SDK::Metrics::Instrument::Counter.new( + name, + unit: unit, + description: description, + advice: advice, + meter: self + ) + register_instrument(name, instrument) + + @meter_provider&.each_metric_reader do |metric_reader| + build_and_add_metric_stream(metric_reader.metric_store, instrument, nil) end + + instrument end def create_histogram(name, unit: nil, description: nil, advice: nil) - register_instrument(name) do - SDK::Metrics::Instrument::Histogram.new( - name, - unit: unit, - description: description, - advice: advice, - meter_provider: self - ) + instrument = SDK::Metrics::Instrument::Histogram.new( + name, + unit: unit, + description: description, + advice: advice, + meter: self + ) + register_instrument(name, instrument) + + @meter_provider&.each_metric_reader do |metric_reader| + build_and_add_metric_stream(metric_reader.metric_store, instrument, nil) end + + instrument end def create_up_down_counter(name, unit: nil, description: nil, advice: nil) - register_instrument(name) do - SDK::Metrics::Instrument::UpDownCounter.new( - name, - unit: unit, - description: description, - advice: advice, - meter_provider: self - ) + instrument = SDK::Metrics::Instrument::UpDownCounter.new( + name, + unit: unit, + description: description, + advice: advice, + meter: self + ) + register_instrument(name, instrument) + + @meter_provider&.each_metric_reader do |metric_reader| + build_and_add_metric_stream(metric_reader.metric_store, instrument, nil) end + + instrument end def create_observable_counter(name, unit: nil, description: nil, callbacks: nil) - register_instrument(name) do - SDK::Metrics::Instrument::ObservableCounter.new( - name, - unit: unit, - description: description, - callbacks: callbacks, - meter_provider: self - ) - end + instrument = SDK::Metrics::Instrument::ObservableCounter.new( + name, + unit: unit, + description: description, + callbacks: callbacks, + meter: self + ) + register_instrument(name, instrument) end def create_observable_gauge(name, unit: nil, description: nil, callbacks: nil) - register_instrument(name) do - SDK::Metrics::Instrument::ObservableGauge.new( - name, - unit: unit, - description: description, - callbacks: callbacks, - meter_provider: self - ) - end + instrument = SDK::Metrics::Instrument::ObservableGauge.new( + name, + unit: unit, + description: description, + callbacks: callbacks, + meter: self + ) + register_instrument(name, instrument) end def create_observable_up_down_counter(name, unit: nil, description: nil, callbacks: nil) - register_instrument(name) do - SDK::Metrics::Instrument::ObservableUpDownCounter.new( - name, - unit: unit, - description: description, - callbacks: callbacks, - meter_provider: self - ) + instrument = SDK::Metrics::Instrument::ObservableUpDownCounter.new( + name, + unit: unit, + description: description, + callbacks: callbacks, + meter: self + ) + register_instrument(name, instrument) + end + + # @api private + def register_metric_store(metric_store, aggregation: nil) + each_instrument do |_name, instrument| + build_and_add_metric_stream(metric_store, instrument, aggregation) + end + end + + private + + def build_and_add_metric_stream(metric_store, instrument, aggregation) + metric_stream = build_metric_stream(instrument, aggregation) + + metric_store.add_metric_stream(metric_stream) + instrument.add_metric_stream(metric_stream) + end + + def build_metric_stream(instrument, aggregation) + aggregation ||= build_default_aggregation_for(instrument) + + SDK::Metrics::State::MetricStream.new( + instrument.name, + instrument.description, + instrument.unit, + instrument.kind, + @meter_provider.resource, + instrumentation_scope, + aggregation + ) + end + + def build_default_aggregation_for(instrument) + case instrument + when Instrument::Counter + Aggregation::Sum.new + when Instrument::Histogram + Aggregation::ExplicitBucketHistogram.new + when Instrument::UpDownCounter + Aggregation::Sum.new + when Instrument::ObservableCounter + # TODO: ? + when Instrument::ObservableGauge + # TODO: ? + when Instrument::ObservableUpDownCounter + # TODO: ? 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 index 05128bc5c8..559ace3151 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb @@ -39,7 +39,7 @@ def meter(name, version: nil, schema_url: nil, attributes: nil) name, version: version, schema_url: schema_url, - attributes: attributes + attributes: attributes, meter_provider: self ) end @@ -117,16 +117,7 @@ def add_metric_reader(metric_reader, aggregation: nil) @metric_readers.push(metric_reader) @meter_registry.each_value do |meter| - meter.each_instrument do |_name, instrument| - metric_stream = build_metric_stream( - meter, - instrument, - aggregation || build_default_aggregation_for(instrument) - ) - - instrument.add_metric_stream(metric_stream) - metric_reader.metric_store.add_metric_stream(metric_stream) - end + meter.register_metric_store(metric_reader.metric_store, aggregation: aggregation) end end @@ -152,38 +143,6 @@ def build_key_for_meter(name, version, schema_url) Key.new(name, version, schema_url) end - - def build_metric_stream(meter, instrument, aggregation) - aggregation ||= default_aggregation_for(instrument) - - SDK::Metrics::State::MetricStream.new( - instrument.name, - instrument.description, - instrument.unit, - instrument.kind, - @resource, - meter.instrumentation_scope, - aggregation - ) - end - - # https://opentelemetry.io/docs/specs/otel/metrics/sdk/#default-aggregation - def build_default_aggregation_for(instrument) - case instrument - when Instrument::Counter - Aggregation::Sum.new - when Instrument::Histogram - Aggregation::ExplicitBucketHistogram.new - when Instrument::UpDownCounter - Aggregation::Sum.new - when Instrument::ObservableCounter - # TODO: ? - when Instrument::ObservableGauge - # TODO: ? - when Instrument::ObservableUpDownCounter - # TODO: ? - end - end end 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 8856556144..aa010d34ed 100644 --- a/metrics_sdk/test/opentelemetry/sdk/metrics/meter_provider_test.rb +++ b/metrics_sdk/test/opentelemetry/sdk/metrics/meter_provider_test.rb @@ -128,19 +128,19 @@ _(metric_reader_b.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 = build_metric_reader - # OpenTelemetry.meter_provider.add_metric_reader(metric_reader_a) + it 'associates the metric store with instruments created after the metric reader' do + metric_reader_a = build_metric_reader + OpenTelemetry.meter_provider.add_metric_reader(metric_reader_a) - # metric_reader_b = build_metric_reader - # OpenTelemetry.meter_provider.add_metric_reader(metric_reader_b) + metric_reader_b = build_metric_reader + OpenTelemetry.meter_provider.add_metric_reader(metric_reader_b) - # instrument = OpenTelemetry.meter_provider.meter('test-meter').create_counter('test-instrument') + instrument = OpenTelemetry.meter_provider.meter('test-meter').create_counter('test-instrument') - # _(instrument.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) - # end + _(instrument.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) + end end # # TODO: OpenTelemetry.meter_provider.add_view From a76cb3ee0049bfdd9fea2720e2935dad10a329c2 Mon Sep 17 00:00:00 2001 From: Elias Rodrigues Date: Sun, 13 Aug 2023 17:35:31 -0300 Subject: [PATCH 29/32] Add empty LastValue aggregation class --- .../sdk/metrics/aggregation/last_value.rb | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 metrics_sdk/lib/opentelemetry/sdk/metrics/aggregation/last_value.rb diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/aggregation/last_value.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/aggregation/last_value.rb new file mode 100644 index 0000000000..39c194db75 --- /dev/null +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/aggregation/last_value.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +# Copyright The OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +module OpenTelemetry + module SDK + module Metrics + module Aggregation + class LastValue + # TODO: implement aggregation code + def initialize + end + end + end + end + end +end From e0b50efb9efbd710e02e8310643fad57ec6c945e Mon Sep 17 00:00:00 2001 From: Elias Rodrigues Date: Sun, 13 Aug 2023 17:35:49 -0300 Subject: [PATCH 30/32] Define default aggregation for instruments --- metrics_sdk/lib/opentelemetry/sdk/metrics/meter.rb | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/meter.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/meter.rb index 77558ba02c..d5d7ebb675 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/meter.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/meter.rb @@ -109,7 +109,7 @@ def create_observable_up_down_counter(name, unit: nil, description: nil, callbac end # @api private - def register_metric_store(metric_store, aggregation: nil) + def register_metric_store(metric_store, aggregation) each_instrument do |_name, instrument| build_and_add_metric_stream(metric_store, instrument, aggregation) end @@ -143,15 +143,17 @@ def build_default_aggregation_for(instrument) when Instrument::Counter Aggregation::Sum.new when Instrument::Histogram + # TODO: check instrument's advice for explicit_bucket_boundaries + # and use it to create the default ExplicitBucketHistogram Aggregation::ExplicitBucketHistogram.new when Instrument::UpDownCounter Aggregation::Sum.new when Instrument::ObservableCounter - # TODO: ? + Aggregation::Sum.new when Instrument::ObservableGauge - # TODO: ? + Aggregation::LastValue.new when Instrument::ObservableUpDownCounter - # TODO: ? + Aggregation::Sum.new end end end From 19addccbc12558b43fca852ca1533d6a667c009a Mon Sep 17 00:00:00 2001 From: Elias Rodrigues Date: Sun, 13 Aug 2023 17:37:20 -0300 Subject: [PATCH 31/32] Make add_metric_stream methods similiar Also, - Remove .dup that I _think_ is not needed - Rename snapshot variable to metric_data --- .../metrics/instrument/synchronous_instrument.rb | 1 + metrics_api/lib/opentelemetry/metrics/meter_provider.rb | 2 +- metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb | 3 +-- .../lib/opentelemetry/sdk/metrics/state/metric_store.rb | 6 +++--- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/metrics_api/lib/opentelemetry/metrics/instrument/synchronous_instrument.rb b/metrics_api/lib/opentelemetry/metrics/instrument/synchronous_instrument.rb index a7cfaa6082..a3a34a5248 100644 --- a/metrics_api/lib/opentelemetry/metrics/instrument/synchronous_instrument.rb +++ b/metrics_api/lib/opentelemetry/metrics/instrument/synchronous_instrument.rb @@ -28,6 +28,7 @@ def initialize(name, unit: nil, description: nil, advice: nil, meter: nil) def add_metric_stream(metric_stream) @mutex.synchronize do @metric_streams.push(metric_stream) + nil end end diff --git a/metrics_api/lib/opentelemetry/metrics/meter_provider.rb b/metrics_api/lib/opentelemetry/metrics/meter_provider.rb index 3a851d1d4a..d78542ed87 100644 --- a/metrics_api/lib/opentelemetry/metrics/meter_provider.rb +++ b/metrics_api/lib/opentelemetry/metrics/meter_provider.rb @@ -13,7 +13,7 @@ class MeterProvider private_constant :NOOP_METER, :Key - attr_reader :resource, :metric_readers + attr_reader :resource def initialize(resource: nil) @resource = resource diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb index 559ace3151..bdbe2115bc 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb @@ -115,9 +115,8 @@ def add_metric_reader(metric_reader, aggregation: nil) OpenTelemetry.logger.warn('calling MetricProvider#add_metric_reader after shutdown.') else @metric_readers.push(metric_reader) - @meter_registry.each_value do |meter| - meter.register_metric_store(metric_reader.metric_store, aggregation: aggregation) + meter.register_metric_store(metric_reader.metric_store, aggregation) end end diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/state/metric_store.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/state/metric_store.rb index d4caaaeac6..9365493f2a 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/state/metric_store.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/state/metric_store.rb @@ -24,18 +24,18 @@ def initialize def collect @mutex.synchronize do @epoch_end_time = now_in_nano - snapshot = @metric_streams.map do |metric_stream| + metric_data = @metric_streams.map do |metric_stream| metric_stream.collect(@epoch_start_time, @epoch_end_time) end @epoch_start_time = @epoch_end_time - snapshot + metric_data end end def add_metric_stream(metric_stream) @mutex.synchronize do - @metric_streams = @metric_streams.dup.push(metric_stream) + @metric_streams.push(metric_stream) nil end end From 63b6015f8cf4664c16467f6bae71e9e26b4d77d0 Mon Sep 17 00:00:00 2001 From: Elias Rodrigues Date: Sun, 13 Aug 2023 17:38:30 -0300 Subject: [PATCH 32/32] Make Aggregation::Sum test examples more verbose --- .../sdk/metrics/aggregation/sum.rb | 40 ++-- .../sdk/metrics/aggregation/sum_test.rb | 208 +++++++++++++----- 2 files changed, 180 insertions(+), 68 deletions(-) diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/aggregation/sum.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/aggregation/sum.rb index 96bcd6ab10..424fef3483 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/aggregation/sum.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/aggregation/sum.rb @@ -11,41 +11,43 @@ module Aggregation # Contains the implementation of the Sum aggregation # https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#sum-aggregation class Sum + attr_reader :aggregation_temporality, :monotonic + def initialize(aggregation_temporality: :delta, monotonic: true) @aggregation_temporality = aggregation_temporality @monotonic = monotonic - @data_points = {} + @number_data_points = {} + end + + def update(increment, attributes) + @number_data_points[attributes] ||= build_number_data_point(attributes) + @number_data_points[attributes].value += increment + nil end - def collect(start_time, end_time) - if @aggregation_temporality == :delta - # Set timestamps and 'move' data point values to result. - ndps = @data_points.each_value do |ndp| - ndp.start_time_unix_nano = start_time - ndp.time_unix_nano = end_time + def collect(start_time_unix_nano, end_time_unix_nano) + if aggregation_temporality == :delta + ndps = @number_data_points.map do |_attributes, ndp| + ndp.start_time_unix_nano = start_time_unix_nano + ndp.time_unix_nano = end_time_unix_nano + ndp end - @data_points.clear + @number_data_points.clear ndps else - # Update timestamps and take a snapshot. - @data_points.values.map! do |ndp| - ndp.start_time_unix_nano ||= start_time # Start time of a data point is from the first observation. - ndp.time_unix_nano = end_time + @number_data_points.map do |_attributes, ndp| + # Start time of data point is from the first observation. + ndp.start_time_unix_nano ||= start_time_unix_nano + ndp.time_unix_nano = end_time_unix_nano ndp.dup end end end - def update(increment, attributes) - @data_points[attributes] ||= build_data_point(attributes) - @data_points[attributes].value += increment - nil - end - private - def build_data_point(attributes) + def build_number_data_point(attributes) NumberDataPoint.new( attributes, nil, diff --git a/metrics_sdk/test/opentelemetry/sdk/metrics/aggregation/sum_test.rb b/metrics_sdk/test/opentelemetry/sdk/metrics/aggregation/sum_test.rb index 3c0a3931e3..fc07c55989 100644 --- a/metrics_sdk/test/opentelemetry/sdk/metrics/aggregation/sum_test.rb +++ b/metrics_sdk/test/opentelemetry/sdk/metrics/aggregation/sum_test.rb @@ -7,69 +7,179 @@ require 'test_helper' describe OpenTelemetry::SDK::Metrics::Aggregation::Sum do - let(:sum_aggregation) { OpenTelemetry::SDK::Metrics::Aggregation::Sum.new(aggregation_temporality: aggregation_temporality) } - let(:aggregation_temporality) { :delta } - - # Time in nano - let(:start_time) { (Time.now.to_r * 1_000_000_000).to_i } - let(:end_time) { ((Time.now + 60).to_r * 1_000_000_000).to_i } - - it 'sets the timestamps' do - sum_aggregation.update(0, {}) - ndp = sum_aggregation.collect(start_time, end_time)[0] - _(ndp.start_time_unix_nano).must_equal(start_time) - _(ndp.time_unix_nano).must_equal(end_time) + let(:start_time_unix_nano) { now_in_nano } + let(:end_time_unix_nano) { start_time_unix_nano + 60*(10**9) } + + describe '.new' do + it 'defaults to aggregation_temporality :delta' do + sum = build_sum + + assert(sum.aggregation_temporality == :delta) + end + + it 'defaults to monotonic true' do + sum = build_sum + + assert(sum.monotonic == true) + end end - it 'aggregates and collects' do - sum_aggregation.update(1, {}) - sum_aggregation.update(2, {}) + describe '#aggregation_temporality' do + it 'returns aggregation_temporality' do + sum = build_sum(aggregation_temporality: :delta) + assert(sum.aggregation_temporality == :delta) - sum_aggregation.update(2, 'foo' => 'bar') - sum_aggregation.update(2, 'foo' => 'bar') + sum = build_sum(aggregation_temporality: :cumulative) + assert(sum.aggregation_temporality == :cumulative) + end + end - ndps = sum_aggregation.collect(start_time, end_time) - _(ndps[0].value).must_equal(3) - _(ndps[0].attributes).must_equal({}) + describe '#monotonic' do + it 'returns monotonic' do + sum = build_sum(monotonic: true) + assert(sum.monotonic == true) - _(ndps[1].value).must_equal(4) - _(ndps[1].attributes).must_equal('foo' => 'bar') + sum = build_sum(monotonic: false) + assert(sum.monotonic == false) + end end - it 'does not aggregate between collects' do - sum_aggregation.update(1, {}) - sum_aggregation.update(2, {}) - ndps = sum_aggregation.collect(start_time, end_time) + describe '#update' do + describe 'when number data point does not exist' do + it 'creates a new one and set increment' do + sum = build_sum + + sum.update(5, { 'service' => 'aaa' }) + sum.update(1, { 'service' => 'bbb' }) + + number_data_points = sum.collect(start_time_unix_nano, end_time_unix_nano) + assert(number_data_points.size == 2) - sum_aggregation.update(1, {}) - # Assert that the recent update does not - # impact the already collected metrics - _(ndps[0].value).must_equal(3) + assert(number_data_points[0].value == 5) + assert(number_data_points[0].attributes == { 'service' => 'aaa' }) - ndps = sum_aggregation.collect(start_time, end_time) - # Assert that we are not accumulating values - # between calls to collect - _(ndps[0].value).must_equal(1) + assert(number_data_points[1].value == 1) + assert(number_data_points[1].attributes == { 'service' => 'bbb' }) + end + end + + describe 'when number data point exists' do + it 'updates the existing one adding increment' do + sum = build_sum + + sum.update(5, { 'service' => 'aaa' }) + sum.update(1, { 'service' => 'bbb' }) + sum.update(10, { 'service' => 'aaa' }) + + number_data_points = sum.collect(start_time_unix_nano, end_time_unix_nano) + assert(number_data_points.size == 2) + + assert(number_data_points[0].value == 15) + assert(number_data_points[0].attributes == { 'service' => 'aaa' }) + + assert(number_data_points[1].value == 1) + assert(number_data_points[1].attributes == { 'service' => 'bbb' }) + end + end end - describe 'when aggregation_temporality is not delta' do - let(:aggregation_temporality) { :not_delta } + describe '#collect' do + describe 'when aggregation_temporality is :delta' do + it'sets timestamps, returns array of number data points and clears, not aggregating between calls' do + sum = build_sum(aggregation_temporality: :delta) + + sum.update(5, { 'service' => 'aaa' }) + sum.update(1, { 'service' => 'bbb' }) + + number_data_points = sum.collect(start_time_unix_nano, end_time_unix_nano) + assert(number_data_points.size == 2) + + assert(number_data_points[0].value == 5) + assert(number_data_points[0].attributes == { 'service' => 'aaa' }) + assert(number_data_points[0].start_time_unix_nano == start_time_unix_nano) + assert(number_data_points[0].time_unix_nano == end_time_unix_nano) - it 'allows metrics to accumulate' do - sum_aggregation.update(1, {}) - sum_aggregation.update(2, {}) - ndps = sum_aggregation.collect(start_time, end_time) + assert(number_data_points[1].value == 1) + assert(number_data_points[1].attributes == { 'service' => 'bbb' }) + assert(number_data_points[1].start_time_unix_nano == start_time_unix_nano) + assert(number_data_points[1].time_unix_nano == end_time_unix_nano) - sum_aggregation.update(1, {}) - # Assert that the recent update does not - # impact the already collected metrics - _(ndps[0].value).must_equal(3) + new_start_time_unix_nano = start_time_unix_nano + 10*(10**9) + new_end_time_unix_nano = end_time_unix_nano + 10*(10**9) - ndps = sum_aggregation.collect(start_time, end_time) - # Assert that we are accumulating values - # and not just capturing the delta since - # the previous collect call - _(ndps[0].value).must_equal(4) + number_data_points = sum.collect(new_start_time_unix_nano, new_end_time_unix_nano) + assert(number_data_points.empty?) + + sum.update(10, { 'service' => 'aaa' }) + + number_data_points = sum.collect(new_start_time_unix_nano, new_end_time_unix_nano) + assert(number_data_points.size == 1) + + assert(number_data_points[0].value == 10) + assert(number_data_points[0].attributes == { 'service' => 'aaa' }) + assert(number_data_points[0].start_time_unix_nano == new_start_time_unix_nano) + assert(number_data_points[0].time_unix_nano == new_end_time_unix_nano) + + number_data_points = sum.collect(new_start_time_unix_nano, new_end_time_unix_nano) + assert(number_data_points.empty?) + end end + + describe 'when aggregation_temporality is not :delta' do + it 'sets timestamps, returns array of number data points but does not clear, aggregating between calls' do + sum = build_sum(aggregation_temporality: :anything) + + sum.update(5, { 'service' => 'aaa' }) + sum.update(1, { 'service' => 'bbb' }) + + number_data_points = sum.collect(start_time_unix_nano, end_time_unix_nano) + assert(number_data_points.size == 2) + + assert(number_data_points[0].value == 5) + assert(number_data_points[0].attributes == { 'service' => 'aaa' }) + assert(number_data_points[0].start_time_unix_nano == start_time_unix_nano) + assert(number_data_points[0].time_unix_nano == end_time_unix_nano) + + assert(number_data_points[1].value == 1) + assert(number_data_points[1].attributes == { 'service' => 'bbb' }) + assert(number_data_points[1].start_time_unix_nano == start_time_unix_nano) + assert(number_data_points[1].time_unix_nano == end_time_unix_nano) + + sum.update(10, { 'service' => 'aaa' }) + sum.update(3, { 'service' => 'ccc' }) + + new_start_time_unix_nano = start_time_unix_nano + 10*(10**9) + new_end_time_unix_nano = end_time_unix_nano + 10*(10**9) + + number_data_points = sum.collect(new_start_time_unix_nano, new_end_time_unix_nano) + assert(number_data_points.size == 3) + + assert(number_data_points[0].value == 15) + assert(number_data_points[0].attributes == { 'service' => 'aaa' }) + assert(number_data_points[0].start_time_unix_nano == start_time_unix_nano) + assert(number_data_points[0].time_unix_nano == new_end_time_unix_nano) + + assert(number_data_points[1].value == 1) + assert(number_data_points[1].attributes == { 'service' => 'bbb' }) + assert(number_data_points[1].start_time_unix_nano == start_time_unix_nano) + assert(number_data_points[1].time_unix_nano == new_end_time_unix_nano) + + assert(number_data_points[2].value == 3) + assert(number_data_points[2].attributes == { 'service' => 'ccc' }) + assert(number_data_points[2].start_time_unix_nano == new_start_time_unix_nano) # new start time + assert(number_data_points[2].time_unix_nano == new_end_time_unix_nano) + + number_data_points = sum.collect(new_start_time_unix_nano, new_end_time_unix_nano) + assert(number_data_points.size == 3) + end + end + end + + def now_in_nano + (Time.now.to_r * 1_000_000_000).to_i + end + + def build_sum(**kwargs) + OpenTelemetry::SDK::Metrics::Aggregation::Sum.new(**kwargs) end end