Merge branch 'main' into update-exporter-appraisals

Author: kaylareopelle

metrics_api/lib/opentelemetry/metrics/meter.rb

@@ -15,8 +15,6 @@ class Meter
       UP_DOWN_COUNTER = Instrument::UpDownCounter.new
       OBSERVABLE_UP_DOWN_COUNTER = Instrument::ObservableUpDownCounter.new
 
-      NAME_REGEX = /\A[a-zA-Z][-.\w]{0,62}\z/
-
       private_constant(:COUNTER, :OBSERVABLE_COUNTER, :HISTOGRAM, :OBSERVABLE_GAUGE, :UP_DOWN_COUNTER, :OBSERVABLE_UP_DOWN_COUNTER)
 
       DuplicateInstrumentError = Class.new(OpenTelemetry::Error)
@@ -56,23 +54,12 @@ def create_observable_up_down_counter(name, callback:, unit: nil, description: n
       private
 
       def create_instrument(kind, name, unit, description, callback)
-        raise InstrumentNameError if name.nil?
-        raise InstrumentNameError if name.empty?
-        raise InstrumentNameError unless NAME_REGEX.match?(name)
-        raise InstrumentUnitError if unit && (!unit.ascii_only? || unit.size > 63)
-        raise InstrumentDescriptionError if description && (description.size > 1023 || !utf8mb3_encoding?(description.dup))
-
         @mutex.synchronize do
           OpenTelemetry.logger.warn("duplicate instrument registration occurred for instrument #{name}") if @instrument_registry.include? name
 
           @instrument_registry[name] = yield
         end
       end
-
-      def utf8mb3_encoding?(string)
-        string.force_encoding('UTF-8').valid_encoding? &&
-          string.each_char { |c| return false if c.bytesize >= 4 }
-      end
     end
   end
 end

metrics_api/test/opentelemetry/metrics/meter_test.rb

@@ -7,11 +7,6 @@
 require 'test_helper'
 
 describe OpenTelemetry::Metrics::Meter do
-  INSTRUMENT_NAME_ERROR = OpenTelemetry::Metrics::Meter::InstrumentNameError
-  INSTRUMENT_UNIT_ERROR = OpenTelemetry::Metrics::Meter::InstrumentUnitError
-  INSTRUMENT_DESCRIPTION_ERROR = OpenTelemetry::Metrics::Meter::InstrumentDescriptionError
-  DUPLICATE_INSTRUMENT_ERROR = OpenTelemetry::Metrics::Meter::DuplicateInstrumentError
-
   let(:meter_provider) { OpenTelemetry::Metrics::MeterProvider.new }
   let(:meter) { meter_provider.meter('test-meter') }
 
@@ -24,50 +19,34 @@
       end
     end
 
-    it 'instrument name must not be nil' do
-      _(-> { meter.create_counter(nil) }).must_raise(INSTRUMENT_NAME_ERROR)
-    end
-
-    it 'instument name must not be an empty string' do
-      _(-> { meter.create_counter('') }).must_raise(INSTRUMENT_NAME_ERROR)
-    end
-
-    it 'instrument name must have an alphabetic first character' do
-      _(meter.create_counter('one_counter'))
-      _(-> { meter.create_counter('1_counter') }).must_raise(INSTRUMENT_NAME_ERROR)
-    end
-
-    it 'instrument name must not exceed 63 character limit' do
-      long_name = 'a' * 63
-      meter.create_counter(long_name)
-      _(-> { meter.create_counter(long_name + 'a') }).must_raise(INSTRUMENT_NAME_ERROR)
+    it 'test create_counter' do
+      counter = meter.create_counter('test')
+      _(counter.class).must_equal(OpenTelemetry::Metrics::Instrument::Counter)
     end
 
-    it 'instrument name must belong to alphanumeric characters, _, ., and -' do
-      meter.create_counter('a_-..-_a')
-      _(-> { meter.create_counter('a@') }).must_raise(INSTRUMENT_NAME_ERROR)
-      _(-> { meter.create_counter('a!') }).must_raise(INSTRUMENT_NAME_ERROR)
+    it 'test create_histogram' do
+      counter = meter.create_histogram('test')
+      _(counter.class).must_equal(OpenTelemetry::Metrics::Instrument::Histogram)
     end
 
-    it 'instrument unit must be ASCII' do
-      _(-> { meter.create_counter('a_counter', unit: 'á') }).must_raise(INSTRUMENT_UNIT_ERROR)
+    it 'test create_up_down_counter' do
+      counter = meter.create_up_down_counter('test')
+      _(counter.class).must_equal(OpenTelemetry::Metrics::Instrument::UpDownCounter)
     end
 
-    it 'instrument unit must not exceed 63 characters' do
-      long_unit = 'a' * 63
-      meter.create_counter('a_counter', unit: long_unit)
-      _(-> { meter.create_counter('b_counter', unit: long_unit + 'a') }).must_raise(INSTRUMENT_UNIT_ERROR)
+    it 'test create_observable_counter' do
+      counter = meter.create_observable_counter('test', callback: -> {})
+      _(counter.class).must_equal(OpenTelemetry::Metrics::Instrument::ObservableCounter)
     end
 
-    it 'instrument description must be utf8mb3' do
-      _(-> { meter.create_counter('a_counter', description: '💩'.dup) }).must_raise(INSTRUMENT_DESCRIPTION_ERROR)
-      _(-> { meter.create_counter('b_counter', description: "\xc2".dup) }).must_raise(INSTRUMENT_DESCRIPTION_ERROR)
+    it 'test create_observable_gauge' do
+      counter = meter.create_observable_gauge('test', callback: -> {})
+      _(counter.class).must_equal(OpenTelemetry::Metrics::Instrument::ObservableGauge)
     end
 
-    it 'instrument description must not exceed 1023 characters' do
-      long_description = 'a' * 1023
-      meter.create_counter('a_counter', description: long_description)
-      _(-> { meter.create_counter('b_counter', description: long_description + 'a') }).must_raise(INSTRUMENT_DESCRIPTION_ERROR)
+    it 'test create_observable_up_down_counter' do
+      counter = meter.create_observable_up_down_counter('test', callback: -> {})
+      _(counter.class).must_equal(OpenTelemetry::Metrics::Instrument::ObservableUpDownCounter)
     end
   end
 end

metrics_sdk/lib/opentelemetry/sdk/metrics/aggregation/explicit_bucket_histogram.rb

@@ -25,7 +25,8 @@ def initialize(
             boundaries: DEFAULT_BOUNDARIES,
             record_min_max: true
           )
-            @aggregation_temporality = aggregation_temporality
+
+            @aggregation_temporality = aggregation_temporality.to_sym
             @boundaries = boundaries && !boundaries.empty? ? boundaries.sort : nil
             @record_min_max = record_min_max
           end

metrics_sdk/lib/opentelemetry/sdk/metrics/aggregation/sum.rb

@@ -15,7 +15,7 @@ class Sum
 
           def initialize(aggregation_temporality: ENV.fetch('OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE', :delta))
             # TODO: the default should be :cumulative, see issue #1555
-            @aggregation_temporality = aggregation_temporality
+            @aggregation_temporality = aggregation_temporality.to_sym
           end
 
           def collect(start_time, end_time, data_points)

metrics_sdk/lib/opentelemetry/sdk/metrics/meter.rb

@@ -11,6 +11,8 @@ module SDK
     module Metrics
       # {Meter} is the SDK implementation of {OpenTelemetry::Metrics::Meter}.
       class Meter < OpenTelemetry::Metrics::Meter
+        NAME_REGEX = /\A[a-zA-Z][-.\w]{0,62}\z/
+
         # @api private
         #
         # Returns a new {Meter} instance.
@@ -34,6 +36,12 @@ def add_metric_reader(metric_reader)
         end
 
         def create_instrument(kind, name, unit, description, callback)
+          raise InstrumentNameError if name.nil?
+          raise InstrumentNameError if name.empty?
+          raise InstrumentNameError unless NAME_REGEX.match?(name)
+          raise InstrumentUnitError if unit && (!unit.ascii_only? || unit.size > 63)
+          raise InstrumentDescriptionError if description && (description.size > 1023 || !utf8mb3_encoding?(description.dup))
+
           super do
             case kind
             when :counter then OpenTelemetry::SDK::Metrics::Instrument::Counter.new(name, unit, description, @instrumentation_scope, @meter_provider)
@@ -45,6 +53,11 @@ def create_instrument(kind, name, unit, description, callback)
             end
           end
         end
+
+        def utf8mb3_encoding?(string)
+          string.force_encoding('UTF-8').valid_encoding? &&
+            string.each_char { |c| return false if c.bytesize >= 4 }
+        end
       end
     end
   end

metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb

@@ -37,6 +37,7 @@ def meter(name, version: nil)
             OpenTelemetry.logger.warn 'calling MeterProvider#meter after shutdown, a noop meter will be returned.'
             OpenTelemetry::Metrics::Meter.new
           else
+            OpenTelemetry.logger.warn "Invalid meter name provided: #{name.nil? ? 'nil' : 'empty'} value" if name.to_s.empty?
             @mutex.synchronize { @meter_registry[Key.new(name, version)] ||= Meter.new(name, version, self) }
           end
         end

metrics_sdk/test/opentelemetry/sdk/metrics/aggregation/explicit_bucket_histogram_test.rb

@@ -22,6 +22,27 @@
   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 }
 
+  describe '#initialize' do
+    it 'defaults to the delta aggregation temporality' do
+      exp = OpenTelemetry::SDK::Metrics::Aggregation::ExplicitBucketHistogram.new
+      _(exp.instance_variable_get(:@aggregation_temporality)).must_equal :delta
+    end
+
+    it 'sets parameters from the environment and converts them to symbols' do
+      exp = OpenTelemetry::TestHelpers.with_env('OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE' => 'potato') do
+        OpenTelemetry::SDK::Metrics::Aggregation::ExplicitBucketHistogram.new
+      end
+      _(exp.instance_variable_get(:@aggregation_temporality)).must_equal :potato
+    end
+
+    it 'prefers explicit parameters rather than the environment and converts them to symbols' do
+      exp = OpenTelemetry::TestHelpers.with_env('OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE' => 'potato') do
+        OpenTelemetry::SDK::Metrics::Aggregation::ExplicitBucketHistogram.new(aggregation_temporality: 'pickles')
+      end
+      _(exp.instance_variable_get(:@aggregation_temporality)).must_equal :pickles
+    end
+  end
+
   describe '#collect' do
     it 'returns all the data points' do
       ebh.update(0, {}, data_points)

metrics_sdk/test/opentelemetry/sdk/metrics/aggregation/sum_test.rb

@@ -15,6 +15,27 @@
   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 }
 
+  describe '#initialize' do
+    it 'defaults to the delta aggregation temporality' do
+      exp = OpenTelemetry::SDK::Metrics::Aggregation::ExplicitBucketHistogram.new
+      _(exp.instance_variable_get(:@aggregation_temporality)).must_equal :delta
+    end
+
+    it 'sets parameters from the environment and converts them to symbols' do
+      exp = OpenTelemetry::TestHelpers.with_env('OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE' => 'potato') do
+        OpenTelemetry::SDK::Metrics::Aggregation::ExplicitBucketHistogram.new
+      end
+      _(exp.instance_variable_get(:@aggregation_temporality)).must_equal :potato
+    end
+
+    it 'prefers explicit parameters rather than the environment and converts them to symbols' do
+      exp = OpenTelemetry::TestHelpers.with_env('OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE' => 'potato') do
+        OpenTelemetry::SDK::Metrics::Aggregation::ExplicitBucketHistogram.new(aggregation_temporality: 'pickles')
+      end
+      _(exp.instance_variable_get(:@aggregation_temporality)).must_equal :pickles
+    end
+  end
+
   it 'sets the timestamps' do
     sum_aggregation.update(0, {}, data_points)
     ndp = sum_aggregation.collect(start_time, end_time, data_points)[0]

metrics_sdk/test/opentelemetry/sdk/metrics/meter_test.rb

@@ -58,4 +58,65 @@
       _(instrument).must_be_instance_of OpenTelemetry::SDK::Metrics::Instrument::ObservableUpDownCounter
     end
   end
+
+  describe 'creating an instrument' do
+    INSTRUMENT_NAME_ERROR = OpenTelemetry::Metrics::Meter::InstrumentNameError
+    INSTRUMENT_UNIT_ERROR = OpenTelemetry::Metrics::Meter::InstrumentUnitError
+    INSTRUMENT_DESCRIPTION_ERROR = OpenTelemetry::Metrics::Meter::InstrumentDescriptionError
+    DUPLICATE_INSTRUMENT_ERROR = OpenTelemetry::Metrics::Meter::DuplicateInstrumentError
+
+    it 'duplicate instrument registration logs a warning' do
+      OpenTelemetry::TestHelpers.with_test_logger do |log_stream|
+        meter.create_counter('a_counter')
+        meter.create_counter('a_counter')
+        _(log_stream.string).must_match(/duplicate instrument registration occurred for instrument a_counter/)
+      end
+    end
+
+    it 'instrument name must not be nil' do
+      _(-> { meter.create_counter(nil) }).must_raise(INSTRUMENT_NAME_ERROR)
+    end
+
+    it 'instument name must not be an empty string' do
+      _(-> { meter.create_counter('') }).must_raise(INSTRUMENT_NAME_ERROR)
+    end
+
+    it 'instrument name must have an alphabetic first character' do
+      _(meter.create_counter('one_counter'))
+      _(-> { meter.create_counter('1_counter') }).must_raise(INSTRUMENT_NAME_ERROR)
+    end
+
+    it 'instrument name must not exceed 63 character limit' do
+      long_name = 'a' * 63
+      meter.create_counter(long_name)
+      _(-> { meter.create_counter(long_name + 'a') }).must_raise(INSTRUMENT_NAME_ERROR)
+    end
+
+    it 'instrument name must belong to alphanumeric characters, _, ., and -' do
+      meter.create_counter('a_-..-_a')
+      _(-> { meter.create_counter('a@') }).must_raise(INSTRUMENT_NAME_ERROR)
+      _(-> { meter.create_counter('a!') }).must_raise(INSTRUMENT_NAME_ERROR)
+    end
+
+    it 'instrument unit must be ASCII' do
+      _(-> { meter.create_counter('a_counter', unit: 'á') }).must_raise(INSTRUMENT_UNIT_ERROR)
+    end
+
+    it 'instrument unit must not exceed 63 characters' do
+      long_unit = 'a' * 63
+      meter.create_counter('a_counter', unit: long_unit)
+      _(-> { meter.create_counter('b_counter', unit: long_unit + 'a') }).must_raise(INSTRUMENT_UNIT_ERROR)
+    end
+
+    it 'instrument description must be utf8mb3' do
+      _(-> { meter.create_counter('a_counter', description: '💩'.dup) }).must_raise(INSTRUMENT_DESCRIPTION_ERROR)
+      _(-> { meter.create_counter('b_counter', description: "\xc2".dup) }).must_raise(INSTRUMENT_DESCRIPTION_ERROR)
+    end
+
+    it 'instrument description must not exceed 1023 characters' do
+      long_description = 'a' * 1023
+      meter.create_counter('a_counter', description: long_description)
+      _(-> { meter.create_counter('b_counter', description: long_description + 'a') }).must_raise(INSTRUMENT_DESCRIPTION_ERROR)
+    end
+  end
 end