Skip to content

Commit

Permalink
Merge branch 'main' into truffle-ruby-intermittent-failure
Browse files Browse the repository at this point in the history
  • Loading branch information
kaylareopelle authored Oct 21, 2024
2 parents 363b808 + 4c9f7e1 commit 9e991cb
Show file tree
Hide file tree
Showing 9 changed files with 138 additions and 54 deletions.
13 changes: 0 additions & 13 deletions metrics_api/lib/opentelemetry/metrics/meter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
57 changes: 18 additions & 39 deletions metrics_api/test/opentelemetry/metrics/meter_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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') }

Expand All @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
13 changes: 13 additions & 0 deletions metrics_sdk/lib/opentelemetry/sdk/metrics/meter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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)
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
21 changes: 21 additions & 0 deletions metrics_sdk/test/opentelemetry/sdk/metrics/aggregation/sum_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
61 changes: 61 additions & 0 deletions metrics_sdk/test/opentelemetry/sdk/metrics/meter_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 9e991cb

Please sign in to comment.