Skip to content

Commit cb05fa5

Browse files
xuan-cao-swikaylareopelle
xuan-cao-swi
and
kaylareopelle
authored
Update Instrument validation and move to SDK (#1735)
Co-authored-by: Kayla Reopelle <[email protected]>
1 parent 044336d commit cb05fa5

File tree

4 files changed

+92
-52
lines changed

4 files changed

+92
-52
lines changed

‎metrics_api/lib/opentelemetry/metrics/meter.rb‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,6 @@ class Meter
1515
UP_DOWN_COUNTER = Instrument::UpDownCounter.new
1616
OBSERVABLE_UP_DOWN_COUNTER = Instrument::ObservableUpDownCounter.new
1717

18-
NAME_REGEX = /\A[a-zA-Z][-.\w]{0,62}\z/
19-
2018
private_constant(:COUNTER, :OBSERVABLE_COUNTER, :HISTOGRAM, :OBSERVABLE_GAUGE, :UP_DOWN_COUNTER, :OBSERVABLE_UP_DOWN_COUNTER)
2119

2220
DuplicateInstrumentError = Class.new(OpenTelemetry::Error)
@@ -56,23 +54,12 @@ def create_observable_up_down_counter(name, callback:, unit: nil, description: n
5654
private
5755

5856
def create_instrument(kind, name, unit, description, callback)
59-
raise InstrumentNameError if name.nil?
60-
raise InstrumentNameError if name.empty?
61-
raise InstrumentNameError unless NAME_REGEX.match?(name)
62-
raise InstrumentUnitError if unit && (!unit.ascii_only? || unit.size > 63)
63-
raise InstrumentDescriptionError if description && (description.size > 1023 || !utf8mb3_encoding?(description.dup))
64-
6557
@mutex.synchronize do
6658
OpenTelemetry.logger.warn("duplicate instrument registration occurred for instrument #{name}") if @instrument_registry.include? name
6759

6860
@instrument_registry[name] = yield
6961
end
7062
end
71-
72-
def utf8mb3_encoding?(string)
73-
string.force_encoding('UTF-8').valid_encoding? &&
74-
string.each_char { |c| return false if c.bytesize >= 4 }
75-
end
7663
end
7764
end
7865
end

‎metrics_api/test/opentelemetry/metrics/meter_test.rb‎

Lines changed: 18 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,6 @@
77
require 'test_helper'
88

99
describe OpenTelemetry::Metrics::Meter do
10-
INSTRUMENT_NAME_ERROR = OpenTelemetry::Metrics::Meter::InstrumentNameError
11-
INSTRUMENT_UNIT_ERROR = OpenTelemetry::Metrics::Meter::InstrumentUnitError
12-
INSTRUMENT_DESCRIPTION_ERROR = OpenTelemetry::Metrics::Meter::InstrumentDescriptionError
13-
DUPLICATE_INSTRUMENT_ERROR = OpenTelemetry::Metrics::Meter::DuplicateInstrumentError
14-
1510
let(:meter_provider) { OpenTelemetry::Metrics::MeterProvider.new }
1611
let(:meter) { meter_provider.meter('test-meter') }
1712

@@ -24,50 +19,34 @@
2419
end
2520
end
2621

27-
it 'instrument name must not be nil' do
28-
_(-> { meter.create_counter(nil) }).must_raise(INSTRUMENT_NAME_ERROR)
29-
end
30-
31-
it 'instument name must not be an empty string' do
32-
_(-> { meter.create_counter('') }).must_raise(INSTRUMENT_NAME_ERROR)
33-
end
34-
35-
it 'instrument name must have an alphabetic first character' do
36-
_(meter.create_counter('one_counter'))
37-
_(-> { meter.create_counter('1_counter') }).must_raise(INSTRUMENT_NAME_ERROR)
38-
end
39-
40-
it 'instrument name must not exceed 63 character limit' do
41-
long_name = 'a' * 63
42-
meter.create_counter(long_name)
43-
_(-> { meter.create_counter(long_name + 'a') }).must_raise(INSTRUMENT_NAME_ERROR)
22+
it 'test create_counter' do
23+
counter = meter.create_counter('test')
24+
_(counter.class).must_equal(OpenTelemetry::Metrics::Instrument::Counter)
4425
end
4526

46-
it 'instrument name must belong to alphanumeric characters, _, ., and -' do
47-
meter.create_counter('a_-..-_a')
48-
_(-> { meter.create_counter('a@') }).must_raise(INSTRUMENT_NAME_ERROR)
49-
_(-> { meter.create_counter('a!') }).must_raise(INSTRUMENT_NAME_ERROR)
27+
it 'test create_histogram' do
28+
counter = meter.create_histogram('test')
29+
_(counter.class).must_equal(OpenTelemetry::Metrics::Instrument::Histogram)
5030
end
5131

52-
it 'instrument unit must be ASCII' do
53-
_(-> { meter.create_counter('a_counter', unit: 'á') }).must_raise(INSTRUMENT_UNIT_ERROR)
32+
it 'test create_up_down_counter' do
33+
counter = meter.create_up_down_counter('test')
34+
_(counter.class).must_equal(OpenTelemetry::Metrics::Instrument::UpDownCounter)
5435
end
5536

56-
it 'instrument unit must not exceed 63 characters' do
57-
long_unit = 'a' * 63
58-
meter.create_counter('a_counter', unit: long_unit)
59-
_(-> { meter.create_counter('b_counter', unit: long_unit + 'a') }).must_raise(INSTRUMENT_UNIT_ERROR)
37+
it 'test create_observable_counter' do
38+
counter = meter.create_observable_counter('test', callback: -> {})
39+
_(counter.class).must_equal(OpenTelemetry::Metrics::Instrument::ObservableCounter)
6040
end
6141

62-
it 'instrument description must be utf8mb3' do
63-
_(-> { meter.create_counter('a_counter', description: '💩'.dup) }).must_raise(INSTRUMENT_DESCRIPTION_ERROR)
64-
_(-> { meter.create_counter('b_counter', description: "\xc2".dup) }).must_raise(INSTRUMENT_DESCRIPTION_ERROR)
42+
it 'test create_observable_gauge' do
43+
counter = meter.create_observable_gauge('test', callback: -> {})
44+
_(counter.class).must_equal(OpenTelemetry::Metrics::Instrument::ObservableGauge)
6545
end
6646

67-
it 'instrument description must not exceed 1023 characters' do
68-
long_description = 'a' * 1023
69-
meter.create_counter('a_counter', description: long_description)
70-
_(-> { meter.create_counter('b_counter', description: long_description + 'a') }).must_raise(INSTRUMENT_DESCRIPTION_ERROR)
47+
it 'test create_observable_up_down_counter' do
48+
counter = meter.create_observable_up_down_counter('test', callback: -> {})
49+
_(counter.class).must_equal(OpenTelemetry::Metrics::Instrument::ObservableUpDownCounter)
7150
end
7251
end
7352
end

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ module SDK
1111
module Metrics
1212
# {Meter} is the SDK implementation of {OpenTelemetry::Metrics::Meter}.
1313
class Meter < OpenTelemetry::Metrics::Meter
14+
NAME_REGEX = /\A[a-zA-Z][-.\w]{0,62}\z/
15+
1416
# @api private
1517
#
1618
# Returns a new {Meter} instance.
@@ -34,6 +36,12 @@ def add_metric_reader(metric_reader)
3436
end
3537

3638
def create_instrument(kind, name, unit, description, callback)
39+
raise InstrumentNameError if name.nil?
40+
raise InstrumentNameError if name.empty?
41+
raise InstrumentNameError unless NAME_REGEX.match?(name)
42+
raise InstrumentUnitError if unit && (!unit.ascii_only? || unit.size > 63)
43+
raise InstrumentDescriptionError if description && (description.size > 1023 || !utf8mb3_encoding?(description.dup))
44+
3745
super do
3846
case kind
3947
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)
4553
end
4654
end
4755
end
56+
57+
def utf8mb3_encoding?(string)
58+
string.force_encoding('UTF-8').valid_encoding? &&
59+
string.each_char { |c| return false if c.bytesize >= 4 }
60+
end
4861
end
4962
end
5063
end

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

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,4 +58,65 @@
5858
_(instrument).must_be_instance_of OpenTelemetry::SDK::Metrics::Instrument::ObservableUpDownCounter
5959
end
6060
end
61+
62+
describe 'creating an instrument' do
63+
INSTRUMENT_NAME_ERROR = OpenTelemetry::Metrics::Meter::InstrumentNameError
64+
INSTRUMENT_UNIT_ERROR = OpenTelemetry::Metrics::Meter::InstrumentUnitError
65+
INSTRUMENT_DESCRIPTION_ERROR = OpenTelemetry::Metrics::Meter::InstrumentDescriptionError
66+
DUPLICATE_INSTRUMENT_ERROR = OpenTelemetry::Metrics::Meter::DuplicateInstrumentError
67+
68+
it 'duplicate instrument registration logs a warning' do
69+
OpenTelemetry::TestHelpers.with_test_logger do |log_stream|
70+
meter.create_counter('a_counter')
71+
meter.create_counter('a_counter')
72+
_(log_stream.string).must_match(/duplicate instrument registration occurred for instrument a_counter/)
73+
end
74+
end
75+
76+
it 'instrument name must not be nil' do
77+
_(-> { meter.create_counter(nil) }).must_raise(INSTRUMENT_NAME_ERROR)
78+
end
79+
80+
it 'instument name must not be an empty string' do
81+
_(-> { meter.create_counter('') }).must_raise(INSTRUMENT_NAME_ERROR)
82+
end
83+
84+
it 'instrument name must have an alphabetic first character' do
85+
_(meter.create_counter('one_counter'))
86+
_(-> { meter.create_counter('1_counter') }).must_raise(INSTRUMENT_NAME_ERROR)
87+
end
88+
89+
it 'instrument name must not exceed 63 character limit' do
90+
long_name = 'a' * 63
91+
meter.create_counter(long_name)
92+
_(-> { meter.create_counter(long_name + 'a') }).must_raise(INSTRUMENT_NAME_ERROR)
93+
end
94+
95+
it 'instrument name must belong to alphanumeric characters, _, ., and -' do
96+
meter.create_counter('a_-..-_a')
97+
_(-> { meter.create_counter('a@') }).must_raise(INSTRUMENT_NAME_ERROR)
98+
_(-> { meter.create_counter('a!') }).must_raise(INSTRUMENT_NAME_ERROR)
99+
end
100+
101+
it 'instrument unit must be ASCII' do
102+
_(-> { meter.create_counter('a_counter', unit: 'á') }).must_raise(INSTRUMENT_UNIT_ERROR)
103+
end
104+
105+
it 'instrument unit must not exceed 63 characters' do
106+
long_unit = 'a' * 63
107+
meter.create_counter('a_counter', unit: long_unit)
108+
_(-> { meter.create_counter('b_counter', unit: long_unit + 'a') }).must_raise(INSTRUMENT_UNIT_ERROR)
109+
end
110+
111+
it 'instrument description must be utf8mb3' do
112+
_(-> { meter.create_counter('a_counter', description: '💩'.dup) }).must_raise(INSTRUMENT_DESCRIPTION_ERROR)
113+
_(-> { meter.create_counter('b_counter', description: "\xc2".dup) }).must_raise(INSTRUMENT_DESCRIPTION_ERROR)
114+
end
115+
116+
it 'instrument description must not exceed 1023 characters' do
117+
long_description = 'a' * 1023
118+
meter.create_counter('a_counter', description: long_description)
119+
_(-> { meter.create_counter('b_counter', description: long_description + 'a') }).must_raise(INSTRUMENT_DESCRIPTION_ERROR)
120+
end
121+
end
61122
end

0 commit comments

Comments
 (0)