Skip to content

Commit

Permalink
fix: Coerce aggregation_temporality to symbol (#1741)
Browse files Browse the repository at this point in the history
The OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE env var sets the
temporality as a string. However, checks throughout the metrics SDK and
the OTLP exporter expect the temporality to be a symbol.

Now, when aggregations that use this env var are initialized, the
temporality will be coerced into a symbol.
  • Loading branch information
kaylareopelle authored Oct 16, 2024
1 parent 15b39ff commit 044336d
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 2 deletions.
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
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

0 comments on commit 044336d

Please sign in to comment.