Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: add kind enum type for aggregation_temporality that only allow two types #1769

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -618,7 +618,7 @@
exemplars: nil
)
],
aggregation_temporality: Opentelemetry::Proto::Metrics::V1::AggregationTemporality::AGGREGATION_TEMPORALITY_DELTA
aggregation_temporality: Opentelemetry::Proto::Metrics::V1::AggregationTemporality::AGGREGATION_TEMPORALITY_CUMULATIVE
)
),
Opentelemetry::Proto::Metrics::V1::Metric.new(
Expand All @@ -642,7 +642,7 @@
max: 10
)
],
aggregation_temporality: Opentelemetry::Proto::Metrics::V1::AggregationTemporality::AGGREGATION_TEMPORALITY_DELTA
aggregation_temporality: Opentelemetry::Proto::Metrics::V1::AggregationTemporality::AGGREGATION_TEMPORALITY_CUMULATIVE
)
)
]
Expand Down
1 change: 1 addition & 0 deletions metrics_sdk/lib/opentelemetry/sdk/metrics/aggregation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ module Aggregation
end
end

require 'opentelemetry/sdk/metrics/aggregation/aggregation_temporality'
require 'opentelemetry/sdk/metrics/aggregation/number_data_point'
require 'opentelemetry/sdk/metrics/aggregation/histogram_data_point'
require 'opentelemetry/sdk/metrics/aggregation/explicit_bucket_histogram'
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
# frozen_string_literal: true

# Copyright The OpenTelemetry Authors
#
# SPDX-License-Identifier: Apache-2.0

module OpenTelemetry
module SDK
module Metrics
module Aggregation
# AggregationTemporality represents the temporality of
# data point ({NumberDataPoint} and {HistogramDataPoint}) in {Metrics}.
# It determine whether the data point will be cleared for each metrics pull/export.
class AggregationTemporality
class << self
private :new

# Returns a newly created {AggregationTemporality} with temporality == DELTA
#
# @return [AggregationTemporality]
def delta
new(DELTA)
end

# Returns a newly created {AggregationTemporality} with temporality == CUMULATIVE
#
# @return [AggregationTemporality]
def cumulative
new(CUMULATIVE)
end
end

attr_reader :temporality

# @api private
# The constructor is private and only for use internally by the class.
# Users should use the {delta} and {cumulative} factory methods to obtain
# a {AggregationTemporality} instance.
#
# @param [Integer] temporality One of the status codes below
def initialize(temporality)
@temporality = temporality
end

def delta?
@temporality == :delta
end

def cumulative?
@temporality == :cumulative
end

# delta: data point will be cleared after each metrics pull/export.
DELTA = :delta

# cumulative: data point will NOT be cleared after metrics pull/export.
CUMULATIVE = :cumulative
end
end
end
end
end
10 changes: 6 additions & 4 deletions metrics_sdk/lib/opentelemetry/sdk/metrics/aggregation/drop.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,8 @@ module Metrics
module Aggregation
# Contains the implementation of the Drop aggregation
class Drop
attr_reader :aggregation_temporality

def initialize(aggregation_temporality: :delta)
@aggregation_temporality = aggregation_temporality
def initialize
@aggregation_temporality = nil
end

def collect(start_time, end_time, data_points)
Expand All @@ -30,6 +28,10 @@ def update(increment, attributes, data_points)
)
nil
end

def aggregation_temporality
nil
end
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,24 +14,22 @@ class ExplicitBucketHistogram
DEFAULT_BOUNDARIES = [0, 5, 10, 25, 50, 75, 100, 250, 500, 1000].freeze
private_constant :DEFAULT_BOUNDARIES

attr_reader :aggregation_temporality

# The default value for boundaries represents the following buckets:
# (-inf, 0], (0, 5.0], (5.0, 10.0], (10.0, 25.0], (25.0, 50.0],
# (50.0, 75.0], (75.0, 100.0], (100.0, 250.0], (250.0, 500.0],
# (500.0, 1000.0], (1000.0, +inf)
def initialize(
aggregation_temporality: ENV.fetch('OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE', :delta), # TODO: the default should be :cumulative, see issue #1555
aggregation_temporality: ENV.fetch('OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE', :cumulative),
boundaries: DEFAULT_BOUNDARIES,
record_min_max: true
)
@aggregation_temporality = aggregation_temporality.to_sym
@aggregation_temporality = aggregation_temporality.to_sym == :delta ? AggregationTemporality.delta : AggregationTemporality.cumulative
@boundaries = boundaries && !boundaries.empty? ? boundaries.sort : nil
@record_min_max = record_min_max
end

def collect(start_time, end_time, data_points)
if @aggregation_temporality == :delta
if @aggregation_temporality.delta?
# Set timestamps and 'move' data point values to result.
hdps = data_points.values.map! do |hdp|
hdp.start_time_unix_nano = start_time
Expand Down Expand Up @@ -87,6 +85,10 @@ def update(amount, attributes, data_points)
nil
end

def aggregation_temporality
@aggregation_temporality.temporality
end

private

def empty_bucket_counts
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,12 @@ module Metrics
module Aggregation
# Contains the implementation of the LastValue aggregation
class LastValue
attr_reader :aggregation_temporality

def initialize(aggregation_temporality: :delta)
@aggregation_temporality = aggregation_temporality
@aggregation_temporality = aggregation_temporality == :cumulative ? AggregationTemporality.cumulative : AggregationTemporality.delta
end

def collect(start_time, end_time, data_points)
if @aggregation_temporality == :delta
if @aggregation_temporality.delta?
# Set timestamps and 'move' data point values to result.
ndps = data_points.values.map! do |ndp|
ndp.start_time_unix_nano = start_time
Expand Down Expand Up @@ -46,6 +44,10 @@ def update(increment, attributes, data_points)
)
nil
end

def aggregation_temporality
@aggregation_temporality.temporality
end
end
end
end
Expand Down
13 changes: 7 additions & 6 deletions metrics_sdk/lib/opentelemetry/sdk/metrics/aggregation/sum.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,12 @@ module Aggregation
# Contains the implementation of the Sum aggregation
# https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#sum-aggregation
class Sum
attr_reader :aggregation_temporality

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.to_sym
def initialize(aggregation_temporality: ENV.fetch('OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE', :cumulative))
@aggregation_temporality = aggregation_temporality.to_sym == :delta ? AggregationTemporality.delta : AggregationTemporality.cumulative
end

def collect(start_time, end_time, data_points)
if @aggregation_temporality == :delta
if @aggregation_temporality.delta?
# Set timestamps and 'move' data point values to result.
ndps = data_points.values.map! do |ndp|
ndp.start_time_unix_nano = start_time
Expand Down Expand Up @@ -50,6 +47,10 @@ def update(increment, attributes, data_points)
ndp.value += increment
nil
end

def aggregation_temporality
@aggregation_temporality.temporality
end
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
_(last_snapshot[0].data_points[3].value).must_equal(4)
_(last_snapshot[0].data_points[3].attributes).must_equal('d' => 'e')

_(last_snapshot[0].aggregation_temporality).must_equal(:delta)
_(last_snapshot[0].aggregation_temporality).must_equal(:cumulative)
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,19 @@

require 'test_helper'

describe OpenTelemetry::SDK::Metrics::Aggregation::LastValue do
describe OpenTelemetry::SDK::Metrics::Aggregation::Drop do
let(:data_points) { {} }
let(:drop_aggregation) { OpenTelemetry::SDK::Metrics::Aggregation::Drop.new(aggregation_temporality: aggregation_temporality) }
let(:drop_aggregation) { OpenTelemetry::SDK::Metrics::Aggregation::Drop.new }
let(:aggregation_temporality) { :delta }

# Time in nano
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
# drop aggregation doesn't care about aggregation_temporality since all data will be dropped
kaylareopelle marked this conversation as resolved.
Show resolved Hide resolved
end

it 'sets the timestamps' do
drop_aggregation.update(0, {}, data_points)
ndp = drop_aggregation.collect(start_time, end_time, data_points)[0]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,21 +25,67 @@
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
_(exp.aggregation_temporality).must_equal :cumulative
end

it 'sets parameters from the environment to cumulative' do
exp = OpenTelemetry::TestHelpers.with_env('OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE' => 'cumulative') do
OpenTelemetry::SDK::Metrics::Aggregation::ExplicitBucketHistogram.new
end
_(exp.aggregation_temporality).must_equal :cumulative
end

it 'sets parameters from the environment to delta' do
exp = OpenTelemetry::TestHelpers.with_env('OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE' => 'delta') do
OpenTelemetry::SDK::Metrics::Aggregation::ExplicitBucketHistogram.new
end
_(exp.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
_(exp.aggregation_temporality).must_equal :cumulative
end

it 'invalid aggregation_temporality from parameters return default to cumulative' do
exp = OpenTelemetry::SDK::Metrics::Aggregation::ExplicitBucketHistogram.new(aggregation_temporality: 'pickles')
_(exp.aggregation_temporality).must_equal :cumulative
end

it 'valid aggregation_temporality delta from parameters' do
exp = OpenTelemetry::SDK::Metrics::Aggregation::ExplicitBucketHistogram.new(aggregation_temporality: 'delta')
_(exp.aggregation_temporality).must_equal :delta
end

it 'valid aggregation_temporality cumulative from parameters' do
exp = OpenTelemetry::SDK::Metrics::Aggregation::ExplicitBucketHistogram.new(aggregation_temporality: 'cumulative')
_(exp.aggregation_temporality).must_equal :cumulative
end

it 'valid aggregation_temporality delta as symbol from parameters' do
exp = OpenTelemetry::SDK::Metrics::Aggregation::ExplicitBucketHistogram.new(aggregation_temporality: :delta)
_(exp.aggregation_temporality).must_equal :delta
end

it 'valid aggregation_temporality cumulative as symbol from parameters' do
exp = OpenTelemetry::SDK::Metrics::Aggregation::ExplicitBucketHistogram.new(aggregation_temporality: :cumulative)
_(exp.aggregation_temporality).must_equal :cumulative
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
_(exp.aggregation_temporality).must_equal :cumulative
end

it 'function arguments have higher priority than environment' do
exp = OpenTelemetry::TestHelpers.with_env('OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE' => 'cumulative') do
OpenTelemetry::SDK::Metrics::Aggregation::ExplicitBucketHistogram.new(aggregation_temporality: :delta)
end
_(exp.aggregation_temporality).must_equal :delta
end
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,28 @@
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::LastValue.new
_(exp.aggregation_temporality).must_equal :delta
end

it 'valid aggregation_temporality delta as symbol from parameters' do
exp = OpenTelemetry::SDK::Metrics::Aggregation::LastValue.new(aggregation_temporality: :delta)
_(exp.aggregation_temporality).must_equal :delta
end

it 'valid aggregation_temporality cumulative as symbol from parameters' do
exp = OpenTelemetry::SDK::Metrics::Aggregation::LastValue.new(aggregation_temporality: :cumulative)
_(exp.aggregation_temporality).must_equal :cumulative
end

it 'invalid aggregation_temporality pickles as symbol from parameters return to defaults delta' do
exp = OpenTelemetry::SDK::Metrics::Aggregation::LastValue.new(aggregation_temporality: :pickles)
_(exp.aggregation_temporality).must_equal :delta
end
end

it 'sets the timestamps' do
last_value_aggregation.update(0, {}, data_points)
ndp = last_value_aggregation.collect(start_time, end_time, data_points)[0]
Expand Down
Loading
Loading