Skip to content

Commit

Permalink
Update Instrument validation and move to SDK
Browse files Browse the repository at this point in the history
  • Loading branch information
@xuan-cao-swi
xuan-cao-swi committed Sep 15, 2024
1 parent e16803c commit e693326
Show file tree
Hide file tree
Showing 4 changed files with 92 additions and 52 deletions.
13 changes: 0 additions & 13 deletions metrics_api/lib/opentelemetry/metrics/meter.rb
View file
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
View file
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
13 changes: 13 additions & 0 deletions metrics_sdk/lib/opentelemetry/sdk/metrics/meter.rb
View file
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
61 changes: 61 additions & 0 deletions metrics_sdk/test/opentelemetry/sdk/metrics/meter_test.rb
View file
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 e693326

Please sign in to comment.