diff --git a/metrics_api/lib/opentelemetry/internal/proxy_instrument.rb b/metrics_api/lib/opentelemetry/internal/proxy_instrument.rb index 117a400778..d6b38c22c0 100644 --- a/metrics_api/lib/opentelemetry/internal/proxy_instrument.rb +++ b/metrics_api/lib/opentelemetry/internal/proxy_instrument.rb @@ -8,19 +8,22 @@ module OpenTelemetry module Internal # @api private class ProxyInstrument - def initialize(kind, name, unit, desc, callable) + def initialize(kind, name, unit, desc, callable, advice = Metrics::Meter::EMPTY_ADVICE) @kind = kind @name = name @unit = unit @desc = desc @callable = callable + @advice = advice @delegate = nil end def upgrade_with(meter) @delegate = case @kind - when :counter, :histogram, :up_down_counter + when :counter, :up_down_counter meter.send("create_#{@kind}", @name, unit: @unit, description: @desc) + when :histogram + meter.send("create_#{@kind}", @name, unit: @unit, description: @desc, advice: @advice) when :observable_counter, :observable_gauge, :observable_up_down_counter meter.send("create_#{@kind}", @name, unit: @unit, description: @desc, callback: @callback) end diff --git a/metrics_api/lib/opentelemetry/internal/proxy_meter.rb b/metrics_api/lib/opentelemetry/internal/proxy_meter.rb index 74d497c684..65e51d4a35 100644 --- a/metrics_api/lib/opentelemetry/internal/proxy_meter.rb +++ b/metrics_api/lib/opentelemetry/internal/proxy_meter.rb @@ -38,13 +38,13 @@ def delegate=(meter) private - def create_instrument(kind, name, unit, description, callback) + def create_instrument(kind, name, unit, description, callback, advice = nil) super do - next ProxyInstrument.new(kind, name, unit, description, callback) if @delegate.nil? + next ProxyInstrument.new(kind, name, unit, description, callback, advice) 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 :histogram then @delegate.create_histogram(name, unit: unit, description: description, advice: advice) 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) diff --git a/metrics_api/lib/opentelemetry/metrics/meter.rb b/metrics_api/lib/opentelemetry/metrics/meter.rb index f44b586110..300437adf1 100644 --- a/metrics_api/lib/opentelemetry/metrics/meter.rb +++ b/metrics_api/lib/opentelemetry/metrics/meter.rb @@ -17,6 +17,8 @@ class Meter private_constant(:COUNTER, :OBSERVABLE_COUNTER, :HISTOGRAM, :OBSERVABLE_GAUGE, :UP_DOWN_COUNTER, :OBSERVABLE_UP_DOWN_COUNTER) + EMPTY_ADVICE = {}.freeze + DuplicateInstrumentError = Class.new(OpenTelemetry::Error) InstrumentNameError = Class.new(OpenTelemetry::Error) InstrumentUnitError = Class.new(OpenTelemetry::Error) @@ -31,8 +33,8 @@ def create_counter(name, unit: nil, description: nil) create_instrument(:counter, name, unit, description, nil) { COUNTER } end - def create_histogram(name, unit: nil, description: nil) - create_instrument(:histogram, name, unit, description, nil) { HISTOGRAM } + def create_histogram(name, unit: nil, description: nil, advice: EMPTY_ADVICE) + create_instrument(:histogram, name, unit, description, nil, advice) { HISTOGRAM } end def create_up_down_counter(name, unit: nil, description: nil) @@ -53,7 +55,7 @@ def create_observable_up_down_counter(name, callback:, unit: nil, description: n private - def create_instrument(kind, name, unit, description, callback) + def create_instrument(kind, name, unit, description, callback, advice = EMPTY_ADVICE) @mutex.synchronize do OpenTelemetry.logger.warn("duplicate instrument registration occurred for instrument #{name}") if @instrument_registry.include? name diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/histogram.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/histogram.rb index 5c8e00f157..92e839a38e 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/histogram.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/histogram.rb @@ -10,6 +10,12 @@ module Metrics module Instrument # {Histogram} is the SDK implementation of {OpenTelemetry::Metrics::Histogram}. class Histogram < OpenTelemetry::SDK::Metrics::Instrument::SynchronousInstrument + def initialize(name, unit, description, instrumentation_scope, meter_provider, advice = OpenTelemetry::Metrics::Meter::EMPTY_ADVICE) + @advice = advice + + super + end + # Returns the instrument kind as a Symbol # # @return [Symbol] @@ -32,6 +38,12 @@ def record(amount, attributes: {}) nil end + def register_with_new_metric_store(metric_store, aggregation: default_aggregation) + aggregation = OpenTelemetry::SDK::Metrics::Aggregation::ExplicitBucketHistogram.new(boundaries: @advice[:explicit_bucket_boundaries]) if @advice&.key?(:explicit_bucket_boundaries) + + super + end + private def default_aggregation diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/synchronous_instrument.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/synchronous_instrument.rb index f5bf321f0a..db5905afb1 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/synchronous_instrument.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/instrument/synchronous_instrument.rb @@ -11,7 +11,7 @@ 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) + def initialize(name, unit, description, instrumentation_scope, meter_provider, advice = OpenTelemetry::Metrics::Meter::EMPTY_ADVICE) @name = name @unit = unit @description = description diff --git a/metrics_sdk/lib/opentelemetry/sdk/metrics/meter.rb b/metrics_sdk/lib/opentelemetry/sdk/metrics/meter.rb index 1307817ec6..07e92e212a 100644 --- a/metrics_sdk/lib/opentelemetry/sdk/metrics/meter.rb +++ b/metrics_sdk/lib/opentelemetry/sdk/metrics/meter.rb @@ -35,7 +35,7 @@ def add_metric_reader(metric_reader) end end - def create_instrument(kind, name, unit, description, callback) + def create_instrument(kind, name, unit, description, callback, advice = OpenTelemetry::Metrics::Meter::EMPTY_ADVICE) raise InstrumentNameError if name.nil? raise InstrumentNameError if name.empty? raise InstrumentNameError unless NAME_REGEX.match?(name) @@ -46,7 +46,7 @@ def create_instrument(kind, name, unit, description, callback) 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 :histogram then OpenTelemetry::SDK::Metrics::Instrument::Histogram.new(name, unit, description, @instrumentation_scope, @meter_provider, advice) 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) diff --git a/metrics_sdk/test/opentelemetry/sdk/metrics/instrument/histogram_test.rb b/metrics_sdk/test/opentelemetry/sdk/metrics/instrument/histogram_test.rb index 771ffaef83..031c455c6a 100644 --- a/metrics_sdk/test/opentelemetry/sdk/metrics/instrument/histogram_test.rb +++ b/metrics_sdk/test/opentelemetry/sdk/metrics/instrument/histogram_test.rb @@ -35,4 +35,29 @@ _(last_snapshot[0].data_points[0].attributes).must_equal('foo' => 'bar') _(last_snapshot[0].aggregation_temporality).must_equal(:delta) end + + it 'can apply advice to change the explicit bucket boundaries' do + histogram = meter.create_histogram('histogram', unit: 's', description: 'test', advice: { explicit_bucket_boundaries: [5, 10, 15] }) + + histogram.record(0) + histogram.record(5) + histogram.record(5) + histogram.record(10) + histogram.record(20) + + metric_exporter.pull + last_snapshot = metric_exporter.metric_snapshots + + _(last_snapshot[0].data_points[0].bucket_counts).must_equal([3, 1, 0, 1]) + _(last_snapshot[0].name).must_equal('histogram') + _(last_snapshot[0].unit).must_equal('s') + _(last_snapshot[0].description).must_equal('test') + _(last_snapshot[0].instrumentation_scope.name).must_equal('test') + _(last_snapshot[0].data_points[0].count).must_equal(5) + _(last_snapshot[0].data_points[0].sum).must_equal(40) + _(last_snapshot[0].data_points[0].min).must_equal(0) + _(last_snapshot[0].data_points[0].max).must_equal(20) + _(last_snapshot[0].data_points[0].attributes).must_equal({}) + _(last_snapshot[0].aggregation_temporality).must_equal(:delta) + end end diff --git a/metrics_sdk/test/opentelemetry/sdk/metrics/meter_test.rb b/metrics_sdk/test/opentelemetry/sdk/metrics/meter_test.rb index e8415d8d35..4342bdbd5b 100644 --- a/metrics_sdk/test/opentelemetry/sdk/metrics/meter_test.rb +++ b/metrics_sdk/test/opentelemetry/sdk/metrics/meter_test.rb @@ -23,6 +23,19 @@ instrument = meter.create_histogram('a_histogram', unit: 'minutes', description: 'useful description') _(instrument).must_be_instance_of OpenTelemetry::SDK::Metrics::Instrument::Histogram end + + it 'accepts advice' do + advice = { wisdom: 'this too shall pass' } + instrument = meter.create_histogram('histogram', description: 'stuff', unit: 'things', advice: advice) + + _(instrument.instance_variable_get(:@advice)).must_equal(advice) + end + + it 'does not require advice' do + instrument = meter.create_histogram('histogram', description: 'stuff', unit: 'things') + + assert_equal({}, instrument.instance_variable_get(:@advice)) + end end describe '#create_up_down_counter' do diff --git a/metrics_sdk/test/test_helper.rb b/metrics_sdk/test/test_helper.rb index 99aa4deee0..c1e4c53c0b 100644 --- a/metrics_sdk/test/test_helper.rb +++ b/metrics_sdk/test/test_helper.rb @@ -13,6 +13,10 @@ require 'minitest/autorun' require 'pry' +# The metrics test output will include an error about a missing OTLP exporter +# unless we set the traces exporter to 'none' +ENV['OTEL_TRACES_EXPORTER'] = 'none' + # reset_metrics_sdk is a test helper used to clear # SDK configuration state between calls def reset_metrics_sdk