Skip to content

Commit

Permalink
fix: Coerce aggregation_temporality to symbol
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 committed Oct 7, 2024
1 parent e16803c commit 6d9fb2a
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ def initialize(
record_min_max: true
)
@data_points = {}
@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
@data_points = {}
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,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, {})
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 @@ -14,6 +14,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, {})
ndp = sum_aggregation.collect(start_time, end_time)[0]
Expand Down

0 comments on commit 6d9fb2a

Please sign in to comment.